Allow webhook CA bundle to be taken from secret

changes/unreleased/Changed-20221021-155603.yaml

@@ -0,0 +1,5 @@
+kind: Changed
+body: Allow webhook CA bundle to be taken from secret instead of helm chart parameter
+time: 2022-10-21T15:56:03.734031786-03:00
+custom:
+  Issue: "273"

cmd/operator/main.go

@@ -16,6 +16,7 @@
 package main
 
 import (
+	"context"
 	"flag"
 	"fmt"
 	"log"
@@ -73,7 +74,7 @@ type FlagConfig struct {
 	EnableProfiler       bool
 	ServiceAccountName   string
 	PrefixName           string // Prefix of the name of all objects created when the operator was deployed
-	GenerateWebhookCert  bool   // true if the operator needs to generate the webhook cert
+	WebhookCertSecret    string // when this is empty we will generate the webhook cert
 	LogArgs              *Logging
 }
 
@@ -124,8 +125,9 @@ func (fc *FlagConfig) setFlagArgs() {
 		"The name of the serviceAccount to use.")
 	flag.StringVar(&fc.PrefixName, "prefix-name", "verticadb-operator",
 		"The common prefix for all objects created during the operator deployment")
-	flag.BoolVar(&fc.GenerateWebhookCert, "generate-webhook-cert", false,
-		"The operator will generate the cert to be used for the webhook")
+	flag.StringVar(&fc.WebhookCertSecret, "webhook-cert-secret", "",
+		"Specifies the secret that contains the webhook cert. If this option is omitted, "+
+			"then the operator will generate the certificate.")
 	fc.LogArgs = &Logging{}
 	fc.LogArgs.setLoggingFlagArgs()
 }
@@ -153,6 +155,8 @@ func getIsWebhookEnabled() bool {
 		return DefaultEnabled
 	}
 	enabled, err := strconv.ParseBool(enableWebhook)
+	setupLog.Info(fmt.Sprintf("Parsed %s env var", enableWebhook),
+		"value", enableWebhook, "enabled", enabled, "err", err)
 	if err != nil {
 		return DefaultEnabled
 	}
@@ -284,6 +288,31 @@ func addWebhooksToManager(mgr manager.Manager) {
 	}
 }
 
