From 3adf07aab4fc45bed027622ffc02c2c5e1edb035 Mon Sep 17 00:00:00 2001 From: Andy Bursavich Date: Thu, 27 Aug 2020 23:23:50 -0700 Subject: [PATCH] add EnvFrom-like VarsFrom secrets and configmaps Fixes #5 --- docs/api.md | 38 +- go.mod | 2 +- manifest/customresourcedefinition.yaml | 32 +- pkg/api/v1alpha1/configmapsecret_types.go | 39 +- pkg/api/v1alpha1/zz_generated.deepcopy.go | 74 ++++ pkg/controllers/configmapsecret_controller.go | 239 ++++++++--- .../configmapsecret_controller_test.go | 388 +++++++++++++++++- pkg/controllers/setup_test.go | 24 +- 8 files changed, 762 insertions(+), 74 deletions(-) diff --git a/docs/api.md b/docs/api.md index ce2c7c2..94a16fa 100644 --- a/docs/api.md +++ b/docs/api.md @@ -9,8 +9,11 @@ * [ConfigMapSecretSpec](#configmapsecretspec) * [ConfigMapSecretStatus](#configmapsecretstatus) * [ConfigMapTemplate](#configmaptemplate) +* [ConfigMapVarsSource](#configmapvarssource) +* [SecretVarsSource](#secretvarssource) * [TemplateMetadata](#templatemetadata) * [TemplateVariable](#templatevariable) +* [VarsFromSource](#varsfromsource) ## ConfigMapSecret @@ -67,6 +70,7 @@ ConfigMapSecretSpec defines the desired state of a ConfigMapSecret. | Field | Description | Type | Required | | ----- | ----------- | ---- | -------- | | template | Template that describes the config that will be rendered.

Variable references $(VAR_NAME) in template data are expanded using the ConfigMapSecret's variables. If a variable cannot be resolved, the reference in the input data will be unchanged. The $(VAR_NAME) syntax can be escaped with a double $$, ie: $$(VAR_NAME). Escaped references will never be expanded, regardless of whether the variable exists or not. | [ConfigMapTemplate](#configmaptemplate) | false | +| varsFrom | List of sources to populate template variables. Keys defined in a source must consist of alphanumeric characters, '-', '_' or '.'. When a key exists in multiple sources, the value associated with the last source will take precedence. Values defined by Vars with a duplicate key will take precedence. | [][VarsFromSource](#varsfromsource) | false | | vars | List of template variables. | [][TemplateVariable](#templatevariable) | false | [Back to TOC](#table-of-contents) @@ -94,6 +98,26 @@ ConfigMapTemplate is a ConfigMap template. [Back to TOC](#table-of-contents) +## ConfigMapVarsSource + +ConfigMapVarsSource selects a ConfigMap to populate template variables with. + +| Field | Description | Type | Required | +| ----- | ----------- | ---- | -------- | +| optional | Specify whether the ConfigMap must be defined. | *bool | false | + +[Back to TOC](#table-of-contents) + +## SecretVarsSource + +SecretVarsSource selects a Secret to populate template variables with. + +| Field | Description | Type | Required | +| ----- | ----------- | ---- | -------- | +| optional | Specify whether the Secret must be defined. | *bool | false | + +[Back to TOC](#table-of-contents) + ## TemplateMetadata TemplateMetadata is a stripped down version of the standard object metadata. @@ -113,8 +137,20 @@ TemplateVariable is a template variable. | Field | Description | Type | Required | | ----- | ----------- | ---- | -------- | | name | Name of the template variable. | string | true | -| value | Variable references $(VAR_NAME) are expanded using the previous defined environment variables in the ConfigMapSecret. If a variable cannot be resolved, the reference in the input string will be unchanged. The $(VAR_NAME) syntax can be escaped with a double $$, ie: $$(VAR_NAME). Escaped references will never be expanded, regardless of whether the variable exists or not.

Defaults to \"\". | string | false | +| value | Variable references $(VAR_NAME) are expanded using the previous defined environment variables in the ConfigMapSecret. If a variable cannot be resolved, the reference in the input string will be unchanged. The $(VAR_NAME) syntax can be escaped with a double $$, ie: $$(VAR_NAME). Escaped references will never be expanded, regardless of whether the variable exists or not. | string | false | | secretValue | SecretValue selects a value by its key in a Secret. | *[corev1.SecretKeySelector](https://pkg.go.dev/k8s.io/api/core/v1#SecretKeySelector) | false | | configMapValue | ConfigMapValue selects a value by its key in a ConfigMap. | *[corev1.ConfigMapKeySelector](https://pkg.go.dev/k8s.io/api/core/v1#ConfigMapKeySelector) | false | [Back to TOC](#table-of-contents) + +## VarsFromSource + +VarsFromSource represents the source of a set of template variables. + +| Field | Description | Type | Required | +| ----- | ----------- | ---- | -------- | +| prefix | An optional identifier to prepend to each key. | string | false | +| secretRef | The Secret to select. | *[SecretVarsSource](#secretvarssource) | false | +| configMapRef | The ConfigMap to select. | *[ConfigMapVarsSource](#configmapvarssource) | false | + +[Back to TOC](#table-of-contents) diff --git a/go.mod b/go.mod index 8cba3ee..d40196d 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/Azure/go-autorest/autorest v0.11.4 // indirect github.com/go-logr/logr v0.2.1-0.20200730175230-ee2de8da5be6 github.com/go-logr/zapr v0.2.0 - github.com/google/go-cmp v0.5.2 // indirect + github.com/google/go-cmp v0.5.2 github.com/google/gofuzz v1.2.0 // indirect github.com/googleapis/gnostic v0.5.1 // indirect github.com/imdario/mergo v0.3.11 // indirect diff --git a/manifest/customresourcedefinition.yaml b/manifest/customresourcedefinition.yaml index 47a9f51..10e4f02 100644 --- a/manifest/customresourcedefinition.yaml +++ b/manifest/customresourcedefinition.yaml @@ -103,12 +103,42 @@ spec: - key type: object value: - description: "Variable references $(VAR_NAME) are expanded using the previous defined environment variables in the ConfigMapSecret. If a variable cannot be resolved, the reference in the input string will be unchanged. The $(VAR_NAME) syntax can be escaped with a double $$, ie: $$(VAR_NAME). Escaped references will never be expanded, regardless of whether the variable exists or not. \n Defaults to \"\"." + description: 'Variable references $(VAR_NAME) are expanded using the previous defined environment variables in the ConfigMapSecret. If a variable cannot be resolved, the reference in the input string will be unchanged. The $(VAR_NAME) syntax can be escaped with a double $$, ie: $$(VAR_NAME). Escaped references will never be expanded, regardless of whether the variable exists or not.' type: string required: - name type: object type: array + varsFrom: + description: List of sources to populate template variables. Keys defined in a source must consist of alphanumeric characters, '-', '_' or '.'. When a key exists in multiple sources, the value associated with the last source will take precedence. Values defined by Vars with a duplicate key will take precedence. + items: + description: VarsFromSource represents the source of a set of template variables. + properties: + configMapRef: + description: The ConfigMap to select. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + optional: + description: Specify whether the ConfigMap must be defined. + type: boolean + type: object + prefix: + description: An optional identifier to prepend to each key. + type: string + secretRef: + description: The Secret to select. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + optional: + description: Specify whether the Secret must be defined. + type: boolean + type: object + type: object + type: array type: object status: description: 'Observed state of the ConfigMapSecret. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status' diff --git a/pkg/api/v1alpha1/configmapsecret_types.go b/pkg/api/v1alpha1/configmapsecret_types.go index c0842be..0293134 100644 --- a/pkg/api/v1alpha1/configmapsecret_types.go +++ b/pkg/api/v1alpha1/configmapsecret_types.go @@ -58,6 +58,13 @@ type ConfigMapSecretSpec struct { // regardless of whether the variable exists or not. Template ConfigMapTemplate `json:"template,omitempty"` + // List of sources to populate template variables. + // Keys defined in a source must consist of alphanumeric characters, '-', '_' or '.'. + // When a key exists in multiple sources, the value associated with the last + // source will take precedence. Values defined by Vars with a duplicate key + // will take precedence. + VarsFrom []VarsFromSource `json:"varsFrom,omitempty"` + // List of template variables. Vars []TemplateVariable `json:"vars,omitempty"` } @@ -116,8 +123,6 @@ type TemplateVariable struct { // the reference in the input string will be unchanged. The $(VAR_NAME) syntax // can be escaped with a double $$, ie: $$(VAR_NAME). Escaped references will // never be expanded, regardless of whether the variable exists or not. - // - // Defaults to "". Value string `json:"value,omitempty"` // SecretValue selects a value by its key in a Secret. @@ -127,6 +132,36 @@ type TemplateVariable struct { ConfigMapValue *corev1.ConfigMapKeySelector `json:"configMapValue,omitempty"` } +// VarsFromSource represents the source of a set of template variables. +type VarsFromSource struct { + // An optional identifier to prepend to each key. + Prefix string `json:"prefix,omitempty"` + + // The Secret to select. + SecretRef *SecretVarsSource `json:"secretRef,omitempty"` + + // The ConfigMap to select. + ConfigMapRef *ConfigMapVarsSource `json:"configMapRef,omitempty"` +} + +// SecretVarsSource selects a Secret to populate template variables with. +type SecretVarsSource struct { + // The Secret to select. + corev1.LocalObjectReference `json:",inline"` + + // Specify whether the Secret must be defined. + Optional *bool `json:"optional,omitempty"` +} + +// ConfigMapVarsSource selects a ConfigMap to populate template variables with. +type ConfigMapVarsSource struct { + // The ConfigMap to select. + corev1.LocalObjectReference `json:",inline"` + + // Specify whether the ConfigMap must be defined. + Optional *bool `json:"optional,omitempty"` +} + // ConfigMapSecretStatus describes the observed state of a ConfigMapSecret. type ConfigMapSecretStatus struct { // The generation observed by the ConfigMapSecret controller. diff --git a/pkg/api/v1alpha1/zz_generated.deepcopy.go b/pkg/api/v1alpha1/zz_generated.deepcopy.go index 26eb44c..6b516b8 100644 --- a/pkg/api/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/api/v1alpha1/zz_generated.deepcopy.go @@ -93,6 +93,13 @@ func (in *ConfigMapSecretList) DeepCopyObject() runtime.Object { func (in *ConfigMapSecretSpec) DeepCopyInto(out *ConfigMapSecretSpec) { *out = *in in.Template.DeepCopyInto(&out.Template) + if in.VarsFrom != nil { + in, out := &in.VarsFrom, &out.VarsFrom + *out = make([]VarsFromSource, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.Vars != nil { in, out := &in.Vars, &out.Vars *out = make([]TemplateVariable, len(*in)) @@ -172,6 +179,48 @@ func (in *ConfigMapTemplate) DeepCopy() *ConfigMapTemplate { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ConfigMapVarsSource) DeepCopyInto(out *ConfigMapVarsSource) { + *out = *in + out.LocalObjectReference = in.LocalObjectReference + if in.Optional != nil { + in, out := &in.Optional, &out.Optional + *out = new(bool) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConfigMapVarsSource. +func (in *ConfigMapVarsSource) DeepCopy() *ConfigMapVarsSource { + if in == nil { + return nil + } + out := new(ConfigMapVarsSource) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SecretVarsSource) DeepCopyInto(out *SecretVarsSource) { + *out = *in + out.LocalObjectReference = in.LocalObjectReference + if in.Optional != nil { + in, out := &in.Optional, &out.Optional + *out = new(bool) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecretVarsSource. +func (in *SecretVarsSource) DeepCopy() *SecretVarsSource { + if in == nil { + return nil + } + out := new(SecretVarsSource) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TemplateMetadata) DeepCopyInto(out *TemplateMetadata) { *out = *in @@ -225,3 +274,28 @@ func (in *TemplateVariable) DeepCopy() *TemplateVariable { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *VarsFromSource) DeepCopyInto(out *VarsFromSource) { + *out = *in + if in.SecretRef != nil { + in, out := &in.SecretRef, &out.SecretRef + *out = new(SecretVarsSource) + (*in).DeepCopyInto(*out) + } + if in.ConfigMapRef != nil { + in, out := &in.ConfigMapRef, &out.ConfigMapRef + *out = new(ConfigMapVarsSource) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VarsFromSource. +func (in *VarsFromSource) DeepCopy() *VarsFromSource { + if in == nil { + return nil + } + out := new(VarsFromSource) + in.DeepCopyInto(out) + return out +} diff --git a/pkg/controllers/configmapsecret_controller.go b/pkg/controllers/configmapsecret_controller.go index b8fcd47..6d7aee5 100644 --- a/pkg/controllers/configmapsecret_controller.go +++ b/pkg/controllers/configmapsecret_controller.go @@ -9,6 +9,8 @@ import ( "errors" "fmt" "reflect" + "sort" + "strings" "sync" "github.com/go-logr/logr" @@ -20,6 +22,8 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation" + "k8s.io/client-go/tools/record" "k8s.io/client-go/util/workqueue" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -37,9 +41,10 @@ var pkgLog = log.Log.WithName("controllers") // ConfigMapSecret reconciles a ConfigMapSecret object type ConfigMapSecret struct { - client client.Client - scheme *runtime.Scheme - logger logr.Logger + client client.Client + scheme *runtime.Scheme + logger logr.Logger + recorder record.EventRecorder mu sync.RWMutex secrets refMap @@ -67,12 +72,19 @@ func (r *ConfigMapSecret) InjectScheme(scheme *runtime.Scheme) error { return nil } +// InjectEventRecorder injects the recorder into the reconciler. +func (r *ConfigMapSecret) InjectEventRecorder(recorder record.EventRecorder) error { + r.recorder = recorder + return nil +} + // SetupWithManager sets up the reconciler with the manager. func (r *ConfigMapSecret) SetupWithManager(manager manager.Manager) error { if r.logger == nil { - if err := r.InjectLogger(pkgLog); err != nil { - return err - } + r.logger = pkgLog + } + if r.recorder == nil { + r.recorder = manager.GetEventRecorderFor("configmapsecret-controller") } return builder.ControllerManagedBy(manager). @@ -177,7 +189,7 @@ func (r *ConfigMapSecret) Reconcile(req reconcile.Request) (reconcile.Result, er return reconcile.Result{}, err } // Set the Secret and ConfigMap references for the instance - secretNames, configMapNames := varRefs(cms.Spec.Vars) + secretNames, configMapNames := varRefs(cms.Spec.VarsFrom, cms.Spec.Vars) r.setRefs(cms.Namespace, cms.Name, secretNames, configMapNames) // Sync and cleanup @@ -360,6 +372,43 @@ func (r *ConfigMapSecret) makeVariables(ctx context.Context, cms *v1alpha1.Confi configMaps := make(map[string]*corev1.ConfigMap) secrets := make(map[string]*corev1.Secret) + for _, v := range cms.Spec.VarsFrom { + var ( + kind, name string + invalidKeys []string + srcVars map[string]string + ) + switch { + case v.SecretRef != nil: + kind = "Secret" + name = v.SecretRef.Name + srcVars, invalidKeys, err = r.secretValues(ctx, secrets, cms.Namespace, v.Prefix, *v.SecretRef) + case v.ConfigMapRef != nil: + kind = "ConfigMap" + name = v.ConfigMapRef.Name + srcVars, invalidKeys, err = r.configMapValues(ctx, configMaps, cms.Namespace, v.Prefix, *v.ConfigMapRef) + } + if err != nil { + return nil, err + } + for k, v := range srcVars { + vars[k] = v + } + if len(invalidKeys) > 0 { + sort.Strings(invalidKeys) + r.recorder.Eventf( + cms, + corev1.EventTypeWarning, + "InvalidTemplateVariableNames", + "Keys [%s] from the VarsFrom %s %s/%s were skipped since they are considered invalid template variable names.", + strings.Join(invalidKeys, ", "), + kind, + cms.Namespace, + name, + ) + } + } + for _, v := range cms.Spec.Vars { val := v.Value found := true @@ -368,9 +417,9 @@ func (r *ConfigMapSecret) makeVariables(ctx context.Context, cms *v1alpha1.Confi case val != "": val = expansion.Expand(val, mappingFn) case v.SecretValue != nil: - val, found, err = r.secretValue(ctx, secrets, cms.Namespace, v.SecretValue) + val, found, err = r.secretValue(ctx, secrets, cms.Namespace, *v.SecretValue) case v.ConfigMapValue != nil: - val, found, err = r.configMapValue(ctx, configMaps, cms.Namespace, v.ConfigMapValue) + val, found, err = r.configMapValue(ctx, configMaps, cms.Namespace, *v.ConfigMapValue) } if err != nil { @@ -386,54 +435,116 @@ func (r *ConfigMapSecret) makeVariables(ctx context.Context, cms *v1alpha1.Confi return vars, nil } -func (r *ConfigMapSecret) secretValue(ctx context.Context, cache map[string]*corev1.Secret, namespace string, ref *corev1.SecretKeySelector) (value string, found bool, err error) { +func (r *ConfigMapSecret) secret(ctx context.Context, cache map[string]*corev1.Secret, namespace string, ref v1alpha1.SecretVarsSource) (secret *corev1.Secret, err error) { name := ref.Name - key := ref.Key - optional := ref.Optional != nil && *ref.Optional - secret, found := cache[name] - if !found { - secret = &corev1.Secret{} - err := r.client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, secret) - if err != nil { - if apierrors.IsNotFound(err) { - if optional { - return "", false, nil - } - return "", false, &configError{err} + if found { + return secret, nil + } + secret = &corev1.Secret{} + err = r.client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, secret) + if err != nil { + if apierrors.IsNotFound(err) { + if ref.Optional != nil && *ref.Optional { + return nil, nil } - return "", false, err + return nil, &configError{err} + } + return nil, err + } + cache[name] = secret + return secret, nil +} + +func (r *ConfigMapSecret) secretValues(ctx context.Context, cache map[string]*corev1.Secret, namespace, prefix string, ref v1alpha1.SecretVarsSource) (values map[string]string, invalidKeys []string, err error) { + secret, err := r.secret(ctx, cache, namespace, ref) + if secret == nil || err != nil { + return nil, nil, err + } + values = make(map[string]string, len(secret.Data)) + for k, v := range secret.Data { + switch k, valid := validPrefixedKey(prefix, k); valid { + case true: + values[k] = string(v) + case false: + invalidKeys = append(invalidKeys, k) } - cache[name] = secret + } + return values, invalidKeys, nil +} + +func (r *ConfigMapSecret) secretValue(ctx context.Context, cache map[string]*corev1.Secret, namespace string, ref corev1.SecretKeySelector) (value string, found bool, err error) { + key := ref.Key + secret, err := r.secret(ctx, cache, namespace, v1alpha1.SecretVarsSource{ + LocalObjectReference: ref.LocalObjectReference, + Optional: ref.Optional, + }) + if secret == nil || err != nil { + return "", false, err } if buf, found := secret.Data[key]; found { return string(buf), true, nil } - if optional { + if ref.Optional != nil && *ref.Optional { return "", false, nil } - return "", false, newConfigError("Couldn't find key %s in Secret %s/%s", key, namespace, name) + return "", false, newConfigError("Couldn't find key %s in Secret %s/%s", key, namespace, ref.Name) } -func (r *ConfigMapSecret) configMapValue(ctx context.Context, cache map[string]*corev1.ConfigMap, namespace string, ref *corev1.ConfigMapKeySelector) (value string, found bool, err error) { +func (r *ConfigMapSecret) configMap(ctx context.Context, cache map[string]*corev1.ConfigMap, namespace string, ref v1alpha1.ConfigMapVarsSource) (configMap *corev1.ConfigMap, err error) { name := ref.Name - key := ref.Key - optional := ref.Optional != nil && *ref.Optional - configMap, found := cache[name] - if !found { - configMap = &corev1.ConfigMap{} - err := r.client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, configMap) - if err != nil { - if apierrors.IsNotFound(err) { - if optional { - return "", false, nil - } - return "", false, &configError{err} + if found { + return configMap, nil + } + configMap = &corev1.ConfigMap{} + err = r.client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, configMap) + if err != nil { + if apierrors.IsNotFound(err) { + if ref.Optional != nil && *ref.Optional { + return nil, nil } - return "", false, err + return nil, &configError{err} } - cache[name] = configMap + return nil, err + } + cache[name] = configMap + return configMap, nil +} + +func (r *ConfigMapSecret) configMapValues(ctx context.Context, cache map[string]*corev1.ConfigMap, namespace, prefix string, ref v1alpha1.ConfigMapVarsSource) (values map[string]string, invalidKeys []string, err error) { + configMap, err := r.configMap(ctx, cache, namespace, ref) + if configMap == nil || err != nil { + return nil, nil, err + } + values = make(map[string]string, len(configMap.Data)+len(configMap.BinaryData)) + for k, v := range configMap.Data { + switch k, valid := validPrefixedKey(prefix, k); valid { + case true: + values[k] = v + case false: + invalidKeys = append(invalidKeys, k) + } + } + for k, v := range configMap.BinaryData { + switch k, valid := validPrefixedKey(prefix, k); valid { + case true: + values[k] = string(v) + case false: + invalidKeys = append(invalidKeys, k) + } + } + return values, invalidKeys, nil +} + +func (r *ConfigMapSecret) configMapValue(ctx context.Context, cache map[string]*corev1.ConfigMap, namespace string, ref corev1.ConfigMapKeySelector) (value string, found bool, err error) { + key := ref.Key + configMap, err := r.configMap(ctx, cache, namespace, v1alpha1.ConfigMapVarsSource{ + LocalObjectReference: ref.LocalObjectReference, + Optional: ref.Optional, + }) + if configMap == nil || err != nil { + return "", false, err } if str, found := configMap.Data[key]; found { return str, true, nil @@ -441,10 +552,10 @@ func (r *ConfigMapSecret) configMapValue(ctx context.Context, cache map[string]* if buf, found := configMap.BinaryData[key]; found { return string(buf), true, nil } - if optional { + if ref.Optional != nil && *ref.Optional { return "", false, nil } - return "", false, newConfigError("Couldn't find key %s in ConfigMap %s/%s", key, namespace, name) + return "", false, newConfigError("Couldn't find key %s in ConfigMap %s/%s", key, namespace, ref.Name) } func (r *ConfigMapSecret) syncSuccessStatus(ctx context.Context, log logr.Logger, cms *v1alpha1.ConfigMapSecret) error { @@ -474,6 +585,16 @@ func (r *ConfigMapSecret) syncStatus(ctx context.Context, log logr.Logger, cms * return nil } +func validPrefixedKey(prefix, key string) (string, bool) { + if prefix != "" { + key = prefix + key + } + if errMsgs := validation.IsEnvVarName(key); len(errMsgs) > 0 { + return key, false + } + return key, true +} + func getOwner(secret *corev1.Secret) *metav1.OwnerReference { owner := metav1.GetControllerOf(secret) if owner == nil || owner.Kind != "ConfigMapSecret" { @@ -510,19 +631,33 @@ func toReqs(namespace string, names map[string]bool) []reconcile.Request { return reqs } -func varRefs(vars []v1alpha1.TemplateVariable) (secrets, configMaps map[string]bool) { +func varRefs(varsFrom []v1alpha1.VarsFromSource, vars []v1alpha1.TemplateVariable) (secrets, configMaps map[string]bool) { + addSecret := func(name string) { + if secrets == nil { + secrets = make(map[string]bool) + } + secrets[name] = true + } + addConfigMap := func(name string) { + if configMaps == nil { + configMaps = make(map[string]bool) + } + configMaps[name] = true + } + for _, v := range varsFrom { + if v.SecretRef != nil { + addSecret(v.SecretRef.Name) + } + if v.ConfigMapRef != nil { + addConfigMap(v.ConfigMapRef.Name) + } + } for _, v := range vars { if v.SecretValue != nil { - if secrets == nil { - secrets = make(map[string]bool) - } - secrets[v.SecretValue.Name] = true + addSecret(v.SecretValue.Name) } if v.ConfigMapValue != nil { - if configMaps == nil { - configMaps = make(map[string]bool) - } - configMaps[v.ConfigMapValue.Name] = true + addConfigMap(v.ConfigMapValue.Name) } } return secrets, configMaps diff --git a/pkg/controllers/configmapsecret_controller_test.go b/pkg/controllers/configmapsecret_controller_test.go index b943979..23ecc9c 100644 --- a/pkg/controllers/configmapsecret_controller_test.go +++ b/pkg/controllers/configmapsecret_controller_test.go @@ -10,13 +10,14 @@ import ( "testing" "time" - "github.com/onsi/gomega" + "github.com/google/go-cmp/cmp" + "github.com/onsi/gomega" // TODO: remove gomega + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" "github.com/machinezone/configmapsecrets/pkg/api/v1alpha1" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" ) const timeout = time.Second * 10 @@ -739,6 +740,357 @@ func TestReconciler(t *testing.T) { parallel: true, }, + { + name: "varsfrom-secrets", + steps: []step{ + createSecretStep(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "varsfrom-secrets-foo", + Namespace: "default", + }, + StringData: map[string]string{ + "FOO": "abc", + "BAR": "ijk", + "BAZ": "pqr", + "QUX": "xyz", + }, + }), + createSecretStep(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "varsfrom-secrets-baz", + Namespace: "default", + }, + StringData: map[string]string{ + "TEST_BAZ": "baz", + }, + }), + createConfigMapSecretStep(&v1alpha1.ConfigMapSecret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "varsfrom-secrets", + Namespace: "default", + }, + Spec: v1alpha1.ConfigMapSecretSpec{ + Template: v1alpha1.ConfigMapTemplate{ + Data: map[string]string{ + "foo": "foo: $(TEST_FOO)", + "bar": "bar: $(TEST_BAR)", + "baz": "baz: $(TEST_BAZ)", + "qux": "qux: $(TEST_QUX)", + }, + }, + VarsFrom: []v1alpha1.VarsFromSource{ + { + Prefix: "TEST_", + SecretRef: &v1alpha1.SecretVarsSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "varsfrom-secrets-foo", + }, + }, + }, + { + SecretRef: &v1alpha1.SecretVarsSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "varsfrom-secrets-baz", + }, + Optional: boolPtr(true), + }, + }, + }, + Vars: []v1alpha1.TemplateVariable{ + { + Name: "TEST_QUX", + Value: "var", + }, + }, + }, + }), + checkSecretStep(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "varsfrom-secrets", + Namespace: "default", + }, + Data: map[string][]byte{ + "foo": []byte("foo: abc"), + "bar": []byte("bar: ijk"), + "baz": []byte("baz: baz"), + "qux": []byte("qux: var"), + }, + }), + checkStatusStep(true, types.NamespacedName{ + Name: "varsfrom-secrets", + Namespace: "default", + }), + }, + subTests: []test{ + { + name: "update-secret", + steps: []step{ + updateSecretStep( + types.NamespacedName{ + Name: "varsfrom-secrets-foo", + Namespace: "default", + }, + func(obj *corev1.Secret) { + obj.Data = nil + obj.StringData = map[string]string{ + "FOO": "abc", + "BAR": "bar", + "BAZ": "pqr", + "QUX": "xyz", + } + }, + ), + checkSecretStep(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "varsfrom-secrets", + Namespace: "default", + }, + Data: map[string][]byte{ + "foo": []byte("foo: abc"), + "bar": []byte("bar: bar"), + "baz": []byte("baz: baz"), + "qux": []byte("qux: var"), + }, + }), + checkStatusStep(true, types.NamespacedName{ + Name: "varsfrom-secrets", + Namespace: "default", + }), + }, + }, + { + name: "delete-optional-secret", + steps: []step{ + deleteSecretStep(types.NamespacedName{ + Name: "varsfrom-secrets-baz", + Namespace: "default", + }), + checkSecretStep(&corev1.Secret{ + // TODO: why is this test flaky? + ObjectMeta: metav1.ObjectMeta{ + Name: "varsfrom-secrets", + Namespace: "default", + }, + Data: map[string][]byte{ + "foo": []byte("foo: abc"), + "bar": []byte("bar: bar"), + "baz": []byte("baz: pqr"), + "qux": []byte("qux: var"), + }, + }), + checkStatusStep(true, types.NamespacedName{ + Name: "varsfrom-secrets", + Namespace: "default", + }), + }, + }, + { + name: "delete-vars", + steps: []step{ + updateConfigMapSecretStep( + types.NamespacedName{ + Name: "varsfrom-secrets", + Namespace: "default", + }, + func(obj *v1alpha1.ConfigMapSecret) { + obj.Spec.Vars = nil + }, + ), + checkSecretStep(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "varsfrom-secrets", + Namespace: "default", + }, + Data: map[string][]byte{ + "foo": []byte("foo: abc"), + "bar": []byte("bar: bar"), + "baz": []byte("baz: pqr"), + "qux": []byte("qux: xyz"), + }, + }), + checkStatusStep(true, types.NamespacedName{ + Name: "varsfrom-secrets", + Namespace: "default", + }), + }, + }, + }, + parallel: true, + }, + + { + name: "varsfrom-configmaps", + steps: []step{ + createConfigMapStep(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "varsfrom-configmaps-foo", + Namespace: "default", + }, + Data: map[string]string{ + "FOO": "abc", + "BAR": "ijk", + }, + BinaryData: map[string][]byte{ + "BAZ": []byte("pqr"), + "QUX": []byte("xyz"), + }, + }), + createConfigMapStep(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "varsfrom-configmaps-baz", + Namespace: "default", + }, + Data: map[string]string{ + "TEST_BAZ": "baz", + }, + }), + createConfigMapSecretStep(&v1alpha1.ConfigMapSecret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "varsfrom-configmaps", + Namespace: "default", + }, + Spec: v1alpha1.ConfigMapSecretSpec{ + Template: v1alpha1.ConfigMapTemplate{ + Data: map[string]string{ + "foo": "foo: $(TEST_FOO)", + "bar": "bar: $(TEST_BAR)", + "baz": "baz: $(TEST_BAZ)", + "qux": "qux: $(TEST_QUX)", + }, + }, + VarsFrom: []v1alpha1.VarsFromSource{ + { + Prefix: "TEST_", + ConfigMapRef: &v1alpha1.ConfigMapVarsSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "varsfrom-configmaps-foo", + }, + }, + }, + { + ConfigMapRef: &v1alpha1.ConfigMapVarsSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "varsfrom-configmaps-baz", + }, + Optional: boolPtr(true), + }, + }, + }, + Vars: []v1alpha1.TemplateVariable{ + { + Name: "TEST_QUX", + Value: "var", + }, + }, + }, + }), + checkSecretStep(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "varsfrom-configmaps", + Namespace: "default", + }, + Data: map[string][]byte{ + "foo": []byte("foo: abc"), + "bar": []byte("bar: ijk"), + "baz": []byte("baz: baz"), + "qux": []byte("qux: var"), + }, + }), + checkStatusStep(true, types.NamespacedName{ + Name: "varsfrom-configmaps", + Namespace: "default", + }), + }, + subTests: []test{ + { + name: "update-secret", + steps: []step{ + updateConfigMapStep( + types.NamespacedName{ + Name: "varsfrom-configmaps-foo", + Namespace: "default", + }, + func(obj *corev1.ConfigMap) { + obj.Data["BAR"] = "bar" + }, + ), + checkSecretStep(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "varsfrom-configmaps", + Namespace: "default", + }, + Data: map[string][]byte{ + "foo": []byte("foo: abc"), + "bar": []byte("bar: bar"), + "baz": []byte("baz: baz"), + "qux": []byte("qux: var"), + }, + }), + checkStatusStep(true, types.NamespacedName{ + Name: "varsfrom-configmaps", + Namespace: "default", + }), + }, + }, + { + name: "delete-optional-configmap", + steps: []step{ + deleteConfigMapStep(types.NamespacedName{ + Name: "varsfrom-configmaps-baz", + Namespace: "default", + }), + checkSecretStep(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "varsfrom-configmaps", + Namespace: "default", + }, + Data: map[string][]byte{ + "foo": []byte("foo: abc"), + "bar": []byte("bar: bar"), + "baz": []byte("baz: pqr"), + "qux": []byte("qux: var"), + }, + }), + checkStatusStep(true, types.NamespacedName{ + Name: "varsfrom-configmaps", + Namespace: "default", + }), + }, + }, + { + name: "delete-vars", + steps: []step{ + updateConfigMapSecretStep( + types.NamespacedName{ + Name: "varsfrom-configmaps", + Namespace: "default", + }, + func(obj *v1alpha1.ConfigMapSecret) { + obj.Spec.Vars = nil + }, + ), + checkSecretStep(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "varsfrom-configmaps", + Namespace: "default", + }, + Data: map[string][]byte{ + "foo": []byte("foo: abc"), + "bar": []byte("bar: bar"), + "baz": []byte("baz: pqr"), + "qux": []byte("qux: xyz"), + }, + }), + checkStatusStep(true, types.NamespacedName{ + Name: "varsfrom-configmaps", + Namespace: "default", + }), + }, + }, + }, + parallel: true, + }, + { name: "render-failure", steps: []step{ @@ -840,10 +1192,14 @@ func checkStatusStep(ok bool, key types.NamespacedName) step { return func(ctx context.Context, t *testing.T, r *testReconciler) { t.Run("check-status", func(t *testing.T) { var obj v1alpha1.ConfigMapSecret - eventually(t, timeout, r.wait(key), func(g *gomega.WithT) { + eventually(t, timeout, r.wait(key), func(t T) { obj = v1alpha1.ConfigMapSecret{} // reset - g.Expect(r.api.Get(ctx, key, &obj)).NotTo(gomega.HaveOccurred(), "Get ConfigMapSecret") - g.Expect(obj.Status.ObservedGeneration).To(gomega.Equal(obj.Generation), "Observed Generation") + if err := r.api.Get(ctx, key, &obj); err != nil { + t.Fatalf("failed to get ConfigMapSecret: %v", err) + } + if gen, obs := obj.Generation, obj.Status.ObservedGeneration; gen != obs { + t.Fatalf("ObservedGeneration doesn't match Generation; %d != %d", obs, gen) + } }) g := gomega.NewWithT(t) g.Expect(obj.Status.Conditions).To(gomega.HaveLen(1), "Conditions") @@ -943,16 +1299,26 @@ func deleteSecretStep(key types.NamespacedName) step { } } +var bytesToString = cmp.Transformer("bytesToString", func(b []byte) string { return string(b) }) + func checkSecretStep(want *corev1.Secret) step { return func(ctx context.Context, t *testing.T, r *testReconciler) { t.Run("check-secret", func(t *testing.T) { key := types.NamespacedName{Name: want.GetName(), Namespace: want.GetNamespace()} - eventually(t, timeout, r.wait(key), func(g *gomega.WithT) { + eventually(t, timeout, r.wait(key), func(t T) { got := &corev1.Secret{} - g.Expect(r.api.Get(ctx, key, got)).NotTo(gomega.HaveOccurred(), "Secret exists") - g.Expect(got.GetLabels()).To(gomega.Equal(want.GetLabels()), "Secret labels match") - g.Expect(got.GetAnnotations()).To(gomega.Equal(want.GetAnnotations()), "Secret annotations match") - g.Expect(got.Data).To(gomega.Equal(want.Data), "Secret data matches") + if err := r.api.Get(ctx, key, got); err != nil { + t.Fatalf("failed to get secret: %v", err) + } + if diff := cmp.Diff(want.Labels, got.Labels); diff != "" { + t.Errorf("unexpected labels diff:\n\n%v", diff) + } + if diff := cmp.Diff(want.Annotations, got.Annotations); diff != "" { + t.Errorf("unexpected annotations diff:\n\n%v", diff) + } + if diff := cmp.Diff(want.Data, got.Data, bytesToString); diff != "" { + t.Errorf("unexpected data diff:\n\n%v", diff) + } }) }) } diff --git a/pkg/controllers/setup_test.go b/pkg/controllers/setup_test.go index 1662173..d0f60a3 100644 --- a/pkg/controllers/setup_test.go +++ b/pkg/controllers/setup_test.go @@ -15,7 +15,7 @@ import ( "github.com/go-logr/zapr" "github.com/machinezone/configmapsecrets/pkg/api/v1alpha1" "github.com/machinezone/configmapsecrets/pkg/mzlog" - "github.com/onsi/gomega" + "github.com/onsi/gomega" // TODO: remove gomega "go.uber.org/zap/zapcore" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -79,6 +79,7 @@ func newTestReconciler(t *testing.T) *testReconciler { ctx, cancel := context.WithCancel(context.TODO()) mgr, err := manager.New(cfg, manager.Options{ Scheme: scheme, + Logger: nil, }) g.Expect(err).NotTo(gomega.HaveOccurred()) @@ -137,7 +138,12 @@ func (r *testReconciler) wait(key types.NamespacedName) <-chan struct{} { return r.waiter(key) } -func eventually(t *testing.T, timeout time.Duration, wait <-chan struct{}, test func(g *gomega.WithT)) { +type T interface { + Errorf(format string, args ...interface{}) + Fatalf(format string, args ...interface{}) +} + +func eventually(t *testing.T, timeout time.Duration, wait <-chan struct{}, test func(t T)) { for { // only final test if succeeds or panics s := &stubT{} @@ -149,7 +155,7 @@ func eventually(t *testing.T, timeout time.Duration, wait <-chan struct{}, test } } }() - test(gomega.NewWithT(s)) + test(s) }() if !s.fail { return // success @@ -161,15 +167,21 @@ func eventually(t *testing.T, timeout time.Duration, wait <-chan struct{}, test timer.Stop() case <-timer.C: // final test: succeed or fail - test(gomega.NewWithT(t)) + test(t) return } } } -type stubT struct{ fail bool } +type stubT struct { + fail bool +} -func (t *stubT) Fatalf(format string, args ...interface{}) { +func (t *stubT) Errorf(format string, args ...interface{}) { t.fail = true +} + +func (t *stubT) Fatalf(format string, args ...interface{}) { + t.Errorf(format, args...) panic(t) }