From 4d0855ace493d7f60b760535f2059b290416e79e Mon Sep 17 00:00:00 2001 From: Sergey Smolnikov Date: Wed, 19 Feb 2025 10:22:42 +0100 Subject: [PATCH] Replaced property useAzureIdentity with Credentials enum to be expandable --- .../templates/radixapplication.yaml | 28 ++++++++++++------- .../templates/radixdeployment.yaml | 14 ++++++---- json-schema/radixapplication.json | 26 +++++++++++------ .../oauthproxyresourcemanager_test.go | 4 +-- pkg/apis/deployment/secretrefs_test.go | 6 ++-- pkg/apis/radix/v1/radixapptypes.go | 18 ++++++++++-- pkg/apis/radix/v1/zz_generated.deepcopy.go | 5 ---- pkg/apis/utils/labels/labels_test.go | 13 ++++----- 8 files changed, 71 insertions(+), 43 deletions(-) diff --git a/charts/radix-operator/templates/radixapplication.yaml b/charts/radix-operator/templates/radixapplication.yaml index 919217e2b..406be8a01 100644 --- a/charts/radix-operator/templates/radixapplication.yaml +++ b/charts/radix-operator/templates/radixapplication.yaml @@ -178,6 +178,15 @@ spec: Cookie.Refresh must be 0, and both SetXAuthRequestHeaders and SetAuthorizationHeader must be false if this setting is true. type: boolean type: object + credentials: + default: secret + description: Credentials defines credentials type for + authenticating. Default is a Secret, which represents + a client secret. + enum: + - secret + - azureWorkloadIdentity + type: string loginUrl: description: |- Defines the authentication endpoint of the identity provider. @@ -243,11 +252,6 @@ spec: X-Auth-Request-Email and X-Auth-Request-Preferred-Username request headers. The access token is passed in the X-Auth-Request-Access-Token header. type: boolean - useAzureIdentity: - description: UseAzureIdentity defines that credentials - for authenticating using Azure Workload Identity instead - of using a ClientSecret. - type: boolean type: object type: object dockerfileName: @@ -340,6 +344,15 @@ spec: Cookie.Refresh must be 0, and both SetXAuthRequestHeaders and SetAuthorizationHeader must be false if this setting is true. type: boolean type: object + credentials: + default: secret + description: Credentials defines credentials type + for authenticating. Default is a Secret, which + represents a client secret. + enum: + - secret + - azureWorkloadIdentity + type: string loginUrl: description: |- Defines the authentication endpoint of the identity provider. @@ -407,11 +420,6 @@ spec: X-Auth-Request-Email and X-Auth-Request-Preferred-Username request headers. The access token is passed in the X-Auth-Request-Access-Token header. type: boolean - useAzureIdentity: - description: UseAzureIdentity defines that credentials - for authenticating using Azure Workload Identity - instead of using a ClientSecret. - type: boolean type: object type: object dockerfileName: diff --git a/charts/radix-operator/templates/radixdeployment.yaml b/charts/radix-operator/templates/radixdeployment.yaml index c220677e7..1396df420 100644 --- a/charts/radix-operator/templates/radixdeployment.yaml +++ b/charts/radix-operator/templates/radixdeployment.yaml @@ -138,6 +138,15 @@ spec: Cookie.Refresh must be 0, and both SetXAuthRequestHeaders and SetAuthorizationHeader must be false if this setting is true. type: boolean type: object + credentials: + default: secret + description: Credentials defines credentials type for + authenticating. Default is a Secret, which represents + a client secret. + enum: + - secret + - azureWorkloadIdentity + type: string loginUrl: description: |- Defines the authentication endpoint of the identity provider. @@ -203,11 +212,6 @@ spec: X-Auth-Request-Email and X-Auth-Request-Preferred-Username request headers. The access token is passed in the X-Auth-Request-Access-Token header. type: boolean - useAzureIdentity: - description: UseAzureIdentity defines that credentials - for authenticating using Azure Workload Identity instead - of using a ClientSecret. - type: boolean type: object type: object dnsAppAlias: diff --git a/json-schema/radixapplication.json b/json-schema/radixapplication.json index 707e326e0..c7e3a3dfb 100644 --- a/json-schema/radixapplication.json +++ b/json-schema/radixapplication.json @@ -152,6 +152,15 @@ }, "type": "object" }, + "credentials": { + "default": "secret", + "description": "Credentials defines credentials type for authenticating. Default is a Secret, which represents a client secret.", + "enum": [ + "secret", + "azureWorkloadIdentity" + ], + "type": "string" + }, "loginUrl": { "description": "Defines the authentication endpoint of the identity provider.\nMust be set if OIDC.SkipDiscovery is true", "type": "string" @@ -219,10 +228,6 @@ "setXAuthRequestHeaders": { "description": "Defines if claims from the access token is added to the X-Auth-Request-User, X-Auth-Request-Groups,\nX-Auth-Request-Email and X-Auth-Request-Preferred-Username request headers.\nThe access token is passed in the X-Auth-Request-Access-Token header.", "type": "boolean" - }, - "useAzureIdentity": { - "description": "UseAzureIdentity defines that credentials for authenticating using Azure Workload Identity instead of using a ClientSecret.", - "type": "boolean" } }, "type": "object" @@ -315,6 +320,15 @@ }, "type": "object" }, + "credentials": { + "default": "secret", + "description": "Credentials defines credentials type for authenticating. Default is a Secret, which represents a client secret.", + "enum": [ + "secret", + "azureWorkloadIdentity" + ], + "type": "string" + }, "loginUrl": { "description": "Defines the authentication endpoint of the identity provider.\nMust be set if OIDC.SkipDiscovery is true", "type": "string" @@ -382,10 +396,6 @@ "setXAuthRequestHeaders": { "description": "Defines if claims from the access token is added to the X-Auth-Request-User, X-Auth-Request-Groups,\nX-Auth-Request-Email and X-Auth-Request-Preferred-Username request headers.\nThe access token is passed in the X-Auth-Request-Access-Token header.", "type": "boolean" - }, - "useAzureIdentity": { - "description": "UseAzureIdentity defines that credentials for authenticating using Azure Workload Identity instead of using a ClientSecret.", - "type": "boolean" } }, "type": "object" diff --git a/pkg/apis/deployment/oauthproxyresourcemanager_test.go b/pkg/apis/deployment/oauthproxyresourcemanager_test.go index ffd877ae1..c1f7f3132 100644 --- a/pkg/apis/deployment/oauthproxyresourcemanager_test.go +++ b/pkg/apis/deployment/oauthproxyresourcemanager_test.go @@ -210,7 +210,7 @@ func (s *OAuthProxyResourceManagerTestSuite) Test_Sync_UseClientSecretOrIdentity }, {name: "Created the service account with Authentication, OAuth2, true useAzureIdentity", rd: utils.NewDeploymentBuilder().WithAppName(appName).WithEnvironment(envName). WithComponent(utils.NewDeployComponentBuilder().WithName(componentName1).WithPublicPort("http").WithIdentity(identity). - WithAuthentication(&v1.Authentication{OAuth2: &v1.OAuth2{ClientID: oauth2ClientId, UseAzureIdentity: pointers.Ptr(true)}})).BuildRD(), + WithAuthentication(&v1.Authentication{OAuth2: &v1.OAuth2{ClientID: oauth2ClientId, Credentials: v1.AzureWorkloadIdentity}})).BuildRD(), expectedAuxOauthDeployCount: 1, expectedSa: auxOAuthServiceAccount, }, @@ -221,7 +221,7 @@ func (s *OAuthProxyResourceManagerTestSuite) Test_Sync_UseClientSecretOrIdentity }, {name: "Not overridden the existing service account with Authentication, OAuth2, true useAzureIdentity", rd: utils.NewDeploymentBuilder().WithAppName(appName).WithEnvironment(envName). WithComponent(utils.NewDeployComponentBuilder().WithName(componentName1).WithPublicPort("http").WithIdentity(identity). - WithAuthentication(&v1.Authentication{OAuth2: &v1.OAuth2{ClientID: oauth2ClientId, UseAzureIdentity: pointers.Ptr(true)}})).BuildRD(), + WithAuthentication(&v1.Authentication{OAuth2: &v1.OAuth2{ClientID: oauth2ClientId, Credentials: v1.AzureWorkloadIdentity}})).BuildRD(), expectedAuxOauthDeployCount: 1, existingSa: auxOAuthServiceAccount, expectedSa: auxOAuthServiceAccount, diff --git a/pkg/apis/deployment/secretrefs_test.go b/pkg/apis/deployment/secretrefs_test.go index fecdb0831..c3f31cac8 100644 --- a/pkg/apis/deployment/secretrefs_test.go +++ b/pkg/apis/deployment/secretrefs_test.go @@ -346,15 +346,15 @@ func Test_GetRadixComponentsForEnv_AzureKeyVaultUseIAzureIdentity(t *testing.T) {name: "empty when commonConfig is empty and environmentConfig is empty", commonConfig: nil, configureEnvironment: true, environmentConfig: nil, expected: nil}, {name: "empty when commonConfig is empty and environmentConfig is not set", commonConfig: nil, configureEnvironment: false, environmentConfig: nil, expected: nil}, {name: "use commonConfig when environmentConfig is empty", commonConfig: []v1.RadixAzureKeyVault{{Name: "key-vault-1", UseAzureIdentity: pointers.Ptr(true), Items: createRadixAzureKeyVaultItem()}}, configureEnvironment: true, environmentConfig: nil, expected: pointers.Ptr(true)}, - {name: "use commonConfig when environmentConfig.UseAzureIdentity is empty", commonConfig: []v1.RadixAzureKeyVault{{Name: "key-vault-1", UseAzureIdentity: pointers.Ptr(true), Items: createRadixAzureKeyVaultItem()}}, configureEnvironment: true, environmentConfig: []v1.RadixAzureKeyVault{{Name: "key-vault-1", UseAzureIdentity: nil, Items: []v1.RadixAzureKeyVaultItem{}}}, expected: pointers.Ptr(true)}, - {name: "override non-empty commonConfig with environmentConfig.UseAzureIdentity", + {name: "use commonConfig when environmentConfig.Credentials is empty", commonConfig: []v1.RadixAzureKeyVault{{Name: "key-vault-1", UseAzureIdentity: pointers.Ptr(true), Items: createRadixAzureKeyVaultItem()}}, configureEnvironment: true, environmentConfig: []v1.RadixAzureKeyVault{{Name: "key-vault-1", UseAzureIdentity: nil, Items: []v1.RadixAzureKeyVaultItem{}}}, expected: pointers.Ptr(true)}, + {name: "override non-empty commonConfig with environmentConfig.Credentials", commonConfig: []v1.RadixAzureKeyVault{{Name: "key-vault-1", UseAzureIdentity: pointers.Ptr(false), Items: createRadixAzureKeyVaultItem()}}, configureEnvironment: true, environmentConfig: []v1.RadixAzureKeyVault{{Name: "key-vault-1", UseAzureIdentity: pointers.Ptr(true), Items: createRadixAzureKeyVaultItem()}}, expected: pointers.Ptr(true)}, {name: "override empty commonConfig with environmentConfig", commonConfig: nil, configureEnvironment: true, environmentConfig: []v1.RadixAzureKeyVault{{Name: "key-vault-1", UseAzureIdentity: pointers.Ptr(true), Items: createRadixAzureKeyVaultItem()}}, expected: pointers.Ptr(true)}, - {name: "override empty commonConfig.UseAzureIdentity with environmentConfig", commonConfig: []v1.RadixAzureKeyVault{{Name: "key-vault-1", UseAzureIdentity: nil, Items: []v1.RadixAzureKeyVaultItem{}}}, + {name: "override empty commonConfig.Credentials with environmentConfig", commonConfig: []v1.RadixAzureKeyVault{{Name: "key-vault-1", UseAzureIdentity: nil, Items: []v1.RadixAzureKeyVaultItem{}}}, configureEnvironment: true, environmentConfig: []v1.RadixAzureKeyVault{{Name: "key-vault-1", UseAzureIdentity: pointers.Ptr(true), Items: createRadixAzureKeyVaultItem()}}, expected: pointers.Ptr(true)}, } diff --git a/pkg/apis/radix/v1/radixapptypes.go b/pkg/apis/radix/v1/radixapptypes.go index 065a8e179..2ed743d26 100644 --- a/pkg/apis/radix/v1/radixapptypes.go +++ b/pkg/apis/radix/v1/radixapptypes.go @@ -917,6 +917,16 @@ type RadixPrivateImageHubCredential struct { Email string `json:"email"` } +// CredentialsType defines the type of credentials +type CredentialsType string + +const ( + // Secret defines the client secret as a type of credentials + Secret CredentialsType = "secret" + // AzureWorkloadIdentity defines the Azure workload identity as a type of credentials + AzureWorkloadIdentity CredentialsType = "azureWorkloadIdentity" +) + // RadixVolumeMount defines an external storage resource. type RadixVolumeMount struct { // Deprecated: use BlobFuse2 instead. @@ -1379,9 +1389,11 @@ type OAuth2 struct { // +optional RedisStore *OAuth2RedisStore `json:"redisStore,omitempty"` - // UseAzureIdentity defines that credentials for authenticating using Azure Workload Identity instead of using a ClientSecret. + // Credentials defines credentials type for authenticating. Default is a Secret, which represents a client secret. + // +kubebuilder:validation:Enum=secret;azureWorkloadIdentity + // +kubebuilder:default:=secret // +optional - UseAzureIdentity *bool `json:"useAzureIdentity,omitempty"` + Credentials CredentialsType `json:"credentials,omitempty"` } // OAuth2Cookie defines properties for the oauth cookie. @@ -1903,7 +1915,7 @@ func (oauth2 *OAuth2) GetUseAzureIdentity() bool { if oauth2 == nil { return false } - return oauth2.UseAzureIdentity != nil && *oauth2.UseAzureIdentity + return oauth2.Credentials == AzureWorkloadIdentity } // GetSessionStoreType Returns the session store type diff --git a/pkg/apis/radix/v1/zz_generated.deepcopy.go b/pkg/apis/radix/v1/zz_generated.deepcopy.go index 2b972aa58..d671b7661 100644 --- a/pkg/apis/radix/v1/zz_generated.deepcopy.go +++ b/pkg/apis/radix/v1/zz_generated.deepcopy.go @@ -588,11 +588,6 @@ func (in *OAuth2) DeepCopyInto(out *OAuth2) { *out = new(OAuth2RedisStore) **out = **in } - if in.UseAzureIdentity != nil { - in, out := &in.UseAzureIdentity, &out.UseAzureIdentity - *out = new(bool) - **out = **in - } return } diff --git a/pkg/apis/utils/labels/labels_test.go b/pkg/apis/utils/labels/labels_test.go index db2705df4..d373e75e5 100644 --- a/pkg/apis/utils/labels/labels_test.go +++ b/pkg/apis/utils/labels/labels_test.go @@ -3,9 +3,8 @@ package labels import ( "testing" - "github.com/equinor/radix-common/utils/pointers" "github.com/equinor/radix-operator/pkg/apis/kube" - v1 "github.com/equinor/radix-operator/pkg/apis/radix/v1" + "github.com/equinor/radix-operator/pkg/apis/radix/v1" "github.com/stretchr/testify/assert" kubelabels "k8s.io/apimachinery/pkg/labels" ) @@ -60,14 +59,14 @@ func Test_ForOAuthProxyServiceAccountWithWorkloadIdentity(t *testing.T) { assert.Equal(t, kubelabels.Set(nil), actual, "Not expected labels when there is no OAuth2") actual = ForOauthProxyServiceAccountWithRadixIdentity(&v1.OAuth2{}) - assert.Equal(t, kubelabels.Set(nil), actual, "Not expected labels when there is no UseAzureIdentity") + assert.Equal(t, kubelabels.Set(nil), actual, "Not expected labels when there is no Credentials") - actual = ForOauthProxyServiceAccountWithRadixIdentity(&v1.OAuth2{UseAzureIdentity: pointers.Ptr(false), ClientID: "any-client-id"}) - assert.Equal(t, kubelabels.Set(nil), actual, "Not expected labels when UseAzureIdentity is false") + actual = ForOauthProxyServiceAccountWithRadixIdentity(&v1.OAuth2{Credentials: v1.Secret, ClientID: "any-client-id"}) + assert.Equal(t, kubelabels.Set(nil), actual, "Not expected labels when Credentials is Secret") - actual = ForOauthProxyServiceAccountWithRadixIdentity(&v1.OAuth2{UseAzureIdentity: pointers.Ptr(true), ClientID: "any-client-id"}) + actual = ForOauthProxyServiceAccountWithRadixIdentity(&v1.OAuth2{Credentials: v1.AzureWorkloadIdentity, ClientID: "any-client-id"}) expected := kubelabels.Set{"azure.workload.identity/use": "true"} - assert.Equal(t, expected, actual, "Expected labels when UseAzureIdentity is true") + assert.Equal(t, expected, actual, "Expected labels when Credentials is AzureWorkloadIdentity") } func Test_ForPodWithRadixIdentity(t *testing.T) {