From b15d9b610229d903b42b782cc5014c4f0fab39e0 Mon Sep 17 00:00:00 2001 From: Miguel Soriano Date: Mon, 4 May 2026 17:33:01 +0200 Subject: [PATCH] feat: stop using clusers-service for frontend externalauth reads All customer provided properties are now being stored in Cosmos during external auth creation. For previously existing external auth resources in the RP we ran a a controller that persisted all potentially missing customer properties to Cosmos by retrieving the information from Clusters Service and persisting it in Cosmos. With this, we can now remove all read interaction with Clusters Service from the RP Frontend. This simplifies the read path on Frontend and it will allow us to later fully disconnect the RP Frontend from Clusters Service when we disconnect the write path. --- backend/pkg/app/backend.go | 9 - ...rnal_auth_customer_properties_migration.go | 145 ----------- ...auth_customer_properties_migration_test.go | 233 ------------------ frontend/pkg/frontend/external_auth.go | 107 ++------ frontend/pkg/frontend/routes.go | 1 + internal/ocm/convert.go | 103 -------- 6 files changed, 19 insertions(+), 579 deletions(-) delete mode 100644 backend/pkg/controllers/externalauthpropertiescontroller/external_auth_customer_properties_migration.go delete mode 100644 backend/pkg/controllers/externalauthpropertiescontroller/external_auth_customer_properties_migration_test.go diff --git a/backend/pkg/app/backend.go b/backend/pkg/app/backend.go index db32f83d5db..1bca9feacbb 100644 --- a/backend/pkg/app/backend.go +++ b/backend/pkg/app/backend.go @@ -41,7 +41,6 @@ import ( "github.com/Azure/ARO-HCP/backend/pkg/controllers/clusterpropertiescontroller" "github.com/Azure/ARO-HCP/backend/pkg/controllers/controllerutils" "github.com/Azure/ARO-HCP/backend/pkg/controllers/datadumpcontrollers" - "github.com/Azure/ARO-HCP/backend/pkg/controllers/externalauthpropertiescontroller" "github.com/Azure/ARO-HCP/backend/pkg/controllers/metricscontrollers" "github.com/Azure/ARO-HCP/backend/pkg/controllers/mismatchcontrollers" "github.com/Azure/ARO-HCP/backend/pkg/controllers/nodepoolpropertiescontroller" @@ -521,13 +520,6 @@ func (b *Backend) runBackendControllersUnderLeaderElection(ctx context.Context, backendInformers, ) - externalAuthCustomerPropertiesMigrationController := externalauthpropertiescontroller.NewExternalAuthCustomerPropertiesMigrationController( - b.options.CosmosDBClient, - b.options.ClustersServiceClient, - activeOperationLister, - backendInformers, - ) - le, err := leaderelection.NewLeaderElector(leaderelection.LeaderElectionConfig{ Lock: b.options.LeaderElectionLock, LeaseDuration: leaderElectionLeaseDuration, @@ -583,7 +575,6 @@ func (b *Backend) runBackendControllersUnderLeaderElection(ctx context.Context, go triggerNodePoolUpgradeController.Run(ctx, 20) go nodePoolPropertiesSyncController.Run(ctx, 20) go nodePoolCustomerPropertiesMigrationController.Run(ctx, 20) - go externalAuthCustomerPropertiesMigrationController.Run(ctx, 20) go operationPhaseMetricsController.Run(ctx, 1) go clusterMetricsController.Run(ctx, 1) go nodePoolMetricsController.Run(ctx, 1) diff --git a/backend/pkg/controllers/externalauthpropertiescontroller/external_auth_customer_properties_migration.go b/backend/pkg/controllers/externalauthpropertiescontroller/external_auth_customer_properties_migration.go deleted file mode 100644 index d71eb652011..00000000000 --- a/backend/pkg/controllers/externalauthpropertiescontroller/external_auth_customer_properties_migration.go +++ /dev/null @@ -1,145 +0,0 @@ -package externalauthpropertiescontroller - -// Copyright 2026 Microsoft Corporation -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -import ( - "context" - "fmt" - "time" - - "github.com/Azure/ARO-HCP/backend/pkg/controllers/controllerutils" - "github.com/Azure/ARO-HCP/backend/pkg/informers" - "github.com/Azure/ARO-HCP/backend/pkg/listers" - "github.com/Azure/ARO-HCP/internal/api" - "github.com/Azure/ARO-HCP/internal/database" - "github.com/Azure/ARO-HCP/internal/ocm" - "github.com/Azure/ARO-HCP/internal/utils" -) - -// externalAuthCustomerPropertiesMigrationController is a ExternalAuth controller that migrates properties (customer properties) -// from cluster-service to cosmos DB. It uses the .platform.vmSize attribute to know that customerProperties are missing. -// Old records will lack those fields and once we read from cluster-service, we'll have the information we need. -type externalAuthCustomerPropertiesMigrationController struct { - cooldownChecker controllerutils.CooldownChecker - - externalAuthLister listers.ExternalAuthLister - cosmosClient database.DBClient - clusterServiceClient ocm.ClusterServiceClientSpec -} - -var _ controllerutils.ExternalAuthSyncer = (*externalAuthCustomerPropertiesMigrationController)(nil) - -func NewExternalAuthCustomerPropertiesMigrationController( - cosmosClient database.DBClient, - clusterServiceClient ocm.ClusterServiceClientSpec, - activeOperationLister listers.ActiveOperationLister, - informers informers.BackendInformers, -) controllerutils.Controller { - _, externalAuthLister := informers.ExternalAuths() - - syncer := &externalAuthCustomerPropertiesMigrationController{ - cooldownChecker: controllerutils.DefaultActiveOperationPrioritizingCooldown(activeOperationLister), - externalAuthLister: externalAuthLister, - cosmosClient: cosmosClient, - clusterServiceClient: clusterServiceClient, - } - - controller := controllerutils.NewExternalAuthWatchingController( - "ExternalAuthCustomerPropertiesMigration", - cosmosClient, - informers, - 60*time.Minute, // Check every 60 minutes - syncer, - ) - - return controller -} - -func (c *externalAuthCustomerPropertiesMigrationController) CooldownChecker() controllerutils.CooldownChecker { - return c.cooldownChecker -} - -func (c *externalAuthCustomerPropertiesMigrationController) NeedsWork(ctx context.Context, existingExternalAuth *api.HCPOpenShiftClusterExternalAuth) bool { - // Check if we have a Clusters Service's ExternalAuth service ID to query. We will lack this information for newly created records when we - // transition to async Clusters Service's ExternalAuth creation. - if len(existingExternalAuth.ServiceProviderProperties.ClusterServiceID.String()) == 0 { - return false - } - - // We use .properties.issuer.url as the marker to know if customer properties - // need to be migrated for the ExternalAuth being processed. - // .properties.issuer.url is a required attribute at ARM API level, so its - // absence in Cosmos signals that the customer properties of the ExternalAuth are not - // migrated into Cosmos yet and we need to migrate them. - needsIssuer := len(existingExternalAuth.Properties.Issuer.URL) == 0 - return needsIssuer -} - -func (c *externalAuthCustomerPropertiesMigrationController) SyncOnce(ctx context.Context, key controllerutils.HCPExternalAuthKey) error { - logger := utils.LoggerFromContext(ctx) - - // do the super cheap cache check first - cachedExternalAuth, err := c.externalAuthLister.Get(ctx, key.SubscriptionID, key.ResourceGroupName, key.HCPClusterName, key.HCPExternalAuthName) - if database.IsNotFoundError(err) { - // we'll be re-fired if it is created again - return nil - } - if err != nil { - return utils.TrackError(fmt.Errorf("failed to get externalAuth from cache: %w", err)) - } - if !c.NeedsWork(ctx, cachedExternalAuth) { - // if the cache doesn't need work, then we'll be retriggered if those values change when the cache updates. - // if the values don't change, then we still have no work to do. - return nil - } - - // Get the externalAuth from Cosmos - externalAuthCRUD := c.cosmosClient.HCPClusters(key.SubscriptionID, key.ResourceGroupName).ExternalAuth(key.HCPClusterName) - existingExternalAuth, err := externalAuthCRUD.Get(ctx, key.HCPExternalAuthName) - if database.IsNotFoundError(err) { - return nil // externalAuth doesn't exist, no work to do - } - if err != nil { - return utils.TrackError(fmt.Errorf("failed to get externalAuth: %w", err)) - } - // check if we need to do work again. Sometimes the live data is more fresh than the cache and obviates the need to any work - if !c.NeedsWork(ctx, existingExternalAuth) { - return nil - } - - // Fetch the ExternalAuth from Cluster Service - csExternalAuth, err := c.clusterServiceClient.GetExternalAuth(ctx, existingExternalAuth.ServiceProviderProperties.ClusterServiceID) - if err != nil { - return utils.TrackError(fmt.Errorf("failed to get externalAuth from Cluster Service: %w", err)) - } - - // Use ConvertCStoExternalAuth to convert the externalAuth and extract the Properties (customer properties) - convertedExternalAuth, err := ocm.ConvertCStoExternalAuth(existingExternalAuth.ID, csExternalAuth) - if err != nil { - return utils.TrackError(fmt.Errorf("failed to convert externalAuth from Cluster Service: %w", err)) - } - - // Update only the Properties from the converted externalAuth - existingExternalAuth.Properties = convertedExternalAuth.Properties - - // Write the updated externalAuth back to Cosmos - if _, err := externalAuthCRUD.Replace(ctx, existingExternalAuth, nil); err != nil { - return utils.TrackError(fmt.Errorf("failed to replace externalAuth: %w", err)) - } - - logger.Info("migrated externalAuth properties from Cluster Service to Cosmos") - - return nil -} diff --git a/backend/pkg/controllers/externalauthpropertiescontroller/external_auth_customer_properties_migration_test.go b/backend/pkg/controllers/externalauthpropertiescontroller/external_auth_customer_properties_migration_test.go deleted file mode 100644 index 89e4549f4a5..00000000000 --- a/backend/pkg/controllers/externalauthpropertiescontroller/external_auth_customer_properties_migration_test.go +++ /dev/null @@ -1,233 +0,0 @@ -// Copyright 2026 Microsoft Corporation -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package externalauthpropertiescontroller - -import ( - "context" - "fmt" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "go.uber.org/mock/gomock" - - azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" - - arohcpv1alpha1 "github.com/openshift-online/ocm-sdk-go/arohcp/v1alpha1" - - "github.com/Azure/ARO-HCP/backend/pkg/controllers/controllerutils" - "github.com/Azure/ARO-HCP/backend/pkg/listertesting" - "github.com/Azure/ARO-HCP/internal/api" - "github.com/Azure/ARO-HCP/internal/api/arm" - "github.com/Azure/ARO-HCP/internal/databasetesting" - "github.com/Azure/ARO-HCP/internal/ocm" -) - -const ( - testSubscriptionID = "00000000-0000-0000-0000-000000000000" - testResourceGroupName = "test-rg" - testClusterName = "test-cluster" - testExternalAuthName = "test-external-auth" - testClusterServiceIDStr = "/api/aro_hcp/v1alpha1/clusters/abc123" - testExternalAuthCSIDStr = testClusterServiceIDStr + "/external_auth_config/external_auths/ea123" - testMigrationIssuerURL = "https://login.example.com/tenant/v2.0" -) - -func TestExternalAuthCustomerPropertiesMigrationController_SyncOnce(t *testing.T) { - testCases := []struct { - name string - cachedExternalAuth *api.HCPOpenShiftClusterExternalAuth // nil means use same as existingCosmosExternalAuth - existingCosmosExternalAuth *api.HCPOpenShiftClusterExternalAuth - csExternalAuth *arohcpv1alpha1.ExternalAuth - csError error - expectCSCall bool - expectError bool - expectedIssuerURL string - }{ - { - name: "cache indicates no work needed - early return without cosmos lookup", - cachedExternalAuth: newTestExternalAuthForMigration(func(ea *api.HCPOpenShiftClusterExternalAuth) { - ea.Properties.Issuer.URL = testMigrationIssuerURL - }), - existingCosmosExternalAuth: newTestExternalAuthForMigration(func(ea *api.HCPOpenShiftClusterExternalAuth) { - ea.Properties.Issuer.URL = testMigrationIssuerURL - }), - expectCSCall: false, - expectError: false, - expectedIssuerURL: testMigrationIssuerURL, - }, - { - name: "cache says work needed but live data says no work needed", - cachedExternalAuth: newTestExternalAuthForMigration(func(ea *api.HCPOpenShiftClusterExternalAuth) {}), - existingCosmosExternalAuth: newTestExternalAuthForMigration(func(ea *api.HCPOpenShiftClusterExternalAuth) { - ea.Properties.Issuer.URL = testMigrationIssuerURL - }), - expectCSCall: false, - expectError: false, - expectedIssuerURL: testMigrationIssuerURL, - }, - { - name: "error reading from cluster-service", - existingCosmosExternalAuth: newTestExternalAuthForMigration(func(ea *api.HCPOpenShiftClusterExternalAuth) {}), - csError: fmt.Errorf("connection refused"), - expectCSCall: true, - expectError: true, - expectedIssuerURL: "", - }, - { - name: "success - migrate issuer URL when missing", - existingCosmosExternalAuth: newTestExternalAuthForMigration(func(ea *api.HCPOpenShiftClusterExternalAuth) {}), - csExternalAuth: newTestFullCSExternalAuth(), - expectCSCall: true, - expectError: false, - expectedIssuerURL: testMigrationIssuerURL, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - ctx := context.Background() - ctrl := gomock.NewController(t) - - mockDB := databasetesting.NewMockDBClient() - - externalAuthCRUD := mockDB.HCPClusters(testSubscriptionID, testResourceGroupName).ExternalAuth(testClusterName) - _, err := externalAuthCRUD.Create(ctx, tc.existingCosmosExternalAuth, nil) - require.NoError(t, err) - - cachedExternalAuth := tc.cachedExternalAuth - if cachedExternalAuth == nil { - cachedExternalAuth = tc.existingCosmosExternalAuth - } - sliceExternalAuthLister := &listertesting.SliceExternalAuthLister{ - ExternalAuths: []*api.HCPOpenShiftClusterExternalAuth{cachedExternalAuth}, - } - - mockCSClient := ocm.NewMockClusterServiceClientSpec(ctrl) - - if tc.expectCSCall { - mockCSClient.EXPECT(). - GetExternalAuth(gomock.Any(), api.Must(api.NewInternalID(testExternalAuthCSIDStr))). - Return(tc.csExternalAuth, tc.csError) - } - - syncer := &externalAuthCustomerPropertiesMigrationController{ - cooldownChecker: &alwaysSyncCooldownChecker{}, - externalAuthLister: sliceExternalAuthLister, - cosmosClient: mockDB, - clusterServiceClient: mockCSClient, - } - - key := controllerutils.HCPExternalAuthKey{ - SubscriptionID: testSubscriptionID, - ResourceGroupName: testResourceGroupName, - HCPClusterName: testClusterName, - HCPExternalAuthName: testExternalAuthName, - } - err = syncer.SyncOnce(ctx, key) - - if tc.expectError { - require.Error(t, err) - } else { - require.NoError(t, err) - } - - updatedExternalAuth, err := externalAuthCRUD.Get(ctx, testExternalAuthName) - require.NoError(t, err) - assert.Equal(t, tc.expectedIssuerURL, updatedExternalAuth.Properties.Issuer.URL) - }) - } -} - -type alwaysSyncCooldownChecker struct{} - -func (c *alwaysSyncCooldownChecker) CanSync(ctx context.Context, key any) bool { - return true -} - -func newTestExternalAuthForMigration(opts func(*api.HCPOpenShiftClusterExternalAuth)) *api.HCPOpenShiftClusterExternalAuth { - ea := newTestExternalAuthWithClusterServiceID() - ea.Properties = api.HCPOpenShiftClusterExternalAuthProperties{} - if opts != nil { - opts(ea) - } - return ea -} - -func newTestExternalAuthWithClusterServiceID() *api.HCPOpenShiftClusterExternalAuth { - resourceID := api.Must(azcorearm.ParseResourceID( - "/subscriptions/" + testSubscriptionID + - "/resourceGroups/" + testResourceGroupName + - "/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/" + testClusterName + - "/externalAuths/" + testExternalAuthName)) - csID := api.Must(api.NewInternalID(testExternalAuthCSIDStr)) - return &api.HCPOpenShiftClusterExternalAuth{ - ProxyResource: arm.NewProxyResource(resourceID), - ServiceProviderProperties: api.HCPOpenShiftClusterExternalAuthServiceProviderProperties{ - ClusterServiceID: csID, - }, - } -} - -func newTestFullCSExternalAuth() *arohcpv1alpha1.ExternalAuth { - externalAuth, err := arohcpv1alpha1.NewExternalAuth(). - ID("ea123"). - HREF(testExternalAuthCSIDStr). - Issuer(arohcpv1alpha1.NewTokenIssuer(). - URL(testMigrationIssuerURL). - CA("testCAPem"). - Audiences( - "87654321-4321-4321-4321-abcdefghijkl", - "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", - )). - Claim(arohcpv1alpha1.NewExternalAuthClaim(). - Mappings(arohcpv1alpha1.NewTokenClaimMappings(). - UserName(arohcpv1alpha1.NewUsernameClaim(). - Claim("sub"). - Prefix("prefix-"). - PrefixPolicy("Prefix")). - Groups(arohcpv1alpha1.NewGroupsClaim(). - Claim("groups"). - Prefix("grp-"))). - ValidationRules( - arohcpv1alpha1.NewTokenClaimValidationRule(). - Claim("tid"). - RequiredValue("expected-tenant"), - arohcpv1alpha1.NewTokenClaimValidationRule(). - Claim("scp"). - RequiredValue("api.read"), - )). - Clients( - arohcpv1alpha1.NewExternalAuthClientConfig(). - ID("11111111-1111-1111-1111-111111111111"). - Component(arohcpv1alpha1.NewClientComponent(). - Name("console"). - Namespace("openshift-console")). - ExtraScopes("openid", "profile"). - Type(arohcpv1alpha1.ExternalAuthClientTypeConfidential), - arohcpv1alpha1.NewExternalAuthClientConfig(). - ID("22222222-2222-2222-2222-222222222222"). - Component(arohcpv1alpha1.NewClientComponent(). - Name("cli"). - Namespace("openshift-console")). - ExtraScopes("offline_access"). - Type(arohcpv1alpha1.ExternalAuthClientTypePublic), - ). - Build() - if err != nil { - panic(err) - } - return externalAuth -} diff --git a/frontend/pkg/frontend/external_auth.go b/frontend/pkg/frontend/external_auth.go index 9ede4a17667..54c8e54ed54 100644 --- a/frontend/pkg/frontend/external_auth.go +++ b/frontend/pkg/frontend/external_auth.go @@ -28,7 +28,6 @@ import ( azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" "github.com/Azure/azure-sdk-for-go/sdk/data/azcosmos" - arohcpv1alpha1 "github.com/openshift-online/ocm-sdk-go/arohcp/v1alpha1" ocmerrors "github.com/openshift-online/ocm-sdk-go/errors" "github.com/Azure/ARO-HCP/internal/api" @@ -71,7 +70,6 @@ func (f *Frontend) GetExternalAuth(writer http.ResponseWriter, request *http.Req func (f *Frontend) ArmResourceListExternalAuths(writer http.ResponseWriter, request *http.Request) error { ctx := request.Context() - logger := utils.LoggerFromContext(ctx) versionedInterface, err := VersionFromContext(ctx) if err != nil { @@ -82,23 +80,25 @@ func (f *Frontend) ArmResourceListExternalAuths(writer http.ResponseWriter, requ resourceGroupName := request.PathValue(PathSegmentResourceGroupName) resourceName := request.PathValue(PathSegmentResourceName) - internalCluster, err := f.dbClient.HCPClusters(subscriptionID, resourceGroupName).Get(ctx, resourceName) + // Verify the parent cluster exists so we return 404 instead of an empty list for a non-existent cluster (Cosmos List is prefix-based) + _, err = f.dbClient.HCPClusters(subscriptionID, resourceGroupName).Get(ctx, resourceName) if err != nil { return utils.TrackError(err) } - if internalCluster.ServiceProviderProperties.ClusterServiceID == nil { - return utils.TrackError(fmt.Errorf("cluster %s has no ClusterServiceID", internalCluster.ID)) - } pagedResponse := arm.NewPagedResponse() - externalAuthsByClusterServiceID := make(map[string]*api.HCPOpenShiftClusterExternalAuth) internalExternalAuthIterator, err := f.dbClient.HCPClusters(subscriptionID, resourceGroupName).ExternalAuth(resourceName).List(ctx, dbListOptionsFromRequest(request)) if err != nil { return utils.TrackError(err) } for _, externalAuth := range internalExternalAuthIterator.Items(ctx) { - externalAuthsByClusterServiceID[externalAuth.ServiceProviderProperties.ClusterServiceID.ID()] = externalAuth + resultingExternalExternalAuth := versionedInterface.NewHCPOpenShiftClusterExternalAuth(externalAuth) + jsonBytes, err := arm.MarshalJSON(resultingExternalExternalAuth) + if err != nil { + return utils.TrackError(err) + } + pagedResponse.AddValue(jsonBytes) } err = internalExternalAuthIterator.GetError() if err != nil { @@ -111,35 +111,6 @@ func (f *Frontend) ArmResourceListExternalAuths(writer http.ResponseWriter, requ return utils.TrackError(err) } - // Build a Cluster Service query that looks for - // the specific IDs returned by the Cosmos query. - queryIDs := make([]string, 0, len(externalAuthsByClusterServiceID)) - for key := range externalAuthsByClusterServiceID { - queryIDs = append(queryIDs, "'"+key+"'") - } - query := fmt.Sprintf("id in (%s)", strings.Join(queryIDs, ", ")) - logger.Info(fmt.Sprintf("Searching Cluster Service for %q", query)) - - csIterator := f.clusterServiceClient.ListExternalAuths(*internalCluster.ServiceProviderProperties.ClusterServiceID, query) - for csExternalAuth := range csIterator.Items(ctx) { - if internalExternalAuth, ok := externalAuthsByClusterServiceID[csExternalAuth.ID()]; ok { - internalExternalAuth, err = mergeToInternalExternalAuth(csExternalAuth, internalExternalAuth) - if err != nil { - return utils.TrackError(err) - } - resultingExternalExternalAuth := versionedInterface.NewHCPOpenShiftClusterExternalAuth(internalExternalAuth) - jsonBytes, err := arm.MarshalJSON(resultingExternalExternalAuth) - if err != nil { - return utils.TrackError(err) - } - pagedResponse.AddValue(jsonBytes) - } - } - err = csIterator.GetError() - if err != nil { - return utils.TrackError(err) - } - _, err = arm.WriteJSONResponse(writer, http.StatusOK, pagedResponse) if err != nil { return utils.TrackError(err) @@ -177,15 +148,9 @@ func (f *Frontend) CreateOrUpdateExternalAuth(writer http.ResponseWriter, reques updating := oldInternalExternalAuth != nil if updating { - // re-write oldInternalCluster for as long as cluster-service needs to be consulted for pre-existing state. - oldInternalExternalAuth, err = f.readInternalExternalAuthFromClusterService(ctx, oldInternalExternalAuth) - if err != nil { - return utils.TrackError(err) - } if err := checkForProvisioningStateConflict(ctx, f.dbClient, database.OperationRequestUpdate, oldInternalExternalAuth.ID, oldInternalExternalAuth.Properties.ProvisioningState); err != nil { return utils.TrackError(err) } - switch request.Method { case http.MethodPut: return f.updateExternalAuth(writer, request, oldInternalExternalAuth) @@ -351,11 +316,6 @@ func (f *Frontend) createExternalAuth(writer http.ResponseWriter, request *http. if !ok { return fmt.Errorf("unexpected type %T", resultingUncastInternalExternalAuth) } - // TODO this overwrite will transformed into a "set" function as we transition fields to ownership in cosmos - resultingInternalExternalAuth, err = mergeToInternalExternalAuth(csExternalAuth, resultingInternalExternalAuth) - if err != nil { - return utils.TrackError(err) - } responseBytes, err := arm.MarshalJSON(versionedInterface.NewHCPOpenShiftClusterExternalAuth(resultingInternalExternalAuth)) if err != nil { return utils.TrackError(err) @@ -420,6 +380,10 @@ func decodeDesiredExternalAuthReplace(ctx context.Context, oldInternalExternalAu conversion.CopyReadOnlyExternalAuthValues(newInternalExternalAuth, oldInternalExternalAuth) newInternalExternalAuth.SystemData = ensureSystemData(systemData, oldInternalExternalAuth.SystemData) + // Backstop for fields unknown to this API version's SetDefaultValues*. + // See docs/api-version-defaults-and-storage.md. + newInternalExternalAuth.EnsureDefaults() + return newInternalExternalAuth, nil } @@ -469,6 +433,10 @@ func decodeDesiredExternalAuthPatch(ctx context.Context, oldInternalExternalAuth conversion.CopyReadOnlyExternalAuthValues(newInternalExternalAuth, oldInternalExternalAuth) newInternalExternalAuth.SystemData = ensureSystemData(systemData, oldInternalExternalAuth.SystemData) + // Backstop for fields unknown to this API version's SetDefaultValues*. + // See docs/api-version-defaults-and-storage.md. + newInternalExternalAuth.EnsureDefaults() + return newInternalExternalAuth, nil } @@ -508,7 +476,7 @@ func (f *Frontend) updateExternalAuthInCosmos(ctx context.Context, writer http.R } logger.Info(fmt.Sprintf("updating resource %s", oldInternalExternalAuth.ID)) - csExternalAuth, err := f.clusterServiceClient.UpdateExternalAuth(ctx, oldInternalExternalAuth.ServiceProviderProperties.ClusterServiceID, csExternalAuthBuilder) + _, err = f.clusterServiceClient.UpdateExternalAuth(ctx, oldInternalExternalAuth.ServiceProviderProperties.ClusterServiceID, csExternalAuthBuilder) if err != nil { return utils.TrackError(err) } @@ -558,11 +526,6 @@ func (f *Frontend) updateExternalAuthInCosmos(ctx context.Context, writer http.R if !ok { return fmt.Errorf("unexpected type %T", resultingUncastInternalExternalAuth) } - // TODO this overwrite will transformed into a "set" function as we transition fields to ownership in cosmos - resultingInternalExternalAuth, err = mergeToInternalExternalAuth(csExternalAuth, resultingInternalExternalAuth) - if err != nil { - return utils.TrackError(err) - } responseBytes, err := arm.MarshalJSON(versionedInterface.NewHCPOpenShiftClusterExternalAuth(resultingInternalExternalAuth)) if err != nil { return utils.TrackError(err) @@ -685,22 +648,6 @@ func (f *Frontend) addDeleteExternalAuthToTransaction(ctx context.Context, write return nil } -// the necessary conversions for the API version of the request. -// TODO this overwrite will transformed into a "set" function as we transition fields to ownership in cosmos -func mergeToInternalExternalAuth(csEternalAuth *arohcpv1alpha1.ExternalAuth, internalObj *api.HCPOpenShiftClusterExternalAuth) (*api.HCPOpenShiftClusterExternalAuth, error) { - mergedExternalAuth, err := ocm.ConvertCStoExternalAuth(internalObj.ID, csEternalAuth) - if err != nil { - return nil, utils.TrackError(err) - } - - // this does not use conversion.CopyReadOnly* because some ServiceProvider properties come from cluster-service-only or live reads - mergedExternalAuth.SystemData = internalObj.SystemData.DeepCopy() - mergedExternalAuth.Properties.ProvisioningState = internalObj.Properties.ProvisioningState - mergedExternalAuth.ServiceProviderProperties = *internalObj.ServiceProviderProperties.DeepCopy() - - return mergedExternalAuth, nil -} - func (f *Frontend) getInternalExternalAuthFromStorage(ctx context.Context, resourceID *azcorearm.ResourceID) (*api.HCPOpenShiftClusterExternalAuth, error) { internalExternalAuth, err := f.dbClient.HCPClusters(resourceID.SubscriptionID, resourceID.ResourceGroupName).ExternalAuth(resourceID.Parent.Name).Get(ctx, resourceID.Name) if database.IsNotFoundError(err) { @@ -729,23 +676,5 @@ func (f *Frontend) getInternalExternalAuthFromStorage(ctx context.Context, resou } internalExternalAuth.ID = resourceID - return f.readInternalExternalAuthFromClusterService(ctx, internalExternalAuth) - -} - -// readInternalExternalAuthFromClusterService takes an internal ExternalAuth read from cosmos, retrieves the corresponding cluster-service data, -// merges the states together, and returns the internal representation. -func (f *Frontend) readInternalExternalAuthFromClusterService(ctx context.Context, oldInternalExternalAuth *api.HCPOpenShiftClusterExternalAuth) (*api.HCPOpenShiftClusterExternalAuth, error) { - oldClusterServiceExternalAuth, err := f.clusterServiceClient.GetExternalAuth(ctx, oldInternalExternalAuth.ServiceProviderProperties.ClusterServiceID) - if err != nil { - return nil, utils.TrackError(err) - } - - // TODO this overwrite will transformed into a "set" function as we transition fields to ownership in cosmos - oldInternalExternalAuth, err = mergeToInternalExternalAuth(oldClusterServiceExternalAuth, oldInternalExternalAuth) - if err != nil { - return nil, utils.TrackError(err) - } - - return oldInternalExternalAuth, nil + return internalExternalAuth, nil } diff --git a/frontend/pkg/frontend/routes.go b/frontend/pkg/frontend/routes.go index 0a345736410..e05254b5c8f 100644 --- a/frontend/pkg/frontend/routes.go +++ b/frontend/pkg/frontend/routes.go @@ -85,6 +85,7 @@ func (f *Frontend) routes(r prometheus.Registerer) http.Handler { // Resource list endpoints postMuxMiddleware := NewMiddleware( + MiddlewareResourceID, MiddlewareLoggingPostMux, newMiddlewareValidatedAPIVersion(f.apiRegistry).handleRequest, newMiddlewareValidateSubscriptionState(f.dbClient).handleRequest) diff --git a/internal/ocm/convert.go b/internal/ocm/convert.go index e91168ac5af..7bc641ca1f0 100644 --- a/internal/ocm/convert.go +++ b/internal/ocm/convert.go @@ -235,19 +235,6 @@ func convertCustomerManagedEncryptionTypeRPToCS(encryptionTypeRP api.CustomerMan } } -func convertUsernameClaimPrefixPolicyCSToRP(prefixPolicyCS string) (api.UsernameClaimPrefixPolicy, error) { - switch prefixPolicyCS { - case csUsernameClaimPrefixPolicyPrefix: - return api.UsernameClaimPrefixPolicyPrefix, nil - case csUsernameClaimPrefixPolicyNoPrefix: - return api.UsernameClaimPrefixPolicyNoPrefix, nil - case "": - return api.UsernameClaimPrefixPolicyNone, nil - default: - return "", conversionError[api.UsernameClaimPrefixPolicy](prefixPolicyCS) - } -} - func convertUsernameClaimPrefixPolicyRPToCS(prefixPolicyRP api.UsernameClaimPrefixPolicy) (string, error) { switch prefixPolicyRP { case api.UsernameClaimPrefixPolicyPrefix: @@ -343,17 +330,6 @@ func convertKeyManagementModeTypeRPToCS(keyManagementModeRP api.EtcdDataEncrypti } } -func convertExternalAuthClientTypeCSToRP(externalAuthClientTypeCS arohcpv1alpha1.ExternalAuthClientType) (api.ExternalAuthClientType, error) { - switch externalAuthClientTypeCS { - case arohcpv1alpha1.ExternalAuthClientTypeConfidential: - return api.ExternalAuthClientTypeConfidential, nil - case arohcpv1alpha1.ExternalAuthClientTypePublic: - return api.ExternalAuthClientTypePublic, nil - default: - return "", conversionError[api.ExternalAuthClientType](externalAuthClientTypeCS) - } -} - func convertExternalAuthClientTypeRPToCS(externalAuthClientTypeRP api.ExternalAuthClientType) (arohcpv1alpha1.ExternalAuthClientType, error) { switch externalAuthClientTypeRP { case api.ExternalAuthClientTypeConfidential: @@ -1113,85 +1089,6 @@ func BuildCSNodePool(ctx context.Context, nodePool *api.HCPOpenShiftClusterNodeP return nodePoolBuilder, nil } -// ConvertCStoExternalAuth converts a CS ExternalAuth object into HCPOpenShiftClusterExternalAuth object. -func ConvertCStoExternalAuth(resourceID *azcorearm.ResourceID, csExternalAuth *arohcpv1alpha1.ExternalAuth) (*api.HCPOpenShiftClusterExternalAuth, error) { - usernameClaimPrefixPolicy, err := convertUsernameClaimPrefixPolicyCSToRP(csExternalAuth.Claim().Mappings().UserName().PrefixPolicy()) - if err != nil { - return nil, err - } - - externalAuth := &api.HCPOpenShiftClusterExternalAuth{ - ProxyResource: arm.ProxyResource{ - Resource: arm.Resource{ - ID: resourceID, - Name: resourceID.Name, - Type: resourceID.ResourceType.String(), - }, - }, - Properties: api.HCPOpenShiftClusterExternalAuthProperties{ - // TODO fill these out later when CS supports Conditions fully - // Condition: api.ExternalAuthCondition{}, - Issuer: api.TokenIssuerProfile{ - URL: csExternalAuth.Issuer().URL(), - CA: csExternalAuth.Issuer().CA(), - Audiences: csExternalAuth.Issuer().Audiences(), - }, - Claim: api.ExternalAuthClaimProfile{ - Mappings: api.TokenClaimMappingsProfile{ - Username: api.UsernameClaimProfile{ - Claim: csExternalAuth.Claim().Mappings().UserName().Claim(), - Prefix: csExternalAuth.Claim().Mappings().UserName().Prefix(), - PrefixPolicy: usernameClaimPrefixPolicy, - }, - }, - }, - }, - } - - if groups, ok := csExternalAuth.Claim().Mappings().GetGroups(); ok { - externalAuth.Properties.Claim.Mappings.Groups = &api.GroupClaimProfile{ - Claim: groups.Claim(), - Prefix: groups.Prefix(), - } - } - - clients := make([]api.ExternalAuthClientProfile, 0, len(csExternalAuth.Clients())) - for _, client := range csExternalAuth.Clients() { - clientType, err := convertExternalAuthClientTypeCSToRP(client.Type()) - if err != nil { - return nil, err - } - - clients = append(clients, api.ExternalAuthClientProfile{ - Component: api.ExternalAuthClientComponentProfile{ - Name: client.Component().Name(), - AuthClientNamespace: client.Component().Namespace(), - }, - ClientID: client.ID(), - ExtraScopes: client.ExtraScopes(), - Type: clientType, - }) - } - externalAuth.Properties.Clients = clients - - validationRules := make([]api.TokenClaimValidationRule, 0, len(csExternalAuth.Claim().ValidationRules())) - if csExternalAuth.Claim().ValidationRules() != nil { - for _, validationRule := range csExternalAuth.Claim().ValidationRules() { - validationRules = append(validationRules, api.TokenClaimValidationRule{ - // We hard code the type here because CS only supports this type currently and doesn't reference the type. - Type: api.TokenValidationRuleTypeRequiredClaim, - RequiredClaim: api.TokenRequiredClaim{ - Claim: validationRule.Claim(), - RequiredValue: validationRule.RequiredValue(), - }, - }) - } - } - externalAuth.Properties.Claim.ValidationRules = validationRules - - return externalAuth, nil -} - // BuildCSExternalAuth creates a CS ExternalAuthBuilder object from an HCPOpenShiftClusterExternalAuth object. func BuildCSExternalAuth(ctx context.Context, externalAuth *api.HCPOpenShiftClusterExternalAuth, updating bool) (*arohcpv1alpha1.ExternalAuthBuilder, error) { externalAuthBuilder := arohcpv1alpha1.NewExternalAuth()