Merge pull request #262 from weaveworks/guard-cert-with-ch

use channel to trigger cert rotation

charts/tf-controller/Chart.yaml

@@ -2,5 +2,5 @@ apiVersion: v2
 name: tf-controller
 description: The Helm chart for Weave GitOps Terraform Controller
 type: application
-version: 0.3.4
-appVersion: "v0.10.0-rc.5"
+version: 0.3.5
+appVersion: "v0.10.0-rc.6"

charts/tf-controller/templates/deployment.yaml

@@ -31,6 +31,9 @@ spec:
         - --log-encoding=json
         - --enable-leader-election
         - --concurrent={{ .Values.concurrency }}
+        - --ca-cert-validity-duration={{ .Values.caCertValidityDuration }}
+        - --cert-rotation-check-frequency={{ .Values.certRotationCheckFrequency }}
+        - --cert-validity-duration={{ .Values.certValidityDuration }}
         command:
         - /sbin/tini
         - --

charts/tf-controller/values.yaml

@@ -1,11 +1,14 @@
 replicaCount: 1
 concurrency: 1
+caCertValidityDuration: 168h0m
+certRotationCheckFrequency: 30m0s
+certValidityDuration: 6h0m
 logLevel: info
 image:
   repository: ghcr.io/weaveworks/tf-controller
   pullPolicy: IfNotPresent
   # Overrides the image tag whose default is the chart appVersion.
-  tag: "v0.10.0-rc.5"
+  tag: "v0.10.0-rc.6"
 # extraEnv -- Additional container environment variables.
 extraEnv: {}
 imagePullSecrets: []
@@ -22,7 +25,7 @@ serviceAccount:
 runner:
   image:
     repository: ghcr.io/weaveworks/tf-runner
-    tag: "v0.10.0-rc.5"
+    tag: "v0.10.0-rc.6"
   serviceAccount:
     # Specifies whether a service account should be created
     create: true
@@ -32,23 +35,20 @@ runner:
     name: ""
 installCRDs: true
 podAnnotations: {}
-
 # Pod-level security context
 podSecurityContext:
   fsGroup: 1337
-
 # Container-level security context
 securityContext:
   allowPrivilegeEscalation: false
   capabilities:
     drop:
-    - ALL
+      - ALL
   readOnlyRootFilesystem: true
   runAsNonRoot: true
   runAsUser: 65532
   seccompProfile:
     type: RuntimeDefault
-
 resources:
   limits:
     cpu: 1000m

config/manager/kustomization.yaml

@@ -8,4 +8,4 @@ generatorOptions:
 images:
   - name: weaveworks/tf-controller
     newName: ghcr.io/weaveworks/tf-controller
-    newTag: v0.10.0-rc.5
+    newTag: v0.10.0-rc.6

controllers/terraform_controller.go

