stick to consistent tense for err msgs

and make the start/stop time parsing best effort - the slsa spec says its optional so we shouldn't fail the whole attestation if we can't find it Signed-off-by: Bohan Chen <[email protected]>
buildpacks-community · Jan 8, 2024 · 7401311 · 7401311
1 parent 84c91fa
commit 7401311
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 47 deletions.
diff --git a/pkg/reconciler/build/build.go b/pkg/reconciler/build/build.go
@@ -8,7 +8,6 @@ import (
 	"github.com/google/go-containerregistry/pkg/authn"
 	ggcrv1 "github.com/google/go-containerregistry/pkg/v1"
 	intoto "github.com/in-toto/in-toto-golang/in_toto"
-	"github.com/pkg/errors"
 	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
@@ -206,7 +205,7 @@ func (c *Reconciler) reconcile(ctx context.Context, build *buildapi.Build) error
 		if c.FeatureFlags.GenerateSlsaAttestation {
 			attestDigest, err = c.attestBuild(ctx, build, buildMetadata, pod)
 			if err != nil {
-				return fmt.Errorf("attesting build: %v", err)
+				return fmt.Errorf("failed to attest build: %v", err)
 			}
 		}
 
@@ -399,7 +398,7 @@ func (c *Reconciler) buildMetadataFromBuildPod(pod *corev1.Pod) (*cnb.BuildMetad
 			return cnb.DecompressBuildMetadata(status.State.Terminated.Message)
 		}
 	}
