From 772b60333a62d9f877f38110552ce6a1d514ac1c Mon Sep 17 00:00:00 2001 From: ddl-ebrown Date: Thu, 29 Jun 2023 18:42:45 -0700 Subject: [PATCH] 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 | 1 + api/openapi-spec/swagger.json | 19 ++ ...haestus.dominodatalab.com_imagebuilds.yaml | 11 + .../templates/controller/clusterrole.yaml | 6 + 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/api/hephaestus/v1/zz_generated.openapi.go | 41 ++- pkg/buildkit/buildkit.go | 6 + .../imagebuild/component/builddispatcher.go | 17 ++ pkg/controller/support/secrets/secrets.go | 75 +++++ .../support/secrets/secrets_test.go | 263 ++++++++++++++++++ test/functional/helpers_test.go | 15 + .../docker-context/secrets/Dockerfile | 2 + .../docker-context/secrets/archive.tgz | Bin 0 -> 200 bytes 16 files changed, 494 insertions(+), 1 deletion(-) create mode 100644 pkg/controller/support/secrets/secrets.go create mode 100644 pkg/controller/support/secrets/secrets_test.go create mode 100644 test/functional/testdata/docker-context/secrets/Dockerfile create mode 100644 test/functional/testdata/docker-context/secrets/archive.tgz 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/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index bb30c23a..9680e99b 100644 --- a/api/openapi-spec/swagger.json +++ b/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": { 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/deployments/helm/hephaestus/templates/controller/clusterrole.yaml b/deployments/helm/hephaestus/templates/controller/clusterrole.yaml index 5b2d4c7f..d3abd347 100644 --- a/deployments/helm/hephaestus/templates/controller/clusterrole.yaml +++ b/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: 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..4415721e 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-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 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/api/hephaestus/v1/zz_generated.openapi.go b/pkg/api/hephaestus/v1/zz_generated.openapi.go index 058c4480..359ba7e5 100644 --- a/pkg/api/hephaestus/v1/zz_generated.openapi.go +++ b/pkg/api/hephaestus/v1/zz_generated.openapi.go @@ -34,6 +34,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1.ImageCacheStatus": schema_pkg_api_hephaestus_v1_ImageCacheStatus(ref), "github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1.RegistryCredentials": schema_pkg_api_hephaestus_v1_RegistryCredentials(ref), "github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1.SecretCredentials": schema_pkg_api_hephaestus_v1_SecretCredentials(ref), + "github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1.SecretReference": schema_pkg_api_hephaestus_v1_SecretReference(ref), } } @@ -482,11 +483,25 @@ func schema_pkg_api_hephaestus_v1_ImageBuildSpec(ref common.ReferenceCallback) c Format: "", }, }, + "secrets": { + SchemaProps: spec.SchemaProps{ + Description: "Secrets provides references to Kubernetes secrets to expose to individual image builds.", + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1.SecretReference"), + }, + }, + }, + }, + }, }, }, }, Dependencies: []string{ - "github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1.ImageBuildAMQPOverrides", "github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1.RegistryCredentials"}, + "github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1.ImageBuildAMQPOverrides", "github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1.RegistryCredentials", "github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1.SecretReference"}, } } @@ -942,3 +957,27 @@ func schema_pkg_api_hephaestus_v1_SecretCredentials(ref common.ReferenceCallback }, } } + +func schema_pkg_api_hephaestus_v1_SecretReference(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "name": { + SchemaProps: spec.SchemaProps{ + Type: []string{"string"}, + Format: "", + }, + }, + "namespace": { + SchemaProps: spec.SchemaProps{ + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, + } +} 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..0c6aced6 --- /dev/null +++ b/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 +} diff --git a/pkg/controller/support/secrets/secrets_test.go b/pkg/controller/support/secrets/secrets_test.go new file mode 100644 index 00000000..6327fa2d --- /dev/null +++ b/pkg/controller/support/secrets/secrets_test.go @@ -0,0 +1,263 @@ +package secrets + +import ( + "context" + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/rest" + + hephv1 "github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1" +) + +// NOTE: this doesn't cover k8s permissioning for secret access +func TestReadSecrets(t *testing.T) { + for name, tc := range map[string]struct { + RequestedSecrets []hephv1.SecretReference + ClientResponse []runtime.Object + Want map[string][]byte + WantError bool + }{ + "returns data for secret in same namespace": { + RequestedSecrets: []hephv1.SecretReference{{Namespace: "domino-compute", Name: "foo"}}, + ClientResponse: []runtime.Object{&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "domino-compute", + Name: "foo", + Labels: map[string]string{"hephaestus-accessible": "true"}, + }, + Data: map[string][]byte{"bar": []byte("hello")}, + }}, + Want: map[string][]byte{"domino-compute/foo/bar": []byte("hello")}, + }, + "returns data from multiple secrets using unique keys": { + RequestedSecrets: []hephv1.SecretReference{ + {Namespace: "domino-compute", Name: "foo"}, + {Namespace: "domino-compute", Name: "bar"}, + }, + ClientResponse: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "domino-compute", + Name: "foo", + Labels: map[string]string{"hephaestus-accessible": "true"}, + }, + Data: map[string][]byte{"bar": []byte("hello")}, + }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "domino-compute", + Name: "bar", + Labels: map[string]string{"hephaestus-accessible": "true"}, + }, + Data: map[string][]byte{"baz": []byte("goodbye")}, + }}, + Want: map[string][]byte{ + "domino-compute/foo/bar": []byte("hello"), + "domino-compute/bar/baz": []byte("goodbye"), + }, + }, + "returns empty data for empty secret": { + RequestedSecrets: []hephv1.SecretReference{{Namespace: "domino-compute", Name: "foo"}}, + ClientResponse: []runtime.Object{&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "domino-compute", + Name: "foo", + Labels: map[string]string{"hephaestus-accessible": "true"}, + }, + }}, + Want: map[string][]byte{}, + }, + "returns all data within a secret, including multiline data": { + RequestedSecrets: []hephv1.SecretReference{{Namespace: "domino-compute", Name: "groups"}}, + ClientResponse: []runtime.Object{&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "domino-compute", + Name: "groups", + Labels: map[string]string{"hephaestus-accessible": "true"}, + }, + Data: map[string][]byte{ + "atcq": []byte("q-tip, phife, ali shaheed, jarobi"), + "wu-tang": []byte("rza\ngza\nodb\ninspectah deck\nu-god\nghost face\nmethod man"), + }, + }}, + Want: map[string][]byte{ + "domino-compute/groups/atcq": []byte("q-tip, phife, ali shaheed, jarobi"), + "domino-compute/groups/wu-tang": []byte("rza\ngza\nodb\ninspectah deck\nu-god\nghost face\nmethod man"), + }, + }, + "returns data for secrets in different namespace": { + RequestedSecrets: []hephv1.SecretReference{{Namespace: "foo", Name: "bar"}}, + ClientResponse: []runtime.Object{&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "foo", + Name: "bar", + Labels: map[string]string{"hephaestus-accessible": "true"}, + }, + Data: map[string][]byte{"baz": []byte("hello")}, + }}, + Want: map[string][]byte{"foo/bar/baz": []byte("hello")}, + }, + "uses namespace to differentiate secrets": { + RequestedSecrets: []hephv1.SecretReference{{Namespace: "domino-test", Name: "foo"}}, + ClientResponse: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "domino-compute", + Name: "foo", + Labels: map[string]string{"hephaestus-accessible": "true"}, + }, + Data: map[string][]byte{"bar": []byte("hello")}, + }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "domino-test", + Name: "foo", + Labels: map[string]string{"hephaestus-accessible": "true"}, + }, + Data: map[string][]byte{"bar": []byte("goodbye")}, + }, + }, + Want: map[string][]byte{"domino-test/foo/bar": []byte("goodbye")}, + }, + "errors for missing secrets": { + RequestedSecrets: []hephv1.SecretReference{{Namespace: "foo", Name: "bar"}}, + ClientResponse: []runtime.Object{}, + WantError: true, + }, + "requires secrets to have hephaestus-accessible label": { + RequestedSecrets: []hephv1.SecretReference{{Namespace: "foo", Name: "bar"}}, + ClientResponse: []runtime.Object{&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "foo", + Name: "bar", + }, + Data: map[string][]byte{"baz": []byte("hello")}, + }}, + WantError: true, + }, + } { + t.Run(name, func(t *testing.T) { + // Assume a static domino-compute namespace for all ImageBuild requests + img := &hephv1.ImageBuild{ + Status: hephv1.ImageBuildStatus{ + Phase: hephv1.PhaseInitializing, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "image-build-request", + Namespace: "domino-compute", + }, + Spec: hephv1.ImageBuildSpec{ + Secrets: tc.RequestedSecrets, + }, + } + + clientsetFunc = func(*rest.Config) (kubernetes.Interface, error) { + return fake.NewSimpleClientset(tc.ClientResponse...), nil + } + + secretData, err := ReadSecrets(context.Background(), img, logr.Discard(), nil, nil) + + if tc.WantError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tc.Want, secretData) + } + }) + } +} + +func TestReadSecretsTakesOwnership(t *testing.T) { + for name, tc := range map[string]struct { + RequestedSecret hephv1.SecretReference + ReturnedSecret *corev1.Secret + Want map[string][]byte + WantImageBuildOwner bool + }{ + "does not change owner by default": { + RequestedSecret: hephv1.SecretReference{Namespace: "domino-compute", Name: "foo"}, + ReturnedSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "domino-compute", + Name: "foo", + Labels: map[string]string{"hephaestus-accessible": "true"}, + }, + Data: map[string][]byte{"bar": []byte("hello")}, + }, + Want: map[string][]byte{"domino-compute/foo/bar": []byte("hello")}, + }, + "updates owner reference to ImageBuild when hephaestus-owned label set": { + RequestedSecret: hephv1.SecretReference{Namespace: "domino-compute", Name: "foo"}, + ReturnedSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "domino-compute", + Name: "foo", + Labels: map[string]string{ + "hephaestus-owned": "true", + "hephaestus-accessible": "true", + }, + }, + Data: map[string][]byte{"bar": []byte("hello")}, + }, + Want: map[string][]byte{"domino-compute/foo/bar": []byte("hello")}, + WantImageBuildOwner: true, + }, + "does not update owner references across namespaces, but still returns data": { + RequestedSecret: hephv1.SecretReference{Namespace: "domino-other", Name: "foo"}, + ReturnedSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "domino-other", + Name: "foo", + Labels: map[string]string{ + "hephaestus-owned": "true", + "hephaestus-accessible": "true", + }, + }, + Data: map[string][]byte{"bar": []byte("hello")}, + }, + Want: map[string][]byte{"domino-other/foo/bar": []byte("hello")}, + }, + } { + t.Run(name, func(t *testing.T) { + // Assume a static domino-compute namespace for all ImageBuild requests + img := &hephv1.ImageBuild{ + Status: hephv1.ImageBuildStatus{ + Phase: hephv1.PhaseInitializing, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "image-build-request", + Namespace: "domino-compute", + }, + Spec: hephv1.ImageBuildSpec{ + Secrets: []hephv1.SecretReference{tc.RequestedSecret}, + }, + } + + simpleClient := fake.NewSimpleClientset(tc.ReturnedSecret) + clientsetFunc = func(*rest.Config) (kubernetes.Interface, error) { return simpleClient, nil } + + schema, _ := hephv1.SchemeBuilder.Build() + secretData, err := ReadSecrets(context.Background(), img, logr.Discard(), nil, schema) + + assert.NoError(t, err) + assert.Equal(t, tc.Want, secretData) + + updatedSecret, err := simpleClient.CoreV1().Secrets(tc.RequestedSecret.Namespace).Get(context.Background(), tc.RequestedSecret.Name, metav1.GetOptions{}) + assert.NoError(t, err) + + if !tc.WantImageBuildOwner { + assert.Empty(t, updatedSecret.OwnerReferences) + } else { + assert.Equal(t, "ImageBuild", updatedSecret.OwnerReferences[0].Kind) + assert.Equal(t, "image-build-request", updatedSecret.OwnerReferences[0].Name) + } + }) + } +} diff --git a/test/functional/helpers_test.go b/test/functional/helpers_test.go index d47dc4a5..0a0508c2 100644 --- a/test/functional/helpers_test.go +++ b/test/functional/helpers_test.go @@ -409,6 +409,20 @@ func (suite *GenericImageBuilderTestSuite) TestImageBuilding() { assert.Equalf(t, ib.Status.Phase, hephv1.PhaseSucceeded, "failed build with message %q", ib.Status.Conditions[0].Message) }) + suite.T().Run("secrets", func(t *testing.T) { + build := newImageBuild( + secretBuildContext, + "docker-registry:5000/test-ns/test-repo", + nil, + ) + build.Spec.Secrets = []SecretReference{"Namespace": "domino-compute", "Name": "foo"} + + // TODO: how to retrieve secrets in a fake way? + ib := createBuild(t, ctx, suite.hephClient, build) + + assert.Equalf(t, ib.Status.Phase, hephv1.PhaseSucceeded, "failed build with message %q", ib.Status.Conditions[0].Message) + }) + suite.T().Run("build_failure", func(t *testing.T) { build := newImageBuild( errorBuildContext, @@ -500,6 +514,7 @@ const ( errorBuildContext python39JupyterBuildContext multiStageBuildContext + secretBuildContext ) func (c remoteDockerBuildContext) String() string { diff --git a/test/functional/testdata/docker-context/secrets/Dockerfile b/test/functional/testdata/docker-context/secrets/Dockerfile new file mode 100644 index 00000000..54ccfe56 --- /dev/null +++ b/test/functional/testdata/docker-context/secrets/Dockerfile @@ -0,0 +1,2 @@ +FROM alpine:3.16 +RUN --mount=type=secret,id=domino-compute/foo/bar cat /run/secrets/bar diff --git a/test/functional/testdata/docker-context/secrets/archive.tgz b/test/functional/testdata/docker-context/secrets/archive.tgz new file mode 100644 index 0000000000000000000000000000000000000000..c3ca65b78a1d71ccdd6f80ef9fc1911f897b08d2 GIT binary patch literal 200 zcmV;(05|_1iwFQd>7`@<1MSb>3W6{c25_%^it_-ev#kvR@1na3gdSk(MkC#1PNKKZ zKTtQ}okXzT&BqxZ+(uPdWg4YTRGOykok*lHNx?0Y+(BY{oy^T{d<=hmm*;t&x9kN(BI{KE2mk