fix(datastore): mount cert in runner + improve logs + set hostname (#320

)

* fix(datastore): mount CA cert into runner pod

* fix(datastore): improve logs to include SA

* fix(datastore): set hostname in config

deploy/charts/burrito/templates/config.yaml

@@ -26,13 +26,22 @@ Datastore Authorized Service Accounts
 {{- $server := printf "%s/%s" .Release.Namespace "burrito-server" }}
 {{- $datastoreAuthorizedServiceAccounts = append $datastoreAuthorizedServiceAccounts $server }}
 {{- $_ := set $config.datastore "serviceAccounts" $datastoreAuthorizedServiceAccounts }}
+
+{{/*
+TLS certificates
+*/}}
 {{- if .Values.hermitcrab.tls.certManager.use }}
 {{- $_ := set $config.hermitcrab "certificateSecretName" .Values.hermitcrab.tls.certManager.certificate.spec.secretName }}
 {{- else }}
 {{- $_ := set $config.hermitcrab "certificateSecretName" .Values.hermitcrab.tls.secretName }}
 {{- end }}
 {{- $_ := set $config.hermitcrab "enabled" .Values.hermitcrab.enabled }}
 {{- $_ := set $config.datastore "tls" .Values.datastore.tls.enabled }}
+{{- if .Values.datastore.tls.certManager.use }}
+{{- $_ := set $config.datastore "certificateSecretName" .Values.datastore.tls.certManager.certificate.spec.secretName }}
+{{- else }}
+{{- $_ := set $config.datastore "certificateSecretName" .Values.datastore.tls.secretName }}
+{{- end }}
 
 
 apiVersion: v1

deploy/charts/burrito/values.yaml

@@ -61,8 +61,8 @@ config:
       # -- Service account to use for datastore operations (e.g. reading/writing to storage)
       serviceAccounts: []
       storage:
-        # -- Use in-memory storage for testing - not intended for production use, data will be lost on datastore restart 
-        mock: false 
+        # -- Use in-memory storage for testing - not intended for production use, data will be lost on datastore restart
+        mock: false
         gcs:
           # -- GCS bucket name
           bucket: ""
@@ -71,11 +71,13 @@ config:
           storageAccount: ""
           # -- Azure storage container name
           container: ""
-        s3: 
+        s3:
           # -- S3 bucket name
           bucket: ""
       # -- Datastore exposed port
       addr: ":8080"
+      # -- Datastore hostname, used by controller, server and runner to reach the datastore
+      hostname: "burrito-datastore.burrito-system.svc.cluster.local"
 
     server:
       # -- Server exposed port
@@ -92,7 +94,7 @@ config:
       # -- Configmap name to store the SSH known hosts in the runner
       sshKnownHostsConfigMapName: burrito-ssh-known-hosts
 
-hermitcrab: 
+hermitcrab:
   # -- Enable/Disable Hermitcrab (terraform provider cache in cluster)
   enabled: false
   metadata:
@@ -161,11 +163,11 @@ hermitcrab:
     # -- Hermitcrab resources configuration
     resources:
       limits:
-        cpu: '1'
-        memory: '2Gi'
+        cpu: "1"
+        memory: "2Gi"
       requests:
-        cpu: '300m'
-        memory: '256Mi'
+        cpu: "300m"
+        memory: "256Mi"
     # -- Hermitcrab ports configuration
     ports:
       - name: http
@@ -204,7 +206,7 @@ hermitcrab:
     extraVolumeMounts: {}
 
 global:
-  # -- Global metadata configuration 
+  # -- Global metadata configuration
   metadata:
     labels:
       app.kubernetes.io/part-of: burrito
@@ -235,7 +237,7 @@ global:
     resources: {}
     # -- Global ports configuration
     ports: []
-    # -- Global environment variables 
+    # -- Global environment variables
     envFrom: []
     # -- Additional volumes
     extraVolumes: {}
@@ -399,12 +401,12 @@ datastore:
   # -- Service configuration for the Burrito datastore
   service:
     ports:
-    - name: http
-      port: 80
-      targetPort: http
-    - name: https
-      port: 443
-      targetPort: http
+      - name: http
+        port: 80
+        targetPort: http
+      - name: https
+        port: 443
+        targetPort: http
   # -- TLS configuration for the Burrito datastore
   tls:
     # -- Enable/Disable TLS for the Burrito datastore (recommended for production use)

internal/burrito/config/config.go

@@ -20,7 +20,10 @@ type Config struct {
 }
 
 type DatastoreConfig struct {
+	Hostname                  string        `mapstructure:"hostname"`
+	Addr                      string        `mapstructure:"addr"`
 	TLS                       bool          `mapstructure:"tls"`
+	CertificateSecretName     string        `mapstructure:"certificateSecretName"`
 	Storage                   StorageConfig `mapstructure:"storage"`
 	AuthorizedServiceAccounts []string      `mapstructure:"serviceAccounts"`
 }

internal/controllers/manager.go

