From 45a6eab429250b8b527defff1bfbe68335944877 Mon Sep 17 00:00:00 2001 From: cadenmarchese Date: Mon, 29 Apr 2024 12:43:14 -0400 Subject: [PATCH] address comments --- pkg/api/v20191231preview/openshiftcluster_convert.go | 3 +-- pkg/api/v20200430/openshiftcluster_convert.go | 1 - pkg/api/v20210901preview/openshiftcluster_convert.go | 1 - pkg/api/v20220401/openshiftcluster_convert.go | 1 - pkg/api/v20220401/openshiftcluster_validatestatic.go | 4 ++++ pkg/api/v20220904/openshiftcluster_convert.go | 1 - pkg/api/v20230701preview/openshiftcluster_convert.go | 1 - pkg/cluster/install.go | 5 ----- 8 files changed, 5 insertions(+), 12 deletions(-) diff --git a/pkg/api/v20191231preview/openshiftcluster_convert.go b/pkg/api/v20191231preview/openshiftcluster_convert.go index 19ce9d44f50..192f16a115e 100644 --- a/pkg/api/v20191231preview/openshiftcluster_convert.go +++ b/pkg/api/v20191231preview/openshiftcluster_convert.go @@ -46,8 +46,7 @@ func (c openShiftClusterConverter) ToExternal(oc *api.OpenShiftCluster) interfac }, } - if oc.Properties.ServicePrincipalProfile != nil { - out.Properties.ServicePrincipalProfile = &ServicePrincipalProfile{} + if oc.Properties.ServicePrincipalProfile != nil { out.Properties.ServicePrincipalProfile = &ServicePrincipalProfile{ ClientID: oc.Properties.ServicePrincipalProfile.ClientID, ClientSecret: string(oc.Properties.ServicePrincipalProfile.ClientSecret), diff --git a/pkg/api/v20200430/openshiftcluster_convert.go b/pkg/api/v20200430/openshiftcluster_convert.go index 8d82832f682..ef4cfadcc52 100644 --- a/pkg/api/v20200430/openshiftcluster_convert.go +++ b/pkg/api/v20200430/openshiftcluster_convert.go @@ -47,7 +47,6 @@ func (c openShiftClusterConverter) ToExternal(oc *api.OpenShiftCluster) interfac } if oc.Properties.ServicePrincipalProfile != nil { - out.Properties.ServicePrincipalProfile = &ServicePrincipalProfile{} out.Properties.ServicePrincipalProfile = &ServicePrincipalProfile{ ClientID: oc.Properties.ServicePrincipalProfile.ClientID, ClientSecret: string(oc.Properties.ServicePrincipalProfile.ClientSecret), diff --git a/pkg/api/v20210901preview/openshiftcluster_convert.go b/pkg/api/v20210901preview/openshiftcluster_convert.go index 75f9ce135c8..ef1cbae6b16 100644 --- a/pkg/api/v20210901preview/openshiftcluster_convert.go +++ b/pkg/api/v20210901preview/openshiftcluster_convert.go @@ -50,7 +50,6 @@ func (c openShiftClusterConverter) ToExternal(oc *api.OpenShiftCluster) interfac } if oc.Properties.ServicePrincipalProfile != nil { - out.Properties.ServicePrincipalProfile = &ServicePrincipalProfile{} out.Properties.ServicePrincipalProfile = &ServicePrincipalProfile{ ClientID: oc.Properties.ServicePrincipalProfile.ClientID, ClientSecret: string(oc.Properties.ServicePrincipalProfile.ClientSecret), diff --git a/pkg/api/v20220401/openshiftcluster_convert.go b/pkg/api/v20220401/openshiftcluster_convert.go index 8f3c2c8b12c..0ea465c1f49 100644 --- a/pkg/api/v20220401/openshiftcluster_convert.go +++ b/pkg/api/v20220401/openshiftcluster_convert.go @@ -50,7 +50,6 @@ func (c openShiftClusterConverter) ToExternal(oc *api.OpenShiftCluster) interfac } if oc.Properties.ServicePrincipalProfile != nil { - out.Properties.ServicePrincipalProfile = &ServicePrincipalProfile{} out.Properties.ServicePrincipalProfile = &ServicePrincipalProfile{ ClientID: oc.Properties.ServicePrincipalProfile.ClientID, ClientSecret: string(oc.Properties.ServicePrincipalProfile.ClientSecret), diff --git a/pkg/api/v20220401/openshiftcluster_validatestatic.go b/pkg/api/v20220401/openshiftcluster_validatestatic.go index 47c47a39ef3..91b42bc3d4a 100644 --- a/pkg/api/v20220401/openshiftcluster_validatestatic.go +++ b/pkg/api/v20220401/openshiftcluster_validatestatic.go @@ -181,6 +181,10 @@ func (sv openShiftClusterStaticValidator) validateConsoleProfile(path string, cp } func (sv openShiftClusterStaticValidator) validateServicePrincipalProfile(path string, spp *ServicePrincipalProfile) error { + if spp == nil { + return nil + } + valid := uuid.IsValid(spp.ClientID) if !valid { return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, path+".clientId", "The provided client ID '%s' is invalid.", spp.ClientID) diff --git a/pkg/api/v20220904/openshiftcluster_convert.go b/pkg/api/v20220904/openshiftcluster_convert.go index 598debde6f7..60137d5f2e3 100644 --- a/pkg/api/v20220904/openshiftcluster_convert.go +++ b/pkg/api/v20220904/openshiftcluster_convert.go @@ -50,7 +50,6 @@ func (c openShiftClusterConverter) ToExternal(oc *api.OpenShiftCluster) interfac } if oc.Properties.ServicePrincipalProfile != nil { - out.Properties.ServicePrincipalProfile = &ServicePrincipalProfile{} out.Properties.ServicePrincipalProfile = &ServicePrincipalProfile{ ClientID: oc.Properties.ServicePrincipalProfile.ClientID, ClientSecret: string(oc.Properties.ServicePrincipalProfile.ClientSecret), diff --git a/pkg/api/v20230701preview/openshiftcluster_convert.go b/pkg/api/v20230701preview/openshiftcluster_convert.go index d3890c8d272..59ff4b9ddd7 100644 --- a/pkg/api/v20230701preview/openshiftcluster_convert.go +++ b/pkg/api/v20230701preview/openshiftcluster_convert.go @@ -51,7 +51,6 @@ func (c openShiftClusterConverter) ToExternal(oc *api.OpenShiftCluster) interfac } if oc.Properties.ServicePrincipalProfile != nil { - out.Properties.ServicePrincipalProfile = &ServicePrincipalProfile{} out.Properties.ServicePrincipalProfile = &ServicePrincipalProfile{ ClientID: oc.Properties.ServicePrincipalProfile.ClientID, ClientSecret: string(oc.Properties.ServicePrincipalProfile.ClientSecret), diff --git a/pkg/cluster/install.go b/pkg/cluster/install.go index 9987fcf8ec1..b9c34074595 100644 --- a/pkg/cluster/install.go +++ b/pkg/cluster/install.go @@ -187,15 +187,10 @@ func (m *manager) Update(ctx context.Context) error { steps.AuthorizationRetryingAction(m.fpAuthorizer, m.validateResources), steps.Action(m.initializeKubernetesClients), // All init steps are first steps.Action(m.initializeOperatorDeployer), // depends on kube clients - steps.Action(m.initializeClusterSPClients), // Since ServicePrincipalProfile is now a pointer and our converters re-build the struct, // our update path needs to enrich the doc with SPObjectID since it was overwritten by our API on put/patch. steps.AuthorizationRetryingAction(m.fpAuthorizer, m.fixupClusterSPObjectID), - // TODO: this relies on an authorizer that isn't exposed in the manager - // struct, so we'll rebuild the fpAuthorizer and use the error catching - // to advance - steps.AuthorizationRetryingAction(m.fpAuthorizer, m.clusterSPObjectID), // credentials rotation flow steps steps.Action(m.createOrUpdateClusterServicePrincipalRBAC), steps.Action(m.createOrUpdateDenyAssignment),