-	return nil, errors.New(buildapi.CompletionContainerName + " container not found")
+	return nil, fmt.Errorf("%v container not found", buildapi.CompletionContainerName)
 }
 
 func (c *Reconciler) attestBuild(ctx context.Context, build *buildapi.Build, buildMetadata *cnb.BuildMetadata, pod *corev1.Pod) (string, error) {
@@ -414,18 +413,18 @@ func (c *Reconciler) attestBuild(ctx context.Context, build *buildapi.Build, bui
 
 	controllerSecrets, err := c.SecretFetcher.SecretsForSystemServiceAccount(ctx)
 	if err != nil {
-		return "", fmt.Errorf("getting controller secrets: %v", err)
+		return "", fmt.Errorf("failed to get controller secrets: %v", err)
 	}
 
 	buildSecrets, err := c.SecretFetcher.SecretsForServiceAccount(ctx, build.ServiceAccount(), build.Namespace)
 	if err != nil {
-		return "", fmt.Errorf("getting service account secrets: %v", err)
+		return "", fmt.Errorf("failed to get service account secrets: %v", err)
 	}
 
 	secrets := append(controllerSecrets, buildSecrets...)
 	signingKeys, err := secret.FilterAndExtractSLSASecrets(secrets)
 	if err != nil {
-		return "", fmt.Errorf("parsing slsa secrets: %v", err)
+		return "", fmt.Errorf("failed to parse slsa secrets: %v", err)
 	}
 
 	signers := make([]slsa.Signer, len(signingKeys))
@@ -438,7 +437,7 @@ func (c *Reconciler) attestBuild(ctx context.Context, build *buildapi.Build, bui
 			s, err = slsa.NewPKCS8Signer(key.Key, key.SecretName)
 		}
 		if err != nil {
-			return "", fmt.Errorf("creating signer: %v", err)
+			return "", fmt.Errorf("failed to create signer: %v", err)
 		}
 		signers[i] = s
 	}
@@ -450,22 +449,22 @@ func (c *Reconciler) attestBuild(ctx context.Context, build *buildapi.Build, bui
 
 	deps, err := c.attestBuildDeps(ctx, build, pod, secrets)
 	if err != nil {
-		return "", fmt.Errorf("gathering build deps: %v", err)
+		return "", fmt.Errorf("failed to gather build deps: %v", err)
 	}
 
 	statement, err := c.Attester.AttestBuild(build, buildMetadata, pod, keychain, buildId, deps...)
 	if err != nil {
-		return "", fmt.Errorf("generating statement: %v", err)
+		return "", fmt.Errorf("failed to generate statement: %v", err)
 	}
 
 	payload, err := c.Attester.Sign(ctx, statement, signers...)
 	if err != nil {
-		return "", fmt.Errorf("signing statement: %v", err)
+		return "", fmt.Errorf("failed to sign statement: %v", err)
 	}
 
 	_, digest, err := c.Attester.Write(ctx, buildMetadata.LatestImage, payload, keychain)
 	if err != nil {
-		return "", fmt.Errorf("writting attestation: %v", err)
+		return "", fmt.Errorf("failed to write attestation: %v", err)
 	}
 
 	return digest, nil

diff --git a/pkg/slsa/attest.go b/pkg/slsa/attest.go
@@ -46,34 +46,31 @@ type Attester struct {
 func (a *Attester) AttestBuild(build *buildv1alpha2.Build, buildMetadata *cnb.BuildMetadata, pod *corev1.Pod, builderAndAppKeychain authn.Keychain, builderId BuilderID, depFns ...BuilderDependencyFn) (intoto.Statement, error) {
 	builderRepo, builderSha, builderLabels, err := a.ImageReader.Read(builderAndAppKeychain, build.Spec.Builder.Image)
 	if err != nil {
-		return intoto.Statement{}, fmt.Errorf("reading builder image: %v", err)
+		return intoto.Statement{}, fmt.Errorf("failed to read builder image: %v", err)
 	}
 
 	appRepo, appSha, appLabels, err := a.ImageReader.Read(builderAndAppKeychain, buildMetadata.LatestImage)
 	if err != nil {
-		return intoto.Statement{}, fmt.Errorf("reading app image: %v", err)
+		return intoto.Statement{}, fmt.Errorf("failed to read app image: %v", err)
 	}
 
 	source, sourceDigest, err := extractSourceFromLabel(appLabels)
 	if err != nil {
-		return intoto.Statement{}, fmt.Errorf("extracting source from label: %v", err)
+		return intoto.Statement{}, fmt.Errorf("failed to extract source from label: %v", err)
 	}
 
-	start, stop, err := getStartStopTime(pod)
-	if err != nil {
-		return intoto.Statement{}, fmt.Errorf("parsing start/stop time: %v", err)
-	}
+	start, stop := getStartStopTime(pod)
 
 	lifecycle, err := a.LifecycleProvider.Metadata()
 	if err != nil {
-		return intoto.Statement{}, fmt.Errorf("reading lifecycle metadata: %v", err)
+		return intoto.Statement{}, fmt.Errorf("failed to read lifecycle metadata: %v", err)
 	}
 
 	builderDeps := make([]slsav1.ResourceDescriptor, 0)
 	for i, fn := range depFns {
 		dep, err := fn()
 		if err != nil {
-			return intoto.Statement{}, fmt.Errorf("fetching builder dependency #%v: %v", i, err)
+			return intoto.Statement{}, fmt.Errorf("failed to fetch builder dependency #%v: %v", i, err)
 		}
 
 		builderDeps = append(builderDeps, dep)
@@ -161,7 +158,7 @@ func getBuildType(version string) string {
 	return fmt.Sprintf("https://github.com/buildpacks-community/kpack/blob/v%v/docs/slsa.md", version)
 }
 
-func getStartStopTime(pod *corev1.Pod) (*time.Time, *time.Time, error) {
+func getStartStopTime(pod *corev1.Pod) (*time.Time, *time.Time) {
 	var (
 		start *time.Time
 		stop  *time.Time
@@ -171,25 +168,17 @@ func getStartStopTime(pod *corev1.Pod) (*time.Time, *time.Time, error) {
 		if c.Name == buildv1alpha2.PrepareContainerName {
 			if c.State.Terminated != nil && c.State.Terminated.ExitCode == 0 {
 				start = &c.State.Terminated.StartedAt.Time
-			} else {
-				return nil, nil, fmt.Errorf("prepare not finished yet")
 			}
 		}
 
 		if c.Name == buildv1alpha2.CompletionContainerName {
 			if c.State.Terminated != nil && c.State.Terminated.ExitCode == 0 {
 				stop = &c.State.Terminated.FinishedAt.Time
-			} else {
-				return nil, nil, fmt.Errorf("completion not finished yet")
 			}
 		}
 	}
 
-	if start == nil || stop == nil {
-		return nil, nil, fmt.Errorf("failed to extract time")
-	}
-
-	return start, stop, nil
+	return start, stop
 }
 
 func convertMap(orig map[string]string) map[string]interface{} {
@@ -222,7 +211,7 @@ func WithVersionedObject(kind string, obj K8sObject) BuilderDependencyFn {
 		}
 		bytes, err := json.Marshal(versioned)
 		if err != nil {
-			return slsav1.ResourceDescriptor{}, fmt.Errorf("marshalling json: %v", err)
+			return slsav1.ResourceDescriptor{}, fmt.Errorf("failed to marshal json: %v", err)
 		}
 
 		return slsav1.ResourceDescriptor{
@@ -246,7 +235,7 @@ func WithVersionedObjects(kind string, objs []K8sObject) BuilderDependencyFn {
 		}
 		bytes, err := json.Marshal(versioned)
 		if err != nil {
-			return slsav1.ResourceDescriptor{}, fmt.Errorf("marshalling json: %v", err)
+			return slsav1.ResourceDescriptor{}, fmt.Errorf("failed to marshal json: %v", err)
 		}
 
 		return slsav1.ResourceDescriptor{

diff --git a/pkg/slsa/image.go b/pkg/slsa/image.go
@@ -32,17 +32,17 @@ func NewImageReader(fetcher ImageFetcher) *reader {
 func (r *reader) Read(keychain authn.Keychain, repoName string) (string, string, map[string]string, error) {
 	img, id, err := r.fetcher.Fetch(keychain, repoName)
 	if err != nil {
-		return "", "", nil, fmt.Errorf("fetching image: %v", err)
+		return "", "", nil, fmt.Errorf("failed to fetch image: %v", err)
 	}
 
 	ref, err := name.NewDigest(id)
 	if err != nil {
-		return "", "", nil, fmt.Errorf("parsing digest: %v", err)
+		return "", "", nil, fmt.Errorf("failed to parse digest: %v", err)
 	}
 
 	configFile, err := img.ConfigFile()
 	if err != nil {
-		return "", "", nil, fmt.Errorf("getting image config: %v", err)
+		return "", "", nil, fmt.Errorf("failed to get image config: %v", err)
 	}
 
 	sha, found := strings.CutPrefix(ref.DigestStr(), "sha256:")
@@ -62,7 +62,7 @@ func extractSourceFromLabel(labels map[string]string) (string, slsacommon.Digest
 	var p project
 	err := json.Unmarshal([]byte(metadata), &p)
 	if err != nil {
-		return "", nil, fmt.Errorf("unmarshalling json: %v", err)
+		return "", nil, fmt.Errorf("failed to unmarshal json: %v", err)
 	}
 
 	switch p.Source.Type {

diff --git a/pkg/slsa/pkcs8_signer.go b/pkg/slsa/pkcs8_signer.go
@@ -70,7 +70,7 @@ func (s *rsaSigner) Sign(ctx context.Context, data []byte) ([]byte, error) {
 	hasher := hf.New()
 	_, err := hasher.Write(data)
 	if err != nil {
-		return nil, fmt.Errorf("hashing data: %v", err)
+		return nil, fmt.Errorf("failed to hash data: %v", err)
 	}
 
 	return rsa.SignPKCS1v15(nil, s.key, crypto.SHA256, hasher.Sum(nil))

diff --git a/pkg/slsa/sign.go b/pkg/slsa/sign.go
@@ -30,20 +30,20 @@ const (
 func (*Attester) Sign(ctx context.Context, stmt intoto.Statement, signers ...Signer) ([]byte, error) {
 	payload, err := json.Marshal(stmt)
 	if err != nil {
-		return nil, fmt.Errorf("marshalling statement: %v", err)
+		return nil, fmt.Errorf("failed to marshal statement: %v", err)
 	}
 	pae := dsse.PAE(IntotoPayloadType, payload)
 
 	sigs := make([]dsse.Signature, len(signers))
 	for i, signer := range signers {
 		keyId, err := signer.KeyID()
 		if err != nil {
-			return nil, fmt.Errorf("retrieving keyid: %v", err)
+			return nil, fmt.Errorf("failed to retreive keyid: %v", err)
 		}
 
 		sig, err := signer.Sign(ctx, pae)
 		if err != nil {
-			return nil, fmt.Errorf("signing payload using '%v': %v", keyId, err)
+			return nil, fmt.Errorf("failed to sign payload using '%v': %v", keyId, err)
 		}
 
 		sigs[i] = dsse.Signature{
@@ -60,7 +60,7 @@ func (*Attester) Sign(ctx context.Context, stmt intoto.Statement, signers ...Sig
 
 	b, err := json.Marshal(envelope)
 	if err != nil {
-		return nil, fmt.Errorf("marshalling envelope: %v", err)
+		return nil, fmt.Errorf("failed to marshal envelope: %v", err)
 	}
 
 	return b, nil
@@ -76,22 +76,22 @@ func (*Attester) Write(ctx context.Context, digestStr string, payload []byte, ke
 
 	attestation, err := cosignstatic.NewAttestation(payload, opts...)
 	if err != nil {
-		return nil, "", fmt.Errorf(":%v", err)
+		return nil, "", fmt.Errorf("failed to create attestation: %v", err)
 	}
 
 	ref, err := name.ParseReference(digestStr)
 	if err != nil {
-		return nil, "", fmt.Errorf(":%v", err)
+		return nil, "", fmt.Errorf("failed to parse reference: %v", err)
 	}
 
 	attestationTag, err := cosignremote.AttestationTag(ref)
 	if err != nil {
-		return nil, "", fmt.Errorf(":%v", err)
+		return nil, "", fmt.Errorf("failed to get attestation tag:%v", err)
 	}
 
 	annots, err := attestation.Annotations()
 	if err != nil {
-		return nil, "", fmt.Errorf("getting attestation annotations: %v", err)
+		return nil, "", fmt.Errorf("failed to get attestation annotations: %v", err)
 	}
 
 	// Overwrite any existing attestations with a new one. The only time this is
@@ -115,12 +115,12 @@ func (*Attester) Write(ctx context.Context, digestStr string, payload []byte, ke
 
 	err = remote.Write(attestationTag, img, remoteOpts...)
 	if err != nil {
-		return nil, "", fmt.Errorf(":%v", err)
+		return nil, "", fmt.Errorf("failed to write attestation: %v", err)
 	}
 
 	signatureDigest, err := img.Digest()
 	if err != nil {
-		return nil, "", fmt.Errorf(":%v", err)
+		return nil, "", fmt.Errorf("failed to retrieve digest: %v", err)
 	}
 
 	return img, fmt.Sprintf("%v@%v", attestationTag.Context().Name(), signatureDigest.String()), nil