From 03254f7427142a28a6815b463dfa52eb35cf80f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Kalke?= <56382792+MichalKalke@users.noreply.github.com> Date: Thu, 11 Dec 2025 14:16:23 +0100 Subject: [PATCH] Cleanup dockerRegistry status when buildless enabled (#2116) --- .../controllers/serverless_controller_test.go | 5 - .../operator/controllers/testhelper_test.go | 42 --- .../operator/internal/state/registry.go | 12 +- .../operator/internal/state/registry_test.go | 279 ------------------ 4 files changed, 9 insertions(+), 329 deletions(-) delete mode 100644 components/operator/internal/state/registry_test.go diff --git a/components/operator/controllers/serverless_controller_test.go b/components/operator/controllers/serverless_controller_test.go index 219aaf993..aa6c1c0a2 100644 --- a/components/operator/controllers/serverless_controller_test.go +++ b/components/operator/controllers/serverless_controller_test.go @@ -138,11 +138,6 @@ func shouldCreateServerless(h testHelper, serverlessName, serverlessDeploymentNa } func shouldPropagateSpecProperties(h testHelper, expected serverlessData) { - Eventually(h.createCheckRegistrySecretFunc(serverlessRegistrySecret, expected.registrySecretData)). - WithPolling(time.Second * 2). - WithTimeout(time.Second * 10). - Should(BeTrue()) - Eventually(h.createCheckOptionalDependenciesFunc(serverlessDeploymentName, expected)). WithPolling(time.Second * 2). WithTimeout(time.Second * 10). diff --git a/components/operator/controllers/testhelper_test.go b/components/operator/controllers/testhelper_test.go index 4fdb59474..ea5242a57 100644 --- a/components/operator/controllers/testhelper_test.go +++ b/components/operator/controllers/testhelper_test.go @@ -306,25 +306,6 @@ func (d *registrySecretData) toMap() map[string]string { return result } -func (h *testHelper) createCheckRegistrySecretFunc(serverlessRegistrySecret string, expected registrySecretData) func() (bool, error) { - return func() (bool, error) { - var configurationSecret corev1.Secret - - if ok, err := h.getKubernetesObjectFunc( - serverlessRegistrySecret, &configurationSecret); !ok || err != nil { - return ok, err - } - if err := secretContainsSameValues( - expected.toMap(), configurationSecret); err != nil { - return false, err - } - if err := secretContainsRequired(configurationSecret); err != nil { - return false, err - } - return true, nil - } -} - func (h *testHelper) createCheckOptionalDependenciesFunc(deploymentName string, expected serverlessData) func() (bool, error) { return func() (bool, error) { var deploy appsv1.Deployment @@ -369,26 +350,3 @@ func deploymentContainsEnv(deployment appsv1.Deployment, name, value string) err return fmt.Errorf("env %s does not exist", name) } - -func secretContainsRequired(configurationSecret corev1.Secret) error { - for _, k := range []string{"username", "password", "registryAddress", "serverAddress"} { - _, ok := configurationSecret.Data[k] - if !ok { - return fmt.Errorf("values not propagated (%s is required)", k) - } - } - return nil -} - -func secretContainsSameValues(expected map[string]string, configurationSecret corev1.Secret) error { - for k, expectedV := range expected { - v, okV := configurationSecret.Data[k] - if okV == false { - return fmt.Errorf("values not propagated (%s: nil != %s )", k, expectedV) - } - if expectedV != string(v) { - return fmt.Errorf("values not propagated (%s: %s != %s )", k, string(v), expectedV) - } - } - return nil -} diff --git a/components/operator/internal/state/registry.go b/components/operator/internal/state/registry.go index dd63625cc..786d15c65 100644 --- a/components/operator/internal/state/registry.go +++ b/components/operator/internal/state/registry.go @@ -37,6 +37,13 @@ func sFnRegistryConfiguration(ctx context.Context, r *reconciler, s *systemState func configureRegistry(ctx context.Context, r *reconciler, s *systemState) error { + //TODO: This is a temporary solution, delete it when legacy serverless is removed + if !isLegacyEnabled(s.instance.Annotations) { + setK3dRegistryConfig(s) + s.instance.Status.DockerRegistry = "" + return nil + } + switch { case isRegistrySecretName(s.instance.Spec.DockerRegistry): // case: use secret from secretName field @@ -131,9 +138,8 @@ func setExternalRegistryConfig(ctx context.Context, r *reconciler, s *systemStat } func setK3dRegistryConfig(s *systemState) { - if isLegacyEnabled(s.instance.Annotations) { - s.instance.Status.DockerRegistry = v1alpha1.DefaultServerAddress - } + s.instance.Status.DockerRegistry = v1alpha1.DefaultServerAddress + s.flagsBuilder.WithRegistryEnableInternal( getEnableInternal(s.instance.Spec.DockerRegistry), ).WithRegistryAddresses( diff --git a/components/operator/internal/state/registry_test.go b/components/operator/internal/state/registry_test.go deleted file mode 100644 index 5c4d0c50d..000000000 --- a/components/operator/internal/state/registry_test.go +++ /dev/null @@ -1,279 +0,0 @@ -package state - -import ( - "context" - "fmt" - "testing" - - "github.com/kyma-project/serverless/components/operator/api/v1alpha1" - "github.com/kyma-project/serverless/components/operator/internal/chart" - "github.com/kyma-project/serverless/components/operator/internal/warning" - "github.com/stretchr/testify/require" - "go.uber.org/zap" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/ptr" - "sigs.k8s.io/controller-runtime/pkg/client/fake" -) - -func Test_sFnRegistryConfiguration(t *testing.T) { - t.Run("no internal registry status", func(t *testing.T) { - s := &systemState{ - instance: v1alpha1.Serverless{ - Spec: v1alpha1.ServerlessSpec{ - DockerRegistry: &v1alpha1.DockerRegistry{ - EnableInternal: ptr.To[bool](true), - }, - }, - }, - statusSnapshot: v1alpha1.ServerlessStatus{ - DockerRegistry: "", - }, - flagsBuilder: chart.NewFlagsBuilder(), - } - r := &reconciler{ - k8s: k8s{client: fake.NewClientBuilder().Build()}, - log: zap.NewNop().Sugar(), - } - expectedFlags := map[string]interface{}{ - "dockerRegistry": map[string]interface{}{ - "enableInternal": true, - }, - "global": map[string]interface{}{ - "registryNodePort": int64(32_137), - }, - } - - next, result, err := sFnRegistryConfiguration(context.Background(), r, s) - require.NoError(t, err) - require.Nil(t, result) - requireEqualFunc(t, sFnOptionalDependencies, next) - - flags, err := s.flagsBuilder.Build() - require.NoError(t, err) - - require.EqualValues(t, expectedFlags, flags) - require.Equal(t, "", s.instance.Status.DockerRegistry) - require.Equal(t, v1alpha1.StateProcessing, s.instance.Status.State) - }) - - t.Run("internal registry and update", func(t *testing.T) { - s := &systemState{ - instance: v1alpha1.Serverless{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - buildlessModeAnnotation: buildlessModeDisabled, - }, - }, - Spec: v1alpha1.ServerlessSpec{ - DockerRegistry: &v1alpha1.DockerRegistry{ - EnableInternal: ptr.To[bool](true), - }, - }, - }, - statusSnapshot: v1alpha1.ServerlessStatus{ - DockerRegistry: "", - }, - flagsBuilder: chart.NewFlagsBuilder(), - } - r := &reconciler{ - k8s: k8s{client: fake.NewClientBuilder().Build()}, - log: zap.NewNop().Sugar(), - } - expectedFlags := map[string]interface{}{ - "dockerRegistry": map[string]interface{}{ - "enableInternal": true, - }, - "global": map[string]interface{}{ - "registryNodePort": int64(32_137), - }, - } - - next, result, err := sFnRegistryConfiguration(context.Background(), r, s) - require.NoError(t, err) - require.Nil(t, result) - requireEqualFunc(t, sFnOptionalDependencies, next) - - flags, err := s.flagsBuilder.Build() - require.NoError(t, err) - - require.EqualValues(t, expectedFlags, flags) - require.Equal(t, "internal", s.instance.Status.DockerRegistry) - require.Equal(t, v1alpha1.StateProcessing, s.instance.Status.State) - }) - - t.Run("external registry and go to next state", func(t *testing.T) { - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-secret", - Namespace: "kyma-test", - }, - Data: map[string][]byte{ - "username": []byte("username"), - "password": []byte("password"), - "registryAddress": []byte("registryAddress"), - "serverAddress": []byte("serverAddress"), - }, - } - s := &systemState{ - instance: v1alpha1.Serverless{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "kyma-test", - }, - Spec: v1alpha1.ServerlessSpec{ - DockerRegistry: &v1alpha1.DockerRegistry{ - EnableInternal: ptr.To[bool](false), - SecretName: ptr.To[string]("test-secret"), - }, - }, - }, - statusSnapshot: v1alpha1.ServerlessStatus{ - DockerRegistry: string(secret.Data["serverAddress"]), - }, - flagsBuilder: chart.NewFlagsBuilder(), - } - r := &reconciler{ - k8s: k8s{ - client: fake.NewClientBuilder(). - WithRuntimeObjects(secret). - Build(), - }, - } - expectedFlags := map[string]interface{}{ - "dockerRegistry": map[string]interface{}{ - "enableInternal": false, - "username": string(secret.Data["username"]), - "password": string(secret.Data["password"]), - "registryAddress": string(secret.Data["registryAddress"]), - "serverAddress": string(secret.Data["serverAddress"]), - }, - } - - next, result, err := sFnRegistryConfiguration(context.Background(), r, s) - require.NoError(t, err) - require.Nil(t, result) - requireEqualFunc(t, sFnOptionalDependencies, next) - - flags, err := s.flagsBuilder.Build() - require.NoError(t, err) - - require.Equal(t, expectedFlags, flags) - require.Equal(t, string(secret.Data["serverAddress"]), s.instance.Status.DockerRegistry) - require.Equal(t, v1alpha1.StateProcessing, s.instance.Status.State) - }) - - t.Run("k3d registry and update", func(t *testing.T) { - s := &systemState{ - instance: v1alpha1.Serverless{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "some-namespace", - Annotations: map[string]string{ - buildlessModeAnnotation: buildlessModeDisabled, - }, - }, - Spec: v1alpha1.ServerlessSpec{ - DockerRegistry: &v1alpha1.DockerRegistry{ - EnableInternal: ptr.To[bool](false), - }, - }, - }, - statusSnapshot: v1alpha1.ServerlessStatus{ - DockerRegistry: "", - }, - flagsBuilder: chart.NewFlagsBuilder(), - } - r := &reconciler{ - k8s: k8s{ - client: fake.NewClientBuilder().Build(), - }, - } - expectedFlags := map[string]interface{}{ - "dockerRegistry": map[string]interface{}{ - "enableInternal": false, - "registryAddress": v1alpha1.DefaultRegistryAddress, - "serverAddress": v1alpha1.DefaultRegistryAddress, - }, - } - - next, result, err := sFnRegistryConfiguration(context.Background(), r, s) - require.NoError(t, err) - require.Nil(t, result) - requireEqualFunc(t, sFnOptionalDependencies, next) - - flags, err := s.flagsBuilder.Build() - require.NoError(t, err) - - require.Equal(t, expectedFlags, flags) - require.Equal(t, v1alpha1.DefaultRegistryAddress, s.instance.Status.DockerRegistry) - }) - - t.Run("external registry secret not found error", func(t *testing.T) { - s := &systemState{ - instance: v1alpha1.Serverless{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "some-namespace", - }, - Spec: v1alpha1.ServerlessSpec{ - DockerRegistry: &v1alpha1.DockerRegistry{ - EnableInternal: ptr.To[bool](false), - SecretName: ptr.To[string]("test-secret-not-found"), - }, - }, - }, - } - r := &reconciler{ - k8s: k8s{ - client: fake.NewClientBuilder().Build(), - }, - } - - next, result, err := sFnRegistryConfiguration(context.Background(), r, s) - require.EqualError(t, err, "secrets \"test-secret-not-found\" not found") - require.Nil(t, result) - require.Nil(t, next) - - status := s.instance.Status - require.Equal(t, v1alpha1.StateError, status.State) - requireContainsCondition(t, status, - v1alpha1.ConditionTypeConfigured, - metav1.ConditionFalse, - v1alpha1.ConditionReasonConfigurationErr, - "secrets \"test-secret-not-found\" not found", - ) - }) -} - -func Test_addRegistryConfigurationWarnings(t *testing.T) { - - t.Run("enable internal is true and secret name exists", func(t *testing.T) { - s := &systemState{ - warningBuilder: warning.NewBuilder(), - instance: v1alpha1.Serverless{ - Spec: v1alpha1.ServerlessSpec{ - DockerRegistry: &v1alpha1.DockerRegistry{ - EnableInternal: ptr.To[bool](true), - SecretName: ptr.To[string]("test-secret"), - }, - }, - }, - } - addRegistryConfigurationWarnings(s) - require.Equal(t, fmt.Sprintf("Warning: %s", internalEnabledAndSecretNameUsedMessage), s.warningBuilder.Build()) - }) - - t.Run("do not build warning", func(t *testing.T) { - s := &systemState{ - warningBuilder: warning.NewBuilder(), - instance: v1alpha1.Serverless{ - Spec: v1alpha1.ServerlessSpec{ - DockerRegistry: &v1alpha1.DockerRegistry{ - EnableInternal: ptr.To[bool](false), - SecretName: ptr.To[string]("serverless-registry-config"), - }, - }, - }, - } - addRegistryConfigurationWarnings(s) - require.Equal(t, "", s.warningBuilder.Build()) - }) -}