[DOM-47678] Add support for passing buildkit secrets via k8s secrets

api/api-rules/violation_exceptions.list

@@ -3,6 +3,7 @@ API rule violation: list_type_missing,github.com/dominodatalab/hephaestus/pkg/ap
 API rule violation: list_type_missing,github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1,ImageBuildSpec,Images
 API rule violation: list_type_missing,github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1,ImageBuildSpec,ImportRemoteBuildCache
 API rule violation: list_type_missing,github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1,ImageBuildSpec,RegistryAuth
+API rule violation: list_type_missing,github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1,ImageBuildSpec,Secrets
 API rule violation: list_type_missing,github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1,ImageBuildStatus,Conditions
 API rule violation: list_type_missing,github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1,ImageBuildStatus,Transitions
 API rule violation: list_type_missing,github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1,ImageBuildStatusTransitionMessage,ImageURLs

api/openapi-spec/swagger.json

@@ -2946,6 +2946,14 @@
             "default": {},
             "$ref": "#/definitions/.RegistryCredentials"
           }
+        },
+        "secrets": {
+          "description": "Secrets provides references to Kubernetes secrets to expose to individual image builds.",
+          "type": "array",
+          "items": {
+            "default": {},
+            "$ref": "#/definitions/.SecretReference"
+          }
         }
       }
     },
@@ -3193,6 +3201,17 @@
           "type": "string"
         }
       }
+    },
+    ".SecretReference": {
+      "type": "object",
+      "properties": {
+        "name": {
+          "type": "string"
+        },
+        "namespace": {
+          "type": "string"
+        }
+      }
     }
   },
   "securityDefinitions": {

deployments/crds/hephaestus.dominodatalab.com_imagebuilds.yaml

@@ -125,6 +125,17 @@ spec:
                       type: string
                   type: object
                 type: array
+              secrets:
+                description: Secrets provides references to Kubernetes secrets to
+                  expose to individual image builds.
+                items:
+                  properties:
+                    name:
+                      type: string
+                    namespace:
+                      type: string
+                  type: object
+                type: array
             type: object
           status:
             properties:

deployments/helm/hephaestus/templates/controller/clusterrole.yaml

@@ -83,9 +83,15 @@ rules:
       - ""
     resources:
       - nodes
+    verbs:
+      - get
+  - apiGroups:
+      - ""
+    resources:
       - secrets
     verbs:
       - get
+      - update
   - apiGroups:
       - apps
     resources:

deployments/helm/hephaestus/values.yaml

@@ -159,7 +159,7 @@ controller:
     # Defaults to 4.25 mins for fetch retries and an unlimited amount of time to extract.
     fetchAndExtractTimeout: null
 
-    # Secrets (name: path) to expose into builds that request it
+    # Global secrets (name: path) to expose into all image builds
     secrets: {}
 
     # Cloud-based registry credentials configuration

examples/resources/imagebuild.yaml

@@ -8,6 +8,9 @@ spec:
     - username/repo:tag
   buildArgs:
     - ENV=development
+  secrets:
+    - name: mySecret
+      namespace: default
   cacheMode: min
   cacheTag: local-test
   disableCacheExports: false

pkg/api/hephaestus/v1/imagebuild_types.go

@@ -32,6 +32,8 @@ type ImageBuildSpec struct {
 	DisableLocalBuildCache bool `json:"disableBuildCache,omitempty"`
 	// DisableCacheLayerExport will remove the "inline" cache metadata from the image configuration.
 	DisableCacheLayerExport bool `json:"disableCacheExport,omitempty"`
+	// Secrets provides references to Kubernetes secrets to expose to individual image builds.
+	Secrets []SecretReference `json:"secrets,omitempty"`
 }
 
 type ImageBuildTransition struct {

pkg/api/hephaestus/v1/types.go

@@ -26,6 +26,15 @@ const (
 	PhaseFailed Phase = "Failed"
 )
 
+const (
+	// Kubernetes metadata set by clients required to allow reading secrets by Hephaestus.
+	// Safeguards against accidental secret exposure / exfiltration.
+	AccessLabel = "hephaestus-accessible"
+	// Kubernetes metadata set by clients, to manage secret lifetime.
+	// When set, the given secret gets deleted at the same time as the ImageBuild that uses it.
+	OwnedLabel = "hephaestus-owned"
+)
+
 type BasicAuthCredentials struct {
 	Username string `json:"username,omitempty"`
 	Password string `json:"password,omitempty"`
@@ -49,6 +58,11 @@ type RegistryCredentials struct {
 	Secret        *SecretCredentials    `json:"secret,omitempty"`
 }
 
+type SecretReference struct {
+	Name      string `json:"name,omitempty"`
+	Namespace string `json:"namespace,omitempty"`
+}
+
 // ImageBuildStatusTransitionMessage contains information about ImageBuild status transitions.
 //
 // This type is used to publish JSON-formatted messages to one or more configured messaging

pkg/buildkit/buildkit.go

@@ -104,6 +104,7 @@ type BuildOptions struct {
 	ImportCache              []string
 	DisableInlineCacheExport bool
 	Secrets                  map[string]string
+	SecretsData              map[string][]byte
 	FetchAndExtractTimeout   time.Duration
 }
 
@@ -177,6 +178,11 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error {
 		secrets[name] = contents
 	}
 
+	// merge in preloaded data
+	for name, contents := range opts.SecretsData {
+		secrets[name] = contents
+	}
+
 	// build solve options
 	solveOpt := bkclient.SolveOpt{
 		Frontend:      "dockerfile.v0",

pkg/config/config.go

@@ -105,7 +105,7 @@ type Buildkit struct {
 	PoolEndpointWatchTimeout *int64 `json:"poolEndpointWatchTimeout" yaml:"poolEndpointWatchTimeout"`
 	// MTLS parameters.
 	MTLS *BuildkitMTLS `json:"mtls,omitempty" yaml:"mtls,omitempty"`
-	// Secrets provided to buildkitd during the build process.
+	// Global secrets provided to buildkitd during the build process for all image builds.
 	Secrets map[string]string `json:"secrets" yaml:"secrets,omitempty"`
 	// Registries parameters.
 	Registries map[string]RegistryConfig `json:"registries,omitempty" yaml:"registries,omitempty"`

pkg/controller/imagebuild/component/builddispatcher.go

@@ -19,6 +19,7 @@ import (
 	"github.com/dominodatalab/hephaestus/pkg/config"
 	"github.com/dominodatalab/hephaestus/pkg/controller/support/credentials"
 	"github.com/dominodatalab/hephaestus/pkg/controller/support/phase"
+	"github.com/dominodatalab/hephaestus/pkg/controller/support/secrets"
 )
 
 type BuildDispatcherComponent struct {
@@ -91,6 +92,21 @@ func (c *BuildDispatcherComponent) Reconcile(ctx *core.Context) (ctrl.Result, er
 	}
 	c.phase.SetInitializing(ctx, obj)
 
+	// Extracts cluster secrets into data to pass to buildkit
+	log.Info("Processing references to build secrets")
+	secretsReadSeq := txn.StartSegment("cluster-secrets-read")
+	secretsData, err := secrets.ReadSecrets(ctx, obj, log, ctx.Config, ctx.Scheme)
+	if err != nil {
+		err = fmt.Errorf("cluster secrets processing failed: %w", err)
+		txn.NoticeError(newrelic.Error{
+			Message: err.Error(),
+			Class:   "ClusterSecretsReadError",
+		})
+
+		return ctrl.Result{}, c.phase.SetFailed(ctx, obj, err)
+	}
+	secretsReadSeq.End()
+
 	log.Info("Processing and persisting registry credentials")
 	persistCredsSeg := txn.StartSegment("credentials-persist")
 	configDir, helpMessage, err := credentials.Persist(ctx, buildLog, ctx.Config, obj.Spec.RegistryAuth)
@@ -189,6 +205,7 @@ func (c *BuildDispatcherComponent) Reconcile(ctx *core.Context) (ctrl.Result, er
 		ImportCache:              obj.Spec.ImportRemoteBuildCache,
 		DisableInlineCacheExport: obj.Spec.DisableCacheLayerExport,
 		Secrets:                  c.cfg.Secrets,
+		SecretsData:              secretsData,
 		FetchAndExtractTimeout:   c.cfg.FetchAndExtractTimeout,
 	}
 	log.Info("Dispatching image build", "images", buildOpts.Images)

pkg/controller/support/secrets/secrets.go

@@ -0,0 +1,81 @@
+package secrets
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	"github.com/go-logr/logr"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/fields"
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/rest"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+
+	hephv1 "github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1"
+)
+
+// exists only so it can overridden by tests with a fake client
+var clientsetFunc = func(config *rest.Config) (kubernetes.Interface, error) {
+	return kubernetes.NewForConfig(config)
+}
+
+func ReadSecrets(
+	ctx context.Context,
+	obj *hephv1.ImageBuild,
+	log logr.Logger,
+	cfg *rest.Config,
+	scheme *runtime.Scheme,
+) (map[string][]byte, error) {
+	clientset, err := clientsetFunc(cfg)
+	if err != nil {
+		return map[string][]byte{}, fmt.Errorf("failure to get kubernetes client: %w", err)
+	}
+	v1 := clientset.CoreV1()
+
+	// Extracts secrets into data to pass to buildkit
+	secretsData := make(map[string][]byte)
+	for _, secretRef := range obj.Spec.Secrets {
+		secretClient := v1.Secrets(secretRef.Namespace)
+
+		path := strings.Join([]string{secretRef.Namespace, secretRef.Name}, "/")
+		log.Info("Finding secret", "path", path)
+		fields := fields.SelectorFromSet(map[string]string{"Namespace": secretRef.Namespace, "Name": secretRef.Name})
+		// prevent exfiltration of arbitrary secret values by using the presence of this label
+		labels := labels.SelectorFromSet(map[string]string{hephv1.AccessLabel: "true"})
+		secrets, err := secretClient.List(ctx,
+			metav1.ListOptions{FieldSelector: fields.String(), LabelSelector: labels.String()})
+
+		if err != nil {
+			return map[string][]byte{}, fmt.Errorf("failure querying for secret %q: %w", path, err)
+		}
+
+		if len(secrets.Items) == 0 {
+			return map[string][]byte{}, fmt.Errorf("secret %q unreadable or missing required label %q", path, hephv1.AccessLabel)
+		}
+		secret := &secrets.Items[0]
+
+		// adopt the secret resource if hephaestus-owned is true to delete when ImageBuild is deleted
+		if _, ok := secret.Labels[hephv1.OwnedLabel]; ok {
+			log.Info("Taking ownership of secret", "owner", obj.Name, "secret", path)
+
+			// non-fatal error thats logged but ignored
+			if err = controllerutil.SetOwnerReference(obj, secret, scheme); err != nil {
+				log.Info("Ignoring error taking ownership of secret", "secret", path, "error", err)
+			} else if _, err = secretClient.Update(ctx, secret, metav1.UpdateOptions{}); err != nil {
+				log.Info("Ignoring error taking ownership of secret", "secret", path, "error", err)
+			}
+		}
+
+		// builds a path for the secret like {namespace}/{name}/{key} to avoid hash key collisions
+		for filename, data := range secret.Data {
+			name := strings.Join([]string{path, filename}, "/")
+			secretsData[name] = data
+			log.Info("Read secret bytes", "path", name, "bytes", len(data))
+		}
+	}
+
+	return secretsData, nil
+}