From 2104a9601340baee5267a02040d98c8d4820cdd8 Mon Sep 17 00:00:00 2001 From: Martin Linkhorst Date: Mon, 5 Feb 2024 17:16:30 +0100 Subject: [PATCH] add accessor functions to simply working with configuration resources (#576) --- cmd/e2e/basic_test.go | 8 ++++---- cmd/e2e/broken_stack_test.go | 8 ++++---- controller/stack_resources.go | 18 ++++++++---------- controller/stackset.go | 13 ++----------- pkg/apis/zalando.org/v1/types.go | 23 +++++++++++++++++++++++ 5 files changed, 41 insertions(+), 29 deletions(-) diff --git a/cmd/e2e/basic_test.go b/cmd/e2e/basic_test.go index fd20f36d..8cf45d89 100644 --- a/cmd/e2e/basic_test.go +++ b/cmd/e2e/basic_test.go @@ -376,8 +376,8 @@ func verifyStack(t *testing.T, stacksetName, currentVersion string, stacksetSpec for _, rsc := range stacksetSpec.StackTemplate.Spec.ConfigurationResources { // Verify ConfigMaps - if rsc.ConfigMapRef.Name != "" { - configMap, err := waitForConfigMap(t, rsc.ConfigMapRef.Name) + if rsc.IsConfigMap() { + configMap, err := waitForConfigMap(t, rsc.GetName()) require.NoError(t, err) assert.EqualValues(t, stackResourceLabels, configMap.Labels) assert.Contains(t, configMap.Name, stack.Name) @@ -393,8 +393,8 @@ func verifyStack(t *testing.T, stacksetName, currentVersion string, stacksetSpec } // Verify Secrets - if rsc.SecretRef.Name != "" { - secret, err := waitForSecret(t, rsc.SecretRef.Name) + if rsc.IsSecret() { + secret, err := waitForSecret(t, rsc.GetName()) require.NoError(t, err) assert.EqualValues(t, stackResourceLabels, secret.Labels) assert.Contains(t, secret.Name, stack.Name) diff --git a/cmd/e2e/broken_stack_test.go b/cmd/e2e/broken_stack_test.go index 7b2b13f7..76890fe4 100644 --- a/cmd/e2e/broken_stack_test.go +++ b/cmd/e2e/broken_stack_test.go @@ -110,8 +110,8 @@ func TestBrokenStackWithConfigMaps(t *testing.T) { unhealthyStack := fmt.Sprintf("%s-%s", stacksetName, unhealthyVersion) spec = factory.Create(t, unhealthyVersion) for _, cr := range spec.StackTemplate.Spec.ConfigurationResources { - if cr.ConfigMapRef.Name != "" { - err := configMapInterface().Delete(context.Background(), cr.ConfigMapRef.Name, metav1.DeleteOptions{}) + if cr.IsConfigMap() { + err := configMapInterface().Delete(context.Background(), cr.GetName(), metav1.DeleteOptions{}) require.NoError(t, err) } } @@ -190,8 +190,8 @@ func TestBrokenStackWithSecrets(t *testing.T) { unhealthyStack := fmt.Sprintf("%s-%s", stacksetName, unhealthyVersion) spec = factory.Create(t, unhealthyVersion) for _, cr := range spec.StackTemplate.Spec.ConfigurationResources { - if cr.SecretRef.Name != "" { - err := secretInterface().Delete(context.Background(), cr.SecretRef.Name, metav1.DeleteOptions{}) + if cr.IsSecret() { + err := secretInterface().Delete(context.Background(), cr.GetName(), metav1.DeleteOptions{}) require.NoError(t, err) } } diff --git a/controller/stack_resources.go b/controller/stack_resources.go index a68dd422..2cb9e2cd 100644 --- a/controller/stack_resources.go +++ b/controller/stack_resources.go @@ -325,17 +325,16 @@ func (c *StackSetController) ReconcileStackConfigMap( updateObjMeta func(*metav1.ObjectMeta) *metav1.ObjectMeta, ) error { for _, rsc := range stack.Spec.ConfigurationResources { - if rsc.ConfigMapRef.Name == "" { + if !rsc.IsConfigMap() { continue } - rscName := rsc.ConfigMapRef.Name - if err := validateConfigurationResourceName(stack.Name, rscName); err != nil { + if err := validateConfigurationResourceName(stack.Name, rsc.GetName()); err != nil { return err } configMap, err := c.client.CoreV1().ConfigMaps(stack.Namespace). - Get(ctx, rscName, metav1.GetOptions{}) + Get(ctx, rsc.GetName(), metav1.GetOptions{}) if err != nil { return err } @@ -344,7 +343,7 @@ func (c *StackSetController) ReconcileStackConfigMap( for _, owner := range configMap.OwnerReferences { if owner.UID != stack.UID { return fmt.Errorf("ConfigMap already owned by other resource. "+ - "ConfigMap: %s, Stack: %s", rscName, stack.Name) + "ConfigMap: %s, Stack: %s", rsc.GetName(), stack.Name) } } continue @@ -388,17 +387,16 @@ func (c *StackSetController) ReconcileStackSecret( updateObjMeta func(*metav1.ObjectMeta) *metav1.ObjectMeta, ) error { for _, rsc := range stack.Spec.ConfigurationResources { - if rsc.SecretRef.Name == "" { + if !rsc.IsSecret() { continue } - rscName := rsc.SecretRef.Name - if err := validateConfigurationResourceName(stack.Name, rscName); err != nil { + if err := validateConfigurationResourceName(stack.Name, rsc.GetName()); err != nil { return err } secret, err := c.client.CoreV1().Secrets(stack.Namespace). - Get(ctx, rscName, metav1.GetOptions{}) + Get(ctx, rsc.GetName(), metav1.GetOptions{}) if err != nil { return err } @@ -407,7 +405,7 @@ func (c *StackSetController) ReconcileStackSecret( for _, owner := range secret.OwnerReferences { if owner.UID != stack.UID { return fmt.Errorf("Secret already owned by other resource. "+ - "Secret: %s, Stack: %s", rscName, stack.Name) + "Secret: %s, Stack: %s", rsc.GetName(), stack.Name) } } continue diff --git a/controller/stackset.go b/controller/stackset.go index cee50d7a..cfb54164 100644 --- a/controller/stackset.go +++ b/controller/stackset.go @@ -1505,17 +1505,8 @@ func resourceReadyTime(timestamp time.Time, ttl time.Duration) bool { // name is not prefixed by Stack name. func validateAllConfigurationResourcesNames(stack *zv1.Stack) error { for _, rsc := range stack.Spec.ConfigurationResources { - var rscName string - if rsc.ConfigMapRef.Name != "" { - rscName = rsc.ConfigMapRef.Name - } - - if rsc.SecretRef.Name != "" { - rscName = rsc.SecretRef.Name - } - - if !strings.HasPrefix(rscName, stack.Name) { - return fmt.Errorf(configurationResourceNameError, rscName, stack.Name) + if !strings.HasPrefix(rsc.GetName(), stack.Name) { + return fmt.Errorf(configurationResourceNameError, rsc.GetName(), stack.Name) } } return nil diff --git a/pkg/apis/zalando.org/v1/types.go b/pkg/apis/zalando.org/v1/types.go index eae7bb4c..2e8d2aad 100644 --- a/pkg/apis/zalando.org/v1/types.go +++ b/pkg/apis/zalando.org/v1/types.go @@ -444,6 +444,29 @@ type ConfigurationResourcesSpec struct { SecretRef v1.LocalObjectReference `json:"secretRef,omitempty"` } +// GetName returns the name of the ConfigurationResourcesSpec. +func (crs *ConfigurationResourcesSpec) GetName() string { + if crs.IsConfigMap() { + return crs.ConfigMapRef.Name + } + + if crs.IsSecret() { + return crs.SecretRef.Name + } + + return "" +} + +// IsConfigMap returns true if the ConfigurationResourcesSpec is a ConfigMap. +func (crs *ConfigurationResourcesSpec) IsConfigMap() bool { + return crs.ConfigMapRef.Name != "" +} + +// IsSecret returns true if the ConfigurationResourcesSpec is a Secret. +func (crs *ConfigurationResourcesSpec) IsSecret() bool { + return crs.SecretRef.Name != "" +} + // StackSpecInternal is the spec part of the Stack, including `ingress` and // `routegroup` specs inherited from the parent StackSet. // +k8s:deepcopy-gen=true