Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 34 additions & 4 deletions pkg/artifacts/signable.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,14 @@ type Signable interface {
StorageBackend(cfg config.Config) sets.String
Signer(cfg config.Config) string
PayloadFormat(cfg config.Config) formats.PayloadType
Key(interface{}) string
// FullKey returns the full identifier for a signable artifact.
// - For OCI artifact, it is the full representation in the format of `<NAME>@sha256:<DIGEST>`.
// - For TaskRun/PipelineRun artifact, it is `<GROUP>-<VERSION>-<KIND>-<UID>`
FullKey(interface{}) string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bikeshed: I'm okay with Key/ShortKey. I know we've been lax about this, but we should also add godoc describing the difference between the 2 (you can generalize instead of going into the specifics of each impl).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I've added godoc in the struct that contains those the shortKey and fullKey field. Please take a look and let me know what you think. Happy to update it further if needed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want docs here as well since this is where Signable implementors / IDEs are going to look for documentation for the type.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see. Added! Please take a look! Thanks

// ShortKey returns the short version of an artifact identifier.
// - For OCI artifact, it is first 12 chars of the image digest.
// - For TaskRun/PipelineRun artifact, it is `<KIND>-<UID>`.
ShortKey(interface{}) string
Type() string
Enabled(cfg config.Config) bool
}
Expand All @@ -42,11 +49,19 @@ type TaskRunArtifact struct {
Logger *zap.SugaredLogger
}

