From e884fd810521ff7e867ebd01682932c803d3bad9 Mon Sep 17 00:00:00 2001 From: Rajdeep Singh Chauhan Date: Thu, 6 Jun 2024 17:32:30 -0400 Subject: [PATCH] ARO-4373 change BoundServiceAccountSigningKey SecureString to SecureBytes --- pkg/api/openshiftcluster.go | 2 +- pkg/cluster/deploybaseresources.go | 2 +- pkg/cluster/deploybaseresources_test.go | 39 +++++++++---------- pkg/frontend/asyncoperationresult_get.go | 2 +- pkg/frontend/openshiftcluster_get.go | 2 +- pkg/frontend/openshiftcluster_list.go | 2 +- pkg/frontend/openshiftcluster_putorpatch.go | 2 +- .../openshiftclustercredentials_post.go | 2 +- ...nshiftclusterkubeconfigcredentials_post.go | 2 +- pkg/util/oidcbuilder/oidcbuilder.go | 4 +- pkg/util/oidcbuilder/oidcbuilder_test.go | 5 ++- 11 files changed, 32 insertions(+), 32 deletions(-) diff --git a/pkg/api/openshiftcluster.go b/pkg/api/openshiftcluster.go index f869541af..14d91aea8 100644 --- a/pkg/api/openshiftcluster.go +++ b/pkg/api/openshiftcluster.go @@ -264,7 +264,7 @@ type ClusterProfile struct { ResourceGroupID string `json:"resourceGroupId,omitempty"` FipsValidatedModules FipsValidatedModules `json:"fipsValidatedModules,omitempty"` OIDCIssuer OIDCIssuer `json:"oidcIssuer,omitempty"` - BoundServiceAccountSigningKey SecureString `json:"boundServiceAccountSigningKey,omitempty"` + BoundServiceAccountSigningKey SecureBytes `json:"boundServiceAccountSigningKey,omitempty"` } // FeatureProfile represents a feature profile. diff --git a/pkg/cluster/deploybaseresources.go b/pkg/cluster/deploybaseresources.go index af1b405f7..8bfb340c5 100644 --- a/pkg/cluster/deploybaseresources.go +++ b/pkg/cluster/deploybaseresources.go @@ -73,7 +73,7 @@ func (m *manager) createOIDC(ctx context.Context) error { m.doc, err = m.db.PatchWithLease(ctx, m.doc.Key, func(doc *api.OpenShiftClusterDocument) error { doc.OpenShiftCluster.Properties.ClusterProfile.OIDCIssuer = api.OIDCIssuer(oidcBuilder.GetEndpointUrl()) - doc.OpenShiftCluster.Properties.ClusterProfile.BoundServiceAccountSigningKey = api.SecureString(oidcBuilder.GetPrivateKey()) + doc.OpenShiftCluster.Properties.ClusterProfile.BoundServiceAccountSigningKey = api.SecureBytes(oidcBuilder.GetPrivateKey()) return nil }) diff --git a/pkg/cluster/deploybaseresources_test.go b/pkg/cluster/deploybaseresources_test.go index 9fe412c15..3b83bd03a 100644 --- a/pkg/cluster/deploybaseresources_test.go +++ b/pkg/cluster/deploybaseresources_test.go @@ -1419,25 +1419,24 @@ func TestCreateOIDC(t *testing.T) { wantErr string wantBoundServiceAccountSigningKey bool }{ - // TODO: Uncomment the test case after testing the PR on all the environments - // { - // name: "Success - Exit createOIDC for non MIWI clusters", - // oc: &api.OpenShiftClusterDocument{ - // Key: strings.ToLower(resourceID), - // ID: resourceID, - // OpenShiftCluster: &api.OpenShiftCluster{ - // Properties: api.OpenShiftClusterProperties{ - // ClusterProfile: api.ClusterProfile{ - // ResourceGroupID: resourceGroup, - // }, - // ServicePrincipalProfile: &api.ServicePrincipalProfile{ - // SPObjectID: fakeClusterSPObjectId, - // }, - // }, - // }, - // }, - // wantBoundServiceAccountSigningKey: false, - // }, + { + name: "Success - Exit createOIDC for non MIWI clusters", + oc: &api.OpenShiftClusterDocument{ + Key: strings.ToLower(resourceID), + ID: resourceID, + OpenShiftCluster: &api.OpenShiftCluster{ + Properties: api.OpenShiftClusterProperties{ + ClusterProfile: api.ClusterProfile{ + ResourceGroupID: resourceGroup, + }, + ServicePrincipalProfile: &api.ServicePrincipalProfile{ + SPObjectID: fakeClusterSPObjectId, + }, + }, + }, + }, + wantBoundServiceAccountSigningKey: false, + }, { name: "Success - Create and persist OIDC Issuer URL", oc: &api.OpenShiftClusterDocument{ @@ -1586,7 +1585,7 @@ func TestCreateOIDC(t *testing.T) { t.Fatalf("OIDC Issuer URL - %s != %s (wanted)", checkDoc.OpenShiftCluster.Properties.ClusterProfile.OIDCIssuer, tt.wantedOIDCIssuer) } - if checkDoc.OpenShiftCluster.Properties.ClusterProfile.BoundServiceAccountSigningKey == "" && tt.wantBoundServiceAccountSigningKey { + if checkDoc.OpenShiftCluster.Properties.ClusterProfile.BoundServiceAccountSigningKey == nil && tt.wantBoundServiceAccountSigningKey { t.Fatalf("Bound Service Account Token is not as expected - wantBoundServiceAccountSigningKey is %t", tt.wantBoundServiceAccountSigningKey) } }) diff --git a/pkg/frontend/asyncoperationresult_get.go b/pkg/frontend/asyncoperationresult_get.go index 2aa2d0589..bd008be49 100644 --- a/pkg/frontend/asyncoperationresult_get.go +++ b/pkg/frontend/asyncoperationresult_get.go @@ -67,7 +67,7 @@ func (f *frontend) _getAsyncOperationResult(ctx context.Context, r *http.Request if asyncdoc.OpenShiftCluster.Properties.ServicePrincipalProfile != nil { asyncdoc.OpenShiftCluster.Properties.ServicePrincipalProfile.ClientSecret = "" } - asyncdoc.OpenShiftCluster.Properties.ClusterProfile.BoundServiceAccountSigningKey = "" + asyncdoc.OpenShiftCluster.Properties.ClusterProfile.BoundServiceAccountSigningKey = nil return json.MarshalIndent(converter.ToExternal(asyncdoc.OpenShiftCluster), "", " ") } diff --git a/pkg/frontend/openshiftcluster_get.go b/pkg/frontend/openshiftcluster_get.go index 5f0b110ef..7ab993c63 100644 --- a/pkg/frontend/openshiftcluster_get.go +++ b/pkg/frontend/openshiftcluster_get.go @@ -48,7 +48,7 @@ func (f *frontend) _getOpenShiftCluster(ctx context.Context, log *logrus.Entry, if doc.OpenShiftCluster.Properties.ServicePrincipalProfile != nil { doc.OpenShiftCluster.Properties.ServicePrincipalProfile.ClientSecret = "" } - doc.OpenShiftCluster.Properties.ClusterProfile.BoundServiceAccountSigningKey = "" + doc.OpenShiftCluster.Properties.ClusterProfile.BoundServiceAccountSigningKey = nil return json.MarshalIndent(converter.ToExternal(doc.OpenShiftCluster), "", " ") } diff --git a/pkg/frontend/openshiftcluster_list.go b/pkg/frontend/openshiftcluster_list.go index 2d2aaa7f3..cbbee1df2 100644 --- a/pkg/frontend/openshiftcluster_list.go +++ b/pkg/frontend/openshiftcluster_list.go @@ -70,7 +70,7 @@ func (f *frontend) _getOpenShiftClusters(ctx context.Context, log *logrus.Entry, if ocs[i].Properties.ServicePrincipalProfile != nil { ocs[i].Properties.ServicePrincipalProfile.ClientSecret = "" } - ocs[i].Properties.ClusterProfile.BoundServiceAccountSigningKey = "" + ocs[i].Properties.ClusterProfile.BoundServiceAccountSigningKey = nil } nextLink, err := f.buildNextLink(r.Header.Get("Referer"), i.Continuation()) diff --git a/pkg/frontend/openshiftcluster_putorpatch.go b/pkg/frontend/openshiftcluster_putorpatch.go index ffe2ea3f9..2baec9bf0 100644 --- a/pkg/frontend/openshiftcluster_putorpatch.go +++ b/pkg/frontend/openshiftcluster_putorpatch.go @@ -257,7 +257,7 @@ func (f *frontend) _putOrPatchOpenShiftCluster(ctx context.Context, log *logrus. if doc.OpenShiftCluster.Properties.ServicePrincipalProfile != nil { doc.OpenShiftCluster.Properties.ServicePrincipalProfile.ClientSecret = "" } - doc.OpenShiftCluster.Properties.ClusterProfile.BoundServiceAccountSigningKey = "" + doc.OpenShiftCluster.Properties.ClusterProfile.BoundServiceAccountSigningKey = nil // We don't return enriched worker profile data on PUT/PATCH operations doc.OpenShiftCluster.Properties.WorkerProfilesStatus = nil diff --git a/pkg/frontend/openshiftclustercredentials_post.go b/pkg/frontend/openshiftclustercredentials_post.go index bc13d571d..ddf8da8b5 100644 --- a/pkg/frontend/openshiftclustercredentials_post.go +++ b/pkg/frontend/openshiftclustercredentials_post.go @@ -70,7 +70,7 @@ func (f *frontend) _postOpenShiftClusterCredentials(ctx context.Context, r *http if doc.OpenShiftCluster.Properties.ServicePrincipalProfile != nil { doc.OpenShiftCluster.Properties.ServicePrincipalProfile.ClientSecret = "" } - doc.OpenShiftCluster.Properties.ClusterProfile.BoundServiceAccountSigningKey = "" + doc.OpenShiftCluster.Properties.ClusterProfile.BoundServiceAccountSigningKey = nil return json.MarshalIndent(converter.ToExternal(doc.OpenShiftCluster), "", " ") } diff --git a/pkg/frontend/openshiftclusterkubeconfigcredentials_post.go b/pkg/frontend/openshiftclusterkubeconfigcredentials_post.go index 0dee92494..067de1a00 100644 --- a/pkg/frontend/openshiftclusterkubeconfigcredentials_post.go +++ b/pkg/frontend/openshiftclusterkubeconfigcredentials_post.go @@ -73,7 +73,7 @@ func (f *frontend) _postOpenShiftClusterKubeConfigCredentials(ctx context.Contex if doc.OpenShiftCluster.Properties.ServicePrincipalProfile != nil { doc.OpenShiftCluster.Properties.ServicePrincipalProfile.ClientSecret = "" } - doc.OpenShiftCluster.Properties.ClusterProfile.BoundServiceAccountSigningKey = "" + doc.OpenShiftCluster.Properties.ClusterProfile.BoundServiceAccountSigningKey = nil return json.MarshalIndent(converter.ToExternal(doc.OpenShiftCluster), "", " ") } diff --git a/pkg/util/oidcbuilder/oidcbuilder.go b/pkg/util/oidcbuilder/oidcbuilder.go index a2935ac6b..ff9ba6f49 100644 --- a/pkg/util/oidcbuilder/oidcbuilder.go +++ b/pkg/util/oidcbuilder/oidcbuilder.go @@ -53,8 +53,8 @@ func (b *OIDCBuilder) GetEndpointUrl() string { return b.endpointURL } -func (b *OIDCBuilder) GetPrivateKey() string { - return string(b.privateKey) +func (b *OIDCBuilder) GetPrivateKey() []byte { + return b.privateKey } func (b *OIDCBuilder) GetBlobContainerURL() string { diff --git a/pkg/util/oidcbuilder/oidcbuilder_test.go b/pkg/util/oidcbuilder/oidcbuilder_test.go index c5127430a..ab7b3aee5 100644 --- a/pkg/util/oidcbuilder/oidcbuilder_test.go +++ b/pkg/util/oidcbuilder/oidcbuilder_test.go @@ -7,6 +7,7 @@ import ( "crypto/x509" "encoding/pem" "errors" + "reflect" "testing" "github.com/golang/mock/gomock" @@ -135,8 +136,8 @@ func TestEnsureOIDCDocs(t *testing.T) { t.Fatalf("GetEndpointUrl doesn't match the original endpointURL - %s != %s (wanted)", tt.oidcbuilder.GetEndpointUrl(), tt.oidcbuilder.endpointURL) } - if tt.oidcbuilder.GetPrivateKey() != string(tt.oidcbuilder.privateKey) { - t.Fatalf("GetPrivateKey doesn't match the original endpointURL - %s != %s (wanted)", tt.oidcbuilder.GetPrivateKey(), string(tt.oidcbuilder.privateKey)) + if !reflect.DeepEqual(tt.oidcbuilder.privateKey, tt.oidcbuilder.GetPrivateKey()) { + t.Fatalf("GetPrivateKey doesn't match the original privateKey") } if tt.oidcbuilder.GetBlobContainerURL() != tt.oidcbuilder.blobContainerURL {