Skip to content

Commit

Permalink
Merge pull request #1208 from stormqueen1990/feat/support-builder-sig…
Browse files Browse the repository at this point in the history
…ning

Add support for signing Builders and ClusterBuilders
  • Loading branch information
tomkennedy513 authored Oct 2, 2023
2 parents 2a98bed + 4a76d00 commit 08a114c
Show file tree
Hide file tree
Showing 18 changed files with 1,875 additions and 148 deletions.
4 changes: 3 additions & 1 deletion cmd/completion/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"path/filepath"
"strings"

"github.com/sigstore/cosign/v2/pkg/oci/remote"

"github.com/BurntSushi/toml"
"github.com/buildpacks/lifecycle/platform/files"
"github.com/google/go-containerregistry/pkg/authn"
Expand Down Expand Up @@ -154,7 +156,7 @@ func main() {

func signImage(report files.Report, keychain authn.Keychain) error {
if hasCosign() {
cosignSigner := cosign.NewImageSigner(logger, sign.SignCmd)
cosignSigner := cosign.NewImageSigner(sign.SignCmd, remote.SignatureTag)

annotations, err := mapKeyValueArgs(cosignAnnotations)
if err != nil {
Expand Down
13 changes: 11 additions & 2 deletions cmd/controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ import (
"os"
"time"

"github.com/pivotal/kpack/pkg/secret"

"github.com/pivotal/kpack/pkg/cosign"
"github.com/sigstore/cosign/v2/cmd/cosign/cli/sign"
ociremote "github.com/sigstore/cosign/v2/pkg/oci/remote"

"github.com/Masterminds/semver/v3"
"go.uber.org/zap"
"golang.org/x/sync/errgroup"
Expand Down Expand Up @@ -194,18 +200,21 @@ func main() {
KpackVersion: cmd.Identifer,
LifecycleProvider: lifecycleProvider,
KeychainFactory: keychainFactory,
ImageSigner: cosign.NewImageSigner(sign.SignCmd, ociremote.SignatureTag),
}

podProgressLogger := &buildchange.ProgressLogger{
K8sClient: k8sClient,
}

secretFetcher := &secret.Fetcher{Client: k8sClient}

buildController := build.NewController(ctx, options, k8sClient, buildInformer, podInformer, metadataRetriever, buildpodGenerator, podProgressLogger, keychainFactory, *injectedSidecarSupport)
imageController := image.NewController(ctx, options, k8sClient, imageInformer, buildInformer, duckBuilderInformer, sourceResolverInformer, pvcInformer, *enablePriorityClasses)
sourceResolverController := sourceresolver.NewController(ctx, options, sourceResolverInformer, gitResolver, blobResolver, registryResolver)
builderController, builderResync := builder.NewController(ctx, options, builderInformer, builderCreator, keychainFactory, clusterStoreInformer, buildpackInformer, clusterBuildpackInformer, clusterStackInformer)
builderController, builderResync := builder.NewController(ctx, options, builderInformer, builderCreator, keychainFactory, clusterStoreInformer, buildpackInformer, clusterBuildpackInformer, clusterStackInformer, secretFetcher)
buildpackController := buildpack.NewController(ctx, options, keychainFactory, buildpackInformer, remoteStoreReader)
clusterBuilderController, clusterBuilderResync := clusterbuilder.NewController(ctx, options, clusterBuilderInformer, builderCreator, keychainFactory, clusterStoreInformer, clusterBuildpackInformer, clusterStackInformer)
clusterBuilderController, clusterBuilderResync := clusterbuilder.NewController(ctx, options, clusterBuilderInformer, builderCreator, keychainFactory, clusterStoreInformer, clusterBuildpackInformer, clusterStackInformer, secretFetcher)
clusterBuildpackController := clusterbuildpack.NewController(ctx, options, keychainFactory, clusterBuildpackInformer, remoteStoreReader)
clusterStoreController := clusterstore.NewController(ctx, options, keychainFactory, clusterStoreInformer, remoteStoreReader)
clusterStackController := clusterstack.NewController(ctx, options, keychainFactory, clusterStackInformer, remoteStackReader)
Expand Down
6 changes: 4 additions & 2 deletions pkg/apis/build/v1alpha2/builder_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ type BuilderRecord struct {
ObservedStoreGeneration int64
ObservedStackGeneration int64
OS string
SignaturePaths []CosignSignature
}

func (bs *BuilderStatus) BuilderRecord(record BuilderRecord) {
Expand All @@ -33,10 +34,11 @@ func (bs *BuilderStatus) BuilderRecord(record BuilderRecord) {
bs.ObservedStoreGeneration = record.ObservedStoreGeneration
bs.ObservedStackGeneration = record.ObservedStackGeneration
bs.OS = record.OS
bs.SignaturePaths = record.SignaturePaths
}

func (cb *BuilderStatus) ErrorCreate(err error) {
cb.Status = corev1alpha1.Status{
func (bs *BuilderStatus) ErrorCreate(err error) {
bs.Status = corev1alpha1.Status{
Conditions: corev1alpha1.Conditions{
{
Type: corev1alpha1.ConditionReady,
Expand Down
7 changes: 7 additions & 0 deletions pkg/apis/build/v1alpha2/builder_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ type NamespacedBuilderSpec struct {
BackwardsCompatibleServiceAccount string `json:"serviceAccount,omitempty"`
}

// +k8s:openapi-gen=true
type CosignSignature struct {
SigningSecret string `json:"signingSecret"`
TargetDigest string `json:"targetDigest"`
}

// +k8s:openapi-gen=true
type BuilderStatus struct {
corev1alpha1.Status `json:",inline"`
Expand All @@ -65,6 +71,7 @@ type BuilderStatus struct {
ObservedStackGeneration int64 `json:"observedStackGeneration,omitempty"`
ObservedStoreGeneration int64 `json:"observedStoreGeneration,omitempty"`
OS string `json:"os,omitempty"`
SignaturePaths []CosignSignature `json:"signaturePaths,omitempty"`
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
Expand Down
18 changes: 17 additions & 1 deletion pkg/cnb/create_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (

"github.com/google/go-containerregistry/pkg/authn"
ggcrv1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/pivotal/kpack/pkg/cosign"
corev1 "k8s.io/api/core/v1"

buildapi "github.com/pivotal/kpack/pkg/apis/build/v1alpha2"
corev1alpha1 "github.com/pivotal/kpack/pkg/apis/core/v1alpha1"
Expand All @@ -25,10 +27,12 @@ type RemoteBuilderCreator struct {
LifecycleProvider LifecycleProvider
KpackVersion string
KeychainFactory registry.KeychainFactory
ImageSigner cosign.BuilderSigner
}

func (r *RemoteBuilderCreator) CreateBuilder(ctx context.Context, builderKeychain authn.Keychain, stackKeychain authn.Keychain, fetcher RemoteBuildpackFetcher, clusterStack *buildapi.ClusterStack, spec buildapi.BuilderSpec) (buildapi.BuilderRecord, error) {
func (r *RemoteBuilderCreator) CreateBuilder(ctx context.Context, builderKeychain authn.Keychain, stackKeychain authn.Keychain, fetcher RemoteBuildpackFetcher, clusterStack *buildapi.ClusterStack, spec buildapi.BuilderSpec, serviceAccountSecrets []*corev1.Secret) (buildapi.BuilderRecord, error) {
buildImage, _, err := r.RegistryClient.Fetch(stackKeychain, clusterStack.Status.BuildImage.LatestImage)

if err != nil {
return buildapi.BuilderRecord{}, err
}
Expand Down Expand Up @@ -76,6 +80,17 @@ func (r *RemoteBuilderCreator) CreateBuilder(ctx context.Context, builderKeychai
return buildapi.BuilderRecord{}, err
}

var (
signaturePaths = make([]buildapi.CosignSignature, 0)
)

if len(serviceAccountSecrets) > 0 {
signaturePaths, err = r.ImageSigner.SignBuilder(ctx, identifier, serviceAccountSecrets, builderKeychain)
if err != nil {
return buildapi.BuilderRecord{}, err
}
}

builder := buildapi.BuilderRecord{
Image: identifier,
Stack: corev1alpha1.BuildStack{
Expand All @@ -87,6 +102,7 @@ func (r *RemoteBuilderCreator) CreateBuilder(ctx context.Context, builderKeychai
ObservedStackGeneration: clusterStack.Status.ObservedGeneration,
ObservedStoreGeneration: fetcher.ClusterStoreObservedGeneration(),
OS: config.OS,
SignaturePaths: signaturePaths,
}

return builder, nil
Expand Down
87 changes: 77 additions & 10 deletions pkg/cnb/create_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,12 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
KpackVersion: "v1.2.3 (git sha: abcdefg123456)",
KeychainFactory: keychainFactory,
LifecycleProvider: lifecycleProvider,
ImageSigner: &fakeBuilderSigner{
signBuilder: func(ctx context.Context, s string, secrets []*corev1.Secret, keychain authn.Keychain) ([]buildapi.CosignSignature, error) {
// no-op
return nil, nil
},
},
}

addBuildpack = func(t *testing.T, id, version, homepage, api string, stacks []corev1alpha1.BuildpackStack) {
Expand Down Expand Up @@ -346,7 +352,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
})

it("creates a custom builder", func() {
builderRecord, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec)
builderRecord, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{})
require.NoError(t, err)

assert.Len(t, builderRecord.Buildpacks, 4)
Expand Down Expand Up @@ -636,11 +642,12 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
})

it("creates images deterministically ", func() {
original, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec)
original, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{})
require.NoError(t, err)

for i := 1; i <= 50; i++ {
other, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec)
other, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{})

require.NoError(t, err)

require.Equal(t, original.Image, other.Image)
Expand Down Expand Up @@ -670,7 +677,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
},
}

_, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec)
_, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{})
require.EqualError(t, err, "validating buildpack io.buildpack.unsupported.stack@v4: stack io.buildpacks.stacks.some-stack is not supported")
})

Expand All @@ -694,7 +701,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
}},
}}

_, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec)
_, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{})
require.EqualError(t, err, "validating buildpack io.buildpack.unsupported.mixin@v4: stack missing mixin(s): something-missing-mixin, something-missing-mixin2")
})

