Improved deployments and crd logic. Added cert-controller reconcile tests

Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>

Makefile

@@ -102,12 +102,8 @@ build-%: generate ## Build binary for the specified arch
 	@$(INFO) go build $*
 	@CGO_ENABLED=0 GOOS=linux GOARCH=$* \
 		go build -o '$(OUTPUT_DIR)/external-secrets-linux-$*' main.go
-	@$(OK) go build $*
-	@$(INFO) go build $*
 	@CGO_ENABLED=0 GOOS=linux GOARCH=$* \
 		go build -o 'webhook/$(OUTPUT_DIR)/external-secrets-webhook-linux-$*' webhook/main.go
-	@$(OK) go build $*
-	@$(INFO) go build $*
 	@CGO_ENABLED=0 GOOS=linux GOARCH=$* \
 		go build -o 'webhook/certcontroller/$(OUTPUT_DIR)/external-secrets-cert-controller-linux-$*' webhook/certcontroller/main.go
 	@$(OK) go build $*

config/crds/kustomization.yaml

@@ -1,11 +0,0 @@
-resources:
-- bases/external-secrets.io_clustersecretstores.yaml
-- bases/external-secrets.io_secretstores.yaml
-- bases/external-secrets.io_externalsecrets.yaml
-
-patchesStrategicMerge:
-- patches/webhook_in_externalsecrets.yaml
-- patches/webhook_in_clustersecretstores.yaml
-- patches/webhook_in_secretstores.yaml
-
-configurations: []

deploy/charts/external-secrets/README.md

@@ -42,6 +42,7 @@ The command removes all the Kubernetes components associated with the chart and
 | certController.fullnameOverride | string | `""` |  |
 | certController.image.pullPolicy | string | `"IfNotPresent"` |  |
 | certController.image.repository | string | `"ghcr.io/external-secrets/external-secrets-cert-controller"` |  |
+| certController.image.tag | string | `""` |  |
 | certController.imagePullSecrets | list | `[]` |  |
 | certController.nameOverride | string | `""` |  |
 | certController.nodeSelector | object | `{}` |  |
@@ -106,6 +107,7 @@ The command removes all the Kubernetes components associated with the chart and
 | webhook.prometheus.enabled | bool | `false` | Specifies whether to expose Service resource for collecting Prometheus metrics |
 | webhook.prometheus.service.port | int | `8080` |  |
 | webhook.rbac.create | bool | `true` | Specifies whether role and rolebinding resources should be created. |
+| webhook.replicaCount | int | `1` |  |
 | webhook.resources | object | `{}` |  |
 | webhook.securityContext | object | `{}` |  |
 | webhook.serviceAccount.annotations | object | `{}` | Annotations to add to the service account. |

deploy/charts/external-secrets/templates/cert-controller-deployment.yaml

@@ -10,7 +10,7 @@ metadata:
     {{- toYaml . | nindent 4 }}
   {{- end }}
 spec:
-  replicas: {{ .Values.certController.replicaCount }}
+  replicas: 1
   selector:
     matchLabels:
       {{- include "external-secrets-cert-controller.selectorLabels" . | nindent 6 }}
@@ -43,6 +43,11 @@ spec:
           {{- end }}
           image: "{{ .Values.certController.image.repository }}:{{ .Values.certController.image.tag | default .Chart.AppVersion }}"
           imagePullPolicy: {{ .Values.certController.image.pullPolicy }}
+          args:
+          - --service-name={{ include "external-secrets.fullname" . }}-webhook
+          - --service-namespace={{ .Release.Namespace }}
+          - --secret-name={{ include "external-secrets.fullname" . }}-webhook
+          - --secret-namespace={{ .Release.Namespace }}
           {{- range $key, $value := .Values.certController.extraArgs }}
             {{- if $value }}
           - --{{ $key }}={{ $value }}
@@ -62,18 +67,18 @@ spec:
           resources:
             {{- toYaml . | nindent 12 }}
           {{- end }}
-      {{- with .Values.webhook.nodeSelector }}
+      {{- with .Values.certController.nodeSelector }}
       nodeSelector:
         {{- toYaml . | nindent 8 }}
       {{- end }}
-      {{- with .Values.webhook.affinity }}
+      {{- with .Values.certController.affinity }}
       affinity:
         {{- toYaml . | nindent 8 }}
       {{- end }}
-      {{- with .Values.webhook.tolerations }}
+      {{- with .Values.certController.tolerations }}
       tolerations:
         {{- toYaml . | nindent 8 }}
       {{- end }}
-      {{- if .Values.webhook.priorityClassName }}
-      priorityClassName: {{ .Values.webhook.priorityClassName }}
+      {{- if .Values.certController.priorityClassName }}
+      priorityClassName: {{ .Values.certController.priorityClassName }}
       {{- end }}

deploy/charts/external-secrets/templates/cert-controller-rbac.yaml

@@ -19,14 +19,6 @@ rules:
   - apiGroups:
     - ""
     resources:
-    - "services"
-    verbs:
-    - "get"
-    - "list"
-    - "watch"
-  - apiGroups:
-    - ""
-    resources:
     - "secrets"
     verbs:
     - "get"

deploy/charts/external-secrets/templates/rbac.yaml

@@ -40,24 +40,6 @@ rules:
     - "list"
     - "watch"
   - apiGroups:
-    - "apiextensions.k8s.io"
-    resources:
-    - "customresourcedefinitions"
-    verbs:
-    - "get"
-    - "list"
-    - "watch"
-    - "update"
-    - "patch"
-  - apiGroups:
-    - ""
-    resources:
-    - "services"
-    verbs:
-    - "get"
-    - "list"
-    - "watch"
-  - apiGroups:
     - ""
     resources:
     - "configmaps"

deploy/charts/external-secrets/templates/webhook-deployment.yaml

@@ -45,6 +45,7 @@ spec:
           imagePullPolicy: {{ .Values.webhook.image.pullPolicy }}
           args:
           - --dns-name={{ include "external-secrets.fullname" . }}-webhook.{{ .Release.Namespace }}.svc
+          - --cert-dir={{ .Values.webhook.certDir }}
           {{- range $key, $value := .Values.webhook.extraArgs }}
             {{- if $value }}
           - --{{ $key }}={{ $value }}
@@ -75,6 +76,7 @@ spec:
           volumeMounts:
           - name: certs
             mountPath: {{ .Values.webhook.certDir }}
+            readOnly: true
       volumes:
       - name: certs
         secret:

deploy/charts/external-secrets/templates/webhook-service.yaml

@@ -5,6 +5,12 @@ metadata:
   labels:
     {{- include "external-secrets-webhook.labels" . | nindent 4 }}
     external-secrets.io/component : webhook
+  {{- if .Values.webhook.prometheus.enabled}}
+  annotations:
+    prometheus.io/path: "/metrics"
+    prometheus.io/scrape: "true"
+    prometheus.io/port: {{ .Values.prometheus.service.port | quote }}
+  {{- end }}
 spec:
   type: ClusterIP
   ports:
@@ -12,9 +18,11 @@ spec:
     targetPort: 9443
     protocol: TCP
     name: webhook
+  {{- if .Values.webhook.prometheus.enabled}}
   - port: {{ .Values.webhook.prometheus.service.port}}
     targetPort: {{ .Values.webhook.prometheus.service.port}}
     protocol: TCP
     name: metrics
+  {{- end }}
   selector:
     {{- include "external-secrets-webhook.selectorLabels" . | nindent 4 }}

deploy/charts/external-secrets/values.yaml

@@ -88,6 +88,7 @@ affinity: {}
 priorityClassName: ""
 
 webhook:
+  replicaCount: 1
   certDir: /tmp/k8s-webhook-server/serving-certs
   image:
     repository: ghcr.io/external-secrets/external-secrets-webhook
@@ -155,6 +156,7 @@ certController:
   image:
     repository: ghcr.io/external-secrets/external-secrets-cert-controller
     pullPolicy: IfNotPresent
+    tag: ""
   imagePullSecrets: []
   nameOverride: ""
   fullnameOverride: ""

main.go