@@ -99,10 +99,15 @@ type TerraformReconciler struct {
 // For more details, check Reconcile and its Result here:
 // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.10.0/pkg/reconcile
 func (r *TerraformReconciler) Reconcile(ctx context.Context, req ctrl.Request) (retResult ctrl.Result, retErr error) {
-	// TODO need to think about many controller instances are sharing the same secret
 
-	// should be blocked if cert is not ready yet
+	// Should be blocked if cert is not ready yet
+	<-r.CertRotator.Ready
+
 	if r.CertRotator.IsCertReady(ctx) == false {
+		// Trigger the CA rotation
+		if len(r.CertRotator.TriggerCARotation) < cap(r.CertRotator.TriggerCARotation) {
+			r.CertRotator.TriggerCARotation <- struct{}{}
+		}
 		return ctrl.Result{Requeue: true}, fmt.Errorf("server cert not ready yet")
 	}
 
@@ -1682,8 +1687,7 @@ func (r *TerraformReconciler) getRunnerConnection(ctx context.Context, terraform
 	// if the cert is not valid, refresh it, recreate the pod and wait for the ip to
 	// be available before connecting
 	if !isCertValid {
-		nsSAN := fmt.Sprintf("*.%s.pod.cluster.local", terraform.Namespace)
-		if err := r.CertRotator.RefreshRunnerCertIfNeeded(ctx, nsSAN, &tlsSecret); err != nil {
+		if err := r.CertRotator.GenerateRunnerCertForNamespace(ctx, terraform.Namespace, &tlsSecret); err != nil {
 			return nil, fmt.Errorf("failed to refresh cert before opening runner connection: %w", err)
 		}
 
@@ -1905,8 +1909,7 @@ func (r *TerraformReconciler) reconcileRunnerSecret(ctx context.Context, terrafo
 
 	// this hostname will be used to generate a cert valid
 	// for all pods in the given namespace
-	hostname := fmt.Sprintf("*.%s.pod.cluster.local", terraform.Namespace)
-	if err := r.CertRotator.RefreshRunnerCertIfNeeded(ctx, hostname, &tlsSecret); err != nil {
+	if err := r.CertRotator.GenerateRunnerCertForNamespace(ctx, terraform.Namespace, &tlsSecret); err != nil {
 		return err
 	}
 

mtls/grpc.go

@@ -31,8 +31,7 @@ func StartGRPCServerForTesting(ctx context.Context, server *runner.TerraformRunn
 		},
 	}
 
-	hostname := fmt.Sprintf("*.%s.pod.cluster.local", namespace)
-	if err := rotator.RefreshRunnerCertIfNeeded(ctx, hostname, tlsSecret); err != nil {
+	if err := rotator.GenerateRunnerCertForNamespace(ctx, namespace, tlsSecret); err != nil {
 		return err
 	}
 

mtls/rotator.go

@@ -72,6 +72,7 @@ type CertRotator struct {
 	CertValidityDuration   time.Duration
 	RotationCheckFrequency time.Duration
 	LookaheadInterval      time.Duration
+	TriggerCARotation      chan struct{}
 }
 
 // AddRotator adds the CertRotator to the manager
@@ -96,21 +97,32 @@ func (cr *CertRotator) Start(ctx context.Context) error {
 
 	// explicitly rotate on the first round so that the certificate
 	// can be bootstrapped, otherwise manager exits before a cert can be written
-	if err := cr.refreshCAandServerCertIfNeeded(); err != nil {
+	if err := cr.refreshCAAndServerCertIfNeeded(); err != nil {
 		crLog.Error(err, "could not refresh cert on startup")
 		return err
 	}
 
 	close(cr.Ready)
 
+	cr.TriggerCARotation = make(chan struct{}, 1)
+	defer func() {
+		close(cr.TriggerCARotation)
+	}()
+
 	ticker := time.NewTicker(cr.RotationCheckFrequency)
 
 tickerLoop:
 	for {
 		select {
+		case <-cr.TriggerCARotation:
+			cr.Ready = make(chan struct{})
+			if err := cr.refreshCAAndServerCertIfNeeded(); err != nil {
+				crLog.Error(err, "error rotating certs via trigger")
+			}
+			close(cr.Ready)
 		case <-ticker.C:
-			if err := cr.refreshCAandServerCertIfNeeded(); err != nil {
-				crLog.Error(err, "error rotating certs")
+			if err := cr.refreshCAAndServerCertIfNeeded(); err != nil {
+				crLog.Error(err, "error rotating certs via ticker")
 			}
 		case <-ctx.Done():
 			break tickerLoop
@@ -138,8 +150,8 @@ func (cr *CertRotator) IsCertReady(ctx context.Context) bool {
 	return true
 }
 
-// refreshCAandServerCertIfNeeded returns whether there's any error when refreshing the certs if needed.
-func (cr *CertRotator) refreshCAandServerCertIfNeeded() error {
+// refreshCAAndServerCertIfNeeded returns whether there's any error when refreshing the certs if needed.
+func (cr *CertRotator) refreshCAAndServerCertIfNeeded() error {
 	refreshFn := func() (bool, error) {
 		ctx := context.Background()
 
@@ -263,28 +275,31 @@ func (cr *CertRotator) refreshCerts(ctx context.Context, refreshCA bool, secret
 	return nil
 }
 
-func (cr *CertRotator) RefreshRunnerCertIfNeeded(ctx context.Context, hostname string, tlsCertSecret *corev1.Secret) error {
+func (cr *CertRotator) GenerateRunnerCertForNamespace(ctx context.Context, namespace string, tlsCertSecret *corev1.Secret) error {
+	hostname := fmt.Sprintf("*.%s.pod.cluster.local", namespace)
 
 	if err := cr.client.Get(ctx, client.ObjectKeyFromObject(tlsCertSecret), tlsCertSecret); err != nil {
 		if !apierrors.IsNotFound(err) {
 			return err
 		}
+
 		// create the runner tls secret
 		if err := cr.client.Create(ctx, tlsCertSecret); err != nil {
 			return err
 		}
 	}
 
-	var caArtifacts *KeyPairArtifacts
+	// Validity is from -1 to cr.CertValidityDuration
 	now := time.Now()
 	begin := now.Add(-1 * time.Hour)
 	end := now.Add(cr.CertValidityDuration)
-	caSecret := &corev1.Secret{}
-	if err := cr.client.Get(ctx, cr.SecretKey, caSecret); err != nil {
+
+	var caSecret corev1.Secret
+	if err := cr.client.Get(ctx, cr.SecretKey, &caSecret); err != nil {
 		return err
 	}
 
-	caArtifacts, err := parseArtifacts(caCertName, caKeyName, caSecret)
+	caArtifacts, err := parseArtifacts(caCertName, caKeyName, &caSecret)
 	if err != nil {
 		return err
 	}