Cert rotation for e2e chart

.github/workflows/build.yaml

@@ -113,3 +113,18 @@ jobs:
  env:
  T: integration
  DEPLOY_METHOD: chart
+ integration-rotation-enabled:
+ runs-on: ubuntu-20.04
+ steps:
+ - uses: actions/checkout@v2
+ - name: Set up Go
+ uses: actions/setup-go@v2
+ with:
+ go-version: '1.21'
+ - id: test-runner
+ uses: ./.github/actions/tests
+ env:
+ T: integration
+ DEPLOY_METHOD: chart
+ HELM_INSTALL_FLAGS_FLAGS: --set certificates.certReload.enabled=true
+

admission-webhook/cert_reloader.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+ "context"
  "crypto/tls"
  "sync"
 
@@ -56,7 +57,8 @@ func (cr *CertReloader) GetCertificateFunc() func(*tls.ClientHelloInfo) (*tls.Ce
  }
 }
 
-func watchCertFiles(certLoader CertLoader) {
+func watchCertFiles(ctx context.Context, certLoader CertLoader) {
+ logrus.Infof("Starting certificate watcher on path %v and %v", certLoader.CertPath(), certLoader.KeyPath())
  watcher, err := fsnotify.NewWatcher()
  if err != nil {
  logrus.Errorf("error creating watcher: %v", err)
@@ -69,23 +71,25 @@ func watchCertFiles(certLoader CertLoader) {
  select {
  case event, ok := <-watcher.Events:
  if !ok {
+ logrus.Errorf("watcher events returned !ok: %v", err)
  return
  }
- if event.Op&fsnotify.Write == fsnotify.Write || event.Op&fsnotify.Rename == fsnotify.Rename {
- logrus.Infof("detected change in certificate file: %v", event.Name)
- _, err := certLoader.LoadCertificate()
- if err != nil {
- logrus.Errorf("error reloading certificate: %v", err)
- } else {
- logrus.Infof("successfully reloaded certificate")
- }
+ logrus.Infof("detected change in certificate file: %v", event.Name)
+ _, err := certLoader.LoadCertificate()
+ if err != nil {
+ logrus.Errorf("error reloading certificate: %v", err)
+ } else {
+ logrus.Infof("successfully reloaded certificate")
  }
  case err, ok := <-watcher.Errors:
  if !ok {
  logrus.Errorf("watcher error returned !ok: %v", err)
  return
  }
  logrus.Errorf("watcher error: %v", err)
+ case <-ctx.Done():
+ logrus.Info("stopping certificate watcher")
+ return
  }
  }
  }()

admission-webhook/cert_reloader_test.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+ "context"
  "crypto/tls"
  "os"
  "testing"
@@ -86,6 +87,7 @@ func TestWatchingCertFiles(t *testing.T) {
  defer os.Remove(tmpKeyFile.Name())
 
  loadCertFuncChan := make(chan bool)
+ done := make(chan bool)
 
  cl := &mockCertLoader{
  certPath: tmpCertFile.Name(),
@@ -97,18 +99,19 @@ func TestWatchingCertFiles(t *testing.T) {
  }
 
  go func() {
- defer close(loadCertFuncChan)
-
  called := <-loadCertFuncChan
  assert.True(t, called)
+ done <- true
  }()
 
- watchCertFiles(cl)
+ ctx, cancel := context.WithCancel(context.Background())
+ defer cancel()
+ watchCertFiles(ctx, cl)
 
  newCertData, _ := os.ReadFile("testdata/cert.pem")
  if err := os.WriteFile(tmpCertFile.Name(), newCertData, 0644); err != nil {
  t.Fatalf("Failed to write new data to cert file: %v", err)
  }
 
- <-loadCertFuncChan
+ <-done
 }

admission-webhook/integration_tests/integration_test.go

@@ -2,7 +2,6 @@ package integrationtests
 
 import (
  "context"
- "encoding/base64"
  "encoding/json"
  "fmt"
  "html/template"
@@ -11,7 +10,9 @@ import (
  "path"
  "reflect"
  "strconv"
+ "strings"
  "testing"
+ "time"
 
  "github.com/stretchr/testify/assert"
  corev1 "k8s.io/api/core/v1"
@@ -368,20 +369,30 @@ func TestDeployV1CredSpecGetAllVersions(t *testing.T) {
 
 func TestPossibleToUpdatePodWithNewCert(t *testing.T) {
  /** TODO:
- * - update the webhook pod to use the new flag
- * - make a request to create a pod to make sure it works (already done)
- * - get a blessed certificate from the API server
- * (https://github.com/kubernetes-sigs/windows-gmsa/blob/141/admission-webhook/deploy/create-signed-cert.sh)
- * - update existing secret in place and wait for the pod to get new secrets which can take time
- * (https://kubernetes.io/docs/concepts/configuration/secret/#using-secrets-as-files-from-a-pod) - similar to what you are doing here
- * - kubectl exec into the running pod to see that the secret changed
- * (using utils like https://github.com/ycheng-kareo/windows-gmsa/blob/watch-reload-cert/admission-webhook/integration_tests/kube.go#L199)
- * - make a request to create a pod to verify that it still works (pod := waitForPodToComeUp(t, testConfig.Namespace, "app="+testName))
- * - add a separate test to verify that requests to the webhook always return during this process
- */
+ * - add a separate test to verify that requests to the webhook always return during this process
+ */
+
+ webHookNs := os.Getenv("NAMESPACE")
+ webHookDeploymentName := os.Getenv("DEPLOYMENT_NAME")
+ webhook, err := kubeClient(t).AppsV1().Deployments(webHookNs).Get(context.Background(), webHookDeploymentName, metav1.GetOptions{})
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ rotationEnabled := false
+ for arg := range webhook.Spec.Template.Spec.Containers[0].Args {
+ if strings.Contains(webhook.Spec.Template.Spec.Containers[0].Args[arg], "--cert-reload=true") {
+ rotationEnabled = true
+ }
+ }
+
+ if !rotationEnabled {
+ t.Skip("Skipping test as rotation is not enabled")
+ }
+
  testName := "possible-to-update-pod-with-new-cert"
  credSpecTemplates := []string{"credspec-0"}
- newSecretTemplate := "new-secret"
+
  templates := []string{"credspecs-users-rbac-role", "service-account", "sa-rbac-binding", "single-pod-with-container-level-gmsa"}
 
  testConfig, tearDownFunc := integrationTestSetup(t, testName, credSpecTemplates, templates)
@@ -390,16 +401,66 @@ func TestPossibleToUpdatePodWithNewCert(t *testing.T) {
  pod := waitForPodToComeUp(t, testConfig.Namespace, "app="+testName)
  assert.Equal(t, expectedCredSpec0, extractContainerCredSpecContents(t, pod, testName))
 
- // read test cert & key
- newCert, _ := os.ReadFile("../testdata/cert.pem")
- newKey, _ := os.ReadFile("../testdata/key.pem")
- testConfig.Cert = base64.StdEncoding.EncodeToString(newCert)
- testConfig.Key = base64.StdEncoding.EncodeToString(newKey)
+ deployMethod := os.Getenv("DEPLOY_METHOD")
+ if deployMethod == "chart" {
+ runCommandOrFail(t, "cmctl", "renew", webHookDeploymentName, "-n", webHookNs)
 
- // apply the new cert & key pair
- renderedTemplate := renderTemplate(t, testConfig, newSecretTemplate)
- success, _, _ := applyManifest(t, renderedTemplate)
- assert.True(t, success)
+ var (
+ timeout = 30 * time.Second
+ start = time.Now()
+ )
+
+ for {
+ success, stdout, stderr := runKubectlCommand(t, "get", "certificaterequest", webHookNs+"-2", "--namespace", webHookNs, "-o", "jsonpath='{.status.conditions[?(@.type==\"Ready\")].status}'")
+ if !success {
+ fmt.Printf("Warning: command failed with %s, %s\n", stdout, stderr)
+ continue
+ }
+
+ if stdout == "'True'" {
+ break
+ } else {
+ fmt.Printf("Warning: status was %s", stdout)
+ }
+
+ if time.Since(start) >= timeout {
+ t.Fatal("Timeout: Unable to get the certificate request status")
+ }
+
+ time.Sleep(1 * time.Second)
+ }
+ } else {
+ /** TODO:
+ * - get a blessed certificate from the API server
+ * (https://github.com/kubernetes-sigs/windows-gmsa/blob/141/admission-webhook/deploy/create-signed-cert.sh)
+ * runCommandOrFail(t, fmt."create-signed-cert.sh --service $NAME --namespace $NAMESPACE --certs-dir $CERTS_DIR", testConfig.Namespace)
+ * - update existing secret in place and wait for the pod to get new secrets which can take time
+ * (https://kubernetes.io/docs/concepts/configuration/secret/#using-secrets-as-files-from-a-pod) - similar to what you are doing here
+ * - kubectl exec into the running pod to see that the secret changed
+ * (using utils like https://github.com/ycheng-kareo/windows-gmsa/blob/watch-reload-cert/admission-webhook/integration_tests/kube.go#L199)
+ **/
+
+ t.Skip("Non chart deployment method not supported for this test")
+ }
+
+ // it takes ~60 seconds for the webhook to pick up the new certificate
+ // so this first run makes sure the old cert still works
+ testName2 := testName + "after-rotation"
+ testConfig2, tearDownFunc2 := integrationTestSetup(t, testName2, credSpecTemplates, templates)
+ defer tearDownFunc2()
+
+ pod2 := waitForPodToComeUp(t, testConfig2.Namespace, "app="+testName2)
+ assert.Equal(t, expectedCredSpec0, extractContainerCredSpecContents(t, pod2, testName2))
+
+ // sleep a bit to ensure the the secret has been propagated to the pod
+ time.Sleep(90 * time.Second)
+
+ testName3 := testName + "after-rotation-propagated"
+ testConfig3, tearDownFunc3 := integrationTestSetup(t, testName3, credSpecTemplates, templates)
+ defer tearDownFunc3()
+
+ pod3 := waitForPodToComeUp(t, testConfig3.Namespace, "app="+testName3)
+ assert.Equal(t, expectedCredSpec0, extractContainerCredSpecContents(t, pod3, testName3))
 }
 
 /* Helpers */

admission-webhook/make/helm.mk

@@ -9,6 +9,10 @@ endif
 install-helm:
  curl https://raw.githubusercontent.com/helm/helm/master/scripts/get-helm-3 | bash
 
+.PHONY: install-cmctl
+install-cmctl:
+ go install github.com/cert-manager/cmctl/v2@latest
+
 .PHONY: helm-chart
 helm-chart:
  $(HELM) package ../charts/gmsa -d ../charts/repo/
@@ -36,8 +40,10 @@ remove_chart:
 # the deploy script as documented in the README, using $K8S_GMSA_DEPLOY_CHART_REPO and
 # $K8S_GMSA_DEPLOY_CHART_VERSION env variables to build the download URL. If VERSION is
 # not set then latest is used.
+# the HELM_INSTALL_FLAGS_FLAGS env var can be set to eg run only specific tests, e.g.:
+# HELM_INSTALL_FLAGS_FLAGS='--set certificates.certReload.enabled=true' make deploy_chart
 .PHONY: _deploy_chart
-_deploy_chart: _copy_image _deploy_certmanager
+_deploy_chart: _copy_image _deploy_certmanager install-cmctl
 ifeq ($(K8S_GMSA_CHART),)
  @ echo "Cannot call target $@ without setting K8S_GMSA_CHART"
  exit 1

admission-webhook/webhook.go

@@ -104,13 +104,14 @@ func (webhook *webhook) start(port int, tlsConfig *tlsConfig, listeningChan chan
  err = webhook.server.Serve(keepAliveListener)
  } else {
  if webhook.config.EnableCertReload {
+ logrus.Infof("Webhook certificate reload enabled")
  certReloader := NewCertReloader(tlsConfig.crtPath, tlsConfig.keyPath)
  _, err = certReloader.LoadCertificate()
  if err != nil {
  return err
  }
 
- go watchCertFiles(certReloader)
+ go watchCertFiles(context.Background(), certReloader)
 
  webhook.server.TLSConfig = &tls.Config{
  GetCertificate: certReloader.GetCertificateFunc(),

charts/gmsa/templates/issuer.yaml

@@ -13,13 +13,40 @@ spec:
  kind: Issuer
  name: {{ .Release.Name }}
  secretName: {{ .Values.certificates.secretName }}
+ {{- if .Values.certificates.certReload.enabled }}
+ privateKey:
+ rotationPolicy: Always
+ {{- end }}
 ---
 {{ template "cert-manager.apiversion" . }}
 kind: Issuer
 metadata:
  name: {{ .Release.Name }}
  namespace: {{ .Release.Namespace }}
  labels: {{ include "gmsa.chartref" . | nindent 4 }}
+spec:
+ ca:
+ secretName: {{ .Release.Name }}-root-ca
+---
+{{ template "cert-manager.apiversion" . }}
+kind: ClusterIssuer
+metadata:
+ name: {{ .Release.Name }}-ca
 spec:
  selfSigned: {}
-{{- end -}}
+---
+{{ template "cert-manager.apiversion" . }}
+kind: Certificate
+metadata:
+ name: {{ .Release.Name }}-ca
+ namespace: {{ .Release.Namespace }}
+spec:
+ isCA: true
+ commonName: {{ .Release.Name }}-ca
+ secretName: {{ .Release.Name }}-root-ca
+ issuerRef:
+ name: {{ .Release.Name }}-ca
+ kind: ClusterIssuer
+ group: cert-manager.io
+---
+{{- end -}}

charts/gmsa/templates/mutatingwebhook.yaml

@@ -4,7 +4,7 @@ metadata:
  name: {{ .Release.Name }}
  {{- if .Values.certificates.certManager.enabled }}
  annotations:
- cert-manager.io/inject-ca-from: {{ .Release.Namespace }}/{{ .Release.Name }}
+ cert-manager.io/inject-ca-from: {{ .Release.Namespace }}/{{ .Release.Name }}-ca
  {{- end }}
  labels: {{ include "gmsa.chartref" . | nindent 4 }}
 webhooks:

charts/gmsa/templates/validatingwebhook.yaml

@@ -4,7 +4,7 @@ metadata:
  name: {{ .Release.Name }}
  {{- if .Values.certificates.certManager.enabled }}
  annotations:
- cert-manager.io/inject-ca-from: {{ .Release.Namespace }}/{{ .Release.Name }}
+ cert-manager.io/inject-ca-from: {{ .Release.Namespace }}/{{ .Release.Name }}-ca
  {{- end }}
  labels: {{ include "gmsa.chartref" . | nindent 4 }}
 webhooks: