Add secrets support to ImageBuild CR

 - Adds the ability to pass k8s secret names by namespace / name to be
   consumed by the ImageBuild request using buildkit secrets. The
   `secrets` field is added to the CR as optional, and is fully
   backwards compatible with previous requests.

   Previously, the Helm chart supported exposing service level secrets
   into *all* builds, but this adds supports on a per request basis.

   Buildkit secrets must be mounted in via Dockerfile syntax:
   https://docs.docker.com/engine/reference/builder/#run---mounttypesecret

   Consumption via Dockerfile is therefore similar to:

   RUN --mount=type=secret,id=domino-compute/mysecret/foo cat /run/secrets/foo

 - For the secret to be accessible by Hephaestus, it must have the
   label `hephaestus-accessible: "true"`. This prevents the build
   service from having access to arbitrary secrets in the cluster and
   requires clients to specifically opt-in.

 - Additionally the `hephaestus-owned: "true"` label can be added to
   secrets to help manage their lifecycle. When set, the secret will be
   updated to specify the attached ImageBuild as the owner -- when
   ImageBuild resources are routinely purged by the service, those
   secrets will be cleaned up at the same time. This removes the burden
   of secret cleanup from clients, but changes the cleanup timing to be
   non-determinstic.

   The ClusterRole for Hephaestus is updated to allow for secret
   resource updates to support this feature.

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:

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/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,75 @@
+package secrets
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	"github.com/go-logr/logr"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"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)
+		secret, err := secretClient.Get(ctx, secretRef.Name, metav1.GetOptions{})
+
+		path := strings.Join([]string{secretRef.Namespace, secretRef.Name}, "/")
+		log.Info("Reading secret", "path", path)
+
+		if err != nil {
+			return map[string][]byte{}, fmt.Errorf("failure loading secret %q: %w", path, err)
+		}
+
+		// prevent exfiltration of arbitrary secret values by using the presence of this lable
+		if _, ok := secret.Labels[hephv1.AccessLabel]; !ok {
+			return map[string][]byte{}, fmt.Errorf("secret %q inaccessible - requires label %q", path, hephv1.AccessLabel)
+		}
+
+		// 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
+}