Annotation-driven deprecated table config validation on create + update

[xiangfu0]

Summary

Reject explicit use of deprecated table-config keys on both create and update, driven by a single source of truth on the SPI getters instead of a hand-maintained rule list.

• New @DeprecatedConfig(replacement, since) annotation in pinot-spi. Placed on the deprecated getter; since is the Pinot release the field was first marked @Deprecated.
• DeprecatedTableConfigValidationUtils discovers rules at class-load via a Jackson-aware reflection walk over TableConfig (recursing into nested BaseJsonConfig types, Collection<X>, Map<?,V>, honoring @JsonProperty renames). The hand-maintained list is gone.
• Severity is version-aware: a rule whose since.major.minor equals the running PinotVersion.major.minor is reported as a warning (one-release grace period). Older rules are errors that block the request. Unknown current version → safe default of error.
• Update paths (PUT /tables/{name}, PUT /tableConfigs/{name}, validate POSTs against existing tables) diff the incoming JSON against the byte-faithful stored ZNRecord JSON (new helper TableConfigSerDeUtils.toRawJsonNode + ZKMetadataProvider.getTableConfigZNRecord) — not against existingConfig.toJsonNode(), which would silently strip @JsonIgnore-d / @JsonInclude(NON_DEFAULT) deprecated keys and turn every legacy PUT into a false positive. Re-submitting an unchanged legacy value is a no-op; only newly introduced or value-changed deprecated paths fire.
• Warnings surface via a new optional deprecationWarnings: List<String> field on ConfigSuccessResponse and the validate endpoint JSON. Errors continue to throw 400.

Deprecated table-config keys covered

Sorted from earliest deprecation to latest. On the current 1.6.0-SNAPSHOT release line, everything older than 1.6 is an error; 1.6.0 deprecations are warnings (one-release grace period).

Since	JSON path	Replacement	Severity on 1.6
0.3.0	routing.routingTableBuilderName	Use routing.segmentPrunerTypes and routing.instanceSelectorType	error
0.7.1	tableIndexConfig.streamConfigs	Use ingestionConfig.streamIngestionConfig.streamConfigMaps	error
0.8.0	segmentsConfig.segmentPushFrequency	Use ingestionConfig.batchIngestionConfig.segmentIngestionFrequency	error
0.8.0	segmentsConfig.segmentPushType	Use ingestionConfig.batchIngestionConfig.segmentIngestionType	error
0.9.0	fieldConfigList[*].indexType	Use fieldConfigList[].indexTypes	error
0.12.0	tableIndexConfig.jsonIndexColumns	Use tableIndexConfig.jsonIndexConfigs	error
1.1.0	segmentsConfig.replicasPerPartition	Use segmentsConfig.replication	error
1.1.0	instanceAssignmentConfigMap[*].replicaGroupPartitionConfig.minimizeDataMovement	Remove this field; it will be removed in a future release	error
1.3.0	segmentsConfig.replicaGroupStrategyConfig	Use segmentAssignmentConfigMap	error
1.3.0	segmentsConfig.minimizeDataMovement	Use instanceAssignmentConfigMap	error
1.4.0	upsertConfig.enableSnapshot	Use upsertConfig.snapshot	error
1.4.0	upsertConfig.enablePreload	Use upsertConfig.preload	error
1.4.0	upsertConfig.allowPartialUpsertConsumptionDuringCommit	Use ingestionConfig.streamIngestionConfig.parallelSegmentConsumptionPolicy	error
1.4.0	dedupConfig.enablePreload	Use dedupConfig.preload	error
1.4.0	dedupConfig.allowDedupConsumptionDuringCommit	Use ingestionConfig.streamIngestionConfig.parallelSegmentConsumptionPolicy	error
1.6.0	tableIndexConfig.createInvertedIndexDuringSegmentGeneration	Remove this field; it is ignored	warning

since was determined per field by walking git log / git tag --contains against the upstream apache/pinot history to find the first release tag that ships the @Deprecated annotation (or the original @deprecated Javadoc when that came first).

Behavior

• Create (POST): every present rule fires. Errors → 400; warnings → server WARN log + deprecationWarnings field.
• Update (PUT): diff against the raw stored ZK JSON. Only paths newly added or whose value changed fire. Legacy values that were already on the table and re-submitted unchanged pass silently.
• Validate (POST): runs in create mode if the table doesn't yet exist, update mode otherwise. Returns warnings in the response body.
• Builder-generated payloads: TableConfigBuilder.build() now converts the deprecated _segmentPushType / _segmentPushFrequency setters into modern ingestionConfig.batchIngestionConfig.segmentIngestionType/Frequency, so existing tests and tools that use the builder produce create payloads that pass validation.

