Skip to content

Commit

Permalink
WIP - add first class secrets support
Browse files Browse the repository at this point in the history
  • Loading branch information
ddl-ebrown committed Jul 3, 2023
1 parent 4ae9efe commit 3237e10
Show file tree
Hide file tree
Showing 9 changed files with 145 additions and 0 deletions.
1 change: 1 addition & 0 deletions api/api-rules/violation_exceptions.list
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions deployments/crds/hephaestus.dominodatalab.com_imagebuilds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
3 changes: 3 additions & 0 deletions examples/resources/imagebuild.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ spec:
- username/repo:tag
buildArgs:
- ENV=development
secrets:
- name: mySecret
namespace: default
cacheMode: min
cacheTag: local-test
disableCacheExports: false
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/hephaestus/v1/imagebuild_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 14 additions & 0 deletions pkg/api/hephaestus/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand All @@ -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
Expand Down
20 changes: 20 additions & 0 deletions pkg/api/hephaestus/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions pkg/buildkit/buildkit.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ type BuildOptions struct {
ImportCache []string
DisableInlineCacheExport bool
Secrets map[string]string
SecretsData map[string][]byte
FetchAndExtractTimeout time.Duration
}

Expand Down Expand Up @@ -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",
Expand Down
17 changes: 17 additions & 0 deletions pkg/controller/imagebuild/component/builddispatcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
71 changes: 71 additions & 0 deletions pkg/controller/support/secrets/secrets.go
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit 3237e10

Please sign in to comment.