From 3237e1019d2c88dc050200b00418b3eb1d9105d1 Mon Sep 17 00:00:00 2001 From: ddl-ebrown Date: Thu, 29 Jun 2023 18:42:45 -0700 Subject: [PATCH] WIP - add first class secrets support --- api/api-rules/violation_exceptions.list | 1 + ...haestus.dominodatalab.com_imagebuilds.yaml | 11 +++ examples/resources/imagebuild.yaml | 3 + pkg/api/hephaestus/v1/imagebuild_types.go | 2 + pkg/api/hephaestus/v1/types.go | 14 ++++ .../hephaestus/v1/zz_generated.deepcopy.go | 20 ++++++ pkg/buildkit/buildkit.go | 6 ++ .../imagebuild/component/builddispatcher.go | 17 +++++ pkg/controller/support/secrets/secrets.go | 71 +++++++++++++++++++ 9 files changed, 145 insertions(+) create mode 100644 pkg/controller/support/secrets/secrets.go diff --git a/api/api-rules/violation_exceptions.list b/api/api-rules/violation_exceptions.list index 4b077250..dbef0df7 100644 --- a/api/api-rules/violation_exceptions.list +++ b/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 diff --git a/deployments/crds/hephaestus.dominodatalab.com_imagebuilds.yaml b/deployments/crds/hephaestus.dominodatalab.com_imagebuilds.yaml index cc684a67..74ef5c11 100644 --- a/deployments/crds/hephaestus.dominodatalab.com_imagebuilds.yaml +++ b/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: diff --git a/examples/resources/imagebuild.yaml b/examples/resources/imagebuild.yaml index 0238e9e2..30f33d78 100644 --- a/examples/resources/imagebuild.yaml +++ b/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 diff --git a/pkg/api/hephaestus/v1/imagebuild_types.go b/pkg/api/hephaestus/v1/imagebuild_types.go index b96e1d70..8a92213c 100644 --- a/pkg/api/hephaestus/v1/imagebuild_types.go +++ b/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 { diff --git a/pkg/api/hephaestus/v1/types.go b/pkg/api/hephaestus/v1/types.go index 456b0592..db38ae65 100644 --- a/pkg/api/hephaestus/v1/types.go +++ b/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-access" + // 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 diff --git a/pkg/api/hephaestus/v1/zz_generated.deepcopy.go b/pkg/api/hephaestus/v1/zz_generated.deepcopy.go index e4b23b03..1f281f24 100644 --- a/pkg/api/hephaestus/v1/zz_generated.deepcopy.go +++ b/pkg/api/hephaestus/v1/zz_generated.deepcopy.go @@ -258,6 +258,11 @@ func (in *ImageBuildSpec) DeepCopyInto(out *ImageBuildSpec) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.Secrets != nil { + in, out := &in.Secrets, &out.Secrets + *out = make([]SecretReference, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ImageBuildSpec. @@ -506,3 +511,18 @@ func (in *SecretCredentials) DeepCopy() *SecretCredentials { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SecretReference) DeepCopyInto(out *SecretReference) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecretReference. +func (in *SecretReference) DeepCopy() *SecretReference { + if in == nil { + return nil + } + out := new(SecretReference) + in.DeepCopyInto(out) + return out +} diff --git a/pkg/buildkit/buildkit.go b/pkg/buildkit/buildkit.go index 94aad1c4..998780ee 100644 --- a/pkg/buildkit/buildkit.go +++ b/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", diff --git a/pkg/controller/imagebuild/component/builddispatcher.go b/pkg/controller/imagebuild/component/builddispatcher.go index 6953e98d..862a37e3 100644 --- a/pkg/controller/imagebuild/component/builddispatcher.go +++ b/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) diff --git a/pkg/controller/support/secrets/secrets.go b/pkg/controller/support/secrets/secrets.go new file mode 100644 index 00000000..f717d2cb --- /dev/null +++ b/pkg/controller/support/secrets/secrets.go @@ -0,0 +1,71 @@ +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" +) + +func ReadSecrets( + ctx context.Context, + obj *hephv1.ImageBuild, + log logr.Logger, + cfg *rest.Config, + scheme *runtime.Scheme, +) (map[string][]byte, error) { + clientset, err := kubernetes.NewForConfig(cfg) + if err != nil { + return map[string][]byte{}, fmt.Errorf("failure to get kubernetes client: %w", err) + } + + // TODO: add logging stuff + // TODO: does it make more sense to project the secret into the filesystem somehow and then just use existing secrets? + // Extracts secrets into data to pass to buildkit + secretsData := make(map[string][]byte) + for _, secretRef := range obj.Spec.Secrets { + client := clientset.CoreV1().Secrets(secretRef.Namespace) + + secret, err := client.Get(ctx, secretRef.Name, metav1.GetOptions{}) + + if err != nil { + return map[string][]byte{}, + fmt.Errorf("failure to load secret '%s/%s': %w", secretRef.Namespace, secretRef.Name, 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 '%s/%s' requires label %q", secretRef.Namespace, secretRef.Name, hephv1.AccessLabel) + } + + // TODO: verify ImageBuild takes ownership of the secret and cleans it up after use to prevent cluster cruft + // https://kubernetes.io/blog/2021/05/14/using-finalizers-to-control-deletion/ + if _, ok := secret.Labels[hephv1.OwnedLabel]; ok { + log.Info("taking ownership of secret '%s/%s'", "owner", obj.Name, + "secret", fmt.Sprintf("%s/%s", secretRef.Namespace, secretRef.Name)) + if err = controllerutil.SetOwnerReference(obj, secret, scheme); err != nil { + // non-fatal error thats logged but ignored + log.Error(err, "Ignoring error taking ownership of secret", + "secret", fmt.Sprintf("%s/%s", secretRef.Namespace, secretRef.Name)) + } + } + + // builds a path for the secret like {namespace}/{name}/{key} to avoid hash key collisions + for path, data := range secret.Data { + name := strings.Join([]string{secret.Namespace, secret.Name, path}, "/") + secretsData[name] = data + log.Info("Read secret bytes", "path", name, "bytes", len(data)) + } + } + + return secretsData, nil +}