Testing

• ./mvnw -pl pinot-spi,pinot-common,pinot-controller -am -Dtest='DeprecatedTableConfigValidationUtilsTest+TableConfigSerDeUtilsTest+TableConfigBuilderTest+PinotTableRestletResourceTest#testRejectsDeprecatedConfigOnCreateAndOnUpdateWhenNewlyIntroduced+PinotTableRestletResourceTest#testUpdateAllowsUnchangedLegacyDeprecatedConfig+TableConfigsRestletResourceTest' test
• ./mvnw -pl pinot-spi,pinot-common,pinot-controller spotless:apply checkstyle:check license:check

Coverage includes: annotation discovery walk against TableConfig, diff filtering on update, version-based severity classification, raw-JSON preservation across @JsonIgnore/@JsonInclude(NON_DEFAULT) getters, and round-trip rejection/acceptance through the controller REST endpoints.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 63.75839% with 108 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.63%. Comparing base (a5c2c45) to head (ada4941).

Files with missing lines	Patch %	Lines
...s/config/DeprecatedTableConfigValidationUtils.java	63.87%	45 Missing and 50 partials ⚠️
...not/common/utils/config/TableConfigSerDeUtils.java	78.57%	3 Missing and 3 partials ⚠️
...he/pinot/spi/utils/builder/TableConfigBuilder.java	0.00%	2 Missing and 1 partial ⚠️
...exception/TableConfigVersionConflictException.java	0.00%	2 Missing ⚠️
...ache/pinot/common/metadata/ZKMetadataProvider.java	0.00%	2 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (a5c2c45) and HEAD (ada4941). Click for more details.

HEAD has 14 uploads less than BASE

Flag	BASE (a5c2c45)	HEAD (ada4941)
lane-a	2	1
integration	4	2
java-25	6	3
integration1	2	0
temurin	6	3
lane-b	2	1
unittests	2	1
unittests2	1	0

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #18411      +/-   ##
============================================
- Coverage     67.54%   57.63%   -9.92%     
+ Complexity     1430        1    -1429     
============================================
  Files          3486     2688     -798     
  Lines        224044   164164   -59880     
  Branches      35353    26687    -8666     
============================================
- Hits         151339    94608   -56731     
- Misses        60677    61496     +819     
+ Partials      12028     8060    -3968     

Flag	Coverage Δ
integration	0.00% <ø> (-100.00%)	⬇️
integration1	?
integration2	0.00% <ø> (ø)
java-25	57.63% <63.75%> (-9.92%)	⬇️
lane-a	0.00% <ø> (-100.00%)	⬇️
lane-b	0.00% <ø> (ø)
temurin	57.63% <63.75%> (-9.92%)	⬇️
unittests	57.63% <63.75%> (-9.92%)	⬇️
unittests1	57.63% <63.75%> (+<0.01%)	⬆️
unittests2	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[noob-se7en]

1 major error-handling / 2 medium follow-ups.

[Copilot]

Pull request overview

This PR introduces annotation-driven validation to reject explicit usage of deprecated TableConfig JSON keys on both table/config creation and updates, while preserving backward compatibility for legacy values already stored in ZK by diffing against the raw stored ZNRecord JSON.

Changes:

• Add @DeprecatedConfig(replacement, since) annotations on deprecated SPI config getters and reflectively discover deprecated JSON paths for validation.
• Enforce deprecated-config validation on controller create/update/validate endpoints (with version-aware warning vs error severity) and surface warnings via deprecationWarnings.
• Update builders, tests, and example table-config JSON to use modern fields (ingestion configs, indexTypes, jsonIndexConfigs, etc.), plus add raw-ZK JSON reconstruction utilities for update diffing.

Reviewed changes

Copilot reviewed 50 out of 50 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
pinot-spi/src/main/java/org/apache/pinot/spi/config/DeprecatedConfig.java	Adds the new deprecation annotation used as the single source of truth for deprecated JSON keys.
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/DeprecatedTableConfigValidationUtils.java	Implements reflective rule discovery + create/update-time deprecated-key validation with version-aware severity.
pinot-common/src/main/java/org/apache/pinot/common/utils/config/TableConfigSerDeUtils.java	Adds toRawJsonNode(ZNRecord) to reconstruct raw stored JSON for byte-faithful update diffing.
pinot-common/src/main/java/org/apache/pinot/common/metadata/ZKMetadataProvider.java	Adds getTableConfigZNRecord() helper to fetch raw table config ZNRecord.
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java	Enforces deprecated-config validation on table create/update/validate and returns warnings in responses.
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java	Enforces deprecated-config validation for TableConfigs create/update/validate and returns warnings in responses.
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ConfigSuccessResponse.java	Adds optional deprecationWarnings field to success responses.
pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableConfigBuilder.java	Converts deprecated builder setters (segment push + stream configs) into modern ingestion config fields and omits deprecated serialized keys.
pinot-spi/src/test/java/org/apache/pinot/spi/utils/builder/TableConfigBuilderTest.java	Tests conversion/omission behavior of deprecated fields in builder output JSON.
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/SegmentsValidationAndRetentionConfig.java	Annotates deprecated segment push + other deprecated fields with @DeprecatedConfig.
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/IndexingConfig.java	Annotates deprecated indexing fields (jsonIndexColumns, streamConfigs, etc.) with @DeprecatedConfig.
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/FieldConfig.java	Marks legacy indexType as deprecated config key for validation while preserving deserialization via @JsonCreator.
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/RoutingConfig.java	Annotates deprecated routingTableBuilderName with @DeprecatedConfig.
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/UpsertConfig.java	Annotates deprecated upsert booleans with @DeprecatedConfig and NON_DEFAULT inclusion.
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/DedupConfig.java	Annotates deprecated dedup booleans with @DeprecatedConfig and NON_DEFAULT inclusion.
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/assignment/InstanceReplicaGroupPartitionConfig.java	Annotates deprecated nested minimizeDataMovement with @DeprecatedConfig.
pinot-common/src/test/java/org/apache/pinot/common/utils/config/TableConfigSerDeUtilsTest.java	Adds tests ensuring toRawJsonNode() preserves keys stripped by bean round-trips.
pinot-controller/src/test/java/org/apache/pinot/controller/api/resources/DeprecatedTableConfigValidationUtilsTest.java	Adds unit tests for rule discovery, version severity, create vs update diff behavior.
pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotTableRestletResourceTest.java	Adds REST-level tests for rejecting deprecated keys on create and on update when newly introduced.
pinot-controller/src/test/java/org/apache/pinot/controller/api/TableConfigsRestletResourceTest.java	Adds TableConfigs REST test ensuring deprecated keys are rejected on create.
pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancerClusterStatelessTest.java	Updates test table config creation to use modern ingestion config handling.
pinot-core/src/test/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManagerTest.java	Migrates tests to use ingestion-config helpers instead of deprecated streamConfigs.
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BasePauselessRealtimeIngestionTest.java	Migrates ingestion setup away from deprecated streamConfigs.
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/logicaltable/LogicalTableWithTwoRealtimeTableIntegrationTest.java	Migrates stream config access to ingestion-config utilities.
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/realtime/ingestion/KafkaIncreaseDecreasePartitionsIntegrationTest.java	Refactors test to rely on base-class topic/table wiring rather than manual creation.
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/TableConfigUtilsTest.java	Adjusts legacy conversion test to inject deprecated keys directly (since builder now produces modern ingestion config).
pinot-tools/src/main/java/org/apache/pinot/tools/BootstrapTableTool.java	Adds a null-guard around batch config maps iteration.
pinot-tools/src/main/resources/conf/sample_offline_table_config.json	Updates sample config to modern ingestion fields / removes deprecated keys.
pinot-tools/src/main/resources/conf/sample_realtime_table_config.json	Updates sample config to modern ingestion fields / removes deprecated keys.
pinot-tools/src/main/resources/examples/batch/airlineStats/airlineStats_offline_table_config.json	Removes deprecated segment push keys; adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/batch/baseballStats/baseballStats_offline_table_config.json	Removes deprecated segment push keys; adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/minions/batch/baseballStats/baseballStats_offline_table_config.json	Removes deprecated segment push keys; adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/batch/clickstreamFunnel/clickstreamFunnel_offline_table_config.json	Removes deprecated keys and adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/batch/dimBaseballTeams/dimBaseballTeams_offline_table_config.json	Removes deprecated segment push keys; adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/batch/fineFoodReviews/fineFoodReviews_offline_table_config.json	Migrates field configs to indexTypes and adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/batch/githubEvents/githubEvents_offline_table_config.json	Migrates jsonIndexColumns -> jsonIndexConfigs and adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/batch/githubComplexTypeEvents/githubComplexTypeEvents_offline_table_config.json	Removes deprecated segment push keys; adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/batch/starbucksStores/starbucksStores_offline_table_config.json	Migrates H3 field config to indexTypes and adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/batch/testUnnest/testUnnest_offline_table_config.json	Removes deprecated fields and adds modern ingestion config fields.
pinot-tools/src/main/resources/examples/stream/dailySales/dailySales_realtime_table_config.json	Removes deprecated keys and migrates to modern ingestion config fields.
pinot-tools/src/main/resources/examples/stream/fineFoodReviews/fineFoodReviews_realtime_table_config.json	Removes deprecated keys and migrates to modern ingestion config fields.
pinot-tools/src/main/resources/examples/stream/fineFoodReviews_part_0/fineFoodReviews_part_0_realtime_table_config.json	Removes deprecated keys and migrates to modern ingestion config fields.
pinot-tools/src/main/resources/examples/stream/fineFoodReviews_part_1/fineFoodReviews_part_1_realtime_table_config.json	Removes deprecated keys and migrates to modern ingestion config fields.
pinot-tools/src/main/resources/examples/stream/meetupRsvpJson/meetupRsvpJson_realtime_table_config.json	Migrates jsonIndexColumns -> jsonIndexConfigs.
pinot-tools/src/main/resources/examples/stream/upsertJsonMeetupRsvp/upsertJsonMeetupRsvp_realtime_table_config.json	Migrates jsonIndexColumns -> jsonIndexConfigs.
pinot-tools/src/main/resources/examples/stream/upsertMeetupRsvp/upsertMeetupRsvp_realtime_table_config.json	Migrates upsert deprecated fields to modern enums; migrates indexType -> indexTypes.
pinot-tools/src/main/resources/examples/stream/upsertPartialMeetupRsvp/upsertPartialMeetupRsvp_realtime_table_config.json	Migrates indexType -> indexTypes.
pinot-integration-tests/src/test/resources/chaos-monkey-create-table.json	Removes deprecated segment push keys; adds modern ingestion config fields.
pinot-integration-tests/src/test/resources/dimDayOfWeek_config.json	Removes deprecated segment push keys; adds modern ingestion config fields.
pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/admin/README.md	Documents modern create payload fields and common migrations away from deprecated keys.

Comments suppressed due to low confidence (1)

pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableConfigBuilder.java:238

• setSegmentPushType() / setSegmentPushFrequency() are documented as deprecated in their Javadoc, but they are not annotated with @Deprecated (unlike other deprecated builder APIs in this class, e.g. setLLC).

Annotate these methods with @Deprecated so callers get compiler warnings and IDE tooling consistently flags their use.

  /**
   * @deprecated Use {@code segmentIngestionType} from {@link IngestionConfig#getBatchIngestionConfig()}
   */
  public TableConfigBuilder setSegmentPushType(String segmentPushType) {
    if (REFRESH_SEGMENT_PUSH_TYPE.equalsIgnoreCase(segmentPushType)) {
      _segmentPushType = REFRESH_SEGMENT_PUSH_TYPE;
    } else {
      _segmentPushType = "APPEND";
    }
    return this;
  }

  /**
   * @deprecated Use {@code segmentIngestionFrequency} from {@link IngestionConfig#getBatchIngestionConfig()}
   */
  public TableConfigBuilder setSegmentPushFrequency(String segmentPushFrequency) {
    _segmentPushFrequency = segmentPushFrequency;
    return this;
  }

[xiangfu0]

Found two high-signal deprecation-validation gaps; see inline comments.

[xiangfu0]

Found one high-signal enforcement gap; see inline comment.

[xiangfu0]

Two validate endpoints still echo the full raw config body on parse failures. Because ControllerApplicationException logs 4xx messages, that leaks stream credentials and other secrets into controller logs; inline details below.

[xiangfu0]

This still reflects the entire tableConfigsStr into both the 400 response and the controller log line (ControllerApplicationException logs 4xx messages). TableConfigs payloads can carry stream credentials, so a malformed /tableConfigs/validate request now leaks secrets into logs. Please match the addConfig() scrub here and keep only e.getMessage().

[xiangfu0]

Fixed in 05def21: switched to "Invalid TableConfigs json string. " + e.getMessage() to match addConfig. Also reordered /tableConfigs/validate so the permission check runs BEFORE the ZK-reading validateNoDeprecatedConfigs path, mirroring PinotTableRestletResource.checkTableConfig and preventing an unauthenticated caller from probing table existence.

[xiangfu0]

Same leak on /tables/validate: parse failures log the full raw table config via ControllerApplicationException, including any stream config credentials in the request body. The new PR already scrubbed similar error paths elsewhere, so this validate endpoint needs the same treatment.

[xiangfu0]

Fixed in 05def21: scrubbed the raw tableConfigStr from both the 400 response and the controller log. The catch now uses concatenation ("Invalid table config json string. " + e.getMessage()), matching the addTable/copyTable scrub pattern already in this file.