Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skips the signing of artifacts when valid configuration is not done #1003

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
8 changes: 8 additions & 0 deletions pkg/chains/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,14 @@ func MarkSigned(ctx context.Context, obj objects.TektonObject, ps versioned.Inte
return AddAnnotation(ctx, obj, ps, ChainsAnnotation, "true", annotations)
}

// MarkSkipped marks a Tekton object as skipped because configurations were not set
func MarkSkipped(ctx context.Context, obj objects.TektonObject, ps versioned.Interface, annotations map[string]string) error {
if _, ok := obj.GetAnnotations()[ChainsAnnotation]; ok {
return nil
}
return AddAnnotation(ctx, obj, ps, ChainsAnnotation, "skipped", annotations)
}

func MarkFailed(ctx context.Context, obj objects.TektonObject, ps versioned.Interface, annotations map[string]string) error {
return AddAnnotation(ctx, obj, ps, ChainsAnnotation, "failed", annotations)
}
Expand Down
21 changes: 15 additions & 6 deletions pkg/chains/signing.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ type ObjectSigner struct {
Pipelineclientset versioned.Interface
}

func allSigners(ctx context.Context, sp string, cfg config.Config) map[string]signing.Signer {
func allSigners(ctx context.Context, sp string, cfg config.Config) (map[string]signing.Signer, error) {
l := logging.FromContext(ctx)
all := map[string]signing.Signer{}
neededSigners := map[string]struct{}{
Expand All @@ -64,22 +64,23 @@ func allSigners(ctx context.Context, sp string, cfg config.Config) map[string]si
signer, err := x509.NewSigner(ctx, sp, cfg)
if err != nil {
l.Warnf("error configuring x509 signer: %s", err)
continue
return nil, err
}
all[s] = signer
case signing.TypeKMS:
signer, err := kms.NewSigner(ctx, cfg.Signers.KMS)
if err != nil {
l.Warnf("error configuring kms signer with config %v: %s", cfg.Signers.KMS, err)
continue
return nil, err
}
all[s] = signer
default:
// This should never happen, so panic
l.Panicf("unsupported signer: %s", s)
}
}
return all
fmt.Println(all)
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove this :)

return all, nil
}

// TODO: Hook this up to config.
Expand Down Expand Up @@ -116,7 +117,15 @@ func (o *ObjectSigner) Sign(ctx context.Context, tektonObj objects.TektonObject)
return err
}

signers := allSigners(ctx, o.SecretPath, cfg)
signers, err := allSigners(ctx, o.SecretPath, cfg)
if err != nil {
logger.Info("Skipping the tekton resource...")
if err := MarkSkipped(ctx, tektonObj, o.Pipelineclientset, map[string]string{}); err != nil {
logger.Error(err)
return err
}
return nil
}

var merr *multierror.Error
extraAnnotations := map[string]string{}
Expand Down Expand Up @@ -151,7 +160,7 @@ func (o *ObjectSigner) Sign(ctx context.Context, tektonObj objects.TektonObject)
signer, ok := signers[signerType]
if !ok {
logger.Warnf("No signer %s configured for %s", signerType, signableType.Type())
continue
return nil
}

if payloader.Wrap() {
Expand Down
56 changes: 55 additions & 1 deletion pkg/chains/signing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"context"
"errors"
"fmt"
"github.com/stretchr/testify/assert"
"reflect"
"testing"

Expand Down Expand Up @@ -184,6 +185,56 @@
}
}

func TestSigningObjectsSkipped(t *testing.T) {
ctx, _ := rtesting.SetupFakeContext(t)
ps := fakepipelineclient.Get(ctx)

tro := objects.NewTaskRunObject(&v1beta1.TaskRun{

Check failure on line 192 in pkg/chains/signing_test.go

View workflow job for this annotation

GitHub Actions / lint

SA1019: v1beta1.TaskRun is deprecated: Please use v1.TaskRun instead. (staticcheck)
Copy link
Contributor

Choose a reason for hiding this comment

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

agreed with lint. Why are we using v1beta1 here? I think it should be v1?

ObjectMeta: metav1.ObjectMeta{
Name: "foo",
},
})

tcfg := &config.Config{
Artifacts: config.ArtifactConfigs{
TaskRuns: config.Artifact{
Format: "in-toto",
StorageBackend: sets.New[string]("mock"),
Signer: "x509",
},
},
}
ctx = config.ToContext(ctx, tcfg.DeepCopy())

ts := &ObjectSigner{
Pipelineclientset: ps,
}

tekton.CreateObject(t, ctx, ps, tro)

if err := ts.Sign(ctx, tro); (err != nil) != false {
t.Errorf("Signer.Sign() error = %v", err)
}

// Fetch the updated object
updatedObject, err := tekton.GetObject(t, ctx, ps, tro)
if err != nil {
t.Errorf("error fetching fake object: %v", err)
}

shouldBeSigned := !true
if Reconciled(ctx, ps, updatedObject) != shouldBeSigned {
t.Errorf("IsSigned()=%t, wanted %t", Reconciled(ctx, ps, updatedObject), shouldBeSigned)
}

// Retrieve all annotations
annotations := updatedObject.GetAnnotations()
expectedAnnotation := "chains.tekton.dev/signed"

actualValue := annotations[expectedAnnotation]
assert.Equal(t, "skipped", actualValue)
}

func TestSigner_Transparency(t *testing.T) {
newTaskRun := func(name string) objects.TektonObject {
return objects.NewTaskRunObject(&v1beta1.TaskRun{
Expand Down Expand Up @@ -393,7 +444,10 @@
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx, _ := rtesting.SetupFakeContext(t)
signers := allSigners(ctx, tt.SecretPath, tt.config)
signers, err := allSigners(ctx, tt.SecretPath, tt.config)
if err != nil {
t.Errorf(err.Error())
}
var signerTypes []string
for _, signer := range signers {
signerTypes = append(signerTypes, signer.Type())
Expand Down
2 changes: 1 addition & 1 deletion pkg/chains/verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (tv *TaskRunVerifier) VerifyTaskRun(ctx context.Context, tr *v1beta1.TaskRu
if err != nil {
return err
}
signers := allSigners(ctx, tv.SecretPath, cfg)
signers, _ := allSigners(ctx, tv.SecretPath, cfg)
Copy link
Contributor

Choose a reason for hiding this comment

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

we should capture the error.


for _, signableType := range enabledSignableTypes {
if !signableType.Enabled(cfg) {
Expand Down
Loading