From 0a8677743b7325ccaa0df7606e4ca9dc522a11bb Mon Sep 17 00:00:00 2001 From: Sergey Smolnikov Date: Fri, 8 Nov 2024 16:09:02 +0100 Subject: [PATCH 1/2] fix-public-dns (#701) * Get component env-vars from kube deployment instead of operator code * Fixed unit-tests * Filtered away secrets --- api/defaults/environment_variables.go | 6 -- api/deployments/models/deployment_builder.go | 1 - .../env_vars_controller_test.go | 1 + api/environmentvariables/env_vars_handler.go | 23 ++++++-- .../env_vars_handler_test.go | 59 ++++++++++++------- go.mod | 2 +- go.sum | 4 +- 7 files changed, 62 insertions(+), 34 deletions(-) delete mode 100644 api/defaults/environment_variables.go diff --git a/api/defaults/environment_variables.go b/api/defaults/environment_variables.go deleted file mode 100644 index aedaa91c..00000000 --- a/api/defaults/environment_variables.go +++ /dev/null @@ -1,6 +0,0 @@ -package defaults - -const ( - // RadixDNSZoneEnvironmentVariable The environment variable on a radix app giving the dns zone. Will be equal to OperatorDNSZoneEnvironmentVariable - RadixDNSZoneEnvironmentVariable = "RADIX_DNS_ZONE" -) diff --git a/api/deployments/models/deployment_builder.go b/api/deployments/models/deployment_builder.go index 138210ed..1dec6f3c 100644 --- a/api/deployments/models/deployment_builder.go +++ b/api/deployments/models/deployment_builder.go @@ -46,7 +46,6 @@ func NewDeploymentBuilder() DeploymentBuilder { func (b *deploymentBuilder) WithRadixDeployment(rd *v1.RadixDeployment) DeploymentBuilder { jobName := rd.Labels[kube.RadixJobNameLabel] - b.withComponentSummariesFromRadixDeployment(rd). withEnvironment(rd.Spec.Environment). withNamespace(rd.GetNamespace()). diff --git a/api/environmentvariables/env_vars_controller_test.go b/api/environmentvariables/env_vars_controller_test.go index 550fe632..f1808f96 100644 --- a/api/environmentvariables/env_vars_controller_test.go +++ b/api/environmentvariables/env_vars_controller_test.go @@ -31,6 +31,7 @@ import ( const ( clusterName = "AnyClusterName" + clusterType = "development" appName = "any-app" environmentName = "dev" componentName = "backend" diff --git a/api/environmentvariables/env_vars_handler.go b/api/environmentvariables/env_vars_handler.go index 4f5b84d8..5cfd23b5 100644 --- a/api/environmentvariables/env_vars_handler.go +++ b/api/environmentvariables/env_vars_handler.go @@ -6,14 +6,15 @@ import ( "sort" "strings" - "github.com/equinor/radix-operator/pkg/apis/deployment" - "github.com/rs/zerolog/log" - envvarsmodels "github.com/equinor/radix-api/api/environmentvariables/models" "github.com/equinor/radix-api/models" + "github.com/equinor/radix-common/utils/slice" "github.com/equinor/radix-operator/pkg/apis/kube" v1 "github.com/equinor/radix-operator/pkg/apis/radix/v1" crdUtils "github.com/equinor/radix-operator/pkg/apis/utils" + "github.com/rs/zerolog/log" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" ) @@ -68,7 +69,7 @@ func (eh *envVarsHandler) GetComponentEnvVars(ctx context.Context, appName strin if err != nil { return nil, err } - envVars, err := deployment.GetEnvironmentVariables(ctx, eh.kubeUtil, appName, rd, radixDeployComponent) + envVars, err := eh.getDeploymentEnvironmentVariables(ctx, appName, rd.Spec.Environment, radixDeployComponent) if err != nil { return nil, err } @@ -164,6 +165,20 @@ func (eh *envVarsHandler) ChangeEnvVar(ctx context.Context, appName, envName, co return eh.kubeUtil.ApplyEnvVarsMetadataConfigMap(ctx, namespace, envVarsMetadataConfigMap, envVarsMetadataMap) } +func (eh *envVarsHandler) getDeploymentEnvironmentVariables(ctx context.Context, appName, envName string, radixDeployComponent v1.RadixCommonDeployComponent) ([]corev1.EnvVar, error) { + deployment, err := eh.accounts.UserAccount.Client.AppsV1().Deployments(crdUtils.GetEnvironmentNamespace(appName, envName)).Get(ctx, radixDeployComponent.GetName(), metav1.GetOptions{}) + if err != nil { + return nil, err + } + componentContainer, ok := slice.FindFirst(deployment.Spec.Template.Spec.Containers, func(container corev1.Container) bool { return container.Name == radixDeployComponent.GetName() }) + if !ok { + return nil, fmt.Errorf("container %s not found in deployment", radixDeployComponent.GetName()) + } + return slice.FindAll(componentContainer.Env, func(envVar corev1.EnvVar) bool { + return envVar.ValueFrom == nil || envVar.ValueFrom.SecretKeyRef == nil + }), nil +} + func getComponent(rd *v1.RadixDeployment, componentName string) v1.RadixCommonDeployComponent { for _, component := range rd.Spec.Components { if strings.EqualFold(component.Name, componentName) { diff --git a/api/environmentvariables/env_vars_handler_test.go b/api/environmentvariables/env_vars_handler_test.go index bd70b7fe..0b339c94 100644 --- a/api/environmentvariables/env_vars_handler_test.go +++ b/api/environmentvariables/env_vars_handler_test.go @@ -6,6 +6,8 @@ import ( envvarsmodels "github.com/equinor/radix-api/api/environmentvariables/models" "github.com/equinor/radix-api/models" + "github.com/equinor/radix-common/utils/slice" + "github.com/equinor/radix-operator/pkg/apis/defaults" "github.com/equinor/radix-operator/pkg/apis/kube" operatorutils "github.com/equinor/radix-operator/pkg/apis/utils" "github.com/stretchr/testify/assert" @@ -30,7 +32,7 @@ func Test_GetEnvVars(t *testing.T) { handler := envVarsHandler{ kubeUtil: commonTestUtils.GetKubeUtil(), inClusterClient: nil, - accounts: models.Accounts{}, + accounts: models.Accounts{UserAccount: models.Account{Client: kubeClient}}, } _, err = kubeUtil.GetConfigMap(context.Background(), namespace, kube.GetEnvVarsConfigMapName(componentName)) @@ -43,11 +45,14 @@ func Test_GetEnvVars(t *testing.T) { assert.NoError(t, err) assert.NotEmpty(t, envVars) - assert.Len(t, envVars, 2) - assert.Equal(t, "VAR1", envVars[0].Name) - assert.Equal(t, "val1", envVars[0].Value) - assert.Equal(t, "VAR2", envVars[1].Name) - assert.Equal(t, "val2", envVars[1].Value) + envVarMap := convertToMap(envVars) + assert.Equal(t, "val1", envVarMap["VAR1"], "Invalid or missing VAR1") + assert.Equal(t, "val2", envVarMap["VAR2"], "Invalid or missing VAR2") + _, existsSecret1 := envVarMap["SECRET1"] + assert.False(t, existsSecret1, "Unexpected secret SECRET1 among env-vars") + _, existsSecret2 := envVarMap["SECRET2"] + assert.False(t, existsSecret2, "Unexpected secret SECRET2 among env-vars") + assertRadixEnvVars(t, envVarMap) }) } @@ -69,7 +74,7 @@ func Test_ChangeGetEnvVars(t *testing.T) { handler := envVarsHandler{ kubeUtil: commonTestUtils.GetKubeUtil(), inClusterClient: nil, - accounts: models.Accounts{}, + accounts: models.Accounts{UserAccount: models.Account{Client: kubeClient}}, } _, err = kubeUtil.GetConfigMap(context.Background(), namespace, kube.GetEnvVarsConfigMapName(componentName)) @@ -95,13 +100,11 @@ func Test_ChangeGetEnvVars(t *testing.T) { envVars, err := handler.GetComponentEnvVars(context.Background(), appName, environmentName, componentName) assert.NoError(t, err) assert.NotEmpty(t, envVars) - assert.Len(t, envVars, 3) - assert.Equal(t, "VAR1", envVars[0].Name) - assert.Equal(t, "val1", envVars[0].Value) - assert.Equal(t, "VAR2", envVars[1].Name) - assert.Equal(t, "new-val2", envVars[1].Value) - assert.Equal(t, "VAR3", envVars[2].Name) - assert.Equal(t, "new-val3", envVars[2].Value) + envVarMap := convertToMap(envVars) + assert.Equal(t, "val1", envVarMap["VAR1"], "Invalid or missing VAR1") + assert.Equal(t, "new-val2", envVarMap["VAR2"], "Invalid or missing VAR2") + assert.Equal(t, "new-val3", envVarMap["VAR3"], "Invalid or missing VAR3") + assertRadixEnvVars(t, envVarMap) }) t.Run("Skipped changing not-existing env vars", func(t *testing.T) { t.Parallel() @@ -118,7 +121,7 @@ func Test_ChangeGetEnvVars(t *testing.T) { handler := envVarsHandler{ kubeUtil: commonTestUtils.GetKubeUtil(), inClusterClient: nil, - accounts: models.Accounts{}, + accounts: models.Accounts{UserAccount: models.Account{Client: kubeClient}}, } _, err = kubeUtil.GetConfigMap(context.Background(), namespace, kube.GetEnvVarsConfigMapName(componentName)) @@ -144,10 +147,26 @@ func Test_ChangeGetEnvVars(t *testing.T) { envVars, err := handler.GetComponentEnvVars(context.Background(), appName, environmentName, componentName) assert.NoError(t, err) assert.NotEmpty(t, envVars) - assert.Len(t, envVars, 2) - assert.Equal(t, "VAR1", envVars[0].Name) - assert.Equal(t, "val1", envVars[0].Value) - assert.Equal(t, "VAR2", envVars[1].Name) - assert.Equal(t, "new-val2", envVars[1].Value) + envVarMap := convertToMap(envVars) + assert.Equal(t, "val1", envVarMap["VAR1"], "Invalid or missing VAR1") + assert.Equal(t, "new-val2", envVarMap["VAR2"], "Invalid or missing VAR2") + assertRadixEnvVars(t, envVarMap) }) } + +func convertToMap(envVars []envvarsmodels.EnvVar) map[string]string { + return slice.Reduce(envVars, make(map[string]string), func(acc map[string]string, envVar envvarsmodels.EnvVar) map[string]string { + acc[envVar.Name] = envVar.Value + return acc + }) +} + +func assertRadixEnvVars(t *testing.T, envVarMap map[string]string) { + assert.Equal(t, appName, envVarMap[defaults.RadixAppEnvironmentVariable], "Invalid or missing RADIX_APP env-var") + assert.Equal(t, environmentName, envVarMap[defaults.EnvironmentnameEnvironmentVariable], "Invalid or missing RADIX_ENVIRONMENT env-var") + assert.Equal(t, clusterName, envVarMap[defaults.ClusternameEnvironmentVariable], "Invalid or missing RADIX_CLUSTERNAME env-var") + assert.Equal(t, clusterType, envVarMap[defaults.RadixClusterTypeEnvironmentVariable], "Invalid or missing RADIX_CLUSTER_TYPE env-var") + assert.Equal(t, componentName, envVarMap[defaults.RadixComponentEnvironmentVariable], "Invalid or missing RADIX_COMPONENT env-var") + assert.Equal(t, "any.container.registry", envVarMap[defaults.ContainerRegistryEnvironmentVariable], "Invalid or missing RADIX_CONTAINER_REGISTRY env-var") + assert.Equal(t, "dev.radix.equinor.com", envVarMap[defaults.RadixDNSZoneEnvironmentVariable], "Invalid or missing RADIX_DNS_ZONE env-var") +} diff --git a/go.mod b/go.mod index ca7f705a..59911614 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/cert-manager/cert-manager v1.15.0 github.com/equinor/radix-common v1.9.5 github.com/equinor/radix-job-scheduler v1.11.0 - github.com/equinor/radix-operator v1.64.0 + github.com/equinor/radix-operator v1.65.1 github.com/evanphx/json-patch/v5 v5.9.0 github.com/felixge/httpsnoop v1.0.4 github.com/golang-jwt/jwt/v5 v5.2.1 diff --git a/go.sum b/go.sum index 1c01ef61..076fe307 100644 --- a/go.sum +++ b/go.sum @@ -89,8 +89,8 @@ github.com/equinor/radix-common v1.9.5 h1:p1xldkYUoavwIMguoxxOyVkOXLPA6K8qMsgzez github.com/equinor/radix-common v1.9.5/go.mod h1:+g0Wj0D40zz29DjNkYKVmCVeYy4OsFWKI7Qi9rA6kpY= github.com/equinor/radix-job-scheduler v1.11.0 h1:8wCmXOVl/1cto8q2WJQEE06Cw68/QmfoifYVR49vzkY= github.com/equinor/radix-job-scheduler v1.11.0/go.mod h1:yPXn3kDcMY0Z3kBkosjuefsdY1x2g0NlBeybMmHz5hc= -github.com/equinor/radix-operator v1.64.0 h1:KbP0ZAX8zZSNzgejc7oOo087RkdrlTpBg4myk7zs48o= -github.com/equinor/radix-operator v1.64.0/go.mod h1:uRW9SgVZ94hkpq87npVv2YVviRuXNJ1zgCleya1uvr8= +github.com/equinor/radix-operator v1.65.1 h1:3lcjWEktQREFj4x1RYUUk9TrQvDQqIN3sJSv+XZEYtA= +github.com/equinor/radix-operator v1.65.1/go.mod h1:bYXKrNGkeRL7ctqj4fwytRFsgHj4XR5ZRks7wXwqjXo= github.com/evanphx/json-patch v5.9.0+incompatible h1:fBXyNpNMuTTDdquAq/uisOr2lShz4oaXpDTX2bLe7ls= github.com/evanphx/json-patch v5.9.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= github.com/evanphx/json-patch/v5 v5.9.0 h1:kcBlZQbplgElYIlo/n1hJbls2z/1awpXxpRi0/FOJfg= From e0551d1f5e19c42c8a03abdad3a96e2fd8072a00 Mon Sep 17 00:00:00 2001 From: Sergey Smolnikov Date: Mon, 11 Nov 2024 10:07:30 +0100 Subject: [PATCH 2/2] Restored missing job scheduler aux pod (#703) --- api/models/component.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/models/component.go b/api/models/component.go index 23113a6f..f67d75d3 100644 --- a/api/models/component.go +++ b/api/models/component.go @@ -13,6 +13,7 @@ import ( "github.com/equinor/radix-api/api/utils/predicate" "github.com/equinor/radix-api/api/utils/tlsvalidation" "github.com/equinor/radix-common/utils/slice" + "github.com/equinor/radix-operator/pkg/apis/kube" radixv1 "github.com/equinor/radix-operator/pkg/apis/radix/v1" operatorutils "github.com/equinor/radix-operator/pkg/apis/utils" radixlabels "github.com/equinor/radix-operator/pkg/apis/utils/labels" @@ -59,11 +60,12 @@ func buildComponent( WithHorizontalScalingSummary(GetHpaSummary(ra.Name, radixComponent.GetName(), hpaList, scaledObjects)). WithExternalDNS(getComponentExternalDNS(ra.Name, radixComponent, secretList, certs, certRequests, tlsValidator)) - componentPods := slice.FindAll(podList, predicate.IsPodForComponent(ra.Name, radixComponent.GetName())) var kd *appsv1.Deployment if depl, ok := slice.FindFirst(deploymentList, predicate.IsDeploymentForComponent(ra.Name, radixComponent.GetName())); ok { kd = &depl } + componentPods := append(slice.FindAll(podList, predicate.IsPodForComponent(ra.Name, radixComponent.GetName())), + slice.FindAll(podList, predicate.IsPodForAuxComponent(ra.Name, radixComponent.GetName(), kube.RadixJobTypeManagerAux))...) if rd.Status.ActiveTo.IsZero() { builder.WithPodNames(slice.Map(componentPods, func(pod corev1.Pod) string { return pod.Name }))