stop serializing old externalauth fields

backend/pkg/controllers/metricscontrollers/resource_metrics_controller_test.go

@@ -167,6 +167,7 @@ func TestExternalAuthMetricsHandler_SetsMetrics(t *testing.T) {
 	handler := NewExternalAuthMetricsHandler(reg)
 
 	externalAuth := &api.HCPOpenShiftClusterExternalAuth{
+		CosmosMetadata: arm.CosmosMetadata{ResourceID: api.Must(azcorearm.ParseResourceID("/subscriptions/sub-1/resourceGroups/rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/cluster-1/externalAuths/ea-1"))},
 		ProxyResource: arm.ProxyResource{
 			Resource: arm.Resource{
 				ID:         api.Must(azcorearm.ParseResourceID("/subscriptions/sub-1/resourceGroups/rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/cluster-1/externalAuths/ea-1")),

backend/pkg/controllers/operationcontrollers/test_helpers_test.go

@@ -271,6 +271,7 @@ func (f *externalAuthTestFixture) newCluster() *api.HCPOpenShiftCluster {
 
 func (f *externalAuthTestFixture) newExternalAuth() *api.HCPOpenShiftClusterExternalAuth {
 	return &api.HCPOpenShiftClusterExternalAuth{
+		CosmosMetadata: arm.CosmosMetadata{ResourceID: f.externalAuthResourceID},
 		ProxyResource: arm.ProxyResource{
 			Resource: arm.Resource{
 				ID:   f.externalAuthResourceID,

backend/pkg/informers/informers_test.go

@@ -751,7 +751,8 @@ func controllerInformerTestCase() informerTestCase {
 					"/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/"+clusterName+
 					"/externalAuths/"+externalAuthName)
 			ea := &api.HCPOpenShiftClusterExternalAuth{
-				ProxyResource: arm.NewProxyResource(eaResourceID),
+				CosmosMetadata: arm.CosmosMetadata{ResourceID: eaResourceID},
+				ProxyResource:  arm.NewProxyResource(eaResourceID),
 			}
 			_, err = mockResourcesDBClient.HCPClusters(subscriptionID, resourceGroupName).ExternalAuth(clusterName).Create(ctx, ea, nil)
 			require.NoError(t, err)

backend/pkg/listertesting/slice_listers_test.go

@@ -381,7 +381,8 @@ func newTestExternalAuth(subscriptionID, resourceGroupName, clusterName, externa
 			"/externalAuths/" + externalAuthName,
 	))
 	return &api.HCPOpenShiftClusterExternalAuth{
-		ProxyResource: arm.NewProxyResource(resourceID),
+		CosmosMetadata: arm.CosmosMetadata{ResourceID: resourceID},
+		ProxyResource:  arm.NewProxyResource(resourceID),
 	}
 }
 

frontend/pkg/frontend/external_auth.go

@@ -210,6 +210,7 @@ func decodeDesiredExternalAuthCreate(ctx context.Context) (*api.HCPOpenShiftClus
 
 	// ProxyResource info doesn't to come from the external resource information
 	conversion.CopyReadOnlyProxyResourceValues(&newInternalExternalAuth.ProxyResource, ptr.To(arm.NewProxyResource(resourceID)))
+	newInternalExternalAuth.SetResourceID(resourceID)
 
 	// set fields that were not included during the conversion, because the user does not provide them or because the
 	// data is determined live on read.
@@ -689,8 +690,11 @@ func (f *Frontend) getInternalExternalAuthFromStorage(ctx context.Context, resou
 	// normalize or return a toupper or tolower form of the resource
 	// group or resource name. The resource group name and resource
 	// name must come from the URL and not the request body.
-	if !strings.EqualFold(internalExternalAuth.ID.String(), resourceID.String()) {
-		return nil, fmt.Errorf("unexpected resourceID: %s", internalExternalAuth.ID.String())
+	if internalExternalAuth.ResourceID == nil {
+		return nil, fmt.Errorf("stored externalauth document is missing cosmosMetadata.resourceID")
+	}
+	if !strings.EqualFold(internalExternalAuth.ResourceID.String(), resourceID.String()) {
+		return nil, fmt.Errorf("unexpected resourceID: %s", internalExternalAuth.ResourceID.String())
 	}
 	internalExternalAuth.ID = resourceID
 

internal/api/types_externalauth.go

@@ -17,7 +17,6 @@ package api
 import (
 	"time"
 
-	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
 	azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
 
 	"github.com/Azure/ARO-HCP/internal/api/arm"
@@ -27,16 +26,11 @@ import (
 // OpenShift clusters.
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 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:"-"`
 }
 
 // EnsureDefaults fills in default values for fields that may be absent in
@@ -55,14 +49,6 @@ func (ea *HCPOpenShiftClusterExternalAuth) EnsureDefaults() {
 
 var _ arm.CosmosPersistable = &HCPOpenShiftClusterExternalAuth{}
 
-func (o *HCPOpenShiftClusterExternalAuth) GetCosmosData() *arm.CosmosMetadata {
-	return &arm.CosmosMetadata{
-		CosmosETag:        o.CosmosETag,
-		ResourceID:        o.ID,
-		ExistingCosmosUID: o.ServiceProviderProperties.ExistingCosmosUID,
-	}
-}
-
 // HCPOpenShiftClusterNodePoolProperties represents the property bag of a
 // HCPOpenShiftClusterNodePool resource.
 type HCPOpenShiftClusterExternalAuthProperties struct {
@@ -74,7 +60,6 @@ type HCPOpenShiftClusterExternalAuthProperties struct {
 }
 
 type HCPOpenShiftClusterExternalAuthServiceProviderProperties struct {
-	ExistingCosmosUID string      `json:"-"`
 	ClusterServiceID  *InternalID `json:"clusterServiceID,omitempty"`
 	ActiveOperationID string      `json:"activeOperationId,omitempty"`
 }

internal/api/v20240610preview/conversion_fuzz_test.go

@@ -40,6 +40,14 @@ func TestRoundTripInternalExternalInternal(t *testing.T) {
 		func(j *azcorearm.ResourceID, c randfill.Continue) {
 			*j = *api.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myRg"))
 		},
+		func(j *api.CosmosMetadata, c randfill.Continue) {
+			c.FillNoCustom(j)
+			// ConvertToInternal lowercases the ID, so use the lowercased canonical form
+			// here so the round-trip comparison succeeds.
+			j.ResourceID = api.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/myrg"))
+			j.ExistingCosmosUID = ""
+			j.CosmosETag = ""
+		},
 		func(j *api.HCPOpenShiftClusterCustomerProperties, c randfill.Continue) {
 			c.FillNoCustom(j)
 			// ImageDigestMirrors is a v20251223preview field that does not exist in v20240610preview.
@@ -88,7 +96,6 @@ func TestRoundTripInternalExternalInternal(t *testing.T) {
 			j.ActiveOperationID = ""
 			// ClusterServiceID does not roundtrip through the external type because it is purely an internal detail
 			j.ClusterServiceID = nil
-			j.ExistingCosmosUID = ""
 		},
 		func(j *api.CustomerPlatformProfile, c randfill.Continue) {
 			c.FillNoCustom(j)

internal/api/v20240610preview/external_auth_methods.go

@@ -64,6 +64,7 @@ func (h *ExternalAuth) ConvertToInternal(existing *api.HCPOpenShiftClusterExtern
 
 	if h.ID != nil {
 		out.ID = api.Must(azcorearm.ParseResourceID(strings.ToLower(*h.ID)))
+		out.ResourceID = api.Must(azcorearm.ParseResourceID(strings.ToLower(*h.ID)))
 	}
 	if h.Name != nil {
 		out.Name = *h.Name
@@ -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()
 	}
 
 	out := &ExternalAuth{

internal/api/v20251223preview/conversion_fuzz_test.go

@@ -40,6 +40,14 @@ func TestRoundTripInternalExternalInternal(t *testing.T) {
 		func(j *azcorearm.ResourceID, c randfill.Continue) {
 			*j = *api.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myRg"))
 		},
+		func(j *api.CosmosMetadata, c randfill.Continue) {
+			c.FillNoCustom(j)
+			// ConvertToInternal lowercases the ID, so use the lowercased canonical form
+			// here so the round-trip comparison succeeds.
+			j.ResourceID = api.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/myrg"))
+			j.ExistingCosmosUID = ""
+			j.CosmosETag = ""
+		},
 		func(j *api.ImageDigestMirror, c randfill.Continue) {
 			c.FillNoCustom(j)
 			// MirrorSourcePolicy does not roundtrip through the external type because it is purely an internal detail
@@ -79,7 +87,6 @@ func TestRoundTripInternalExternalInternal(t *testing.T) {
 			j.ActiveOperationID = ""
 			// ClusterServiceID does not roundtrip through the external type because it is purely an internal detail
 			j.ClusterServiceID = nil
-			j.ExistingCosmosUID = ""
 		},
 		func(j *api.CustomerManagedEncryptionProfile, c randfill.Continue) {
 			c.FillNoCustom(j)

internal/api/v20251223preview/external_auth_methods.go

@@ -64,6 +64,7 @@ func (h *ExternalAuth) ConvertToInternal(existing *api.HCPOpenShiftClusterExtern
 
 	if h.ID != nil {
 		out.ID = api.Must(azcorearm.ParseResourceID(strings.ToLower(*h.ID)))
+		out.ResourceID = api.Must(azcorearm.ParseResourceID(strings.ToLower(*h.ID)))
 	}
 	if h.Name != nil {
 		out.Name = *h.Name
@@ -315,8 +316,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()
 	}
 
 	out := &ExternalAuth{

internal/conversion/readonly_externalauth.go

@@ -29,8 +29,11 @@ func CopyReadOnlyProxyResourceValues(dest, src *arm.ProxyResource) {
 func CopyReadOnlyExternalAuthValues(dest, src *api.HCPOpenShiftClusterExternalAuth) {
 	CopyReadOnlyProxyResourceValues(&dest.ProxyResource, &src.ProxyResource)
 
+	// CosmosMetadata is read-only on the API surface; carry over so the
+	// case-preserving ResourceID and CosmosETag survive the replace round-trip.
+	dest.CosmosMetadata = *src.CosmosMetadata.DeepCopy()
+
 	dest.Properties.ProvisioningState = src.Properties.ProvisioningState
 	dest.Properties.Condition = *src.Properties.Condition.DeepCopy()
 	dest.ServiceProviderProperties = *src.ServiceProviderProperties.DeepCopy()
-	dest.CosmosETag = src.CosmosETag
 }

internal/database/convert_cluster_test.go

@@ -140,7 +140,7 @@ func roundTripInternalToCosmosToInternal[InternalAPIType, CosmosAPIType any](t *
 	case *api.HCPOpenShiftClusterNodePool:
 		cast.ServiceProviderProperties.ExistingCosmosUID = ""
 	case *api.HCPOpenShiftClusterExternalAuth:
-		cast.ServiceProviderProperties.ExistingCosmosUID = ""
+		cast.ExistingCosmosUID = ""
 	}
 	//finalJSON, _ := json.MarshalIndent(final, "", "    ")
 

internal/database/convert_defaults_consistency_test.go

@@ -585,21 +585,24 @@ func TestPreExistingDataExternalAuth(t *testing.T) {
 		"/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/cluster/externalAuths/default",
 	))
 
+	internalID := api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/test-cluster/external_auth_config/external_auths/default"))
 	preExistingDoc := &ExternalAuth{
 		TypedDocument: TypedDocument{
 			BaseDocument: BaseDocument{ID: "test-doc-id"},
 			ResourceID:   resourceID,
 		},
 		ExternalAuthProperties: ExternalAuthProperties{
-			IntermediateResourceDoc: &ResourceDocument{
-				ResourceID:        resourceID,
-				InternalID:        api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/test-cluster/external_auth_config/external_auths/default")),
-				ProvisioningState: arm.ProvisioningStateSucceeded,
-			},
-			InternalState: ExternalAuthInternalState{
-				InternalAPI: api.HCPOpenShiftClusterExternalAuth{
-					// PrefixPolicy is intentionally zero-valued to simulate
-					// a pre-existing document that predates the field.
+			HCPOpenShiftClusterExternalAuth: api.HCPOpenShiftClusterExternalAuth{
+				// PrefixPolicy is intentionally zero-valued to simulate
+				// a pre-existing document that predates the field.
+				CosmosMetadata: arm.CosmosMetadata{
+					ResourceID: resourceID,
+				},
+				Properties: api.HCPOpenShiftClusterExternalAuthProperties{
+					ProvisioningState: arm.ProvisioningStateSucceeded,
+				},
+				ServiceProviderProperties: api.HCPOpenShiftClusterExternalAuthServiceProviderProperties{
+					ClusterServiceID: &internalID,
 				},
 			},
 		},

internal/database/convert_externalauth.go

@@ -18,97 +18,54 @@ import (
 	"fmt"
 	"strings"
 
-	"k8s.io/utils/ptr"
-
 	"github.com/Azure/ARO-HCP/internal/api"
-	"github.com/Azure/ARO-HCP/internal/api/arm"
-	"github.com/Azure/ARO-HCP/internal/ocm"
 )
 
 func InternalToCosmosExternalAuth(internalObj *api.HCPOpenShiftClusterExternalAuth) (*ExternalAuth, error) {
 	if internalObj == nil {
 		return nil, nil
 	}
 
+	// CosmosMetadata.ResourceID is the canonical identifier for cosmos-side
+	// concerns (partitioning, document UID, resource-type indexing). Use it
+	// instead of the ProxyResource.ID, which is an ARM-surface concern.
+	cosmosResourceID := internalObj.GetCosmosData().ResourceID
+	if cosmosResourceID == nil {
+		return nil, fmt.Errorf("internalObj is missing CosmosMetadata.ResourceID")
+	}
 	cosmosObj := &ExternalAuth{
 		TypedDocument: TypedDocument{
 			BaseDocument: BaseDocument{
 				ID: internalObj.GetCosmosData().GetCosmosUID(),
 			},
-			PartitionKey: strings.ToLower(internalObj.ID.SubscriptionID),
-			ResourceID:   internalObj.ID,
-			ResourceType: internalObj.ID.ResourceType.String(),
+			PartitionKey: strings.ToLower(cosmosResourceID.SubscriptionID),
+			ResourceID:   cosmosResourceID,
+			ResourceType: cosmosResourceID.ResourceType.String(),
 		},
 		ExternalAuthProperties: ExternalAuthProperties{
-			CosmosMetadata: api.CosmosMetadata{
-				ResourceID: internalObj.ID,
-			},
-			IntermediateResourceDoc: &ResourceDocument{
-				ResourceID:        internalObj.ID,
-				InternalID:        ptr.Deref(internalObj.ServiceProviderProperties.ClusterServiceID, api.InternalID{}),
-				ActiveOperationID: internalObj.ServiceProviderProperties.ActiveOperationID,
-				ProvisioningState: internalObj.Properties.ProvisioningState,
-				Identity:          nil,
-				SystemData:        internalObj.SystemData,
-				Tags:              nil,
-			},
-
-			InternalState: ExternalAuthInternalState{
-				InternalAPI: *internalObj,
-			},
+			HCPOpenShiftClusterExternalAuth: *internalObj,
 		},
 	}
 
-	// some pieces of data in the internalExternalAuth conflict with ResourceDocument fields.  We may evolve over time, but for
-	// now avoid persisting those.
-	cosmosObj.InternalState.InternalAPI.ProxyResource = arm.ProxyResource{}
-	cosmosObj.InternalState.InternalAPI.Properties.ProvisioningState = ""
-	cosmosObj.InternalState.InternalAPI.SystemData = nil
-	// we do this to keep serialization the same so that we can go to n-1 where this field isn't a pointer.
-	// on the reading side, we handle the pointer as expected.
-	cosmosObj.InternalState.InternalAPI.ServiceProviderProperties.ClusterServiceID = &ocm.InternalID{}
-	cosmosObj.InternalState.InternalAPI.ServiceProviderProperties.ActiveOperationID = ""
-
 	return cosmosObj, nil
 }
 
 func CosmosToInternalExternalAuth(cosmosObj *ExternalAuth) (*api.HCPOpenShiftClusterExternalAuth, error) {
 	if cosmosObj == nil {
 		return nil, nil
 	}
-	resourceDoc := cosmosObj.IntermediateResourceDoc
-	if resourceDoc == nil {
-		return nil, fmt.Errorf("resource document cannot be nil")
-	}
-
-	tempInternalAPI := cosmosObj.InternalState.InternalAPI
-	internalObj := &tempInternalAPI
 
-	// some pieces of data are stored on the ResourceDocument, so we need to restore that data
-	internalObj.ProxyResource = arm.ProxyResource{
-		Resource: arm.Resource{
-			ID:         resourceDoc.ResourceID,
-			Name:       resourceDoc.ResourceID.Name,
-			Type:       resourceDoc.ResourceID.ResourceType.String(),
-			SystemData: resourceDoc.SystemData,
-		},
-	}
-	// we carry over the CosmosETag from the cosmos object to the internal object into a
-	// temporary field until we have inlined and serialized CosmosMetadata in
-	// HCPOpenShiftClusterExternalAuth.
-	internalObj.CosmosETag = cosmosObj.CosmosETag
-	internalObj.Properties.ProvisioningState = resourceDoc.ProvisioningState
-	internalObj.SystemData = resourceDoc.SystemData
-	internalObj.ServiceProviderProperties.ExistingCosmosUID = cosmosObj.ID
-	if len(resourceDoc.InternalID.String()) == 0 {
-		// preserve the nil on read
-		internalObj.ServiceProviderProperties.ClusterServiceID = nil
-	} else {
-		internalObj.ServiceProviderProperties.ClusterServiceID = &resourceDoc.InternalID
+	internalObj := cosmosObj.DeepCopy()
+	internalObj.ExistingCosmosUID = cosmosObj.ID
+	internalObj.SetEtag(cosmosObj.CosmosETag)
+	if internalObj.GetResourceID() == nil {
+		if cosmosObj.ResourceID != nil {
+			internalObj.SetResourceID(cosmosObj.ResourceID)
+		} else {
+			return nil, fmt.Errorf("internalObj is missing a resourceID: %T: %q", cosmosObj, cosmosObj.ID)
+		}
 	}
 
-	internalObj.ServiceProviderProperties.ActiveOperationID = resourceDoc.ActiveOperationID
-
 	internalObj.EnsureDefaults()
 
 	return internalObj, nil