Skips the signing of artifacts when valid configuration is not done

Before this patch, if proper configurations where not done, chains
used to sign the tekton resources with the default configurations
which in turn used to create a impression that tekton resources are
signed as it used to add the `chains.tekton.dev/signed=true` annotation

Hence, with this patch, if proper configuration is not done (like for
e.g. private keys are not stored using cosign or x509) then in that case
tekton resources will be skipped from signing, indicating the user to
do proper configurations to sign the resources

Signed-off-by: PuneetPunamiya <[email protected]>

pkg/chains/annotations.go

@@ -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)
 }

pkg/chains/signing.go

@@ -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{}{
@@ -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)
+	return all, nil
 }
 
 // TODO: Hook this up to config.
@@ -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{}
@@ -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() {

pkg/chains/signing_test.go

@@ -17,6 +17,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"github.com/stretchr/testify/assert"
 	"reflect"
 	"testing"
 
@@ -184,6 +185,56 @@ func TestSigner_Sign(t *testing.T) {
 	}
 }
 
+func TestSigningObjectsSkipped(t *testing.T) {
+	ctx, _ := rtesting.SetupFakeContext(t)
+	ps := fakepipelineclient.Get(ctx)
+
+	tro := objects.NewTaskRunObject(&v1beta1.TaskRun{
+		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{
@@ -393,7 +444,10 @@ func TestSigningObjects(t *testing.T) {
 	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())

pkg/chains/verifier.go

@@ -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)
 
 	for _, signableType := range enabledSignableTypes {
 		if !signableType.Enabled(cfg) {