Merge pull request #561 from vasrem/feature/refactor_certificates_for…

…_webhooks

Add more flexibility on how to configure certificates for the admission controllers via helm chart

Makefile

@@ -174,7 +174,7 @@ skopeo:
 fakechroot:
 	if ! which fakechroot; then if [ -f /etc/redhat-release ]; then dnf -y install fakechroot; elif [ -f /etc/lsb-release ]; then sudo apt-get -y update; sudo apt-get -y install fakechroot; fi; fi
 
-deploy-setup: export ENABLE_ADMISSION_CONTROLLER?=false
+deploy-setup: export ADMISSION_CONTROLLERS_ENABLED?=false
 deploy-setup: skopeo install
 	hack/deploy-setup.sh $(NAMESPACE)
 
@@ -218,7 +218,7 @@ test-%: generate vet manifests envtest
 	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir=/tmp -p path)" HOME="$(shell pwd)" go test ./$*/... -coverprofile cover-$*.out -coverpkg ./... -v
 
 # deploy-setup-k8s: export NAMESPACE=sriov-network-operator
-# deploy-setup-k8s: export ENABLE_ADMISSION_CONTROLLER=false
+# deploy-setup-k8s: export ADMISSION_CONTROLLERS_ENABLED=false
 # deploy-setup-k8s: export CNI_BIN_PATH=/opt/cni/bin
 # test-e2e-k8s: test-e2e
 

bindata/manifests/operator-webhook/001-service.yaml

@@ -6,7 +6,7 @@ metadata:
   namespace: {{.Namespace}}
   annotations:
     {{- if eq .ClusterType "openshift" }}
-    service.alpha.openshift.io/serving-cert-secret-name: operator-webhook-service
+    service.alpha.openshift.io/serving-cert-secret-name: {{.OperatorWebhookSecretName}}
     {{- end }}
 spec:
   ports:

bindata/manifests/operator-webhook/003-webhook.yaml

@@ -6,8 +6,10 @@ metadata:
   annotations:
     {{- if eq .ClusterType "openshift" }}
     service.beta.openshift.io/inject-cabundle: "true"
-    {{- else if and (not .CaBundle) (eq .ClusterType "kubernetes") }}
-    cert-manager.io/inject-ca-from: {{.Namespace}}/operator-webhook-service
+    {{- else if and (.CertManagerEnabled) (eq .ClusterType "kubernetes") }}
+    # Limitation: Certificate must be named as the secret it produces to avoid overcomplicating the logic with
+    # more variables.
+    cert-manager.io/inject-ca-from: {{.Namespace}}/{{.OperatorWebhookSecretName}}
     {{- end }}
 webhooks:
   - name: operator-webhook.sriovnetwork.openshift.io
@@ -19,9 +21,9 @@ webhooks:
         name: operator-webhook-service
         namespace: {{.Namespace}}
         path: "/mutating-custom-resource"
-      {{- if and (.CaBundle) (eq .ClusterType "kubernetes") }}
-      caBundle: "{{.CaBundle}}"
-      {{- end}}
+      {{- if and (not .CertManagerEnabled) (eq .ClusterType "kubernetes") }}
+      caBundle: "{{.OperatorWebhookCA}}"
+      {{- end }}
     rules:
       - operations: [ "CREATE", "UPDATE" ]
         apiGroups: ["sriovnetwork.openshift.io"]
@@ -36,8 +38,10 @@ metadata:
   annotations:
     {{- if eq .ClusterType "openshift" }}
     service.beta.openshift.io/inject-cabundle: "true"
-    {{- else if and (not .CaBundle) (eq .ClusterType "kubernetes") }}
-    cert-manager.io/inject-ca-from: {{.Namespace}}/operator-webhook-service
+    {{- else if and (.CertManagerEnabled) (eq .ClusterType "kubernetes") }}
+    # Limitation: Certificate must be named as the secret it produces to avoid overcomplicating the logic with
+    # more variables.
+    cert-manager.io/inject-ca-from: {{.Namespace}}/{{.OperatorWebhookSecretName}}
     {{- end }}
 webhooks:
   - name: operator-webhook.sriovnetwork.openshift.io
@@ -49,8 +53,8 @@ webhooks:
         name: operator-webhook-service
         namespace: {{.Namespace}}
         path: "/validating-custom-resource"
-      {{- if and (.CaBundle) (eq .ClusterType "kubernetes") }}
-      caBundle: "{{.CaBundle}}"
+      {{- if and (not .CertManagerEnabled) (eq .ClusterType "kubernetes") }}
+      caBundle: "{{.OperatorWebhookCA}}"
       {{- end }}
     rules:
       - operations: [ "CREATE", "UPDATE", "DELETE" ]

bindata/manifests/operator-webhook/server.yaml

@@ -86,4 +86,4 @@ spec:
       volumes:
       - name: tls
         secret:
-          secretName: operator-webhook-service
+          secretName: {{.OperatorWebhookSecretName}}

bindata/manifests/webhook/001-service.yaml

@@ -6,7 +6,7 @@ metadata:
   namespace: {{.Namespace}}
   annotations:
     {{- if eq .ClusterType "openshift" }}
-    service.alpha.openshift.io/serving-cert-secret-name: network-resources-injector-secret
+    service.alpha.openshift.io/serving-cert-secret-name: {{.InjectorWebhookSecretName}}
     {{- end }}
 spec:
   ports:

bindata/manifests/webhook/003-webhook.yaml

@@ -6,8 +6,10 @@ metadata:
   annotations:
     {{- if eq .ClusterType "openshift" }}
     service.beta.openshift.io/inject-cabundle: "true"
-    {{- else if and (not .CaBundle) (eq .ClusterType "kubernetes") }}
-    cert-manager.io/inject-ca-from: {{.Namespace}}/network-resources-injector-secret
+    {{- else if and (.CertManagerEnabled) (eq .ClusterType "kubernetes") }}
+    # Limitation: Certificate must be named as the secret it produces to avoid overcomplicating the logic with
+    # more variables.
+    cert-manager.io/inject-ca-from: {{.Namespace}}/{{.InjectorWebhookSecretName}}
     {{- end }}
 webhooks:
   - name: network-resources-injector-config.k8s.io
@@ -19,8 +21,8 @@ webhooks:
         name: network-resources-injector-service
         namespace: {{.Namespace}}
         path: "/mutate"
-      {{- if and (.CaBundle) (eq .ClusterType "kubernetes") }}
-      caBundle: "{{.CaBundle}}"
+      {{- if and (not .CertManagerEnabled) (eq .ClusterType "kubernetes") }}
+      caBundle: "{{.InjectorWebhookCA}}"
       {{- end }}
     rules:
       - operations: [ "CREATE" ]

bindata/manifests/webhook/server.yaml

@@ -86,4 +86,4 @@ spec:
       volumes:
       - name: tls
         secret:
-          secretName: network-resources-injector-secret
+          secretName: {{.InjectorWebhookSecretName}}

controllers/helper.go

@@ -36,6 +36,7 @@ const (
 	mutatingWebhookConfigurationCRDName   = "MutatingWebhookConfiguration"
 	validatingWebhookConfigurationCRDName = "ValidatingWebhookConfiguration"
 	machineConfigCRDName                  = "MachineConfig"
+	trueString                            = "true"
 )
 
 var namespace = os.Getenv("NAMESPACE")

controllers/sriovoperatorconfig_controller.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"strings"
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -69,7 +70,7 @@ func (r *SriovOperatorConfigReconciler) Reconcile(ctx context.Context, req ctrl.
 
 	logger.Info("Reconciling SriovOperatorConfig")
 
-	enableAdmissionController := os.Getenv("ENABLE_ADMISSION_CONTROLLER") == "true"
+	enableAdmissionController := os.Getenv("ADMISSION_CONTROLLERS_ENABLED") == trueString
 	if !enableAdmissionController {
 		logger.Info("SR-IOV Network Resource Injector and Operator Webhook are disabled.")
 	}
@@ -251,9 +252,13 @@ func (r *SriovOperatorConfigReconciler) syncWebhookObjs(ctx context.Context, dc
 		data.Data["SriovNetworkWebhookImage"] = os.Getenv("SRIOV_NETWORK_WEBHOOK_IMAGE")
 		data.Data["ReleaseVersion"] = os.Getenv("RELEASEVERSION")
 		data.Data["ClusterType"] = utils.ClusterType
-		data.Data["CaBundle"] = os.Getenv("WEBHOOK_CA_BUNDLE")
 		data.Data["DevMode"] = os.Getenv("DEV_MODE")
 		data.Data["ImagePullSecrets"] = GetImagePullSecrets()
+		data.Data["CertManagerEnabled"] = strings.ToLower(os.Getenv("ADMISSION_CONTROLLERS_CERTIFICATES_CERT_MANAGER_ENABLED")) == trueString
+		data.Data["OperatorWebhookSecretName"] = os.Getenv("ADMISSION_CONTROLLERS_CERTIFICATES_OPERATOR_SECRET_NAME")
+		data.Data["OperatorWebhookCA"] = os.Getenv("ADMISSION_CONTROLLERS_CERTIFICATES_OPERATOR_CA_CRT")
+		data.Data["InjectorWebhookSecretName"] = os.Getenv("ADMISSION_CONTROLLERS_CERTIFICATES_INJECTOR_SECRET_NAME")
+		data.Data["InjectorWebhookCA"] = os.Getenv("ADMISSION_CONTROLLERS_CERTIFICATES_INJECTOR_CA_CRT")
 
 		data.Data["ExternalControlPlane"] = false
 		if r.OpenshiftContext.IsOpenshiftCluster() {

controllers/suite_test.go

@@ -145,7 +145,9 @@ var _ = BeforeSuite(func(done Done) {
 
 	os.Setenv("RESOURCE_PREFIX", "openshift.io")
 	os.Setenv("NAMESPACE", "openshift-sriov-network-operator")
-	os.Setenv("ENABLE_ADMISSION_CONTROLLER", "true")
+	os.Setenv("ADMISSION_CONTROLLERS_ENABLED", "true")
+	os.Setenv("ADMISSION_CONTROLLERS_CERTIFICATES_OPERATOR_SECRET_NAME", "operator-webhook-cert")
+	os.Setenv("ADMISSION_CONTROLLERS_CERTIFICATES_INJECTOR_SECRET_NAME", "network-resources-injector-cert")
 	os.Setenv("SRIOV_CNI_IMAGE", "mock-image")
 	os.Setenv("SRIOV_INFINIBAND_CNI_IMAGE", "mock-image")
 	os.Setenv("SRIOV_DEVICE_PLUGIN_IMAGE", "mock-image")

deploy/operator.yaml

@@ -66,8 +66,8 @@ spec:
               value: $SRIOV_NETWORK_WEBHOOK_IMAGE
             - name: RESOURCE_PREFIX
               value: $RESOURCE_PREFIX
-            - name: ENABLE_ADMISSION_CONTROLLER
-              value: "$ENABLE_ADMISSION_CONTROLLER"
+            - name: ADMISSION_CONTROLLERS_ENABLED
+              value: "$ADMISSION_CONTROLLERS_ENABLED"
             - name: DEV_MODE
               value: "$DEV_MODE"
             - name: NAMESPACE
@@ -88,5 +88,13 @@ spec:
               value: $CNI_BIN_PATH
             - name: CLUSTER_TYPE
               value: $CLUSTER_TYPE
-            - name: WEBHOOK_CA_BUNDLE
-              value: "$WEBHOOK_CA_BUNDLE"
+            - name: ADMISSION_CONTROLLERS_CERTIFICATES_OPERATOR_SECRET_NAME
+              value: $ADMISSION_CONTROLLERS_CERTIFICATES_OPERATOR_SECRET_NAME
+            - name: ADMISSION_CONTROLLERS_CERTIFICATES_INJECTOR_SECRET_NAME
+              value: $ADMISSION_CONTROLLERS_CERTIFICATES_INJECTOR_SECRET_NAME
+            - name: ADMISSION_CONTROLLERS_CERTIFICATES_CERT_MANAGER_ENABLED
+              value: "$ADMISSION_CONTROLLERS_CERTIFICATES_CERT_MANAGER_ENABLED"
+            - name: ADMISSION_CONTROLLERS_CERTIFICATES_OPERATOR_CA_CRT
+              value: $ADMISSION_CONTROLLERS_CERTIFICATES_OPERATOR_CA_CRT
+            - name: ADMISSION_CONTROLLERS_CERTIFICATES_INJECTOR_CA_CRT
+              value: $ADMISSION_CONTROLLERS_CERTIFICATES_INJECTOR_CA_CRT

deployment/sriov-network-operator/templates/certificate.yaml

@@ -0,0 +1,71 @@
+{{- if .Values.operator.admissionControllers.enabled }}
+{{- if and (.Values.operator.admissionControllers.certificates.certManager.enabled) (.Values.operator.admissionControllers.certificates.certManager.generateSelfSigned) }}
+---
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  name: {{ .Values.operator.admissionControllers.certificates.secretNames.operator }}
+  namespace: {{ .Release.Namespace }}
+spec:
+  dnsNames:
+  - operator-webhook-service.{{ .Release.Namespace }}.svc
+  - operator-webhook-service.{{ .Release.Namespace }}.svc.cluster.local
+  issuerRef:
+    kind: Issuer
+    name: operator-webhook-selfsigned-issuer
+  secretName: {{ .Values.operator.admissionControllers.certificates.secretNames.operator }}
+---
+apiVersion: cert-manager.io/v1
+kind: Issuer
+metadata:
+  name: operator-webhook-selfsigned-issuer
+  namespace: {{ .Release.Namespace }}
+spec:
+  selfSigned: {}
+---
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  name: {{ .Values.operator.admissionControllers.certificates.secretNames.injector }}
+  namespace: {{ .Release.Namespace }}
+spec:
+  dnsNames:
+  - network-resources-injector-service.{{ .Release.Namespace }}.svc
+  - network-resources-injector-service.{{ .Release.Namespace }}.svc.cluster.local
+  issuerRef:
+    kind: Issuer
+    name: network-resources-injector-selfsigned-issuer
+  secretName: {{ .Values.operator.admissionControllers.certificates.secretNames.injector }}
+---
+apiVersion: cert-manager.io/v1
+kind: Issuer
+metadata:
+  name: network-resources-injector-selfsigned-issuer
+  namespace: {{ .Release.Namespace }}
+spec:
+  selfSigned: {}
+{{- else if and (not .Values.operator.admissionControllers.certificates.certManager.enabled) (.Values.operator.admissionControllers.certificates.custom.enabled) }}
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: {{ .Values.operator.admissionControllers.certificates.secretNames.operator }}
+  namespace: {{ .Release.Namespace }}
+type: Opaque
+data:
+  ca.crt: {{ .Values.operator.admissionControllers.certificates.custom.operator.caCrt | b64enc | b64enc | quote }}
+  tls.crt: {{ .Values.operator.admissionControllers.certificates.custom.operator.tlsCrt | b64enc | quote }}
+  tls.key: {{ .Values.operator.admissionControllers.certificates.custom.operator.tlsKey | b64enc | quote }}
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: {{ .Values.operator.admissionControllers.certificates.secretNames.injector }}
+  namespace: {{ .Release.Namespace }}
+type: Opaque
+data:
+  ca.crt: {{ .Values.operator.admissionControllers.certificates.custom.injector.caCrt | b64enc | b64enc | quote }}
+  tls.crt: {{ .Values.operator.admissionControllers.certificates.custom.injector.tlsCrt | b64enc | quote }}
+  tls.key: {{ .Values.operator.admissionControllers.certificates.custom.injector.tlsKey | b64enc | quote }}
+{{- end }}
+{{- end }}

deployment/sriov-network-operator/templates/operator.yaml

@@ -68,8 +68,6 @@ spec:
               value: {{ .Values.images.webhook }}
             - name: RESOURCE_PREFIX
               value: {{ .Values.operator.resourcePrefix }}
-            - name: ENABLE_ADMISSION_CONTROLLER
-              value: {{ .Values.operator.enableAdmissionController | quote }}
             - name: IMAGE_PULL_SECRETS
               value: {{ join "," .Values.imagePullSecrets }}
             - name: NAMESPACE
@@ -90,3 +88,26 @@ spec:
               value: {{ .Values.operator.cniBinPath }}
             - name: CLUSTER_TYPE
               value: {{ .Values.operator.clusterType }}
+            - name: ADMISSION_CONTROLLERS_ENABLED
+              value: {{ .Values.operator.admissionControllers.enabled | quote }}
+        {{- if .Values.operator.admissionControllers.enabled }}
+            - name: ADMISSION_CONTROLLERS_CERTIFICATES_OPERATOR_SECRET_NAME
+              value: {{ .Values.operator.admissionControllers.certificates.secretNames.operator }}
+            - name: ADMISSION_CONTROLLERS_CERTIFICATES_INJECTOR_SECRET_NAME
+              value: {{ .Values.operator.admissionControllers.certificates.secretNames.injector }}
+        {{- if .Values.operator.admissionControllers.certificates.certManager.enabled }}
+            - name: ADMISSION_CONTROLLERS_CERTIFICATES_CERT_MANAGER_ENABLED
+              value: {{ .Values.operator.admissionControllers.certificates.certManager.enabled | quote }}
+        {{- else }}
+            - name: ADMISSION_CONTROLLERS_CERTIFICATES_OPERATOR_CA_CRT
+              valueFrom:
+                secretKeyRef:
+                  name: {{ .Values.operator.admissionControllers.certificates.secretNames.operator }}
+                  key: ca.crt
+            - name: ADMISSION_CONTROLLERS_CERTIFICATES_INJECTOR_CA_CRT
+              valueFrom:
+                secretKeyRef:
+                  name: {{ .Values.operator.admissionControllers.certificates.secretNames.injector }}
+                  key: ca.crt
+        {{- end }}
+        {{- end }}

deployment/sriov-network-operator/values.yaml

@@ -25,9 +25,56 @@ operator:
   nameOverride: ""
   fullnameOverride: ""
   resourcePrefix: "openshift.io"
-  enableAdmissionController: false
   cniBinPath: "/opt/cni/bin"
   clusterType: "kubernetes"
+  admissionControllers:
+    enabled: false
+    certificates:
+      secretNames:
+        operator: "operator-webhook-cert"
+        injector: "network-resources-injector-cert"
+      certManager:
+        # When enabled, makes use of certificates managed by cert-manager.
+        enabled: false
+        # When enabled, certificates are generated via cert-manager and then name will match the name of the secrets
+        # defined above
+        generateSelfSigned: false
+      # If not specified, no secret is created and secrets with the names defined above are expected to exist in the
+      # cluster. In that case, the ca.crt must be base64 encoded twice since it ends up being an env variable.
+      custom:
+        enabled: false
+    #   operator:
+    #     caCrt: |
+    #       -----BEGIN CERTIFICATE-----
+    #       MIIMIICLDCCAdKgAwIBAgIBADAKBggqhkjOPQQDAjB9MQswCQYDVQQGEwJCRTEPMA0G
+    #       ...
+    #       -----END CERTIFICATE-----
+    #     tlsCrt: |
+    #       -----BEGIN CERTIFICATE-----
+    #       MIIMIICLDCCAdKgAwIBAgIBADAKBggqhkjOPQQDAjB9MQswCQYDVQQGEwJCRTEPMA0G
+    #       ...
+    #       -----END CERTIFICATE-----
+    #     tlsKey: |
+    #       -----BEGIN EC PRIVATE KEY-----
+    #       MHcl4wOuDwKQa+upc8GftXE2C//4mKANBC6It01gUaTIpo=
+    #       ...
+    #      -----END EC PRIVATE KEY-----
+    #   injector:
+    #     caCrt: |
+    #       -----BEGIN CERTIFICATE-----
+    #       MIIMIICLDCCAdKgAwIBAgIBADAKBggqhkjOPQQDAjB9MQswCQYDVQQGEwJCRTEPMA0G
+    #       ...
+    #       -----END CERTIFICATE-----
+    #     tlsCrt: |
+    #       -----BEGIN CERTIFICATE-----
+    #       MIIMIICLDCCAdKgAwIBAgIBADAKBggqhkjOPQQDAjB9MQswCQYDVQQGEwJCRTEPMA0G
+    #       ...
+    #       -----END CERTIFICATE-----
+    #     tlsKey: |
+    #       -----BEGIN EC PRIVATE KEY-----
+    #       MHcl4wOuDwKQa+upc8GftXE2C//4mKANBC6It01gUaTIpo=
+    #       ...
+    #      -----END EC PRIVATE KEY-----
 
 # Image URIs for sriov-network-operator components
 images: