stop serializing old externalauth fields#5217
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR is part of the multi-step ExternalAuth storage migration plan (refs #5215/#5216), updating Cosmos DB serialization to the “new-only” (GenericDocument-compatible) shape by inlining HCPOpenShiftClusterExternalAuth and removing legacy nested fields. It also updates integration artifacts and tests to match the new on-disk document structure and to rely on cosmosMetadata.resourceID as the canonical Cosmos identifier.
Changes:
- Inline
api.HCPOpenShiftClusterExternalAuthdirectly into the CosmosExternalAuthdocument shape and update conversion logic to useCosmosMetadata.ResourceIDfor partitioning/UID/resource-type. - Propagate/copy
CosmosMetadataas a read-only value during replace/patch flows; update deepcopy generation to deep copyCosmosMetadata. - Update integration test artifacts and multiple unit/fuzz tests to reflect the new stored JSON shape and required metadata.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test-integration/utils/databasemutationhelpers/per_resource_comparer.go | Ignores additional variable Cosmos/metadata fields in resource comparisons. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/read-new-data/18-cosmosCompare-confirm/externalauth-default.json | Updates expected Cosmos document shape for ExternalAuth after reading new data. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/read-new-data/01-load-old-data/externalauth-default.json | Updates expected old-data load artifact to new ExternalAuth persisted shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/migrate-old-data/99-cosmosCompare-confirm-migration/externalauth-default.json | Updates expected end-state artifact for migration to new ExternalAuth shape. |
| test-integration/frontend/artifacts/FrontendCRUD/Migration/migrate-old-data/01-load-old-data/externalauth-default.json | Updates migrate-old-data load artifact to new ExternalAuth persisted shape. |
| test-integration/frontend/artifacts/FrontendCRUD/ExternalAuth/read-old-data/09-cosmosCompare-confirm-update/externalauth-default.json | Updates expected Cosmos document after ExternalAuth update in old-data scenario. |
| test-integration/frontend/artifacts/FrontendCRUD/ExternalAuth/read-old-data/01-load-old-data/externalauth-default.json | Updates initial old-data artifact for ExternalAuth to new persisted shape. |
| test-integration/frontend/artifacts/FrontendCRUD/ExternalAuth/create-current/06-cosmosCompare-ending-content/externalauth-default.json | Updates expected Cosmos document after current create flow to new shape. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/07-untypedListRecursive-resourcegroup-via-child/basic-external-auth-auth.json | Removes legacy nested internalState content from expected untyped list output. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/05-untypedListRecursive-subscription/basic-external-auth-auth.json | Removes legacy nested internalState content from expected untyped list output. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/04-untypedListRecursive-resourcegroup/basic-external-auth-auth.json | Removes legacy nested internalState content from expected untyped list output. |
| test-integration/frontend/artifacts/DatabaseCRUD/UntypedCRUD/basic/01-load-initial/basic-external-auth-auth.json | Removes legacy nested internalState content from initial untyped CRUD fixture. |
| test-integration/backend/controllers/mismatches/artifacts/nodepool/remove_orphaned_nodepool_descendents/99-cosmosCompare-end-state/externalauth-default.json | Updates mismatch-controller end-state artifact to new ExternalAuth stored shape. |
| test-integration/backend/controllers/mismatches/artifacts/nodepool/remove_orphaned_nodepool_descendents/00-load-initial-state/externalauth-default.json | Updates mismatch-controller initial-state artifact to new ExternalAuth stored shape. |
| test-integration/backend/controllers/mismatches/artifacts/externalauth/remove_orphaned_externalauth_descendents/00-load-initial-state/externalauth-default.json | Updates mismatch-controller initial-state artifact to new ExternalAuth stored shape. |
| test-integration/backend/controllers/mismatches/artifacts/externalauth/present_externalauth/99-cosmosCompare-end-state/externalauth-default.json | Updates present-externalauth end-state artifact to new stored shape. |
| test-integration/backend/controllers/mismatches/artifacts/externalauth/present_externalauth/00-load-initial-state/externalauth-default.json | Updates present-externalauth initial-state artifact to new stored shape. |
| test-integration/backend/controllers/mismatches/artifacts/cluster/remove_orphaned_cluster_descendents/00-load-initial-state/externalauth-default.json | Updates cluster mismatch artifact to new ExternalAuth stored shape. |
| internal/validation/validate_externalauth_comprehensive_test.go | Ensures test ExternalAuth objects include CosmosMetadata.ResourceID. |
| internal/database/types_externalauth.go | Inlines HCPOpenShiftClusterExternalAuth into Cosmos ExternalAuth properties. |
| internal/database/convert_externalauth.go | Updates Cosmos<->internal conversion to rely on CosmosMetadata.ResourceID + embedded type. |
| internal/database/convert_externalauth_test.go | Updates round-trip and ETag preservation tests for new CosmosMetadata/inline layout. |
| internal/database/convert_defaults_consistency_test.go | Adjusts pre-existing-data test fixture to match new inlined storage layout. |
| internal/database/convert_cluster_test.go | Updates generic round-trip helper to clear ExistingCosmosUID from new location for ExternalAuth. |
| internal/conversion/readonly_externalauth.go | Copies CosmosMetadata as read-only state during update/replace flows. |
| internal/api/zz_generated.deepcopy.go | Deep-copies CosmosMetadata on ExternalAuth deepcopy. |
| internal/api/v20251223preview/external_auth_methods.go | Sets internal ResourceID on ConvertToInternal; uses ResourceID when emitting external id. |
| internal/api/v20251223preview/conversion_fuzz_test.go | Updates fuzz round-trip expectations for CosmosMetadata fields. |
| internal/api/v20240610preview/external_auth_methods.go | Sets internal ResourceID on ConvertToInternal; uses ResourceID when emitting external id. |
| internal/api/v20240610preview/conversion_fuzz_test.go | Updates fuzz round-trip expectations for CosmosMetadata fields. |
| internal/api/types_externalauth.go | Adds embedded CosmosMetadata; removes legacy CosmosETag/ExistingCosmosUID plumbing. |
| frontend/pkg/frontend/external_auth.go | Sets CosmosMetadata.ResourceID on create; validates stored ResourceID on read. |
| backend/pkg/listertesting/slice_listers_test.go | Updates test ExternalAuth objects to include CosmosMetadata.ResourceID. |
| backend/pkg/informers/informers_test.go | Updates informer tests to include CosmosMetadata.ResourceID on ExternalAuth. |
| backend/pkg/controllers/operationcontrollers/test_helpers_test.go | Updates controller fixture ExternalAuth to include CosmosMetadata.ResourceID. |
| backend/pkg/controllers/metricscontrollers/resource_metrics_controller_test.go | Updates metrics test ExternalAuth to include CosmosMetadata.ResourceID. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type HCPOpenShiftClusterExternalAuth struct { | ||
| CosmosMetadata `json:"cosmosMetadata"` | ||
|
|
||
| arm.ProxyResource | ||
| Properties HCPOpenShiftClusterExternalAuthProperties `json:"properties"` | ||
| ServiceProviderProperties HCPOpenShiftClusterExternalAuthServiceProviderProperties `json:"serviceProviderProperties,omitempty"` | ||
| // CosmosETag is an in-memory copy of the _etag field read from the Cosmos DB document (BaseDocument) and | ||
| // populated on DB read via the CosmosToInternalExternalAuth() conversion function. | ||
| // We carry it across the API boundary between ExternalAuth (the direct cosmos db type) and HCPOpenShiftClusterExternalAuth (this) | ||
| // so we can populate the CosmosETag in GetCosmosData() so that we can do conditional replaces in cosmos. | ||
| // This can be removed once we have inlined and serialized CosmosMetadata in | ||
| // HCPOpenShiftClusterExternalAuth. | ||
| CosmosETag azcore.ETag `json:"-"` | ||
| } |
| @@ -327,8 +328,8 @@ func (v version) NewHCPOpenShiftClusterExternalAuth(from *api.HCPOpenShiftCluste | |||
| } | |||
|
|
|||
| idString := "" | |||
| if from.ID != nil { | ||
| idString = from.ID.String() | ||
| if from.ResourceID != nil { | ||
| idString = from.ResourceID.String() |
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
replaced by #5395 |
part of #5215, requires #5216 to go to prod before merge
/hold for #5216 to get to prod