+// setupWebhook will setup the webhook in the manager if enabled
+func setupWebhook(ctx context.Context, mgr manager.Manager, restCfg *rest.Config, flagArgs *FlagConfig) error {
+	if getIsWebhookEnabled() {
+		watchNamespace, err := getWatchNamespace()
+		if err != nil {
+			// We cannot setup webhooks if we are watching multiple namespaces
+			// because the webhook config uses a namespaceSelector.
+			setupLog.Info("Disabling webhook since we are not watching a single namespace")
+			return nil
+		}
+		if flagArgs.WebhookCertSecret == "" {
+			if err := security.GenerateWebhookCert(ctx, &setupLog, restCfg, CertDir, flagArgs.PrefixName, watchNamespace); err != nil {
+				return err
+			}
+		} else {
+			if err := security.PatchWebhookCABundleFromSecret(ctx, &setupLog, restCfg, flagArgs.WebhookCertSecret,
+				flagArgs.PrefixName, watchNamespace); err != nil {
+				return err
+			}
+		}
+		addWebhooksToManager(mgr)
+	}
+	return nil
+}
+
 func main() {
 	flagArgs := &FlagConfig{}
 	flagArgs.setFlagArgs()
@@ -329,16 +358,11 @@ func main() {
 		os.Exit(1)
 	}
 
-	ctx := ctrl.SetupSignalHandler()
 	addReconcilersToManager(mgr, restCfg, flagArgs)
-	if getIsWebhookEnabled() {
-		if flagArgs.GenerateWebhookCert {
-			if err := security.GenerateWebhookCert(ctx, &setupLog, restCfg, CertDir, flagArgs.PrefixName, watchNamespace); err != nil {
-				setupLog.Error(err, "generating webhook cert")
-				os.Exit(1)
-			}
-		}
-		addWebhooksToManager(mgr)
+	ctx := ctrl.SetupSignalHandler()
+	if err := setupWebhook(ctx, mgr, restCfg, flagArgs); err != nil {
+		setupLog.Error(err, "unable to setup webhook")
+		os.Exit(1)
 	}
 
 	if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {

config/default/manager_auth_proxy_patch.yaml

@@ -48,3 +48,4 @@ spec:
         - "--level=info"
         - "--dev=false"
         - "--prefix-name=verticadb-operator"
+        - "--webhook-cert-secret=verticadb-operator-controller-manager-service-cert"

config/manager/manager.yaml

@@ -52,6 +52,8 @@ spec:
             valueFrom:
               fieldRef:
                 fieldPath: metadata.namespace
+          - name: ENABLE_WEBHOOKS
+            value: true
       serviceAccountName: controller-manager
       terminationGracePeriodSeconds: 10
       volumes:

helm-charts/verticadb-operator/README.md

@@ -9,9 +9,9 @@ This helm chart will install the operator and an admission controller webhook.
 | rbac_proxy_image.name | Image name of Kubernetes RBAC proxy. | kubebuilder/kube-rbac-proxy:v0.13.0 |
 | rbac_proxy_image.repo | Repo server hosting rbac_proxy_image.name | gcr.io |
 | imagePullSecrets | List of Secret names containing login credentials for above repos | null (pull images anonymously) |
-| webhook.caBundle | A PEM encoded CA bundle that will be used to validate the webhook's server certificate.  If unspecified, system trust roots on the apiserver are used. | |
+| webhook.caBundle | A PEM encoded CA bundle that will be used to validate the webhook's server certificate.  This option is deprecated in favour of providing the CA bundle in the webhook.tlsSecret with the ca.crt key. This option will be removed in a future release.| |
 | webhook.certSource | The webhook requires a TLS certificate to work. This parm defines how the cert is supplied. Valid values are:<br><br>- **internal**: The certs are generated internally by the operator prior to starting the managing controller. The generated cert is self-signed. When it expires, the operator pod will need to be restarted in order to generate a new certificate. This is the default.<br><br>- **cert-manager**: The certs are generated using the cert-manager operator.  This operator needs to be deployed before deploying the operator. Deployment of this chart will create a self-signed cert through cert-manager. The advantage of this over 'internal' is that cert-manager will automatically handle private key rotation when the certificate is about to expire.<br><br>- **secret**: The certs are created prior to installation of this chart and are provided to the operator through a secret. This option gives you the most flexibility as it is entirely up to you how the cert is created.  This option requires the webhook.tlsSecret option to be set. For backwards compatibility, if webhook.tlsSecret is set, it is implicit that this mode is selected. | internal |
-| webhook.tlsSecret | The webhook requires a TLS certficate to work. By default we create a cert internally. If you want full control over the cert that is created you can use this parameter to provide it. When set, it is a name of a secret in the same namespace the chart is being installed in.  The secret must have the keys: tls.key and tls.crt. | |
+| webhook.tlsSecret | The webhook requires a TLS certficate to work. By default we create a cert internally. If you want full control over the cert that is created you can use this parameter to provide it. When set, it is a name of a secret in the same namespace the chart is being installed in.  The secret must have the keys: tls.key and tls.crt. It can also include the key ca.crt. When that key is included the operator will patch it in the CA bundle in the webhook configuration.| |
 | webhook.enable | If true, the webhook will be enabled and its configuration is setup by the helm chart. Setting this to false will disable the webhook. The webhook setup needs privileges to add validatingwebhookconfiguration and mutatingwebhookconfiguration, both are cluster scoped. If you do not have necessary privileges to add these configurations, then this option can be used to skip that and still deploy the operator. | true |
 | logging.filePath | The path to the log file. If omitted, all logging will be written to stdout.  | |
 | logging.maxFileSize | The maximum size, in MB, of the logging file before log rotation occurs. This is only applicable if logging to a file. | 500 |

helm-charts/verticadb-operator/templates/_helpers.tpl

@@ -26,3 +26,18 @@ Choose the webhook certificate source
 {{- .Values.webhook.certSource }}
 {{- end }}
 {{- end }}
+
+{{/*
+Choose the secret that contains the webhook certificate.
+This can be blank if the operator does not get the webhook from a secret (i.e.
+it is generated internally)
+*/}}
+{{- define "vdb-op.certSecret" -}}
+{{- if not (empty .Values.webhook.tlsSecret) }}
+{{- .Values.webhook.tlsSecret }}
+{{- else if eq .Values.webhook.certSource "internal" }}
+{{- "" }}
+{{- else }}
+{{- include "vdb-op.name" . }}-controller-manager-service-cert
+{{- end }}
+{{- end }}

helm-charts/verticadb-operator/values.yaml

@@ -59,14 +59,20 @@ webhook:
   certSource: internal
   # Use this parameter if you want to provide your own cert. tlsSecret is a
   # name of a secret in the same namespace the chart is being installed in.
-  # The secret must have the keys: tls.key and tls.crt.
+  # The secret must have the keys: tls.key and tls.crt. It can also include the
+  # key ca.crt. When that key is included the operator will patch it into the CA
+  # bundle in the webhook configuration.
   #
   # For backwards compatibility, if this is set, then 'certSource = secret' is
   # implied.
   tlsSecret: ""
   # caBundle is a PEM encoded CA bundle that will be used to validate the
-  # webhook's server certificate.  If unspecified, system trust roots on the
-  # apiserver are used. Use this when providing your own cert.
+  # webhook's server certificate.
+  #
+  # This is another way to provide the CA bundle to be patched with in the
+  # webhook configuration. This approach is deprecated in favour of providing
+  # the CA bundle in the tlsSecret with the ca.crt key. This option will be
+  # removed in a future release.
   caBundle: ""
   # If true, the webhook will be enabled and its configuration is setup by the helm chart. 
   # Setting this to false will disable the webhook. The webhook setup needs privileges to add 

scripts/template-helm-chart.sh

@@ -53,14 +53,20 @@ cat >>$TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml << END
 {{ end }}
 END
 # 5. Template the tls secret name
-sed -i 's/secretName: webhook-server-cert/secretName: {{ default "webhook-server-cert" .Values.webhook.tlsSecret }}/' $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
+for fn in verticadb-operator-controller-manager-deployment.yaml \
+    verticadb-operator-serving-cert-certificate.yaml
+do
+  sed -i 's/secretName: webhook-server-cert/secretName: {{ include "vdb-op.certSecret" . }}/' $TEMPLATE_DIR/$fn
+done
 for fn in $TEMPLATE_DIR/verticadb-operator-controller-manager-deployment.yaml
 do
   # Include the secret only if not using webhook.certSource=internal
   perl -i -0777 -pe 's/(.*- name: cert\n.*secret:\n.*defaultMode:.*\n.*secretName:.*)/\{\{- if or (ne .Values.webhook.certSource "internal") (not (empty .Values.webhook.tlsSecret)) \}\}\n$1\n\{\{- end \}\}/g' $fn
   perl -i -0777 -pe 's/(.*- mountPath: .*\n.*name: cert\n.*readOnly:.*)/\{\{- if or (ne .Values.webhook.certSource "internal") (not (empty .Values.webhook.tlsSecret)) \}\}\n$1\n\{\{- end \}\}/g' $fn
-  # Add the --generate-webhook-cert option when cert generation is internal
-  perl -i -0777 -pe 's/(.*- --prefix-name=.*)/$1\n\{\{- if and (eq .Values.webhook.certSource "internal") (empty .Values.webhook.tlsSecret) \}\}\n        - --generate-webhook-cert\n\{\{- end \}\}/g' $fn
+  # Update the --webhook-cert-secret option to include the actual name of the secret
+  perl -i -0777 -pe 's/(- --webhook-cert-secret=)(.*)/$1\{\{ include "vdb-op.certSecret" . \}\}/g' $fn
+  # Set ENABLE_WEBHOOK according to webhook.enable value
+  perl -i -0777 -pe 's/(name: ENABLE_WEBHOOKS\n.*value:) .*/$1 {{ quote .Values.webhook.enable }}/g' $fn
 done
 for fn in verticadb-operator-selfsigned-issuer-issuer.yaml \
     verticadb-operator-serving-cert-certificate.yaml

tests/e2e-extra/disable-webhook/10-assert.yaml

@@ -0,0 +1,27 @@
+# (c) Copyright [2021-2022] Micro Focus or one of its affiliates.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: v1
+kind: Pod
+metadata:
+  labels:
+    control-plane: controller-manager
+status:
+  phase: Running
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: verticadb-operator-controller-manager
+status:
+  readyReplicas: 1

tests/e2e-extra/disable-webhook/10-deploy-operator.yaml

@@ -0,0 +1,18 @@
+# (c) Copyright [2021-2022] Micro Focus or one of its affiliates.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+  - command: sh -c "cd ../../.. && DEPLOY_WITH=helm HELM_OVERRIDES='--set webhook.enable=false' make deploy-operator NAMESPACE=$NAMESPACE"
+    ignoreFailure: true # Ignore because no webhook but make target waits for it

tests/e2e-extra/disable-webhook/15-assert.yaml

@@ -0,0 +1,27 @@
+# (c) Copyright [2021-2022] Micro Focus or one of its affiliates.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: v-disable-webhook-main
+---
+apiVersion: vertica.com/v1beta1
+kind: VerticaDB
+metadata:
+  name: v-disable-webhook
+spec:
+  kSafety: "0"
+status:
+  subclusters:
+    - installCount: 1

tests/e2e-extra/disable-webhook/20-apply-change-that-webhook-would-deny.yaml

@@ -0,0 +1,19 @@
+# (c) Copyright [2021-2022] Micro Focus or one of its affiliates.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: vertica.com/v1beta1
+kind: VerticaDB
+metadata:
+  name: v-disable-webhook
+spec:
+  kSafety: "1"

tests/e2e-extra/disable-webhook/20-assert.yaml

@@ -0,0 +1,19 @@
+# (c) Copyright [2021-2022] Micro Focus or one of its affiliates.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: vertica.com/v1beta1
+kind: VerticaDB
+metadata:
+  name: v-disable-webhook
+spec:
+  kSafety: "1"

tests/e2e-extra/disable-webhook/setup-vdb/base/setup-vdb.yaml

@@ -0,0 +1,31 @@
+# (c) Copyright [2021-2022] Micro Focus or one of its affiliates.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: vertica.com/v1beta1
+kind: VerticaDB
+metadata:
+  name: v-disable-webhook
+spec:
+  image: kustomize-vertica-image
+  communal:
+    includeUIDInPath: true
+  local:
+    requestSize: 100Mi
+  subclusters:
+    - name: main
+      size: 1
+  kSafety: "0"
+  certSecrets: []
+  imagePullSecrets: []
+  volumes: []
+  volumeMounts: []

tests/e2e/custom-cert-webhook-cabundle-as-parm/07-create-creds.yaml

@@ -0,0 +1,18 @@
+# (c) Copyright [2021-2022] Micro Focus or one of its affiliates.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+  - script: kustomize build ../../manifests/communal-creds/overlay | kubectl apply -f - --namespace $NAMESPACE
+  - script: kustomize build ../../manifests/priv-container-creds/overlay | kubectl apply -f - --namespace $NAMESPACE

tests/e2e/custom-cert-webhook-cabundle-as-parm/15-setup-vdb.yaml

@@ -0,0 +1,17 @@
+# (c) Copyright [2021-2022] Micro Focus or one of its affiliates.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+  - command: bash -c "kustomize build setup-vdb/overlay | kubectl -n $NAMESPACE apply -f - "

tests/e2e/custom-cert-webhook/20-assert.yaml → tests/e2e/custom-cert-webhook-cabundle-as-parm/20-assert.yaml

@@ -14,6 +14,6 @@
 apiVersion: vertica.com/v1beta1
 kind: VerticaDB
 metadata:
-  name: v-custom-cert-webhook
+  name: v-custom-cert-webhook-cabundle-as-parm
 spec:
   initPolicy: Create

tests/e2e/custom-cert-webhook/20-update-initpolicy.yaml → tests/e2e/custom-cert-webhook-cabundle-as-parm/20-update-initpolicy.yaml

@@ -14,5 +14,5 @@
 apiVersion: kuttl.dev/v1beta1
 kind: TestStep
 commands:
-  - script: kubectl -n $NAMESPACE get verticadb v-custom-cert-webhook -o json | jq '.spec.initPolicy="Revive"' | kubectl -n $NAMESPACE replace -f -
+  - script: kubectl -n $NAMESPACE get verticadb v-custom-cert-webhook-cabundle-as-parm -o json | jq '.spec.initPolicy="Revive"' | kubectl -n $NAMESPACE replace -f -
     ignoreFailure: true

tests/e2e/custom-cert-webhook/25-assert.yaml → tests/e2e/custom-cert-webhook-cabundle-as-parm/25-assert.yaml

@@ -14,6 +14,6 @@
 apiVersion: vertica.com/v1beta1
 kind: VerticaDB
 metadata:
-  name: v-custom-cert-webhook
+  name: v-custom-cert-webhook-cabundle-as-parm
 spec:
   dbName: vertdb