@@ -98,10 +98,7 @@ func (c *Controllers) Exec() {
 	if err != nil {
 		log.Fatalf("unable to start manager: %s", err)
 	}
-	datastoreClient := datastore.NewDefaultClient()
-	if c.config.Datastore.TLS {
-		datastoreClient.Scheme = "https"
-	}
+	datastoreClient := datastore.NewDefaultClient(c.config.Datastore)
 	config, err := rest.InClusterConfig()
 	if err != nil {
 		panic(err.Error())

internal/controllers/terraformlayer/controller.go

@@ -113,6 +113,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	state, conditions := r.GetState(ctx, layer)
 	lastResult, err := r.Datastore.GetPlan(layer.Namespace, layer.Name, layer.Status.LastRun.Name, "", "short")
 	if err != nil {
+		log.Errorf("failed to get plan for layer %s: %s", layer.Name, err)
 		r.Recorder.Event(layer, corev1.EventTypeNormal, "Reconciliation", "Failed to get last Result")
 		lastResult = []byte("Error getting last Result")
 	}

internal/controllers/terraformrun/pod.go

@@ -3,7 +3,6 @@ package terraformrun
 import (
 	"context"
 	"fmt"
-	"strconv"
 
 	configv1alpha1 "github.com/padok-team/burrito/api/v1alpha1"
 	"github.com/padok-team/burrito/internal/burrito/config"
@@ -55,19 +54,21 @@ func (r *Reconciler) GetLinkedPods(run *configv1alpha1.TerraformRun) (*corev1.Po
 	return list, nil
 }
 
-func (r *Reconciler) ensureHermitcrabSecret(tenantNamespace string) error {
+func (r *Reconciler) ensureCertificateAuthoritySecret(tenantNamespace, caSecretName string) error {
 	secret := &corev1.Secret{}
-	err := r.Client.Get(context.Background(), client.ObjectKey{Namespace: r.Config.Controller.MainNamespace,
-		Name: r.Config.Hermitcrab.CertificateSecretName}, secret)
+	err := r.Client.Get(context.Background(), client.ObjectKey{
+		Namespace: r.Config.Controller.MainNamespace,
+		Name:      caSecretName,
+	}, secret)
 	if err != nil {
 		return err
 	}
 	if _, ok := secret.Data["ca.crt"]; !ok {
-		return fmt.Errorf("ca.crt not found in secret %s", r.Config.Hermitcrab.CertificateSecretName)
+		return fmt.Errorf("ca.crt not found in secret %s", caSecretName)
 	}
 	secret = &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      r.Config.Hermitcrab.CertificateSecretName,
+			Name:      caSecretName,
 			Namespace: tenantNamespace,
 		},
 		Data: map[string][]byte{
@@ -83,38 +84,45 @@ func (r *Reconciler) ensureHermitcrabSecret(tenantNamespace string) error {
 	} else if err != nil {
 		return err
 	}
-	log.Infof("hermitcrab certificate is available in namespace %s", tenantNamespace)
+	log.Infof("CA certificate is available in namespace %s", tenantNamespace)
 	return nil
 }
 
+func mountCA(podSpec *corev1.PodSpec, caSecretName, caName string) {
+	volumeName := fmt.Sprintf("%s-cert", caName)
+	mountPath := fmt.Sprintf("/etc/ssl/certs/%s.crt", caName)
+	caFilename := fmt.Sprintf("%s.crt", caName)
+
+	podSpec.Volumes = append(podSpec.Volumes, corev1.Volume{
+		Name: volumeName,
+		VolumeSource: corev1.VolumeSource{
+			Secret: &corev1.SecretVolumeSource{
+				SecretName: caSecretName,
+				Items: []corev1.KeyToPath{
+					{
+						Key:  "ca.crt",
+						Path: caFilename,
+					},
+				},
+			},
+		},
+	})
+	podSpec.Containers[0].VolumeMounts = append(podSpec.Containers[0].VolumeMounts, corev1.VolumeMount{
+		MountPath: mountPath,
+		Name:      volumeName,
+		SubPath:   caFilename,
+	})
+}
+
 func (r *Reconciler) getPod(run *configv1alpha1.TerraformRun, layer *configv1alpha1.TerraformLayer, repository *configv1alpha1.TerraformRepository) corev1.Pod {
 	defaultSpec := defaultPodSpec(r.Config, layer, repository, run)
 
 	if r.Config.Hermitcrab.Enabled {
-		err := r.ensureHermitcrabSecret(layer.Namespace)
+		err := r.ensureCertificateAuthoritySecret(layer.Namespace, r.Config.Hermitcrab.CertificateSecretName)
 		if err != nil {
 			log.Errorf("failed to ensure HermitCrab secret in namespace %s: %s", layer.Namespace, err)
 		} else {
-			defaultSpec.Volumes = append(defaultSpec.Volumes, corev1.Volume{
-				Name: "hermitcrab-ca-cert",
-				VolumeSource: corev1.VolumeSource{
-					Secret: &corev1.SecretVolumeSource{
-						SecretName: r.Config.Hermitcrab.CertificateSecretName,
-						Items: []corev1.KeyToPath{
-							{
-								Key:  "ca.crt",
-								Path: "hermitcrab-ca.crt",
-							},
-						},
-					},
-				},
-			})
-			defaultSpec.Containers[0].VolumeMounts = append(defaultSpec.Containers[0].VolumeMounts, corev1.VolumeMount{
-				MountPath: "/etc/ssl/certs/hermitcrab-ca.crt",
-				Name:      "hermitcrab-ca-cert",
-				SubPath:   "hermitcrab-ca.crt",
-			})
-
+			mountCA(&defaultSpec, r.Config.Hermitcrab.CertificateSecretName, "burrito-hermitcrab-ca")
 			defaultSpec.Containers[0].Env = append(defaultSpec.Containers[0].Env,
 				corev1.EnvVar{
 					Name:  "HERMITCRAB_ENABLED",
@@ -124,13 +132,21 @@ func (r *Reconciler) getPod(run *configv1alpha1.TerraformRun, layer *configv1alp
 					Name:  "HERMITCRAB_URL",
 					Value: fmt.Sprintf("https://burrito-hermitcrab.%s.svc.cluster.local/v1/providers/", r.Config.Controller.MainNamespace),
 				},
-				corev1.EnvVar{
-					Name:  "BURRITO_DATASTORE_TLS",
-					Value: strconv.FormatBool(r.Config.Datastore.TLS),
-				},
 			)
 		}
 	}
+	if r.Config.Datastore.TLS {
+		err := r.ensureCertificateAuthoritySecret(layer.Namespace, r.Config.Datastore.CertificateSecretName)
+		if err != nil {
+			log.Errorf("failed to ensure Datastore secret in namespace %s: %s", layer.Namespace, err)
+		} else {
+			mountCA(&defaultSpec, r.Config.Datastore.CertificateSecretName, "burrito-datastore-ca")
+			defaultSpec.Containers[0].Env = append(defaultSpec.Containers[0].Env, corev1.EnvVar{
+				Name:  "BURRITO_DATASTORE_TLS",
+				Value: "true",
+			})
+		}
+	}
 	switch Action(run.Spec.Action) {
 	case PlanAction:
 		defaultSpec.Containers[0].Env = append(defaultSpec.Containers[0].Env, corev1.EnvVar{
@@ -299,6 +315,10 @@ func defaultPodSpec(config *config.Config, layer *configv1alpha1.TerraformLayer,
 						Name:  "SSH_KNOWN_HOSTS",
 						Value: "/home/burrito/.ssh/known_hosts",
 					},
+					{
+						Name:  "BURRITO_DATASTORE_HOSTNAME",
+						Value: config.Datastore.Hostname,
+					},
 				},
 			},
 		},

internal/datastore/client/client.go

@@ -2,21 +2,21 @@ package client
 
 import (
 	"bytes"
-	"crypto/tls"
 	"encoding/json"
 	"fmt"
 	"io"
 	"net/http"
 	"net/url"
 	"os"
 
+	"github.com/padok-team/burrito/internal/burrito/config"
 	"github.com/padok-team/burrito/internal/datastore/api"
 	storageerrors "github.com/padok-team/burrito/internal/datastore/storage/error"
+	log "github.com/sirupsen/logrus"
 )
 
 const (
-	DefaultHostname = "burrito-datastore.burrito-system"
-	DefaultPath     = "/var/run/secrets/token/burrito"
+	DefaultTokenPath = "/var/run/secrets/token/burrito"
 )
 
 type Client interface {
@@ -27,39 +27,37 @@ type Client interface {
 }
 
 type DefaultClient struct {
-	Hostname string
-	Path     string
-	Scheme   string
-	client   *http.Client
+	Hostname  string
+	tokenPath string
+	scheme    string
+	client    *http.Client
 }
 
-func NewDefaultClient() *DefaultClient {
+func NewDefaultClient(config config.DatastoreConfig) *DefaultClient {
+	scheme := "http"
+	if config.TLS {
+		log.Info("using TLS for datastore")
+		scheme = "https"
+	}
 	return &DefaultClient{
-		Hostname: DefaultHostname,
-		Path:     DefaultPath,
-		Scheme:   "http",
-		client: &http.Client{
-			Transport: &http.Transport{
-				TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
-			},
-		},
+		Hostname:  config.Hostname,
+		tokenPath: DefaultTokenPath,
+		scheme:    scheme,
+		client:    &http.Client{},
 	}
 }
 
 func (c *DefaultClient) buildRequest(path string, queryParams url.Values, method string, body io.Reader) (*http.Request, error) {
-	url := fmt.Sprintf("%s://%s%s?%s", c.Scheme, c.Hostname, path, queryParams.Encode())
+	url := fmt.Sprintf("%s://%s%s?%s", c.scheme, c.Hostname, path, queryParams.Encode())
 	req, err := http.NewRequest(method, url, body)
 	if err != nil {
 		return nil, err
 	}
-	token, err := os.ReadFile(c.Path)
+	token, err := os.ReadFile(c.tokenPath)
 	if err != nil {
 		return nil, err
 	}
 	req.Header.Set("Authorization", string(token))
-	if err != nil {
-		return nil, err
-	}
 	return req, nil
 }