Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cert rotation for e2e chart #145

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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

22 changes: 13 additions & 9 deletions admission-webhook/cert_reloader.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"context"
"crypto/tls"
"sync"

Expand Down Expand Up @@ -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)
Expand All @@ -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
}
}
}()
Expand Down
11 changes: 7 additions & 4 deletions admission-webhook/cert_reloader_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"context"
"crypto/tls"
"os"
"testing"
Expand Down Expand Up @@ -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(),
Expand All @@ -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
}
105 changes: 83 additions & 22 deletions admission-webhook/integration_tests/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package integrationtests

import (
"context"
"encoding/base64"
"encoding/json"
"fmt"
"html/template"
Expand All @@ -11,7 +10,9 @@ import (
"path"
"reflect"
"strconv"
"strings"
"testing"
"time"

"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -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)
Expand All @@ -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 */
Expand Down
8 changes: 7 additions & 1 deletion admission-webhook/make/helm.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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/
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion admission-webhook/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
29 changes: 28 additions & 1 deletion charts/gmsa/templates/issuer.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 -}}
2 changes: 1 addition & 1 deletion charts/gmsa/templates/mutatingwebhook.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion charts/gmsa/templates/validatingwebhook.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading