From 2174a67575b3e48dd523eb729909a07b8f8bced1 Mon Sep 17 00:00:00 2001 From: Scott Andrews Date: Tue, 16 May 2023 16:06:55 -0400 Subject: [PATCH] Make ExternalSecret a provisioned service (#2263) The Service Binding for Kubernetes project (servicebinding.io) is a spec to make it easier for workloads to consume services. At runtime, the ServiceBinding resource references a service resources and workload resource to connect to the service. The Secret for a service is projected into a workload resource at a well known path. Services can advertise the name of the Secret representing the service on it's status at `.status.binding.name`. Hosting the name of a Secret at this location is the Provisioned Service duck type. It has the effect of decoupling the logical consumption of a service from the physical Secret holding state. Using ServiceBindings with ExternalSecrets today requires the user to directly know and reference the Secret created by the ExternalSecret as the service reference. This PR adds the name of the Secret to the status of the ExternalSecret at a well known location where it is be discovered by a ServiceBinding. With this change, user can reference an ExternalSecret from a ServiceBinding. A ClusterRole is also added with a well known label for the ServiceBinding controller to have permission to watch ExternalSecrets and read the binding Secret. ClusterExternalSecret was not modified as ServiceBindings are limited to the scope of a single namespace. Signed-off-by: Scott Andrews --- .../externalsecret_conversion_test.go | 6 +++++ .../v1alpha1/externalsecret_types.go | 3 +++ .../v1alpha1/zz_generated.deepcopy.go | 1 + .../v1beta1/externalsecret_types.go | 3 +++ .../v1beta1/zz_generated.deepcopy.go | 1 + .../external-secrets.io_externalsecrets.yaml | 20 ++++++++++++++++ deploy/charts/external-secrets/README.md | 1 + .../external-secrets/templates/rbac.yaml | 19 +++++++++++++++ deploy/charts/external-secrets/values.yaml | 4 ++++ deploy/crds/bundle.yaml | 16 +++++++++++++ design/design-crd-spec.md | 3 +++ docs/api/spec.md | 13 ++++++++++ docs/introduction/getting-started.md | 2 ++ .../externalsecret_controller.go | 6 +++++ .../externalsecret_controller_test.go | 24 ++++++++++++++++++- 15 files changed, 121 insertions(+), 1 deletion(-) diff --git a/apis/externalsecrets/v1alpha1/externalsecret_conversion_test.go b/apis/externalsecrets/v1alpha1/externalsecret_conversion_test.go index 171047872..67af85f99 100644 --- a/apis/externalsecrets/v1alpha1/externalsecret_conversion_test.go +++ b/apis/externalsecrets/v1alpha1/externalsecret_conversion_test.go @@ -44,6 +44,9 @@ func newExternalSecretV1Alpha1() *ExternalSecret { Message: "...why wouldn't it be?", }, }, + Binding: corev1.LocalObjectReference{ + Name: "test-target", + }, }, Spec: ExternalSecretSpec{ SecretStoreRef: SecretStoreRef{ @@ -126,6 +129,9 @@ func newExternalSecretV1Beta1() *esv1beta1.ExternalSecret { Message: "...why wouldn't it be?", }, }, + Binding: corev1.LocalObjectReference{ + Name: "test-target", + }, }, Spec: esv1beta1.ExternalSecretSpec{ SecretStoreRef: esv1beta1.SecretStoreRef{ diff --git a/apis/externalsecrets/v1alpha1/externalsecret_types.go b/apis/externalsecrets/v1alpha1/externalsecret_types.go index 1b27a67fc..db70af978 100644 --- a/apis/externalsecrets/v1alpha1/externalsecret_types.go +++ b/apis/externalsecrets/v1alpha1/externalsecret_types.go @@ -222,6 +222,9 @@ type ExternalSecretStatus struct { // +optional Conditions []ExternalSecretStatusCondition `json:"conditions,omitempty"` + + // Binding represents a servicebinding.io Provisioned Service reference to the secret + Binding corev1.LocalObjectReference `json:"binding,omitempty"` } // +kubebuilder:object:root=true diff --git a/apis/externalsecrets/v1alpha1/zz_generated.deepcopy.go b/apis/externalsecrets/v1alpha1/zz_generated.deepcopy.go index 35d7cba8b..56ee4cd3b 100644 --- a/apis/externalsecrets/v1alpha1/zz_generated.deepcopy.go +++ b/apis/externalsecrets/v1alpha1/zz_generated.deepcopy.go @@ -574,6 +574,7 @@ func (in *ExternalSecretStatus) DeepCopyInto(out *ExternalSecretStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + out.Binding = in.Binding } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExternalSecretStatus. diff --git a/apis/externalsecrets/v1beta1/externalsecret_types.go b/apis/externalsecrets/v1beta1/externalsecret_types.go index 0505abc21..39c527c95 100644 --- a/apis/externalsecrets/v1beta1/externalsecret_types.go +++ b/apis/externalsecrets/v1beta1/externalsecret_types.go @@ -411,6 +411,9 @@ type ExternalSecretStatus struct { // +optional Conditions []ExternalSecretStatusCondition `json:"conditions,omitempty"` + + // Binding represents a servicebinding.io Provisioned Service reference to the secret + Binding corev1.LocalObjectReference `json:"binding,omitempty"` } // +kubebuilder:object:root=true diff --git a/apis/externalsecrets/v1beta1/zz_generated.deepcopy.go b/apis/externalsecrets/v1beta1/zz_generated.deepcopy.go index 666e9e032..a8e98e4aa 100644 --- a/apis/externalsecrets/v1beta1/zz_generated.deepcopy.go +++ b/apis/externalsecrets/v1beta1/zz_generated.deepcopy.go @@ -915,6 +915,7 @@ func (in *ExternalSecretStatus) DeepCopyInto(out *ExternalSecretStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + out.Binding = in.Binding } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExternalSecretStatus. diff --git a/config/crds/bases/external-secrets.io_externalsecrets.yaml b/config/crds/bases/external-secrets.io_externalsecrets.yaml index 173b59860..950fb1d0b 100644 --- a/config/crds/bases/external-secrets.io_externalsecrets.yaml +++ b/config/crds/bases/external-secrets.io_externalsecrets.yaml @@ -226,6 +226,16 @@ spec: type: object status: properties: + binding: + description: Binding represents a servicebinding.io Provisioned Service + reference to the secret + 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 + type: object + x-kubernetes-map-type: atomic conditions: items: properties: @@ -657,6 +667,16 @@ spec: type: object status: properties: + binding: + description: Binding represents a servicebinding.io Provisioned Service + reference to the secret + 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 + type: object + x-kubernetes-map-type: atomic conditions: items: properties: diff --git a/deploy/charts/external-secrets/README.md b/deploy/charts/external-secrets/README.md index 8ad7f9d6b..30e14d7eb 100644 --- a/deploy/charts/external-secrets/README.md +++ b/deploy/charts/external-secrets/README.md @@ -121,6 +121,7 @@ The command removes all the Kubernetes components associated with the chart and | prometheus.enabled | bool | `false` | deprecated. will be removed with 0.7.0, use serviceMonitor instead. | | prometheus.service.port | int | `8080` | deprecated. will be removed with 0.7.0, use serviceMonitor instead. | | rbac.create | bool | `true` | Specifies whether role and rolebinding resources should be created. | +| rbac.servicebindings.create | bool | `true` | Specifies whether a clusterrole to give servicebindings read access should be created. | | replicaCount | int | `1` | | | resources | object | `{}` | | | revisionHistoryLimit | int | `10` | Specifies the amount of historic ReplicaSets k8s should keep (see https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#clean-up-policy) | diff --git a/deploy/charts/external-secrets/templates/rbac.yaml b/deploy/charts/external-secrets/templates/rbac.yaml index 99433d4dd..da5d648ca 100644 --- a/deploy/charts/external-secrets/templates/rbac.yaml +++ b/deploy/charts/external-secrets/templates/rbac.yaml @@ -272,4 +272,23 @@ subjects: - kind: ServiceAccount name: {{ include "external-secrets.serviceAccountName" . }} namespace: {{ .Release.Namespace | quote }} +{{- if .Values.rbac.servicebindings.create }} +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: {{ include "external-secrets.fullname" . }}-servicebindings + labels: + servicebinding.io/controller: "true" + {{- include "external-secrets.labels" . | nindent 4 }} +rules: + - apiGroups: + - "external-secrets.io" + resources: + - "externalsecrets" + verbs: + - "get" + - "list" + - "watch" +{{- end }} {{- end }} diff --git a/deploy/charts/external-secrets/values.yaml b/deploy/charts/external-secrets/values.yaml index 20dc6614a..fa3cfc87f 100644 --- a/deploy/charts/external-secrets/values.yaml +++ b/deploy/charts/external-secrets/values.yaml @@ -80,6 +80,10 @@ rbac: # -- Specifies whether role and rolebinding resources should be created. create: true + servicebindings: + # -- Specifies whether a clusterrole to give servicebindings read access should be created. + create: true + ## -- Extra environment variables to add to container. extraEnv: [] diff --git a/deploy/crds/bundle.yaml b/deploy/crds/bundle.yaml index adaa5e611..5228f1853 100644 --- a/deploy/crds/bundle.yaml +++ b/deploy/crds/bundle.yaml @@ -3377,6 +3377,14 @@ spec: type: object status: properties: + binding: + description: Binding represents a servicebinding.io Provisioned Service reference to the secret + 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 + type: object + x-kubernetes-map-type: atomic conditions: items: properties: @@ -3751,6 +3759,14 @@ spec: type: object status: properties: + binding: + description: Binding represents a servicebinding.io Provisioned Service reference to the secret + 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 + type: object + x-kubernetes-map-type: atomic conditions: items: properties: diff --git a/design/design-crd-spec.md b/design/design-crd-spec.md index addc58f3c..cb1ab3a49 100644 --- a/design/design-crd-spec.md +++ b/design/design-crd-spec.md @@ -207,6 +207,9 @@ status: reason: "SecretSynced" message: "Secret was synced" lastTransitionTime: "2019-08-12T12:33:02Z" + # servicebinding.io Provisioned Service reference to the secret + binding: + name: my-secret ``` diff --git a/docs/api/spec.md b/docs/api/spec.md index afb5d2578..9d63b4a00 100644 --- a/docs/api/spec.md +++ b/docs/api/spec.md @@ -2554,6 +2554,19 @@ string (Optional) + + +binding
+ + +Kubernetes core/v1.LocalObjectReference + + + + +

Binding represents a servicebinding.io Provisioned Service reference to the secret

+ +

ExternalSecretStatusCondition diff --git a/docs/introduction/getting-started.md b/docs/introduction/getting-started.md index 41e41250f..fbdb2ebd9 100644 --- a/docs/introduction/getting-started.md +++ b/docs/introduction/getting-started.md @@ -64,6 +64,8 @@ kubectl describe externalsecret example # [...] Name: example Status: + Binding: + Name: secret-to-be-created Conditions: Last Transition Time: 2021-02-24T16:45:23Z Message: Secret was synced diff --git a/pkg/controllers/externalsecret/externalsecret_controller.go b/pkg/controllers/externalsecret/externalsecret_controller.go index 76c704df1..de94a01d6 100644 --- a/pkg/controllers/externalsecret/externalsecret_controller.go +++ b/pkg/controllers/externalsecret/externalsecret_controller.go @@ -287,11 +287,17 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu switch externalSecret.Spec.Target.CreationPolicy { case esv1beta1.CreatePolicyMerge: err = patchSecret(ctx, r.Client, r.Scheme, secret, mutationFunc, externalSecret.Name) + if err == nil { + externalSecret.Status.Binding = v1.LocalObjectReference{Name: secret.Name} + } case esv1beta1.CreatePolicyNone: log.V(1).Info("secret creation skipped due to creationPolicy=None") err = nil default: err = createOrUpdate(ctx, r.Client, secret, mutationFunc, externalSecret.Name) + if err == nil { + externalSecret.Status.Binding = v1.LocalObjectReference{Name: secret.Name} + } } if err != nil { diff --git a/pkg/controllers/externalsecret/externalsecret_controller_test.go b/pkg/controllers/externalsecret/externalsecret_controller_test.go index d97ea0ecd..09be3b10a 100644 --- a/pkg/controllers/externalsecret/externalsecret_controller_test.go +++ b/pkg/controllers/externalsecret/externalsecret_controller_test.go @@ -269,11 +269,31 @@ var _ = Describe("ExternalSecret controller", func() { syncWithoutTargetName := func(tc *testCase) { tc.externalSecret.Spec.Target.Name = "" tc.checkSecret = func(es *esv1beta1.ExternalSecret, secret *v1.Secret) { - // check secret name Expect(secret.ObjectMeta.Name).To(Equal(ExternalSecretName)) + + // check binding secret on external secret + Expect(es.Status.Binding.Name).To(Equal(secret.ObjectMeta.Name)) } } + + // the secret name is reflected on the external secret's status as the binding secret + syncBindingSecret := func(tc *testCase) { + tc.checkSecret = func(es *esv1beta1.ExternalSecret, secret *v1.Secret) { + // check binding secret on external secret + Expect(es.Status.Binding.Name).To(Equal(secret.ObjectMeta.Name)) + } + } + + // their is no binding secret when a secret is not synced + skipBindingSecret := func(tc *testCase) { + tc.externalSecret.Spec.Target.CreationPolicy = esv1beta1.CreatePolicyNone + tc.checkExternalSecret = func(es *esv1beta1.ExternalSecret) { + // check binding secret is not set + Expect(es.Status.Binding.Name).To(BeEmpty()) + } + } + // labels and annotations from the Kind=ExternalSecret // should be copied over to the Kind=Secret syncLabelsAnnotations := func(tc *testCase) { @@ -1991,6 +2011,8 @@ var _ = Describe("ExternalSecret controller", func() { Entry("should create proper hash annotation for the external secret", checkSecretDataHashAnnotation), Entry("should refresh when the hash annotation doesn't correspond to secret data", checkSecretDataHashAnnotationChange), Entry("should use external secret name if target secret name isn't defined", syncWithoutTargetName), + Entry("should expose the secret as a provisioned service binding secret", syncBindingSecret), + Entry("should not expose a provisioned service when no secret is synced", skipBindingSecret), Entry("should set the condition eventually", syncLabelsAnnotations), Entry("should set prometheus counters", checkPrometheusCounters), Entry("should merge with existing secret using creationPolicy=Merge", mergeWithSecret),