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 Jun 30, 2023
1 parent 4ae9efe commit 3a9a795
Show file tree
Hide file tree
Showing 8 changed files with 107 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
11 changes: 11 additions & 0 deletions pkg/api/hephaestus/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ 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"
)

type BasicAuthCredentials struct {
Username string `json:"username,omitempty"`
Password string `json:"password,omitempty"`
Expand All @@ -49,6 +55,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
53 changes: 53 additions & 0 deletions pkg/controller/imagebuild/component/builddispatcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,18 @@ import (
"context"
"fmt"
"os"
"strings"
"sync"
"time"

"github.com/dominodatalab/controller-util/core"
"github.com/go-logr/logr"
"github.com/newrelic/go-agent/v3/newrelic"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

hephv1 "github.com/dominodatalab/hephaestus/pkg/api/hephaestus/v1"
"github.com/dominodatalab/hephaestus/pkg/buildkit"
Expand Down Expand Up @@ -181,6 +185,54 @@ func (c *BuildDispatcherComponent) Reconcile(ctx *core.Context) (ctrl.Result, er
}
clientInitSeg.End()

// TODO: refactor this secret extraction to a helper like the credentials persist call
// configDir, helpMessage, err := credentials.Persist(ctx, buildLog, ctx.Config, obj.Spec.RegistryAuth)
clientset, err := kubernetes.NewForConfig(ctx.Config)
if err != nil {
return ctrl.Result{}, c.phase.SetFailed(ctx, obj, fmt.Errorf("failure to get kubernetes client: %w", err))
}

// TODO: add logging and NewRelic 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 ctrl.Result{}, c.phase.SetFailed(ctx, obj,
fmt.Errorf("failure to load secret %q/%q: %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 ctrl.Result{}, c.phase.SetFailed(ctx, obj,
fmt.Errorf("secret %q/%q 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/
// NOTE:
if err = controllerutil.SetOwnerReference(obj, secret, ctx.Scheme); err != nil {
// non-fatal error thats logged but ignored
log.Error(err, "Ignoring error taking ownership of secret %q/%q", secretRef.Namespace, secretRef.Name)
}
// TODO: this is an alternative way to manually cook up metadata rather than just using controllerutil
// secret.OwnerReferences = append(secret.OwnerReferences)
// _, err = client.Update(ctx, secret, metav1.UpdateOptions{})
// if err != nil {
// log.Info("")
// }

// builds a path from the secret name and the key from the data array like {name}/{key}
for path, data := range secret.Data {
name := strings.Join([]string{secret.Name, path}, "/")
secretsData[name] = data
}
}

buildOpts := buildkit.BuildOptions{
Context: obj.Spec.Context,
Images: obj.Spec.Images,
Expand All @@ -189,6 +241,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

0 comments on commit 3a9a795

Please sign in to comment.