diff --git a/modules/common/configmap/configmap.go b/modules/common/configmap/configmap.go index 97c5892d..97c9d184 100644 --- a/modules/common/configmap/configmap.go +++ b/modules/common/configmap/configmap.go @@ -199,6 +199,50 @@ func EnsureConfigMaps( return nil } +// CreateOrPatchRawConfigMap creates or patches a ConfigMap from raw data +// (map[string]string) without template rendering. Returns the config hash and +// operation result. This is a simpler alternative to EnsureConfigMaps when the +// caller already has the data and doesn't need template machinery. +// When skipSetOwner is true, no controller reference is set on the ConfigMap. +func CreateOrPatchRawConfigMap( + ctx context.Context, + h *helper.Helper, + obj client.Object, + cm *corev1.ConfigMap, + skipSetOwner bool, +) (string, controllerutil.OperationResult, error) { + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: cm.Name, + Namespace: cm.Namespace, + }, + } + + op, err := controllerutil.CreateOrPatch(ctx, h.GetClient(), configMap, func() error { + configMap.Annotations = util.MergeStringMaps(configMap.Annotations, cm.Annotations) + configMap.Labels = util.MergeStringMaps(configMap.Labels, cm.Labels) + configMap.Data = cm.Data + + if !skipSetOwner { + err := controllerutil.SetControllerReference(obj, configMap, h.GetScheme()) + if err != nil { + return err + } + } + return nil + }) + if err != nil { + return "", op, fmt.Errorf("error create/updating configmap: %w", err) + } + + configMapHash, err := Hash(configMap) + if err != nil { + return "", op, fmt.Errorf("error calculating configuration hash: %w", err) + } + + return configMapHash, op, nil +} + // GetConfigMaps - get all configmaps required, verify they exist and add the hash to env and status func GetConfigMaps( ctx context.Context, diff --git a/modules/common/secret/secret.go b/modules/common/secret/secret.go index 9100ea52..e55b1aed 100644 --- a/modules/common/secret/secret.go +++ b/modules/common/secret/secret.go @@ -156,6 +156,57 @@ func CreateOrPatchSecret( return secretHash, op, err } +// CreateOrPatchSecretPreserve creates a secret on first creation and preserves +// existing Data keys on subsequent reconciles. This is useful for generated +// credentials (passwords, cookies) that should only be set once and not +// rotated on every reconcile. Labels and annotations are always updated. +// When skipSetOwner is true, no controller reference is set on the Secret. +func CreateOrPatchSecretPreserve( + ctx context.Context, + h *helper.Helper, + obj client.Object, + secret *corev1.Secret, + skipSetOwner bool, +) (string, controllerutil.OperationResult, error) { + s := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secret.Name, + Namespace: secret.Namespace, + }, + } + + op, err := controllerutil.CreateOrPatch(ctx, h.GetClient(), s, func() error { + s.Annotations = util.MergeStringMaps(s.Annotations, secret.Annotations) + s.Labels = util.MergeStringMaps(s.Labels, secret.Labels) + + // Only set data on initial creation (when the object has no data yet) + if len(s.Data) == 0 { + s.Immutable = secret.Immutable + s.Type = secret.Type + s.Data = secret.Data + s.StringData = secret.StringData + } + + if !skipSetOwner { + err := controllerutil.SetControllerReference(obj, s, h.GetScheme()) + if err != nil { + return err + } + } + return nil + }) + if err != nil { + return "", op, fmt.Errorf("error create/updating secret: %w", err) + } + + secretHash, err := Hash(s) + if err != nil { + return "", "", fmt.Errorf("error calculating configuration hash: %w", err) + } + + return secretHash, op, err +} + // createOrUpdateSecret - create or update existing secrte if it already exists // finally return configuration hash func createOrUpdateSecret( diff --git a/modules/common/statefulset/container.go b/modules/common/statefulset/container.go new file mode 100644 index 00000000..86db4418 --- /dev/null +++ b/modules/common/statefulset/container.go @@ -0,0 +1,62 @@ +/* +Copyright 2026 Red Hat + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package statefulset + +import ( + corev1 "k8s.io/api/core/v1" +) + +// MergeContainersByName merges desired container specs into existing containers +// matched by name. It starts from the desired container and preserves only the +// server-defaulted fields (TerminationMessagePath, TerminationMessagePolicy, +// ImagePullPolicy) from the existing container. All other fields come from the +// desired spec, which ensures that new fields added in future Kubernetes +// versions are not silently dropped. +// +// When container counts differ or a desired container name is not found in +// existing, the existing slice is replaced with the desired containers. +func MergeContainersByName(existing *[]corev1.Container, desired []corev1.Container) { + if len(*existing) != len(desired) { + *existing = desired + return + } + + existingByName := make(map[string]int, len(*existing)) + for i := range *existing { + existingByName[(*existing)[i].Name] = i + } + + for _, d := range desired { + idx, ok := existingByName[d.Name] + if !ok { + *existing = desired + return + } + // Preserve server-defaulted fields from the existing container + // only when the desired spec doesn't explicitly set them. + if d.ImagePullPolicy == "" { + d.ImagePullPolicy = (*existing)[idx].ImagePullPolicy + } + if d.TerminationMessagePath == "" { + d.TerminationMessagePath = (*existing)[idx].TerminationMessagePath + } + if d.TerminationMessagePolicy == "" { + d.TerminationMessagePolicy = (*existing)[idx].TerminationMessagePolicy + } + (*existing)[idx] = d + } +} diff --git a/modules/common/statefulset/container_test.go b/modules/common/statefulset/container_test.go new file mode 100644 index 00000000..14943199 --- /dev/null +++ b/modules/common/statefulset/container_test.go @@ -0,0 +1,337 @@ +/* +Copyright 2026 Red Hat + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package statefulset + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" +) + +func TestMergeContainersByName(t *testing.T) { + tests := []struct { + name string + existing []corev1.Container + desired []corev1.Container + verify func(t *testing.T, containers []corev1.Container) + }{ + { + name: "successful merge preserves server defaults", + existing: []corev1.Container{ + { + Name: "app", + Image: "old-image:v1", + TerminationMessagePath: "/dev/termination-log", + TerminationMessagePolicy: corev1.TerminationMessageReadFile, + ImagePullPolicy: corev1.PullIfNotPresent, + }, + }, + desired: []corev1.Container{ + { + Name: "app", + Image: "new-image:v2", + Env: []corev1.EnvVar{ + {Name: "FOO", Value: "bar"}, + }, + }, + }, + verify: func(t *testing.T, containers []corev1.Container) { + c := containers[0] + if c.Image != "new-image:v2" { + t.Errorf("Image = %q, want %q", c.Image, "new-image:v2") + } + if len(c.Env) != 1 || c.Env[0].Name != "FOO" { + t.Errorf("Env not merged correctly") + } + // Server defaults should be preserved + if c.TerminationMessagePath != "/dev/termination-log" { + t.Errorf("TerminationMessagePath lost: %q", c.TerminationMessagePath) + } + if c.TerminationMessagePolicy != corev1.TerminationMessageReadFile { + t.Errorf("TerminationMessagePolicy lost: %v", c.TerminationMessagePolicy) + } + if c.ImagePullPolicy != corev1.PullIfNotPresent { + t.Errorf("ImagePullPolicy lost: %v", c.ImagePullPolicy) + } + }, + }, + { + name: "multi-container merge by name not order", + existing: []corev1.Container{ + {Name: "sidecar", Image: "sidecar:v1", ImagePullPolicy: corev1.PullAlways}, + {Name: "main", Image: "main:v1", ImagePullPolicy: corev1.PullIfNotPresent}, + }, + desired: []corev1.Container{ + {Name: "main", Image: "main:v2"}, + {Name: "sidecar", Image: "sidecar:v2"}, + }, + verify: func(t *testing.T, containers []corev1.Container) { + // Order should be preserved (existing order) + if containers[0].Name != "sidecar" || containers[0].Image != "sidecar:v2" { + t.Errorf("sidecar not merged: %+v", containers[0]) + } + if containers[1].Name != "main" || containers[1].Image != "main:v2" { + t.Errorf("main not merged: %+v", containers[1]) + } + // ImagePullPolicy preserved + if containers[0].ImagePullPolicy != corev1.PullAlways { + t.Errorf("sidecar ImagePullPolicy lost") + } + if containers[1].ImagePullPolicy != corev1.PullIfNotPresent { + t.Errorf("main ImagePullPolicy lost") + } + }, + }, + { + name: "merges all operator-controlled fields", + existing: []corev1.Container{ + { + Name: "app", + Image: "old:v1", + ImagePullPolicy: corev1.PullAlways, + }, + }, + desired: []corev1.Container{ + { + Name: "app", + Image: "new:v2", + Command: []string{"/bin/sh"}, + Args: []string{"-c", "echo"}, + Env: []corev1.EnvVar{{Name: "K", Value: "V"}}, + Ports: []corev1.ContainerPort{{ContainerPort: 8080}}, + VolumeMounts: []corev1.VolumeMount{ + {Name: "data", MountPath: "/data"}, + }, + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + }, + }, + LivenessProbe: &corev1.Probe{InitialDelaySeconds: 5}, + ReadinessProbe: &corev1.Probe{InitialDelaySeconds: 10}, + Lifecycle: &corev1.Lifecycle{}, + SecurityContext: &corev1.SecurityContext{}, + }, + }, + verify: func(t *testing.T, containers []corev1.Container) { + c := containers[0] + if c.Image != "new:v2" { + t.Errorf("Image not merged") + } + if len(c.Command) != 1 || c.Command[0] != "/bin/sh" { + t.Errorf("Command not merged") + } + if len(c.Args) != 2 { + t.Errorf("Args not merged") + } + if len(c.Env) != 1 { + t.Errorf("Env not merged") + } + if len(c.Ports) != 1 { + t.Errorf("Ports not merged") + } + if len(c.VolumeMounts) != 1 { + t.Errorf("VolumeMounts not merged") + } + if c.Resources.Limits == nil { + t.Errorf("Resources not merged") + } + if c.LivenessProbe == nil { + t.Errorf("LivenessProbe not merged") + } + if c.ReadinessProbe == nil { + t.Errorf("ReadinessProbe not merged") + } + if c.Lifecycle == nil { + t.Errorf("Lifecycle not merged") + } + if c.SecurityContext == nil { + t.Errorf("SecurityContext not merged") + } + // Server default preserved + if c.ImagePullPolicy != corev1.PullAlways { + t.Errorf("ImagePullPolicy lost") + } + }, + }, + { + name: "desired fields override existing for StartupProbe, WorkingDir, EnvFrom", + existing: []corev1.Container{ + { + Name: "app", + Image: "old:v1", + ImagePullPolicy: corev1.PullAlways, + StartupProbe: &corev1.Probe{InitialDelaySeconds: 15}, + WorkingDir: "/old/dir", + EnvFrom: []corev1.EnvFromSource{ + {Prefix: "OLD_"}, + }, + }, + }, + desired: []corev1.Container{ + { + Name: "app", + Image: "new:v2", + StartupProbe: &corev1.Probe{InitialDelaySeconds: 30}, + WorkingDir: "/new/dir", + EnvFrom: []corev1.EnvFromSource{ + {Prefix: "NEW_"}, + }, + }, + }, + verify: func(t *testing.T, containers []corev1.Container) { + c := containers[0] + if c.Image != "new:v2" { + t.Errorf("Image not merged") + } + if c.StartupProbe == nil || c.StartupProbe.InitialDelaySeconds != 30 { + t.Errorf("StartupProbe should come from desired, got %v", c.StartupProbe) + } + if c.WorkingDir != "/new/dir" { + t.Errorf("WorkingDir should come from desired, got %q", c.WorkingDir) + } + if len(c.EnvFrom) != 1 || c.EnvFrom[0].Prefix != "NEW_" { + t.Errorf("EnvFrom should come from desired, got %v", c.EnvFrom) + } + if c.ImagePullPolicy != corev1.PullAlways { + t.Errorf("ImagePullPolicy should be preserved from existing") + } + }, + }, + { + name: "desired without optional fields clears them from existing", + existing: []corev1.Container{ + { + Name: "app", + Image: "old:v1", + ImagePullPolicy: corev1.PullAlways, + StartupProbe: &corev1.Probe{InitialDelaySeconds: 15}, + WorkingDir: "/old/dir", + EnvFrom: []corev1.EnvFromSource{{Prefix: "OLD_"}}, + VolumeDevices: []corev1.VolumeDevice{{Name: "dev", DevicePath: "/dev/xvda"}}, + }, + }, + desired: []corev1.Container{ + { + Name: "app", + Image: "new:v2", + }, + }, + verify: func(t *testing.T, containers []corev1.Container) { + c := containers[0] + if c.StartupProbe != nil { + t.Errorf("StartupProbe should be nil when not in desired") + } + if c.WorkingDir != "" { + t.Errorf("WorkingDir should be empty when not in desired") + } + if c.EnvFrom != nil { + t.Errorf("EnvFrom should be nil when not in desired") + } + if c.VolumeDevices != nil { + t.Errorf("VolumeDevices should be nil when not in desired") + } + if c.ImagePullPolicy != corev1.PullAlways { + t.Errorf("ImagePullPolicy should be preserved from existing") + } + }, + }, + { + name: "count mismatch falls back to replacement", + existing: []corev1.Container{ + {Name: "app", Image: "old:v1"}, + }, + desired: []corev1.Container{ + {Name: "app", Image: "new:v2"}, + {Name: "sidecar", Image: "sidecar:v1"}, + }, + verify: func(t *testing.T, containers []corev1.Container) { + if len(containers) != 2 { + t.Errorf("expected 2 containers after replacement, got %d", len(containers)) + } + if containers[0].Name != "app" || containers[0].Image != "new:v2" { + t.Errorf("first container not replaced correctly: %+v", containers[0]) + } + if containers[1].Name != "sidecar" || containers[1].Image != "sidecar:v1" { + t.Errorf("second container not replaced correctly: %+v", containers[1]) + } + }, + }, + { + name: "name mismatch falls back to replacement", + existing: []corev1.Container{ + {Name: "app", Image: "old:v1"}, + }, + desired: []corev1.Container{ + {Name: "different", Image: "new:v2"}, + }, + verify: func(t *testing.T, containers []corev1.Container) { + if len(containers) != 1 || containers[0].Name != "different" { + t.Errorf("expected replacement with desired, got %+v", containers) + } + }, + }, + { + name: "desired explicit server-default fields are honored", + existing: []corev1.Container{ + { + Name: "app", + Image: "old:v1", + ImagePullPolicy: corev1.PullIfNotPresent, + TerminationMessagePath: "/dev/termination-log", + TerminationMessagePolicy: corev1.TerminationMessageReadFile, + }, + }, + desired: []corev1.Container{ + { + Name: "app", + Image: "new:v2", + ImagePullPolicy: corev1.PullAlways, + TerminationMessagePath: "/custom/path", + TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError, + }, + }, + verify: func(t *testing.T, containers []corev1.Container) { + c := containers[0] + if c.ImagePullPolicy != corev1.PullAlways { + t.Errorf("ImagePullPolicy = %v, want PullAlways (from desired)", c.ImagePullPolicy) + } + if c.TerminationMessagePath != "/custom/path" { + t.Errorf("TerminationMessagePath = %q, want /custom/path (from desired)", c.TerminationMessagePath) + } + if c.TerminationMessagePolicy != corev1.TerminationMessageFallbackToLogsOnError { + t.Errorf("TerminationMessagePolicy = %v, want FallbackToLogsOnError (from desired)", c.TerminationMessagePolicy) + } + }, + }, + { + name: "empty slices succeed", + existing: []corev1.Container{}, + desired: []corev1.Container{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + MergeContainersByName(&tt.existing, tt.desired) + if tt.verify != nil { + tt.verify(t, tt.existing) + } + }) + } +} diff --git a/modules/common/statefulset/statefulset.go b/modules/common/statefulset/statefulset.go index ed353a1c..04fe21c2 100644 --- a/modules/common/statefulset/statefulset.go +++ b/modules/common/statefulset/statefulset.go @@ -56,33 +56,43 @@ func (s *StatefulSet) CreateOrPatch( } op, err := controllerutil.CreateOrPatch(ctx, h.GetClient(), statefulset, func() error { - // selector and VolumeClaimTemplates are immutable so we set this value only if - // a new object is going to be created - if statefulset.CreationTimestamp.IsZero() { - statefulset.Spec.Selector = s.statefulset.Spec.Selector - statefulset.Spec.VolumeClaimTemplates = s.statefulset.Spec.VolumeClaimTemplates - } - - statefulset.Annotations = util.MergeStringMaps(statefulset.Annotations, s.statefulset.Annotations) statefulset.Labels = util.MergeStringMaps(statefulset.Labels, s.statefulset.Labels) - // We need to copy the Spec field by field as Selector and VolumeClaimTemplates are not updatable - // This list needs to be synced StatefulSet to gain ability to set - // those new fields via lib-common - statefulset.Spec.Replicas = s.statefulset.Spec.Replicas - statefulset.Spec.Template = s.statefulset.Spec.Template - statefulset.Spec.ServiceName = s.statefulset.Spec.ServiceName - statefulset.Spec.PodManagementPolicy = s.statefulset.Spec.PodManagementPolicy - statefulset.Spec.UpdateStrategy = s.statefulset.Spec.UpdateStrategy - statefulset.Spec.RevisionHistoryLimit = s.statefulset.Spec.RevisionHistoryLimit - statefulset.Spec.MinReadySeconds = s.statefulset.Spec.MinReadySeconds - statefulset.Spec.PersistentVolumeClaimRetentionPolicy = s.statefulset.Spec.PersistentVolumeClaimRetentionPolicy - - err := controllerutil.SetControllerReference(h.GetBeforeObject(), statefulset, h.GetScheme()) - if err != nil { - return err + statefulset.Annotations = util.MergeStringMaps(statefulset.Annotations, s.statefulset.Annotations) + + // Selector and VolumeClaimTemplates are immutable after creation. + // Preserve the existing values so the full Spec overwrite below + // does not trigger an API error on update. + if !statefulset.CreationTimestamp.IsZero() { + s.statefulset.Spec.Selector = statefulset.Spec.Selector + s.statefulset.Spec.VolumeClaimTemplates = statefulset.Spec.VolumeClaimTemplates } - return nil + // Save existing containers before overwriting the Spec so we can + // merge them below to preserve server-defaulted fields. + existingContainers := statefulset.Spec.Template.Spec.Containers + existingInitContainers := statefulset.Spec.Template.Spec.InitContainers + + // Overwrite the entire Spec with the desired state. This ensures + // any new Kubernetes fields are picked up automatically without + // needing to add individual field copies. + statefulset.Spec = s.statefulset.Spec + + // Merge containers by name to preserve server-defaulted fields + // (e.g. TerminationMessagePath, ImagePullPolicy) and avoid + // unnecessary reconcile loops. Falls back to full replacement if + // container sets don't match by name. + statefulset.Spec.Template.Spec.Containers = existingContainers + MergeContainersByName( + &statefulset.Spec.Template.Spec.Containers, + s.statefulset.Spec.Template.Spec.Containers, + ) + statefulset.Spec.Template.Spec.InitContainers = existingInitContainers + MergeContainersByName( + &statefulset.Spec.Template.Spec.InitContainers, + s.statefulset.Spec.Template.Spec.InitContainers, + ) + + return controllerutil.SetControllerReference(h.GetBeforeObject(), statefulset, h.GetScheme()) }) if err != nil { if k8s_errors.IsNotFound(err) { diff --git a/modules/common/test/functional/configmap_test.go b/modules/common/test/functional/configmap_test.go new file mode 100644 index 00000000..69f874fd --- /dev/null +++ b/modules/common/test/functional/configmap_test.go @@ -0,0 +1,131 @@ +/* +Copyright 2026 Red Hat + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package functional + +import ( + "github.com/google/uuid" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/openstack-k8s-operators/lib-common/modules/common/configmap" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +var _ = Describe("ConfigMap helpers", func() { + var namespace string + + BeforeEach(func() { + namespace = uuid.New().String() + th.CreateNamespace(namespace) + }) + + When("CreateOrPatchRawConfigMap is called", func() { + It("creates a ConfigMap from raw data", func() { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cm", + Namespace: namespace, + Labels: map[string]string{ + "app": "test", + }, + }, + Data: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + } + hash, op, err := configmap.CreateOrPatchRawConfigMap( + ctx, h, th.CreateNamespace("cm-owner"), cm, false, + ) + Expect(err).NotTo(HaveOccurred()) + Expect(op).To(Equal(controllerutil.OperationResultCreated)) + Expect(hash).NotTo(BeEmpty()) + + got := &corev1.ConfigMap{} + Expect(cClient.Get(ctx, types.NamespacedName{ + Name: "test-cm", + Namespace: namespace, + }, got)).To(Succeed()) + + Expect(got.Data).To(HaveKeyWithValue("key1", "value1")) + Expect(got.Data).To(HaveKeyWithValue("key2", "value2")) + Expect(got.Labels).To(HaveKeyWithValue("app", "test")) + }) + + It("patches an existing ConfigMap with new data", func() { + owner := th.CreateNamespace("cm-patch-owner") + + cm1 := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "patch-cm", + Namespace: namespace, + }, + Data: map[string]string{"key1": "old"}, + } + _, _, err := configmap.CreateOrPatchRawConfigMap( + ctx, h, owner, cm1, false, + ) + Expect(err).NotTo(HaveOccurred()) + + cm2 := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "patch-cm", + Namespace: namespace, + }, + Data: map[string]string{"key1": "new", "key2": "added"}, + } + hash2, op, err := configmap.CreateOrPatchRawConfigMap( + ctx, h, owner, cm2, false, + ) + Expect(err).NotTo(HaveOccurred()) + Expect(op).To(Equal(controllerutil.OperationResultUpdated)) + Expect(hash2).NotTo(BeEmpty()) + + got := &corev1.ConfigMap{} + Expect(cClient.Get(ctx, types.NamespacedName{ + Name: "patch-cm", + Namespace: namespace, + }, got)).To(Succeed()) + + Expect(got.Data).To(HaveKeyWithValue("key1", "new")) + Expect(got.Data).To(HaveKeyWithValue("key2", "added")) + }) + + It("returns consistent hash for same data", func() { + owner := th.CreateNamespace("cm-hash-owner") + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hash-cm", + Namespace: namespace, + }, + Data: map[string]string{"k": "v"}, + } + + hash1, _, err := configmap.CreateOrPatchRawConfigMap( + ctx, h, owner, cm, false, + ) + Expect(err).NotTo(HaveOccurred()) + + hash2, _, err := configmap.CreateOrPatchRawConfigMap( + ctx, h, owner, cm, false, + ) + Expect(err).NotTo(HaveOccurred()) + Expect(hash2).To(Equal(hash1)) + }) + }) +}) diff --git a/modules/common/test/functional/secret_test.go b/modules/common/test/functional/secret_test.go new file mode 100644 index 00000000..d7903da6 --- /dev/null +++ b/modules/common/test/functional/secret_test.go @@ -0,0 +1,144 @@ +/* +Copyright 2026 Red Hat + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package functional + +import ( + "github.com/google/uuid" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/openstack-k8s-operators/lib-common/modules/common/secret" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +var _ = Describe("Secret helpers", func() { + var namespace string + + BeforeEach(func() { + namespace = uuid.New().String() + th.CreateNamespace(namespace) + }) + + When("CreateOrPatchSecretPreserve is called", func() { + It("creates a secret with initial data", func() { + owner := th.CreateNamespace("secret-owner") + s := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: namespace, + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + "password": []byte("initial-password"), + }, + } + + hash, op, err := secret.CreateOrPatchSecretPreserve(ctx, h, owner, s, false) + Expect(err).NotTo(HaveOccurred()) + Expect(op).To(Equal(controllerutil.OperationResultCreated)) + Expect(hash).NotTo(BeEmpty()) + + got := &corev1.Secret{} + Expect(cClient.Get(ctx, types.NamespacedName{ + Name: "test-secret", + Namespace: namespace, + }, got)).To(Succeed()) + Expect(got.Data).To(HaveKeyWithValue("password", []byte("initial-password"))) + }) + + It("preserves existing data on subsequent calls", func() { + owner := th.CreateNamespace("secret-preserve-owner") + + // First call: create with initial data + s1 := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "preserve-secret", + Namespace: namespace, + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + "password": []byte("original-password"), + }, + } + _, _, err := secret.CreateOrPatchSecretPreserve(ctx, h, owner, s1, false) + Expect(err).NotTo(HaveOccurred()) + + // Second call: try to update with different data + s2 := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "preserve-secret", + Namespace: namespace, + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + "password": []byte("new-password-should-be-ignored"), + }, + } + _, op, err := secret.CreateOrPatchSecretPreserve(ctx, h, owner, s2, false) + Expect(err).NotTo(HaveOccurred()) + // No change since data was preserved + Expect(op).To(Equal(controllerutil.OperationResultNone)) + + // Verify the original data is preserved + got := &corev1.Secret{} + Expect(cClient.Get(ctx, types.NamespacedName{ + Name: "preserve-secret", + Namespace: namespace, + }, got)).To(Succeed()) + Expect(got.Data).To(HaveKeyWithValue("password", []byte("original-password"))) + }) + + It("adds new labels while preserving existing ones and data", func() { + owner := th.CreateNamespace("secret-labels-owner") + + s1 := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "labels-secret", + Namespace: namespace, + Labels: map[string]string{"version": "v1"}, + }, + Data: map[string][]byte{"key": []byte("value")}, + } + _, _, err := secret.CreateOrPatchSecretPreserve(ctx, h, owner, s1, false) + Expect(err).NotTo(HaveOccurred()) + + s2 := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "labels-secret", + Namespace: namespace, + Labels: map[string]string{"new-label": "yes"}, + }, + Data: map[string][]byte{"key": []byte("new-value")}, + } + _, _, err = secret.CreateOrPatchSecretPreserve(ctx, h, owner, s2, false) + Expect(err).NotTo(HaveOccurred()) + + got := &corev1.Secret{} + Expect(cClient.Get(ctx, types.NamespacedName{ + Name: "labels-secret", + Namespace: namespace, + }, got)).To(Succeed()) + + // Existing labels preserved, new labels added (MergeStringMaps behavior) + Expect(got.Labels).To(HaveKeyWithValue("version", "v1")) + Expect(got.Labels).To(HaveKeyWithValue("new-label", "yes")) + // Data should be preserved (original value) + Expect(got.Data).To(HaveKeyWithValue("key", []byte("value"))) + }) + }) +})