diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 0a0e56e38..7f85cec63 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -77,7 +77,7 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@v6 with: - version: v1.60.3 + version: v1.64.3 verify-code-generation: name: Verify Code Generation diff --git a/Makefile b/Makefile index 43c1f467b..438531249 100644 --- a/Makefile +++ b/Makefile @@ -187,7 +187,7 @@ HAS_KUBECTL := $(shell command -v kubectl;) .PHONY: bootstrap bootstrap: vendor ifndef HAS_GOLANGCI_LINT - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $$(go env GOPATH)/bin v1.60.3 + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $$(go env GOPATH)/bin v1.64.3 endif ifndef HAS_MOCKGEN go install github.com/golang/mock/mockgen@v1.6.0 diff --git a/charts/radix-operator/Chart.yaml b/charts/radix-operator/Chart.yaml index e3e5658a3..6d0c9f03b 100644 --- a/charts/radix-operator/Chart.yaml +++ b/charts/radix-operator/Chart.yaml @@ -1,7 +1,7 @@ apiVersion: v2 name: radix-operator -version: 1.51.1 -appVersion: 1.71.1 +version: 1.52.0 +appVersion: 1.72.0 kubeVersion: ">=1.24.0" description: Radix Operator keywords: diff --git a/charts/radix-operator/templates/radixapplication.yaml b/charts/radix-operator/templates/radixapplication.yaml index 42c1d92d4..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. @@ -335,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. diff --git a/charts/radix-operator/templates/radixdeployment.yaml b/charts/radix-operator/templates/radixdeployment.yaml index f1ed0d7db..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. diff --git a/go.mod b/go.mod index 36772f20f..f7b41165c 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module github.com/equinor/radix-operator go 1.23.0 -toolchain go1.23.4 +toolchain go1.23.6 require ( dario.cat/mergo v1.0.1 diff --git a/json-schema/radixapplication.json b/json-schema/radixapplication.json index d00e75e97..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" @@ -311,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" diff --git a/operator.Dockerfile b/operator.Dockerfile index cb4f375e3..b9f261553 100644 --- a/operator.Dockerfile +++ b/operator.Dockerfile @@ -1,4 +1,4 @@ -FROM --platform=$BUILDPLATFORM docker.io/golang:1.23.4-alpine3.21 AS builder +FROM --platform=$BUILDPLATFORM docker.io/golang:1.23.6-alpine3.21 AS builder ARG TARGETARCH ENV CGO_ENABLED=0 \ GOOS=linux \ diff --git a/pipeline.Dockerfile b/pipeline.Dockerfile index 9db2f114d..60e591c23 100644 --- a/pipeline.Dockerfile +++ b/pipeline.Dockerfile @@ -1,4 +1,4 @@ -FROM --platform=$BUILDPLATFORM docker.io/golang:1.23.4-alpine3.21 AS builder +FROM --platform=$BUILDPLATFORM docker.io/golang:1.23.6-alpine3.21 AS builder ARG TARGETARCH ENV CGO_ENABLED=0 \ GOOS=linux \ diff --git a/pkg/apis/defaults/oauth2.go b/pkg/apis/defaults/oauth2.go index 5464b4a9e..ace66f19f 100644 --- a/pkg/apis/defaults/oauth2.go +++ b/pkg/apis/defaults/oauth2.go @@ -13,6 +13,8 @@ const ( AuthUrlAnnotation = "nginx.ingress.kubernetes.io/auth-url" AuthSigninAnnotation = "nginx.ingress.kubernetes.io/auth-signin" AuthResponseHeadersAnnotation = "nginx.ingress.kubernetes.io/auth-response-headers" + OAuthProxyProviderOIDC = "oidc" + OAuthProxyProviderEntraId = "entra-id" ) // OAuth2Config is implemented by any value that has as MergeWith method diff --git a/pkg/apis/deployment/oauthproxyresourcemanager.go b/pkg/apis/deployment/oauthproxyresourcemanager.go index 11145bc2b..9ba042e5f 100644 --- a/pkg/apis/deployment/oauthproxyresourcemanager.go +++ b/pkg/apis/deployment/oauthproxyresourcemanager.go @@ -31,6 +31,13 @@ import ( "k8s.io/client-go/tools/cache" ) +const ( + oauth2ProxyClientSecretEnvironmentVariable = "OAUTH2_PROXY_CLIENT_SECRET" + oauth2ProxyCookieSecretEnvironmentVariable = "OAUTH2_PROXY_COOKIE_SECRET" + oauth2ProxyRedisPasswordEnvironmentVariable = "OAUTH2_PROXY_REDIS_PASSWORD" + oauth2ProxyEntraIdFederatedTokenAuthEnvironmentVariable = "OAUTH2_PROXY_ENTRA_ID_FEDERATED_TOKEN_AUTH" +) + // NewOAuthProxyResourceManager creates a new OAuthProxyResourceManager func NewOAuthProxyResourceManager(rd *v1.RadixDeployment, rr *v1.RadixRegistration, kubeutil *kube.Kube, oauth2DefaultConfig defaults.OAuth2Config, ingressAnnotationProviders []ingress.AnnotationProvider, oauth2ProxyDockerImage string) AuxiliaryResourceManager { @@ -255,6 +262,12 @@ func (o *oauthProxyResourceManager) install(ctx context.Context, component v1.Ra return err } + if err := createOrUpdateOAuthProxyServiceAccount(ctx, o.kubeutil, o.rd, component); err != nil { + return fmt.Errorf("failed to create OAuth proxy service account: %w", err) + } + if err := garbageCollectServiceAccountNoLongerInSpecForOAuthProxyComponent(ctx, o.kubeutil, o.rd, component); err != nil { + return fmt.Errorf("failed to garbage collect service account no longer in spec for OAuth proxy component: %w", err) + } return o.createOrUpdateDeployment(ctx, component) } @@ -272,6 +285,10 @@ func (o *oauthProxyResourceManager) uninstall(ctx context.Context, component v1. return err } + if err := deleteOAuthProxyServiceAccounts(ctx, o.kubeutil, o.rd.Namespace, component); err != nil { + return err + } + if err := o.deleteSecrets(ctx, component); err != nil { return err } @@ -450,26 +467,28 @@ func (o *oauthProxyResourceManager) createOrUpdateService(ctx context.Context, c func (o *oauthProxyResourceManager) createOrUpdateSecret(ctx context.Context, component v1.RadixCommonDeployComponent) error { secretName := utils.GetAuxiliaryComponentSecretName(component.GetName(), defaults.OAuthProxyAuxiliaryComponentSuffix) - secret, err := o.kubeutil.GetSecret(ctx, o.rd.Namespace, secretName) - + existingSecret, err := o.kubeutil.GetSecret(ctx, o.rd.Namespace, secretName) if err != nil { if !kubeerrors.IsNotFound(err) { return err } - secret, err = o.buildSecretSpec(component) + secret, err := buildOAuthProxySecret(o.rd.Spec.AppName, component) if err != nil { return err } - } else { - oauthutil.MergeAuxComponentResourceLabels(secret, o.rd.Spec.AppName, component) - if component.GetAuthentication().OAuth2.SessionStoreType != v1.SessionStoreRedis { - if secret.Data != nil && len(secret.Data[defaults.OAuthRedisPasswordKeyName]) > 0 { - delete(secret.Data, defaults.OAuthRedisPasswordKeyName) - } - } + _, err = o.kubeutil.CreateSecret(ctx, o.rd.Namespace, secret) + return err } - _, err = o.kubeutil.ApplySecret(ctx, o.rd.Namespace, secret) //nolint:staticcheck // must be updated to use UpdateSecret or CreateSecret + secret := existingSecret.DeepCopy() + oauthutil.MergeAuxComponentResourceLabels(secret, o.rd.Spec.AppName, component) + if _, ok := secret.Data[defaults.OAuthRedisPasswordKeyName]; ok && component.GetAuthentication().GetOAuth2().GetSessionStoreType() != v1.SessionStoreRedis { + delete(secret.Data, defaults.OAuthRedisPasswordKeyName) + } + if _, ok := secret.Data[defaults.OAuthClientSecretKeyName]; ok && component.GetAuthentication().GetOAuth2().GetUseAzureIdentity() { + delete(secret.Data, defaults.OAuthClientSecretKeyName) + } + _, err = o.kubeutil.UpdateSecret(ctx, existingSecret, secret) return err } @@ -537,7 +556,7 @@ func (o *oauthProxyResourceManager) getRoleAndRoleBindingName(prefix, componentN return fmt.Sprintf("%s-%s", prefix, deploymentName) } -func (o *oauthProxyResourceManager) buildSecretSpec(component v1.RadixCommonDeployComponent) (*corev1.Secret, error) { +func buildOAuthProxySecret(appName string, component v1.RadixCommonDeployComponent) (*corev1.Secret, error) { secretName := utils.GetAuxiliaryComponentSecretName(component.GetName(), defaults.OAuthProxyAuxiliaryComponentSuffix) secret := &corev1.Secret{ @@ -547,8 +566,8 @@ func (o *oauthProxyResourceManager) buildSecretSpec(component v1.RadixCommonDepl }, Data: make(map[string][]byte), } - oauthutil.MergeAuxComponentResourceLabels(secret, o.rd.Spec.AppName, component) - cookieSecret, err := o.generateRandomCookieSecret() + oauthutil.MergeAuxComponentResourceLabels(secret, appName, component) + cookieSecret, err := generateRandomCookieSecret() if err != nil { return nil, err } @@ -580,7 +599,7 @@ func (o *oauthProxyResourceManager) buildServiceSpec(component v1.RadixCommonDep return service } -func (o *oauthProxyResourceManager) generateRandomCookieSecret() ([]byte, error) { +func generateRandomCookieSecret() ([]byte, error) { randomBytes := commonutils.GenerateRandomKey(32) // Extra check to make sure correct number of bytes are returned for the random key if len(randomBytes) != 32 { @@ -674,7 +693,13 @@ func (o *oauthProxyResourceManager) getDesiredDeployment(component v1.RadixCommo }, }, } - + if component.GetAuthentication().GetOAuth2().GetUseAzureIdentity() { + desiredDeployment.Spec.Template.Labels = radixlabels.Merge( + desiredDeployment.Spec.Template.GetLabels(), + radixlabels.ForPodWithRadixIdentity(component.GetIdentity()), + ) + desiredDeployment.Spec.Template.Spec.ServiceAccountName = utils.GetOAuthProxyServiceAccountName(component.GetName()) + } oauthutil.MergeAuxComponentResourceLabels(desiredDeployment, o.rd.Spec.AppName, component) return desiredDeployment, nil } @@ -701,7 +726,7 @@ func (o *oauthProxyResourceManager) getEnvVars(component v1.RadixCommonDeployCom } // oauth2-proxy env-vars - envVars = append(envVars, corev1.EnvVar{Name: "OAUTH2_PROXY_PROVIDER", Value: "oidc"}) + envVars = append(envVars, corev1.EnvVar{Name: "OAUTH2_PROXY_PROVIDER", Value: getOAuthProxyProvider(oauth)}) envVars = append(envVars, corev1.EnvVar{Name: "OAUTH2_PROXY_COOKIE_HTTPONLY", Value: "true"}) envVars = append(envVars, corev1.EnvVar{Name: "OAUTH2_PROXY_COOKIE_SECURE", Value: "true"}) envVars = append(envVars, corev1.EnvVar{Name: "OAUTH2_PROXY_PASS_BASIC_AUTH", Value: "false"}) @@ -710,10 +735,16 @@ func (o *oauthProxyResourceManager) getEnvVars(component v1.RadixCommonDeployCom envVars = append(envVars, corev1.EnvVar{Name: "OAUTH2_PROXY_SKIP_CLAIMS_FROM_PROFILE_URL", Value: "true"}) envVars = append(envVars, corev1.EnvVar{Name: "OAUTH2_PROXY_HTTP_ADDRESS", Value: fmt.Sprintf("%s://:%v", "http", defaults.OAuthProxyPortNumber)}) secretName := utils.GetAuxiliaryComponentSecretName(component.GetName(), defaults.OAuthProxyAuxiliaryComponentSuffix) - envVars = append(envVars, o.createEnvVarWithSecretRef("OAUTH2_PROXY_COOKIE_SECRET", secretName, defaults.OAuthCookieSecretKeyName)) - envVars = append(envVars, o.createEnvVarWithSecretRef("OAUTH2_PROXY_CLIENT_SECRET", secretName, defaults.OAuthClientSecretKeyName)) + envVars = append(envVars, o.createEnvVarWithSecretRef(oauth2ProxyCookieSecretEnvironmentVariable, secretName, defaults.OAuthCookieSecretKeyName)) + + if oauth.GetUseAzureIdentity() { + envVars = append(envVars, corev1.EnvVar{Name: oauth2ProxyEntraIdFederatedTokenAuthEnvironmentVariable, Value: "true"}) + } else { + envVars = append(envVars, o.createEnvVarWithSecretRef(oauth2ProxyClientSecretEnvironmentVariable, secretName, defaults.OAuthClientSecretKeyName)) + } + if oauth.SessionStoreType == v1.SessionStoreRedis { - envVars = append(envVars, o.createEnvVarWithSecretRef("OAUTH2_PROXY_REDIS_PASSWORD", secretName, defaults.OAuthRedisPasswordKeyName)) + envVars = append(envVars, o.createEnvVarWithSecretRef(oauth2ProxyRedisPasswordEnvironmentVariable, secretName, defaults.OAuthRedisPasswordKeyName)) } addEnvVarIfSet("OAUTH2_PROXY_CLIENT_ID", oauth.ClientID) @@ -751,6 +782,13 @@ func (o *oauthProxyResourceManager) getEnvVars(component v1.RadixCommonDeployCom return envVars } +func getOAuthProxyProvider(oauth *v1.OAuth2) string { + if oauth.GetUseAzureIdentity() { + return defaults.OAuthProxyProviderEntraId + } + return defaults.OAuthProxyProviderOIDC +} + func (o *oauthProxyResourceManager) createEnvVarWithSecretRef(envVarName, secretName, key string) corev1.EnvVar { return corev1.EnvVar{ Name: envVarName, diff --git a/pkg/apis/deployment/oauthproxyresourcemanager_test.go b/pkg/apis/deployment/oauthproxyresourcemanager_test.go index 9b553313d..c1f7f3132 100644 --- a/pkg/apis/deployment/oauthproxyresourcemanager_test.go +++ b/pkg/apis/deployment/oauthproxyresourcemanager_test.go @@ -15,6 +15,7 @@ import ( "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/utils" + radixlabels "github.com/equinor/radix-operator/pkg/apis/utils/labels" oauthutil "github.com/equinor/radix-operator/pkg/apis/utils/oauth" radixclient "github.com/equinor/radix-operator/pkg/client/clientset/versioned" radixfake "github.com/equinor/radix-operator/pkg/client/clientset/versioned/fake" @@ -67,6 +68,10 @@ func (s *OAuthProxyResourceManagerTestSuite) SetupSuite() { } func (s *OAuthProxyResourceManagerTestSuite) SetupTest() { + s.setupTest() +} + +func (s *OAuthProxyResourceManagerTestSuite) setupTest() { s.kubeClient = kubefake.NewSimpleClientset() s.radixClient = radixfake.NewSimpleClientset() s.kedaClient = kedafake.NewSimpleClientset() @@ -169,6 +174,124 @@ func (s *OAuthProxyResourceManagerTestSuite) Test_Sync_NotPublicOrNoOAuth() { } } +func (s *OAuthProxyResourceManagerTestSuite) Test_Sync_UseClientSecretOrIdentity() { + type scenarioDef struct { + name string + rd *v1.RadixDeployment + expectedSa *corev1.ServiceAccount + expectedAuxOauthDeployCount int + existingSa *corev1.ServiceAccount + } + const ( + appName = "anyapp" + componentAzureClientId = "some-azure-client-id" + oauth2ClientId = "oauth2-client-id" + componentName1 = "comp1" + expectedServiceAccountName = "comp1-aux-oauth-sa" + envName = "qa" + ) + + identity := &v1.Identity{Azure: &v1.AzureIdentity{ClientId: componentAzureClientId}} + auxOAuthServiceAccount := buildServiceAccount(utils.GetEnvironmentNamespace(appName, envName), expectedServiceAccountName, + radixlabels.Merge( + getLabelsForAuxOAuthComponentServiceAccount(componentName1), + radixlabels.ForServiceAccountWithRadixIdentity(&v1.Identity{Azure: &v1.AzureIdentity{ClientId: oauth2ClientId}}), + ), oauth2ClientId) + scenarios := []scenarioDef{ + {name: "Not created service account without Authentication", rd: utils.NewDeploymentBuilder().WithAppName(appName).WithEnvironment(envName).WithComponent(utils.NewDeployComponentBuilder().WithName(componentName1).WithPublicPort("http").WithIdentity(&v1.Identity{Azure: &v1.AzureIdentity{ClientId: componentAzureClientId}})). + BuildRD(), + expectedAuxOauthDeployCount: 0}, + {name: "Not created service account with Authentication, no OAuth2", rd: utils.NewDeploymentBuilder().WithAppName(appName).WithEnvironment(envName).WithComponent(utils.NewDeployComponentBuilder().WithName(componentName1).WithPublicPort("http").WithIdentity(&v1.Identity{Azure: &v1.AzureIdentity{ClientId: componentAzureClientId}}). + WithAuthentication(&v1.Authentication{})).BuildRD(), + expectedAuxOauthDeployCount: 0}, + {name: "Not created service account with Authentication, OAuth2, false useAzureIdentity", rd: utils.NewDeploymentBuilder().WithAppName(appName).WithEnvironment(envName).WithComponent(utils.NewDeployComponentBuilder().WithName(componentName1).WithPublicPort("http").WithIdentity(&v1.Identity{Azure: &v1.AzureIdentity{ClientId: componentAzureClientId}}). + WithAuthentication(&v1.Authentication{OAuth2: &v1.OAuth2{ClientID: oauth2ClientId}})).BuildRD(), + expectedAuxOauthDeployCount: 1, + }, + {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, Credentials: v1.AzureWorkloadIdentity}})).BuildRD(), + expectedAuxOauthDeployCount: 1, + expectedSa: auxOAuthServiceAccount, + }, + {name: "Delete existing service account, with Authentication, OAuth2, no useAzureIdentity", rd: utils.NewDeploymentBuilder().WithAppName(appName).WithEnvironment(envName).WithComponent(utils.NewDeployComponentBuilder().WithName(componentName1).WithPublicPort("http").WithIdentity(&v1.Identity{Azure: &v1.AzureIdentity{ClientId: componentAzureClientId}}). + WithAuthentication(&v1.Authentication{OAuth2: &v1.OAuth2{ClientID: oauth2ClientId}})).BuildRD(), + expectedAuxOauthDeployCount: 1, + existingSa: auxOAuthServiceAccount, + }, + {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, Credentials: v1.AzureWorkloadIdentity}})).BuildRD(), + expectedAuxOauthDeployCount: 1, + existingSa: auxOAuthServiceAccount, + expectedSa: auxOAuthServiceAccount, + }, + } + rr := utils.NewRegistrationBuilder().WithName(appName).BuildRR() + + for _, scenario := range scenarios { + s.Run(scenario.name, func() { + s.setupTest() + sut := &oauthProxyResourceManager{scenario.rd, rr, s.kubeUtil, []ingress.AnnotationProvider{}, defaults.NewOAuth2Config(defaults.WithOAuth2Defaults()), "", zerolog.Nop()} + if scenario.existingSa != nil { + _, err := sut.kubeutil.KubeClient().CoreV1().ServiceAccounts(scenario.existingSa.Namespace).Create(context.Background(), scenario.existingSa, metav1.CreateOptions{}) + s.NoError(err, "Failed to create service account") + } + auxOAuthSecret, err := buildOAuthProxySecret(appName, &scenario.rd.Spec.Components[0]) + auxOAuthSecret.SetNamespace(scenario.rd.Namespace) + s.NoError(err, "Failed to build secret") + auxOAuthSecret.Data[defaults.OAuthClientSecretKeyName] = []byte("some-client-secret") + _, err = s.kubeUtil.KubeClient().CoreV1().Secrets(scenario.rd.Namespace).Create(context.Background(), auxOAuthSecret, metav1.CreateOptions{}) + s.NoError(err, "Failed to create secret") + + err = sut.Sync(context.Background()) + s.Nil(err) + + deploys, err := sut.kubeutil.KubeClient().AppsV1().Deployments(corev1.NamespaceAll).List(context.Background(), metav1.ListOptions{LabelSelector: s.getAppNameSelector(appName)}) + s.NoError(err, "Failed to list deployments") + s.Len(deploys.Items, scenario.expectedAuxOauthDeployCount, "Expected aux oauth deployment count") + saList, err := sut.kubeutil.KubeClient().CoreV1().ServiceAccounts(corev1.NamespaceAll).List(context.Background(), metav1.ListOptions{LabelSelector: getLabelsForAuxOAuthComponentServiceAccount(componentName1).String()}) + s.NoError(err, "Failed to list service accounts") + existsClientSecretEnvVar := false + existsAuxOAuthDeployment := len(deploys.Items) > 0 + if existsAuxOAuthDeployment { + auxOAuthDeployment := deploys.Items[0] + auxOAuthContainer := auxOAuthDeployment.Spec.Template.Spec.Containers[0] + secretName := utils.GetAuxiliaryComponentSecretName(scenario.rd.Spec.Components[0].GetName(), defaults.OAuthProxyAuxiliaryComponentSuffix) + existsClientSecretEnvVar = slice.Any(auxOAuthContainer.Env, func(ev corev1.EnvVar) bool { + return ev.Name == oauth2ProxyClientSecretEnvironmentVariable && ev.ValueFrom != nil && ev.ValueFrom.SecretKeyRef != nil && + ev.ValueFrom.SecretKeyRef.Key == defaults.OAuthClientSecretKeyName && + ev.ValueFrom.SecretKeyRef.LocalObjectReference.Name == secretName + }) + actualSecret, err := sut.kubeutil.KubeClient().CoreV1().Secrets(scenario.rd.Namespace).Get(context.Background(), auxOAuthSecret.GetName(), metav1.GetOptions{}) + s.NoError(err, "Failed to get aux OAuth secret") + _, existsOAuthClientSecret := actualSecret.Data[defaults.OAuthClientSecretKeyName] + if scenario.expectedSa != nil { + s.False(existsOAuthClientSecret, "Unexpected client secret") + } else { + s.True(existsOAuthClientSecret, "Expected client secret") + } + } + if scenario.expectedSa != nil { + s.Len(saList.Items, 1, fmt.Sprintf("Expected service account %s", scenario.expectedSa.GetName())) + s.Equal(scenario.expectedSa, &saList.Items[0], fmt.Sprintf("Expected service account %s", scenario.expectedSa.GetName())) + if existsAuxOAuthDeployment { + s.False(existsClientSecretEnvVar, "Unexpected client secret env var") + } + } else { + s.Len(saList.Items, 0, "Expected no service account") + if existsAuxOAuthDeployment { + s.True(existsClientSecretEnvVar, "Expected client secret env var") + } + } + }) + } +} + +func getLabelsForAuxOAuthComponentServiceAccount(componentName string) labels.Set { + return radixlabels.ForOAuthProxyComponentServiceAccount(&v1.RadixDeployComponent{Name: componentName}) +} + func (s *OAuthProxyResourceManagerTestSuite) Test_Sync_OauthDeploymentReplicas() { auth := &v1.Authentication{OAuth2: &v1.OAuth2{ClientID: "1234"}} appName := "anyapp" @@ -246,7 +369,7 @@ func (s *OAuthProxyResourceManagerTestSuite) Test_Sync_OauthDeploymentReplicas() sut := &oauthProxyResourceManager{test.rd, rr, s.kubeUtil, []ingress.AnnotationProvider{}, s.oauth2Config, "", zerolog.Nop()} err := sut.Sync(context.Background()) s.Nil(err) - deploys, _ := s.kubeClient.AppsV1().Deployments(corev1.NamespaceAll).List(context.Background(), metav1.ListOptions{LabelSelector: s.getAppNameSelector(appName)}) + deploys, _ := sut.kubeutil.KubeClient().AppsV1().Deployments(corev1.NamespaceAll).List(context.Background(), metav1.ListOptions{LabelSelector: s.getAppNameSelector(appName)}) s.Equal(test.expectedReplicas, *deploys.Items[0].Spec.Replicas) }) } @@ -354,9 +477,9 @@ func (s *OAuthProxyResourceManagerTestSuite) Test_Sync_OAuthProxyDeploymentCreat s.Equal("true", s.getEnvVarValueByName("OAUTH2_PROXY_SESSION_COOKIE_MINIMAL", defaultContainer.Env)) s.Equal(returnOAuth.RedisStore.ConnectionURL, s.getEnvVarValueByName("OAUTH2_PROXY_REDIS_CONNECTION_URL", defaultContainer.Env)) secretName := utils.GetAuxiliaryComponentSecretName(componentName, defaults.OAuthProxyAuxiliaryComponentSuffix) - s.Equal(corev1.EnvVarSource{SecretKeyRef: &corev1.SecretKeySelector{Key: defaults.OAuthCookieSecretKeyName, LocalObjectReference: corev1.LocalObjectReference{Name: secretName}}}, s.getEnvVarValueFromByName("OAUTH2_PROXY_COOKIE_SECRET", defaultContainer.Env)) - s.Equal(corev1.EnvVarSource{SecretKeyRef: &corev1.SecretKeySelector{Key: defaults.OAuthClientSecretKeyName, LocalObjectReference: corev1.LocalObjectReference{Name: secretName}}}, s.getEnvVarValueFromByName("OAUTH2_PROXY_CLIENT_SECRET", defaultContainer.Env)) - s.Equal(corev1.EnvVarSource{SecretKeyRef: &corev1.SecretKeySelector{Key: defaults.OAuthRedisPasswordKeyName, LocalObjectReference: corev1.LocalObjectReference{Name: secretName}}}, s.getEnvVarValueFromByName("OAUTH2_PROXY_REDIS_PASSWORD", defaultContainer.Env)) + s.Equal(corev1.EnvVarSource{SecretKeyRef: &corev1.SecretKeySelector{Key: defaults.OAuthCookieSecretKeyName, LocalObjectReference: corev1.LocalObjectReference{Name: secretName}}}, s.getEnvVarValueFromByName(oauth2ProxyCookieSecretEnvironmentVariable, defaultContainer.Env)) + s.Equal(corev1.EnvVarSource{SecretKeyRef: &corev1.SecretKeySelector{Key: defaults.OAuthClientSecretKeyName, LocalObjectReference: corev1.LocalObjectReference{Name: secretName}}}, s.getEnvVarValueFromByName(oauth2ProxyClientSecretEnvironmentVariable, defaultContainer.Env)) + s.Equal(corev1.EnvVarSource{SecretKeyRef: &corev1.SecretKeySelector{Key: defaults.OAuthRedisPasswordKeyName, LocalObjectReference: corev1.LocalObjectReference{Name: secretName}}}, s.getEnvVarValueFromByName(oauth2ProxyRedisPasswordEnvironmentVariable, defaultContainer.Env)) // Env var OAUTH2_PROXY_REDIS_PASSWORD should not be present when SessionStoreType is cookie returnOAuth.SessionStoreType = "cookie" @@ -365,7 +488,7 @@ func (s *OAuthProxyResourceManagerTestSuite) Test_Sync_OAuthProxyDeploymentCreat s.Nil(err) actualDeploys, _ = s.kubeClient.AppsV1().Deployments(corev1.NamespaceAll).List(context.Background(), metav1.ListOptions{}) s.Len(actualDeploys.Items[0].Spec.Template.Spec.Containers[0].Env, 29) - s.False(s.getEnvVarExist("OAUTH2_PROXY_REDIS_PASSWORD", actualDeploys.Items[0].Spec.Template.Spec.Containers[0].Env)) + s.False(s.getEnvVarExist(oauth2ProxyRedisPasswordEnvironmentVariable, actualDeploys.Items[0].Spec.Template.Spec.Containers[0].Env)) } func (s *OAuthProxyResourceManagerTestSuite) Test_Sync_OAuthProxySecretAndRbacCreated() { @@ -1040,14 +1163,14 @@ func (s *OAuthProxyResourceManagerTestSuite) addRole(name, namespace, appName, a } func (s *OAuthProxyResourceManagerTestSuite) addRoleBinding(name, namespace, appName, auxComponentName, auxComponentType string) { - rolebinding := &rbacv1.RoleBinding{ + roleBinding := &rbacv1.RoleBinding{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, Labels: s.buildResourceLabels(appName, auxComponentName, auxComponentType), }, } - _, err := s.kubeClient.RbacV1().RoleBindings(namespace).Create(context.Background(), rolebinding, metav1.CreateOptions{}) + _, err := s.kubeClient.RbacV1().RoleBindings(namespace).Create(context.Background(), roleBinding, metav1.CreateOptions{}) s.Require().NoError(err) } 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/deployment/serviceaccount.go b/pkg/apis/deployment/serviceaccount.go index 1d9bab506..59c19b000 100644 --- a/pkg/apis/deployment/serviceaccount.go +++ b/pkg/apis/deployment/serviceaccount.go @@ -4,18 +4,20 @@ import ( "context" "fmt" + "github.com/equinor/radix-operator/pkg/apis/kube" radixv1 "github.com/equinor/radix-operator/pkg/apis/radix/v1" "github.com/equinor/radix-operator/pkg/apis/utils" radixannotations "github.com/equinor/radix-operator/pkg/apis/utils/annotations" radixlabels "github.com/equinor/radix-operator/pkg/apis/utils/labels" + "github.com/rs/zerolog/log" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kubelabels "k8s.io/apimachinery/pkg/labels" ) func (deploy *Deployment) createOrUpdateServiceAccount(ctx context.Context, component radixv1.RadixCommonDeployComponent) error { - if deploy.isServiceAccountForComponentPreinstalled(component) { + if isServiceAccountForComponentPreinstalled(deploy.radixDeployment, component) { return nil } @@ -23,9 +25,9 @@ func (deploy *Deployment) createOrUpdateServiceAccount(ctx context.Context, comp return nil } - sa := deploy.buildServiceAccountForComponent(component) + sa := buildServiceAccountForComponent(deploy.radixDeployment.Namespace, component) - if err := deploy.verifyServiceAccountForComponentCanBeApplied(ctx, sa, component); err != nil { + if err := verifyServiceAccountForComponentCanBeApplied(ctx, deploy.kubeutil, sa, component); err != nil { return err } @@ -33,8 +35,27 @@ func (deploy *Deployment) createOrUpdateServiceAccount(ctx context.Context, comp return err } -func (deploy *Deployment) verifyServiceAccountForComponentCanBeApplied(ctx context.Context, sa *corev1.ServiceAccount, component radixv1.RadixCommonDeployComponent) error { - existingSa, err := deploy.kubeutil.GetServiceAccount(ctx, sa.GetNamespace(), sa.GetName()) +func createOrUpdateOAuthProxyServiceAccount(ctx context.Context, kubeUtil *kube.Kube, radixDeployment *radixv1.RadixDeployment, component radixv1.RadixCommonDeployComponent) error { + if isServiceAccountForComponentPreinstalled(radixDeployment, component) { + return nil + } + + if !component.GetAuthentication().GetOAuth2().GetUseAzureIdentity() { + return nil + } + + sa := buildServiceAccountForOAuthProxyComponent(radixDeployment.Namespace, component) + + if err := verifyServiceAccountForOAuthProxyComponentCanBeApplied(ctx, kubeUtil, sa, component); err != nil { + return err + } + + _, err := kubeUtil.ApplyServiceAccount(ctx, sa) + return err +} + +func verifyServiceAccountForComponentCanBeApplied(ctx context.Context, kubeUtil *kube.Kube, sa *corev1.ServiceAccount, component radixv1.RadixCommonDeployComponent) error { + existingSa, err := kubeUtil.GetServiceAccount(ctx, sa.GetNamespace(), sa.GetName()) if err != nil { // If service account does not already exist we return nil to indicate that it can be applied if errors.IsNotFound(err) { @@ -50,24 +71,48 @@ func (deploy *Deployment) verifyServiceAccountForComponentCanBeApplied(ctx conte return nil } -func (deploy *Deployment) buildServiceAccountForComponent(component radixv1.RadixCommonDeployComponent) *corev1.ServiceAccount { +func verifyServiceAccountForOAuthProxyComponentCanBeApplied(ctx context.Context, kubeUtil *kube.Kube, sa *corev1.ServiceAccount, component radixv1.RadixCommonDeployComponent) error { + existingSa, err := kubeUtil.GetServiceAccount(ctx, sa.GetNamespace(), sa.GetName()) + if err != nil { + // If service account does not already exist we return nil to indicate that it can be applied + if errors.IsNotFound(err) { + return nil + } + return err + } + + if !isServiceAccountForOAuthProxyComponent(existingSa, component) { + return fmt.Errorf("existing service account does not belong to aux OAuth proxy of the component %s", component.GetName()) + } + + return nil +} + +func buildServiceAccountForComponent(namespace string, component radixv1.RadixCommonDeployComponent) *corev1.ServiceAccount { labels := getComponentServiceAccountLabels(component) - annotations := getComponentServiceAccountAnnotations(component) + name := utils.GetComponentServiceAccountName(component.GetName()) + return buildServiceAccount(namespace, name, labels, component.GetIdentity().GetAzure().GetClientId()) +} + +func buildServiceAccountForOAuthProxyComponent(namespace string, component radixv1.RadixCommonDeployComponent) *corev1.ServiceAccount { + labels := getOAuthProxyComponentServiceAccountLabels(component) + name := utils.GetOAuthProxyServiceAccountName(component.GetName()) + return buildServiceAccount(namespace, name, labels, component.GetAuthentication().GetOAuth2().GetClientID()) +} - sa := corev1.ServiceAccount{ - ObjectMeta: v1.ObjectMeta{ - Name: utils.GetComponentServiceAccountName(component.GetName()), - Namespace: deploy.radixDeployment.Namespace, +func buildServiceAccount(namespace, name string, labels kubelabels.Set, identityClientId string) *corev1.ServiceAccount { + return &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, Labels: labels, - Annotations: annotations, + Annotations: getComponentServiceAccountAnnotations(identityClientId), }, } - - return &sa } func (deploy *Deployment) garbageCollectServiceAccountNoLongerInSpecForComponent(ctx context.Context, component radixv1.RadixCommonDeployComponent) error { - if deploy.isServiceAccountForComponentPreinstalled(component) { + if isServiceAccountForComponentPreinstalled(deploy.radixDeployment, component) { return nil } @@ -89,6 +134,35 @@ func (deploy *Deployment) garbageCollectServiceAccountNoLongerInSpecForComponent return nil } +func garbageCollectServiceAccountNoLongerInSpecForOAuthProxyComponent(ctx context.Context, kubeUtil *kube.Kube, radixDeployment *radixv1.RadixDeployment, component radixv1.RadixCommonDeployComponent) error { + if isServiceAccountForComponentPreinstalled(radixDeployment, component) { + return nil + } + + if component.GetAuthentication().GetOAuth2().GetUseAzureIdentity() { + return nil + } + + return deleteOAuthProxyServiceAccounts(ctx, kubeUtil, radixDeployment.Namespace, component) +} + +func deleteOAuthProxyServiceAccounts(ctx context.Context, kubeUtil *kube.Kube, namespace string, component radixv1.RadixCommonDeployComponent) error { + selector := radixlabels.ForOAuthProxyComponentServiceAccount(component).AsSelector().String() + serviceAccountList, err := kubeUtil.ListServiceAccountsWithSelector(ctx, namespace, selector) + if err != nil { + return err + } + + for _, sa := range serviceAccountList { + if err := kubeUtil.DeleteServiceAccount(ctx, namespace, sa.Name); err != nil { + return err + } + log.Ctx(ctx).Info().Msgf("Deleted serviceAccount: %s in namespace %s", sa.Name, namespace) + } + + return nil +} + func (deploy *Deployment) garbageCollectServiceAccountNoLongerInSpec(ctx context.Context) error { serviceAccounts, err := deploy.kubeutil.ListServiceAccounts(ctx, deploy.radixDeployment.GetNamespace()) if err != nil { @@ -107,7 +181,7 @@ func (deploy *Deployment) garbageCollectServiceAccountNoLongerInSpec(ctx context continue } - if err := deploy.kubeclient.CoreV1().ServiceAccounts(deploy.radixDeployment.GetNamespace()).Delete(ctx, serviceAccount.GetName(), v1.DeleteOptions{}); err != nil { + if err := deploy.kubeclient.CoreV1().ServiceAccounts(deploy.radixDeployment.GetNamespace()).Delete(ctx, serviceAccount.GetName(), metav1.DeleteOptions{}); err != nil { return err } } @@ -118,9 +192,9 @@ func (deploy *Deployment) garbageCollectServiceAccountNoLongerInSpec(ctx context // Is this a component/deployment where the service account is handled elsewhere, // e.g. radix-api and radix-github-webhook -func (deploy *Deployment) isServiceAccountForComponentPreinstalled(component radixv1.RadixCommonDeployComponent) bool { +func isServiceAccountForComponentPreinstalled(radixDeployment *radixv1.RadixDeployment, component radixv1.RadixCommonDeployComponent) bool { isComponent := component.GetType() == radixv1.RadixComponentTypeComponent - return isComponent && (isRadixAPI(deploy.radixDeployment) || isRadixWebHook(deploy.radixDeployment)) + return isComponent && (isRadixAPI(radixDeployment) || isRadixWebHook(radixDeployment)) } func componentRequiresServiceAccount(component radixv1.RadixCommonDeployComponent) bool { @@ -135,6 +209,13 @@ func getComponentServiceAccountLabels(component radixv1.RadixCommonDeployCompone ) } +func getOAuthProxyComponentServiceAccountLabels(component radixv1.RadixCommonDeployComponent) kubelabels.Set { + return radixlabels.Merge( + radixlabels.ForOAuthProxyComponentServiceAccount(component), + radixlabels.ForOauthProxyServiceAccountWithRadixIdentity(component.GetAuthentication().GetOAuth2()), + ) +} + func getComponentServiceAccountIdentifier(componentName string) kubelabels.Set { return radixlabels.Merge( radixlabels.ForComponentName(componentName), @@ -142,11 +223,14 @@ func getComponentServiceAccountIdentifier(componentName string) kubelabels.Set { ) } -func getComponentServiceAccountAnnotations(component radixv1.RadixCommonDeployComponent) map[string]string { - return radixannotations.ForServiceAccountWithRadixIdentity(component.GetIdentity()) +func getComponentServiceAccountAnnotations(identityClientId string) map[string]string { + return radixannotations.ForServiceAccountWithRadixIdentityClientId(identityClientId) } -// isServiceAccountForComponent checks if service account has labels matching labels returned by getComponentServiceAccountIdentifier func isServiceAccountForComponent(sa *corev1.ServiceAccount, componentName string) bool { return getComponentServiceAccountIdentifier(componentName).AsSelector().Matches(kubelabels.Set(sa.Labels)) } + +func isServiceAccountForOAuthProxyComponent(sa *corev1.ServiceAccount, component radixv1.RadixCommonDeployComponent) bool { + return radixlabels.ForOAuthProxyComponentServiceAccount(component).AsSelector().Matches(kubelabels.Set(sa.Labels)) +} diff --git a/pkg/apis/kube/kube.go b/pkg/apis/kube/kube.go index 2dc457b97..6adf51d97 100644 --- a/pkg/apis/kube/kube.go +++ b/pkg/apis/kube/kube.go @@ -83,6 +83,7 @@ const ( RadixPodIsJobAuxObjectLabel = "is-job-aux-object" IsServiceAccountForComponent = "is-service-account-for-component" IsServiceAccountForSubPipelineLabel = "is-service-account-for-subpipeline" + IsServiceAccountForOAuthProxyLabel = "is-service-account-for-oauth-proxy" RadixBatchNameLabel = "radix-batch-name" RadixBatchJobNameLabel = "radix-batch-job-name" RadixBatchTypeLabel = "radix-batch-type" diff --git a/pkg/apis/kube/service_account.go b/pkg/apis/kube/service_account.go index 4650bc298..806e5fe9b 100644 --- a/pkg/apis/kube/service_account.go +++ b/pkg/apis/kube/service_account.go @@ -86,8 +86,7 @@ func (kubeutil *Kube) DeleteServiceAccount(ctx context.Context, namespace, name } else if err != nil { return fmt.Errorf("failed to get service account object: %v", err) } - err = kubeutil.kubeClient.CoreV1().ServiceAccounts(namespace).Delete(ctx, name, metav1.DeleteOptions{}) - if err != nil { + if err = kubeutil.kubeClient.CoreV1().ServiceAccounts(namespace).Delete(ctx, name, metav1.DeleteOptions{}); err != nil { return fmt.Errorf("failed to delete ServiceAccount object: %v", err) } return nil diff --git a/pkg/apis/radix/v1/radixapptypes.go b/pkg/apis/radix/v1/radixapptypes.go index c95236ad3..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. @@ -1378,6 +1388,12 @@ type OAuth2 struct { // Settings for Redis store when SessionStoreType is redis. // +optional RedisStore *OAuth2RedisStore `json:"redisStore,omitempty"` + + // Credentials defines credentials type for authenticating. Default is a Secret, which represents a client secret. + // +kubebuilder:validation:Enum=secret;azureWorkloadIdentity + // +kubebuilder:default:=secret + // +optional + Credentials CredentialsType `json:"credentials,omitempty"` } // OAuth2Cookie defines properties for the oauth cookie. @@ -1886,6 +1902,38 @@ func (component *RadixJobComponent) GetHorizontalScaling() *RadixHorizontalScali return nil } +// GetOAuth2 Returns OAuth2 if exist +func (authentication *Authentication) GetOAuth2() *OAuth2 { + if authentication == nil { + return nil + } + return authentication.OAuth2 +} + +// GetUseAzureIdentity Indicates if the OAuth2 uses the azure identity +func (oauth2 *OAuth2) GetUseAzureIdentity() bool { + if oauth2 == nil { + return false + } + return oauth2.Credentials == AzureWorkloadIdentity +} + +// GetSessionStoreType Returns the session store type +func (oauth2 *OAuth2) GetSessionStoreType() SessionStoreType { + if oauth2 == nil { + return SessionStoreCookie + } + return oauth2.SessionStoreType +} + +// GetClientID Returns the client ID +func (oauth2 *OAuth2) GetClientID() string { + if oauth2 == nil { + return "" + } + return oauth2.ClientID +} + func getEnvironmentConfigByName(environment string, environmentConfigs []RadixCommonEnvironmentConfig) RadixCommonEnvironmentConfig { for _, environmentConfig := range environmentConfigs { if strings.EqualFold(environment, environmentConfig.GetEnvironment()) { diff --git a/pkg/apis/radixvalidators/errors.go b/pkg/apis/radixvalidators/errors.go index 8492f99d0..fbb887619 100644 --- a/pkg/apis/radixvalidators/errors.go +++ b/pkg/apis/radixvalidators/errors.go @@ -544,6 +544,6 @@ func getWebhookErrorWithMessage(err error, message, jobComponentName, environmen return errors.WithMessagef(err, "%s %s in environment %s", message, componentAndEnvironmentNames, environment) } -func MissingAzureIdentityErrorWithMessage(keyVaultName, componentName string) error { +func MissingAzureIdentityForAzureKeyVaultErrorWithMessage(keyVaultName, componentName string) error { return errors.WithMessagef(ErrMissingAzureIdentity, "missing Azure identity for Azure Key Vault %s in the component %s", keyVaultName, componentName) } diff --git a/pkg/apis/radixvalidators/validate_ra.go b/pkg/apis/radixvalidators/validate_ra.go index 067c6bc75..29caf1ff2 100644 --- a/pkg/apis/radixvalidators/validate_ra.go +++ b/pkg/apis/radixvalidators/validate_ra.go @@ -618,7 +618,7 @@ func validateOAuth(oauth *radixv1.OAuth2, component *radixv1.RadixComponent, env } // Validate RedisStore - if oauthWithDefaults.SessionStoreType == radixv1.SessionStoreRedis { + if oauthWithDefaults.GetSessionStoreType() == radixv1.SessionStoreRedis { if redisStore := oauthWithDefaults.RedisStore; redisStore == nil { errors = append(errors, OAuthRedisStoreEmptyErrorWithMessage(componentName, environmentName)) } else if len(strings.TrimSpace(redisStore.ConnectionURL)) == 0 { @@ -1001,7 +1001,7 @@ func validateSecretRefs(commonComponent radixv1.RadixCommonComponent, secretRefs useAzureIdentity := azureKeyVault.UseAzureIdentity if useAzureIdentity != nil && *useAzureIdentity { if !azureIdentityIsSet(commonComponent) { - return MissingAzureIdentityErrorWithMessage(azureKeyVault.Name, commonComponent.GetName()) + return MissingAzureIdentityForAzureKeyVaultErrorWithMessage(azureKeyVault.Name, commonComponent.GetName()) } // TODO: validate for env-chain } diff --git a/pkg/apis/radixvalidators/validate_ra_test.go b/pkg/apis/radixvalidators/validate_ra_test.go index 7f4939f91..b436fa582 100644 --- a/pkg/apis/radixvalidators/validate_ra_test.go +++ b/pkg/apis/radixvalidators/validate_ra_test.go @@ -1939,7 +1939,7 @@ func Test_HorizontalScaling_Validation(t *testing.T) { []error{radixvalidators.ErrMinReplicasGreaterThanMaxReplicas}, }, { - "Copmonent with Trigger config and Resource config should fail", + "Component with Trigger config and Resource config should fail", func(ra *radixv1.RadixApplication) { ra.Spec.Components[0].EnvironmentConfig[0].HorizontalScaling = &radixv1.RadixHorizontalScaling{ MinReplicas: pointers.Ptr(int32(2)), @@ -1957,7 +1957,7 @@ func Test_HorizontalScaling_Validation(t *testing.T) { []error{radixvalidators.ErrCombiningTriggersWithResourcesIsIllegal}, }, { - "Copmonent with 0 replicas is correct when combined with atleast 1 non-resource trigger", + "Component with 0 replicas is correct when combined with atleast 1 non-resource trigger", func(ra *radixv1.RadixApplication) { ra.Spec.Components[0].EnvironmentConfig[0].HorizontalScaling = utils.NewHorizontalScalingBuilder(). WithMinReplicas(0). @@ -1971,7 +1971,7 @@ func Test_HorizontalScaling_Validation(t *testing.T) { nil, }, { - "Copmonent with 0 replicas is invalid with only resource triggers", + "Component with 0 replicas is invalid with only resource triggers", func(ra *radixv1.RadixApplication) { ra.Spec.Components[0].EnvironmentConfig[0].HorizontalScaling = utils.NewHorizontalScalingBuilder(). WithMinReplicas(0). @@ -1983,7 +1983,7 @@ func Test_HorizontalScaling_Validation(t *testing.T) { []error{radixvalidators.ErrInvalidMinimumReplicasConfigurationWithMemoryAndCPUTriggers}, }, { - "Copmonent with multiple definitions in same trigger must fail", + "Component with multiple definitions in same trigger must fail", func(ra *radixv1.RadixApplication) { ra.Spec.Components[0].EnvironmentConfig[0].HorizontalScaling = utils.NewHorizontalScalingBuilder(). WithMinReplicas(1). @@ -1994,7 +1994,7 @@ func Test_HorizontalScaling_Validation(t *testing.T) { []error{radixvalidators.ErrMoreThanOneDefinitionInTrigger}, }, { - "Copmonent with multiple definitions of same type is allowed", + "Component with multiple definitions of same type is allowed", func(ra *radixv1.RadixApplication) { ra.Spec.Components[0].EnvironmentConfig[0].HorizontalScaling = utils.NewHorizontalScalingBuilder(). WithMinReplicas(1). @@ -2006,7 +2006,7 @@ func Test_HorizontalScaling_Validation(t *testing.T) { nil, }, { - "Copmonent must have unique name", + "Component must have unique name", func(ra *radixv1.RadixApplication) { ra.Spec.Components[0].EnvironmentConfig[0].HorizontalScaling = utils.NewHorizontalScalingBuilder(). WithMinReplicas(1). diff --git a/pkg/apis/utils/annotations/annotations.go b/pkg/apis/utils/annotations/annotations.go index b209f0539..1abc4c305 100644 --- a/pkg/apis/utils/annotations/annotations.go +++ b/pkg/apis/utils/annotations/annotations.go @@ -38,20 +38,13 @@ func ForKubernetesDeploymentObservedGeneration(rd *radixv1.RadixDeployment) map[ return map[string]string{kube.RadixDeploymentObservedGeneration: strconv.Itoa(int(rd.ObjectMeta.Generation))} } -// ForServiceAccountWithRadixIdentity returns annotations for configuring a ServiceAccount with external identities, +// ForServiceAccountWithRadixIdentityClientId returns annotations for configuring a ServiceAccount with external identities, // e.g. for Azure Workload Identity: "azure.workload.identity/client-id": "11111111-2222-3333-4444-555555555555" -func ForServiceAccountWithRadixIdentity(identity *radixv1.Identity) map[string]string { - if identity == nil { +func ForServiceAccountWithRadixIdentityClientId(identityClientId string) map[string]string { + if len(identityClientId) == 0 { return nil } - - var annotations map[string]string - - if identity.Azure != nil { - annotations = Merge(annotations, forAzureWorkloadIdentityClientId(identity.Azure.ClientId)) - } - - return annotations + return forAzureWorkloadIdentityClientId(identityClientId) } // ForClusterAutoscalerSafeToEvict returns annotation used by cluster autoscaler to determine if a Pod diff --git a/pkg/apis/utils/annotations/annotations_test.go b/pkg/apis/utils/annotations/annotations_test.go index b3e8da4c1..3531be82b 100644 --- a/pkg/apis/utils/annotations/annotations_test.go +++ b/pkg/apis/utils/annotations/annotations_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/equinor/radix-operator/pkg/apis/kube" - v1 "github.com/equinor/radix-operator/pkg/apis/radix/v1" "github.com/stretchr/testify/assert" ) @@ -30,13 +29,10 @@ func Test_ForRadixDeploymentName(t *testing.T) { } func Test_ForServiceAccountWithRadixIdentity(t *testing.T) { - actual := ForServiceAccountWithRadixIdentity(nil) + actual := ForServiceAccountWithRadixIdentityClientId("") assert.Equal(t, map[string]string(nil), actual) - actual = ForServiceAccountWithRadixIdentity(&v1.Identity{}) - assert.Equal(t, map[string]string(nil), actual) - - actual = ForServiceAccountWithRadixIdentity(&v1.Identity{Azure: &v1.AzureIdentity{ClientId: "anyclientid"}}) + actual = ForServiceAccountWithRadixIdentityClientId("anyclientid") expected := map[string]string{"azure.workload.identity/client-id": "anyclientid"} assert.Equal(t, expected, actual) } diff --git a/pkg/apis/utils/labels/labels.go b/pkg/apis/utils/labels/labels.go index 0229beb5c..c01a8ea26 100644 --- a/pkg/apis/utils/labels/labels.go +++ b/pkg/apis/utils/labels/labels.go @@ -117,13 +117,26 @@ func ForServiceAccountIsForComponent() kubelabels.Set { return kubelabels.Set{ kube.IsServiceAccountForComponent: "true", } -} // ForServiceAccountIsForComponent returns labels indicating that a service account is used by a component or job +} + +// ForServiceAccountIsForSubPipeline returns labels indicating that a service account is used by a subpipeline func ForServiceAccountIsForSubPipeline() kubelabels.Set { return kubelabels.Set{ kube.IsServiceAccountForSubPipelineLabel: "true", } } +// ForOAuthProxyComponentServiceAccount returns labels for configuring a ServiceAccount for an aux OAuth2 proxy +func ForOAuthProxyComponentServiceAccount(component v1.RadixCommonDeployComponent) kubelabels.Set { + return Merge( + kubelabels.Set{ + kube.RadixAuxiliaryComponentLabel: component.GetName(), + kube.RadixAuxiliaryComponentTypeLabel: defaults.OAuthProxyAuxiliaryComponentType, + kube.IsServiceAccountForOAuthProxyLabel: "true", + }, + ) +} + // ForServiceAccountWithRadixIdentity returns labels for configuring a ServiceAccount with external identities, // e.g. for Azure Workload Identity: "azure.workload.identity/use": "true" func ForServiceAccountWithRadixIdentity(identity *v1.Identity) kubelabels.Set { @@ -140,6 +153,15 @@ func ForServiceAccountWithRadixIdentity(identity *v1.Identity) kubelabels.Set { return labels } +// ForOauthProxyServiceAccountWithRadixIdentity returns labels for configuring a ServiceAccount with external identities, +// e.g. for Azure Workload Identity: "azure.workload.identity/use": "true" +func ForOauthProxyServiceAccountWithRadixIdentity(oauth2 *v1.OAuth2) kubelabels.Set { + if !oauth2.GetUseAzureIdentity() { + return nil + } + return forAzureWorkloadUseIdentity() +} + // ForPodWithRadixIdentity returns labels for configuring a Pod with external identities, // e.g. for Azure Workload Identity: "azure.workload.identity/use": "true" func ForPodWithRadixIdentity(identity *v1.Identity) kubelabels.Set { diff --git a/pkg/apis/utils/labels/labels_test.go b/pkg/apis/utils/labels/labels_test.go index d41db6c76..d373e75e5 100644 --- a/pkg/apis/utils/labels/labels_test.go +++ b/pkg/apis/utils/labels/labels_test.go @@ -4,7 +4,7 @@ import ( "testing" "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" ) @@ -54,6 +54,21 @@ func Test_ForServiceAccountWithRadixIdentity(t *testing.T) { assert.Equal(t, expected, actual) } +func Test_ForOAuthProxyServiceAccountWithWorkloadIdentity(t *testing.T) { + actual := ForOauthProxyServiceAccountWithRadixIdentity(nil) + 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 Credentials") + + 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{Credentials: v1.AzureWorkloadIdentity, ClientID: "any-client-id"}) + expected := kubelabels.Set{"azure.workload.identity/use": "true"} + assert.Equal(t, expected, actual, "Expected labels when Credentials is AzureWorkloadIdentity") +} + func Test_ForPodWithRadixIdentity(t *testing.T) { actual := ForPodWithRadixIdentity(nil) assert.Equal(t, kubelabels.Set(nil), actual) diff --git a/pkg/apis/utils/serviceaccount.go b/pkg/apis/utils/serviceaccount.go index d1af775fb..0d8fa3e1d 100644 --- a/pkg/apis/utils/serviceaccount.go +++ b/pkg/apis/utils/serviceaccount.go @@ -1,6 +1,10 @@ package utils -import "fmt" +import ( + "fmt" + + "github.com/equinor/radix-operator/pkg/apis/defaults" +) // GetComponentServiceAccountName Gets unique name for component or job service account func GetComponentServiceAccountName(componentName string) string { @@ -11,3 +15,8 @@ func GetComponentServiceAccountName(componentName string) string { func GetSubPipelineServiceAccountName(environmentName string) string { return fmt.Sprintf("subpipeline-%s-sa", environmentName) } + +// GetOAuthProxyServiceAccountName Gets unique name for OAuth2 proxy service account +func GetOAuthProxyServiceAccountName(componentName string) string { + return fmt.Sprintf("%s-%s-sa", componentName, defaults.OAuthProxyAuxiliaryComponentSuffix) +}