@@ -90,7 +90,7 @@ func main() {
 	}
 	if err = (&secretstore.StoreReconciler{
 		Client:          mgr.GetClient(),
-		Log:             ctrl.Log.WithName("contllers").WithName("SecretStore"),
+		Log:             ctrl.Log.WithName("controllers").WithName("SecretStore"),
 		Scheme:          mgr.GetScheme(),
 		ControllerClass: controllerClass,
 		RequeueInterval: storeRequeueInterval,

pkg/controllers/crds/crds_controller.go

@@ -35,6 +35,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -63,10 +64,11 @@ type Reconciler struct {
 	Log                    logr.Logger
 	Scheme                 *runtime.Scheme
 	recorder               record.EventRecorder
-	SvcLabels              map[string]string
-	SecretLabels           map[string]string
+	SvcName                string
+	SvcNamespace           string
+	SecretName             string
+	SecretNamespace        string
 	CrdResources           []string
-	CertDir                string
 	dnsName                string
 	CAName                 string
 	CAOrganization         string
@@ -124,43 +126,34 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager, opts controller.Options)
 func (r *Reconciler) updateCRD(ctx context.Context, req ctrl.Request) error {
 	crdGVK := schema.GroupVersionKind{Group: "apiextensions.k8s.io", Version: "v1", Kind: "CustomResourceDefinition"}
 
-	svcList := corev1.ServiceList{}
-	err := r.List(context.Background(), &svcList, client.MatchingLabels(r.SvcLabels))
-	if err != nil {
-		return err
-	}
-	if len(svcList.Items) == 0 {
-		return fmt.Errorf("no service matches the labels %v", r.SvcLabels)
+	secret := corev1.Secret{}
+	secretName := types.NamespacedName{
+		Name:      r.SecretName,
+		Namespace: r.SecretNamespace,
 	}
-	if len(svcList.Items) > 1 {
-		return fmt.Errorf("multiple services match labels: %v", svcList.Items)
-	}
-	secretList := corev1.SecretList{}
-	err = r.List(context.Background(), &secretList, client.MatchingLabels(r.SecretLabels))
+	err := r.Get(context.Background(), secretName, &secret)
 	if err != nil {
 		return err
 	}
-	if len(secretList.Items) == 0 {
-		return fmt.Errorf("no secret matches the labels %v", r.SvcLabels)
-	}
-	if len(secretList.Items) > 1 {
-		return fmt.Errorf("multiple secrets match labels: %v", svcList.Items)
-	}
 	updatedResource := &unstructured.Unstructured{}
 	updatedResource.SetGroupVersionKind(crdGVK)
 	if err := r.Get(ctx, req.NamespacedName, updatedResource); err != nil {
 		return err
 	}
-	if err := injectSvcToConversionWebhook(updatedResource, &svcList.Items[0]); err != nil {
+	svc := types.NamespacedName{
+		Name:      r.SvcName,
+		Namespace: r.SvcNamespace,
+	}
+	if err := injectSvcToConversionWebhook(updatedResource, svc); err != nil {
 		return err
 	}
-	r.dnsName = fmt.Sprintf("%v.%v.svc", svcList.Items[0].Name, svcList.Items[0].Namespace)
-	need, err := r.refreshCertIfNeeded(&secretList.Items[0])
+	r.dnsName = fmt.Sprintf("%v.%v.svc", r.SvcName, r.SvcNamespace)
+	need, err := r.refreshCertIfNeeded(&secret)
 	if err != nil {
 		return err
 	}
 	if need {
-		artifacts, err := buildArtifactsFromSecret(&secretList.Items[0])
+		artifacts, err := buildArtifactsFromSecret(&secret)
 		if err != nil {
 			return err
 		}
@@ -174,13 +167,7 @@ func (r *Reconciler) updateCRD(ctx context.Context, req ctrl.Request) error {
 	return nil
 }
 
-func (r *Reconciler) EnsureCertsMounted() bool {
-	certFile := r.CertDir + "/" + certName
-	_, err := os.Stat(certFile)
-	return err == nil
-}
-
-func injectSvcToConversionWebhook(crd *unstructured.Unstructured, service *corev1.Service) error {
+func injectSvcToConversionWebhook(crd *unstructured.Unstructured, svc types.NamespacedName) error {
 	_, found, err := unstructured.NestedMap(crd.Object, "spec", "conversion", "webhook", "clientConfig")
 	if err != nil {
 		return err
@@ -188,10 +175,10 @@ func injectSvcToConversionWebhook(crd *unstructured.Unstructured, service *corev
 	if !found {
 		return errors.New("`conversion.webhook.clientConfig` field not found in CustomResourceDefinition")
 	}
-	if err := unstructured.SetNestedField(crd.Object, service.Name, "spec", "conversion", "webhook", "clientConfig", "service", "name"); err != nil {
+	if err := unstructured.SetNestedField(crd.Object, svc.Name, "spec", "conversion", "webhook", "clientConfig", "service", "name"); err != nil {
 		return err
 	}
-	if err := unstructured.SetNestedField(crd.Object, service.Namespace, "spec", "conversion", "webhook", "clientConfig", "service", "namespace"); err != nil {
+	if err := unstructured.SetNestedField(crd.Object, svc.Namespace, "spec", "conversion", "webhook", "clientConfig", "service", "namespace"); err != nil {
 		return err
 	}
 	return nil

pkg/controllers/crds/crds_controller_test.go

@@ -33,9 +33,11 @@ import (
 
 func newReconciler() Reconciler {
 	return Reconciler{
-		CrdResources: []string{"one", "two", "three"},
-		SvcLabels:    map[string]string{"foo": "bar"},
-		SecretLabels: map[string]string{"foo": "bar"},
+		CrdResources:    []string{"one", "two", "three"},
+		SvcName:         "foo",
+		SvcNamespace:    "default",
+		SecretName:      "foo",
+		SecretNamespace: "default",
 	}
 }
 
@@ -130,7 +132,11 @@ func TestInjectSvcToConversionWebhook(t *testing.T) {
 	u := unstructured.Unstructured{
 		Object: crdunmarshalled,
 	}
-	err = injectSvcToConversionWebhook(&u, &svc)
+	name := types.NamespacedName{
+		Name:      svc.Name,
+		Namespace: svc.Namespace,
+	}
+	err = injectSvcToConversionWebhook(&u, name)
 	if err != nil {
 		t.Errorf("Failed: error when injecting: %v", err)
 	}

pkg/controllers/crds/suite_test.go

@@ -77,10 +77,11 @@ var _ = BeforeSuite(func() {
 		Client:                 k8sClient,
 		Scheme:                 k8sManager.GetScheme(),
 		Log:                    ctrl.Log.WithName("controllers").WithName("CustomResourceDefinition"),
-		SvcLabels:              map[string]string{"foo": "bar"},
-		SecretLabels:           map[string]string{"foo": "bar"},
+		SvcName:                "foo",
+		SvcNamespace:           "default",
+		SecretName:             "foo",
+		SecretNamespace:        "default",
 		CrdResources:           []string{"externalsecrets.test.io", "secretstores.test.io", "clustersecretstores.test.io"},
-		CertDir:                "my/cert/dir",
 		CAName:                 "external-secrets",
 		CAOrganization:         "external-secrets",
 		RestartOnSecretRefresh: false,

webhook/certcontroller/main.go

@@ -52,7 +52,13 @@ func main() {
 	var concurrent int
 	var loglevel string
 	var namespace string
+	var serviceName, serviceNamespace string
+	var secretName, secretNamespace string
 	flag.StringVar(&metricsAddr, "metrics-addr", ":8080", "The address the metric endpoint binds to.")
+	flag.StringVar(&serviceName, "service-name", "external-secrets-webhook", "Webhook service name")
+	flag.StringVar(&serviceNamespace, "service-namespace", "default", "Webhook service namespace")
+	flag.StringVar(&secretName, "secret-name", "external-secrets-webhook", "Secret to store certs for webhook")
+	flag.StringVar(&secretNamespace, "secret-namespace", "default", "namespace of the secret to store certs")
 	flag.BoolVar(&enableLeaderElection, "enable-leader-election", false,
 		"Enable leader election for controller manager. "+
 			"Enabling this will ensure there is only one active controller manager.")
@@ -84,10 +90,11 @@ func main() {
 		Client:                 mgr.GetClient(),
 		Log:                    ctrl.Log.WithName("controllers").WithName("webhook-certs-updater"),
 		Scheme:                 mgr.GetScheme(),
-		SvcLabels:              map[string]string{"external-secrets.io/component": "webhook"},
-		SecretLabels:           map[string]string{"external-secrets.io/component": "webhook"},
+		SvcName:                serviceName,
+		SvcNamespace:           serviceNamespace,
+		SecretName:             secretName,
+		SecretNamespace:        secretNamespace,
 		CrdResources:           []string{"externalsecrets.external-secrets.io", "clustersecretstores.external-secrets.io", "secretstores.external-secrets.io"},
-		CertDir:                "/tmp/k8s-webhook-server/serving-certs",
 		CAName:                 "external-secrets",
 		CAOrganization:         "external-secrets",
 		RestartOnSecretRefresh: false,