Expand Down Expand Up @@ -739,7 +746,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
}},
}}

_, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec)
_, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{})
require.Nil(t, err)
})

Expand All @@ -764,7 +771,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
}},
}}

_, err := subject.CreateBuilder(ctx, builderKeychain, nil, fetcher, stack, clusterBuilderSpec)
_, err := subject.CreateBuilder(ctx, builderKeychain, nil, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{})
require.Error(t, err, "validating buildpack io.buildpack.relaxed.old.mixin@v4: stack missing mixin(s): build:common-mixin, run:common-mixin, another-common-mixin")
})

Expand All @@ -787,7 +794,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
}},
}}

_, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec)
_, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{})
require.EqualError(t, err, "validating buildpack io.buildpack.unsupported.buildpack.api@v4: unsupported buildpack api: 0.1, expecting: 0.2, 0.3")
})

Expand Down Expand Up @@ -830,7 +837,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
}},
}}

_, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec)
_, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{})
require.NoError(t, err)
})
})
Expand All @@ -857,13 +864,73 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
},
}

_, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec)
_, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{})
require.EqualError(t, err, "unsupported platform apis in kpack lifecycle: 0.1, 0.2, 0.999, expecting one of: 0.3, 0.4, 0.5, 0.6, 0.7, 0.8")
})
})

when("signing a builder image", func() {
it("does not populate the signature paths when no secrets were present", func() {
builderRecord, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{})
require.NoError(t, err)
require.NotNil(t, builderRecord)
require.Empty(t, builderRecord.SignaturePaths)
})

it("returns an error if signing fails", func() {
subject.ImageSigner = &fakeBuilderSigner{
signBuilder: func(ctx context.Context, s string, secrets []*corev1.Secret, keychain authn.Keychain) ([]buildapi.CosignSignature, error) {
return nil, fmt.Errorf("failed to sign builder")
},
}

fakeSecret := corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "cosign-creds",
Namespace: "test-namespace",
},
}

_, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{&fakeSecret})
require.Error(t, err)
})

it("populates the signature paths when signing succeeds", func() {
fakeSecret := corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "cosign-creds",
Namespace: "test-namespace",
},
}

subject.ImageSigner = &fakeBuilderSigner{
signBuilder: func(ctx context.Context, s string, secrets []*corev1.Secret, keychain authn.Keychain) ([]buildapi.CosignSignature, error) {
return []buildapi.CosignSignature{
{
SigningSecret: fmt.Sprintf("k8s://%s/%s", fakeSecret.Namespace, fakeSecret.Name),
TargetDigest: "registry.local/test-image:signature-tag",
},
}, nil
},
}

builderRecord, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{&fakeSecret})
require.NoError(t, err)
require.NotNil(t, builderRecord)
require.NotEmpty(t, builderRecord.SignaturePaths)
})
})
})
}

type fakeBuilderSigner struct {
signBuilder func(context.Context, string, []*corev1.Secret, authn.Keychain) ([]buildapi.CosignSignature, error)
}

func (s *fakeBuilderSigner) SignBuilder(ctx context.Context, imageReference string, signingSecrets []*corev1.Secret, builderKeychain authn.Keychain) ([]buildapi.CosignSignature, error) {
return s.signBuilder(ctx, imageReference, signingSecrets, builderKeychain)
}

type fakeLifecycleProvider struct {
metadata LifecycleMetadata
layers map[string]v1.Layer
Expand Down
Loading

0 comments on commit 08a114c

Please sign in to comment.