func (ta *TaskRunArtifact) Key(obj interface{}) string {
var _ Signable = &TaskRunArtifact{}

func (ta *TaskRunArtifact) ShortKey(obj interface{}) string {
tro := obj.(*objects.TaskRunObject)
return "taskrun-" + string(tro.UID)
}

func (ta *TaskRunArtifact) FullKey(obj interface{}) string {
tro := obj.(*objects.TaskRunObject)
gvk := tro.GetGroupVersionKind()
return fmt.Sprintf("%s-%s-%s-%s", gvk.Group, gvk.Version, gvk.Kind, tro.UID)
}

func (ta *TaskRunArtifact) ExtractObjects(obj objects.TektonObject) []interface{} {
return []interface{}{obj}
}
Expand Down Expand Up @@ -75,11 +90,19 @@ type PipelineRunArtifact struct {
Logger *zap.SugaredLogger
}

func (pa *PipelineRunArtifact) Key(obj interface{}) string {
var _ Signable = &PipelineRunArtifact{}

func (pa *PipelineRunArtifact) ShortKey(obj interface{}) string {
pro := obj.(*objects.PipelineRunObject)
return "pipelinerun-" + string(pro.UID)
}

func (pa *PipelineRunArtifact) FullKey(obj interface{}) string {
pro := obj.(*objects.PipelineRunObject)
gvk := pro.GetGroupVersionKind()
return fmt.Sprintf("%s-%s-%s-%s", gvk.Group, gvk.Version, gvk.Kind, pro.UID)
}

func (pa *PipelineRunArtifact) ExtractObjects(obj objects.TektonObject) []interface{} {
return []interface{}{obj}
}
Expand Down Expand Up @@ -109,6 +132,8 @@ type OCIArtifact struct {
Logger *zap.SugaredLogger
}

var _ Signable = &OCIArtifact{}

type image struct {
url string
digest string
Expand Down Expand Up @@ -291,11 +316,16 @@ func (oa *OCIArtifact) Signer(cfg config.Config) string {
return cfg.Artifacts.OCI.Signer
}

func (oa *OCIArtifact) Key(obj interface{}) string {
func (oa *OCIArtifact) ShortKey(obj interface{}) string {
v := obj.(name.Digest)
return strings.TrimPrefix(v.DigestStr(), "sha256:")[:12]
}

func (oa *OCIArtifact) FullKey(obj interface{}) string {
v := obj.(name.Digest)
return v.Name()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should double check the behavior here for annotation based storage - we should probably preserve the old behavior there since IIRC labels/annotations have a fixed key size, and including the full digest may cause issues.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point!! Thanks for catching this @wlynch. You are right. I found the source.

Now, I maintained both ShortKey and FullKey so that downstream code can use whichever is needed. Please let me know what you think or if you have better suggestions.

Thank you!!

}

func (oa *OCIArtifact) Enabled(cfg config.Config) bool {
return cfg.Artifacts.OCI.Enabled()
}
3 changes: 2 additions & 1 deletion pkg/chains/signing.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ func (o *ObjectSigner) Sign(ctx context.Context, tektonObj objects.TektonObject)
for _, backend := range signableType.StorageBackend(cfg).List() {
b := o.Backends[backend]
storageOpts := config.StorageOpts{
Key: signableType.Key(obj),
ShortKey: signableType.ShortKey(obj),
FullKey: signableType.FullKey(obj),
Cert: signer.Cert(),
Chain: signer.Chain(),
PayloadFormat: payloadFormat,
Expand Down
4 changes: 2 additions & 2 deletions pkg/chains/storage/docdb/docdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (b *Backend) StorePayload(ctx context.Context, _ objects.TektonObject, rawP
Signed: rawPayload,
Signature: base64.StdEncoding.EncodeToString([]byte(signature)),
Object: obj,
Name: opts.Key,
Name: opts.ShortKey,
Cert: opts.Cert,
Chain: opts.Chain,
}
Expand Down Expand Up @@ -122,7 +122,7 @@ func (b *Backend) RetrievePayloads(ctx context.Context, _ objects.TektonObject,
}

func (b *Backend) retrieveDocuments(ctx context.Context, opts config.StorageOpts) ([]SignedDocument, error) {
d := SignedDocument{Name: opts.Key}
d := SignedDocument{Name: opts.ShortKey}
if err := b.coll.Get(ctx, &d); err != nil {
return []SignedDocument{}, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/chains/storage/docdb/docdb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func TestBackend_StorePayload(t *testing.T) {
}

// Store the document.
opts := config.StorageOpts{Key: tt.args.key}
opts := config.StorageOpts{ShortKey: tt.args.key}
trObj := objects.NewTaskRunObject(tt.args.tr)
if err := b.StorePayload(ctx, trObj, sb, tt.args.signature, opts); (err != nil) != tt.wantErr {
t.Fatalf("Backend.StorePayload() error = %v, wantErr %v", err, tt.wantErr)
Expand Down
8 changes: 4 additions & 4 deletions pkg/chains/storage/gcs/gcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,17 +187,17 @@ func (b *Backend) retrieveObject(ctx context.Context, object string) (string, er
}

func sigName(tr *v1beta1.TaskRun, opts config.StorageOpts) string {
return fmt.Sprintf(SignatureNameFormat, tr.Namespace, tr.Name, opts.Key)
return fmt.Sprintf(SignatureNameFormat, tr.Namespace, tr.Name, opts.ShortKey)
}

func payloadName(tr *v1beta1.TaskRun, opts config.StorageOpts) string {
return fmt.Sprintf(PayloadNameFormat, tr.Namespace, tr.Name, opts.Key)
return fmt.Sprintf(PayloadNameFormat, tr.Namespace, tr.Name, opts.ShortKey)
}

func certName(tr *v1beta1.TaskRun, opts config.StorageOpts) string {
return fmt.Sprintf(CertNameFormat, tr.Namespace, tr.Name, opts.Key)
return fmt.Sprintf(CertNameFormat, tr.Namespace, tr.Name, opts.ShortKey)
}

func chainName(tr *v1beta1.TaskRun, opts config.StorageOpts) string {
return fmt.Sprintf(ChainNameFormat, tr.Namespace, tr.Name, opts.Key)
return fmt.Sprintf(ChainNameFormat, tr.Namespace, tr.Name, opts.ShortKey)
}
4 changes: 2 additions & 2 deletions pkg/chains/storage/gcs/gcs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestBackend_StorePayload(t *testing.T) {
},
signed: []byte("signed"),
signature: "signature",
opts: config.StorageOpts{Key: "foo.uuid", PayloadFormat: formats.PayloadTypeInTotoIte6},
opts: config.StorageOpts{ShortKey: "foo.uuid", PayloadFormat: formats.PayloadTypeInTotoIte6},
},
},
{
Expand All @@ -71,7 +71,7 @@ func TestBackend_StorePayload(t *testing.T) {
},
signed: []byte("signed"),
signature: "signature",
opts: config.StorageOpts{Key: "foo.uuid", PayloadFormat: formats.PayloadTypeTekton},
opts: config.StorageOpts{ShortKey: "foo.uuid", PayloadFormat: formats.PayloadTypeTekton},
},
},
}
Expand Down
20 changes: 1 addition & 19 deletions pkg/chains/storage/grafeas/grafeas.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,7 @@ func (b *Backend) createOccurrence(ctx context.Context, tr *v1beta1.TaskRun, pay

// create Occurrence_Attestation for OCI
if opts.PayloadFormat == formats.PayloadTypeSimpleSigning {
uri := b.retrieveSingleOCIURI(tr, opts)
occ, err := b.createAttestationOccurrence(ctx, tr, payload, signature, uri)
occ, err := b.createAttestationOccurrence(ctx, tr, payload, signature, opts.FullKey)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -395,23 +394,6 @@ func (b *Backend) findOccurrencesForCriteria(ctx context.Context, projectPath st
return occurences.GetOccurrences(), nil
}

// get resource uri for a single oci image in the format of `IMAGE_URL@IMAGE_DIGEST`
func (b *Backend) retrieveSingleOCIURI(tr *v1beta1.TaskRun, opts config.StorageOpts) string {
imgs := b.retrieveAllArtifactIdentifiers(tr)
for _, img := range imgs {
// get digest part of the image representation
digest := strings.Split(img, "sha256:")[1]

// for oci image, the key in StorageOpts will be the first 12 chars of digest
// so we want to compare
digestKey := digest[:12]
if digestKey == opts.Key {
return img
}
}
return ""
}

// retrieve the uri of all images generated from a specific taskrun in the format of `IMAGE_URL@IMAGE_DIGEST`
func (b *Backend) retrieveAllArtifactIdentifiers(tr *v1beta1.TaskRun) []string {
result := []string{}
Expand Down
17 changes: 5 additions & 12 deletions pkg/chains/storage/grafeas/grafeas_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func TestGrafeasBackend_StoreAndRetrieve(t *testing.T) {
},
payload: []byte("{}"),
signature: "taskrun signature",
opts: config.StorageOpts{Key: "taskrun.uuid", PayloadFormat: formats.PayloadTypeInTotoIte6},
opts: config.StorageOpts{FullKey: "tekton.dev-v1beta1-taskrun-uuid", PayloadFormat: formats.PayloadTypeInTotoIte6},
},
wantErr: false,
},
Expand All @@ -174,12 +174,7 @@ func TestGrafeasBackend_StoreAndRetrieve(t *testing.T) {
},
payload: []byte("oci payload"),
signature: "oci signature",
// The Key field must be the same as the first 12 chars of the image digest.
// Reason:
// Inside chains.Sign function, we set the key field for both artifacts.
// For OCI artifact, it is implemented as the first 12 chars of the image digest.
// https://github.com/tektoncd/chains/blob/v0.8.0/pkg/artifacts/signable.go#L200
opts: config.StorageOpts{Key: "cfe4f0bf41c8", PayloadFormat: formats.PayloadTypeSimpleSigning},
opts: config.StorageOpts{FullKey: "gcr.io/test/kaniko-chains1@sha256:cfe4f0bf41c80609214f9b8ec0408b1afb28b3ced343b944aaa05d47caba3e00", PayloadFormat: formats.PayloadTypeSimpleSigning},
},
wantErr: false,
},
Expand All @@ -193,7 +188,7 @@ func TestGrafeasBackend_StoreAndRetrieve(t *testing.T) {
UID: types.UID("uid3"),
},
},
opts: config.StorageOpts{Key: "taskrun2.uuid", PayloadFormat: formats.PayloadTypeTekton},
opts: config.StorageOpts{FullKey: "tekton.dev-v1beta1-taskrun2-uuid", PayloadFormat: formats.PayloadTypeTekton},
},
wantErr: true,
},
Expand Down Expand Up @@ -251,8 +246,7 @@ func testStoreAndRetrieve(ctx context.Context, t *testing.T, test testConfig, ba
// ----------------
expectSignature := map[string][]string{}
if test.args.opts.PayloadFormat == formats.PayloadTypeSimpleSigning {
uri := backend.retrieveSingleOCIURI(test.args.tr, test.args.opts)
expectSignature[uri] = []string{test.args.signature}
expectSignature[test.args.opts.FullKey] = []string{test.args.signature}
}
if test.args.opts.PayloadFormat == formats.PayloadTypeInTotoIte6 {
allURIs := backend.retrieveAllArtifactIdentifiers(test.args.tr)
Expand All @@ -274,8 +268,7 @@ func testStoreAndRetrieve(ctx context.Context, t *testing.T, test testConfig, ba
// --------------
expectPayload := map[string]string{}
if test.args.opts.PayloadFormat == formats.PayloadTypeSimpleSigning {
uri := backend.retrieveSingleOCIURI(test.args.tr, test.args.opts)
expectPayload[uri] = string(test.args.payload)
expectPayload[test.args.opts.FullKey] = string(test.args.payload)
}
if test.args.opts.PayloadFormat == formats.PayloadTypeInTotoIte6 {
allURIs := backend.retrieveAllArtifactIdentifiers(test.args.tr)
Expand Down
12 changes: 6 additions & 6 deletions pkg/chains/storage/tekton/tekton.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ func (b *Backend) StorePayload(ctx context.Context, obj objects.TektonObject, ra
// Use patch instead of update to prevent race conditions.
patchBytes, err := patch.GetAnnotationsPatch(map[string]string{
// Base64 encode both the signature and the payload
fmt.Sprintf(PayloadAnnotationFormat, opts.Key): base64.StdEncoding.EncodeToString(rawPayload),
fmt.Sprintf(SignatureAnnotationFormat, opts.Key): base64.StdEncoding.EncodeToString([]byte(signature)),
fmt.Sprintf(CertAnnotationsFormat, opts.Key): base64.StdEncoding.EncodeToString([]byte(opts.Cert)),
fmt.Sprintf(ChainAnnotationFormat, opts.Key): base64.StdEncoding.EncodeToString([]byte(opts.Chain)),
fmt.Sprintf(PayloadAnnotationFormat, opts.ShortKey): base64.StdEncoding.EncodeToString(rawPayload),
fmt.Sprintf(SignatureAnnotationFormat, opts.ShortKey): base64.StdEncoding.EncodeToString([]byte(signature)),
fmt.Sprintf(CertAnnotationsFormat, opts.ShortKey): base64.StdEncoding.EncodeToString([]byte(opts.Cert)),
fmt.Sprintf(ChainAnnotationFormat, opts.ShortKey): base64.StdEncoding.EncodeToString([]byte(opts.Chain)),
})
if err != nil {
return err
Expand Down Expand Up @@ -131,9 +131,9 @@ func (b *Backend) RetrievePayloads(ctx context.Context, obj objects.TektonObject
}

func sigName(opts config.StorageOpts) string {
return fmt.Sprintf(SignatureAnnotationFormat, opts.Key)
return fmt.Sprintf(SignatureAnnotationFormat, opts.ShortKey)
}

func payloadName(opts config.StorageOpts) string {
return fmt.Sprintf(PayloadAnnotationFormat, opts.Key)
return fmt.Sprintf(PayloadAnnotationFormat, opts.ShortKey)
}
2 changes: 1 addition & 1 deletion pkg/chains/storage/tekton/tekton_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func TestBackend_StorePayload(t *testing.T) {
if err != nil {
t.Errorf("error marshaling json: %v", err)
}
opts := config.StorageOpts{Key: "mockpayload"}
opts := config.StorageOpts{ShortKey: "mockpayload"}
mockSignature := "mocksignature"
if err := b.StorePayload(ctx, tt.object, payload, mockSignature, opts); (err != nil) != tt.wantErr {
t.Errorf("Backend.StorePayload() error = %v, wantErr %v", err, tt.wantErr)
Expand Down
12 changes: 9 additions & 3 deletions pkg/config/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,16 @@ import "github.com/tektoncd/chains/pkg/chains/formats"

// StorageOpts contains additional information required when storing signatures
type StorageOpts struct {
// Key stands for the identifier of an artifact.
// FullKey stands for the identifier of an artifact.
// - For OCI artifact, it is the full representation in the format of `<NAME>@sha256:<DIGEST>`.
// - For TaskRun/PipelineRun artifact, it is `<GROUP>-<VERSION>-<KIND>-<UID>`
FullKey string

// ShortKey is the short version of an artifact identifier. This is useful for annotation based storage
// because annotation key has limitations (https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/#syntax-and-character-set).
// - For OCI artifact, it is first 12 chars of the image digest.
// - For TaskRun artifact, it is `taskrun-<UID>`
Key string
// - For TaskRun/PipelineRun artifact, it is `<KIND>-<UID>`.
ShortKey string

// Cert is an OPTIONAL property that contains a PEM-encoded x509 certificate.
// If present, this certificate MUST embed the public key that can be used to verify the signature.
Expand Down
2 changes: 1 addition & 1 deletion test/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ func verifySignature(ctx context.Context, t *testing.T, c *clients, tr *v1beta1.

// Initialize the storage options.
opts := config.StorageOpts{
Key: fmt.Sprintf("taskrun-%s", tr.UID),
ShortKey: fmt.Sprintf("taskrun-%s", tr.UID),
}

trObj := objects.NewTaskRunObject(tr)
Expand Down