Use patch instead of update to mutate annotations.

This helps prevent some race conditions, where we try to update an object
when simultaneous updates are happening. They eventually work themselves out
because we continue to retry, but this helps speed things along.

pkg/patch/patch.go

@@ -0,0 +1,21 @@
+package patch
+
+import "encoding/json"
+
+// GetAnnotationsPatch returns patch bytes that can be used with kubectl patch
+func GetAnnotationsPatch(newAnnotations map[string]string) ([]byte, error) {
+	p := patch{
+		Metadata: metadata{
+			Annotations: newAnnotations,
+		},
+	}
+	return json.Marshal(p)
+}
+
+// These are used to get proper json formatting
+type patch struct {
+	Metadata metadata `json:"metadata,omitempty"`
+}
+type metadata struct {
+	Annotations map[string]string `json:"annotations,omitempty"`
+}

pkg/patch/patch_test.go

@@ -0,0 +1,49 @@
+package patch
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestGetAnnotationsPatch(t *testing.T) {
+	tests := []struct {
+		name           string
+		newAnnotations map[string]string
+		want           string
+		wantErr        bool
+	}{
+		{
+			name:           "empty",
+			newAnnotations: map[string]string{},
+			want:           `{"metadata":{}}`,
+		},
+		{
+			name: "one",
+			newAnnotations: map[string]string{
+				"foo": "bar",
+			},
+			want: `{"metadata":{"annotations":{"foo":"bar"}}}`,
+		},
+		{
+			name: "many",
+			newAnnotations: map[string]string{
+				"foo": "bar",
+				"baz": "bat",
+			},
+			want: `{"metadata":{"annotations":{"baz":"bat","foo":"bar"}}}`,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := GetAnnotationsPatch(tt.newAnnotations)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("GetAnnotationsPatch() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			gotStr := string(got)
+			if !reflect.DeepEqual(gotStr, tt.want) {
+				t.Errorf("GetAnnotationsPatch() = %v, want %v", gotStr, tt.want)
+			}
+		})
+	}
+}

pkg/signing/signing.go

@@ -14,11 +14,13 @@ limitations under the License.
 package signing
 
 import (
+	"github.com/tektoncd/chains/pkg/patch"
 	"github.com/tektoncd/chains/pkg/signing/formats"
 	"github.com/tektoncd/chains/pkg/signing/storage"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	versioned "github.com/tektoncd/pipeline/pkg/client/clientset/versioned"
 	"go.uber.org/zap"
+	"k8s.io/apimachinery/pkg/types"
 )
 
 const (
@@ -46,11 +48,14 @@ func IsSigned(tr *v1beta1.TaskRun) bool {
 
 // MarkSigned marks a TaskRun as signed.
 func MarkSigned(tr *v1beta1.TaskRun, ps versioned.Interface) error {
-	if tr.ObjectMeta.Annotations == nil {
-		tr.ObjectMeta.Annotations = map[string]string{}
+	// Use patch instead of update to help prevent race conditions.
+	patchBytes, err := patch.GetAnnotationsPatch(map[string]string{
+		ChainsAnnotation: "true",
+	})
+	if err != nil {
+		return err
 	}
-	tr.ObjectMeta.Annotations[ChainsAnnotation] = "true"
-	if _, err := ps.TektonV1beta1().TaskRuns(tr.Namespace).Update(tr); err != nil {
+	if _, err := ps.TektonV1beta1().TaskRuns(tr.Namespace).Patch(tr.Name, types.MergePatchType, patchBytes); err != nil {
 		return err
 	}
 	return nil
@@ -69,7 +74,7 @@ func (ts *TaskRunSigner) SignTaskRun(tr *v1beta1.TaskRun) error {
 	for _, b := range backends {
 		for payloadType, payload := range payloads {
 			if err := b.StorePayload(payload, payloadType, tr); err != nil {
-				ts.Logger.Errorf("error storing payloadType %s on storageBackend %s for taskRun %s/%s", payloadType, b.Type(), tr.Namespace, tr.Name)
+				ts.Logger.Errorf("error storing payloadType %s on storageBackend %s for taskRun %s/%s: %v", payloadType, b.Type(), tr.Namespace, tr.Name, err)
 				// continue and store others
 			}
 		}

pkg/signing/signing_test.go

@@ -157,6 +157,11 @@ func TestTaskRunSigner_SignTaskRun(t *testing.T) {
 				t.Errorf("TaskRunSigner.SignTaskRun() error = %v", err)
 			}
 
+			// Fetch a new TR!
+			tr, err := ps.TektonV1beta1().TaskRuns(tr.Namespace).Get(tr.Name, metav1.GetOptions{})
+			if err != nil {
+				t.Errorf("error fetching fake taskrun: %v", err)
+			}
 			// Check it is marked as signed
 			if !IsSigned(tr) {
 				t.Errorf("TaskRun %s/%s should be marked as signed, was not", tr.Namespace, tr.Name)
@@ -198,7 +203,7 @@ type mockBackend struct {
 // StorePayload implements the Payloader interface.
 func (b *mockBackend) StorePayload(payload interface{}, payloadType string, tr *v1beta1.TaskRun) error {
 	if b.shouldErr {
-		return errors.New("error storing")
+		return errors.New("mock error storing")
 	}
 	if b.storedPayloads == nil {
 		b.storedPayloads = map[string]interface{}{}

pkg/signing/storage/tekton/tekton.go

@@ -17,9 +17,11 @@ import (
 	"encoding/base64"
 	"encoding/json"
 
+	"github.com/tektoncd/chains/pkg/patch"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	versioned "github.com/tektoncd/pipeline/pkg/client/clientset/versioned"
 	"go.uber.org/zap"
+	"k8s.io/apimachinery/pkg/types"
 )
 
 const (
@@ -45,9 +47,6 @@ func NewStorageBackend(ps versioned.Interface, logger *zap.SugaredLogger) *Backe
 // StorePayload implements the Payloader interface.
 func (b *Backend) StorePayload(payload interface{}, payloadType string, tr *v1beta1.TaskRun) error {
 	b.logger.Infof("Storing payload type %s on TaskRun %s/%s", payloadType, tr.Namespace, tr.Name)
-	if tr.ObjectMeta.Annotations == nil {
-		tr.ObjectMeta.Annotations = map[string]string{}
-	}
 
 	jsonPayload, err := json.Marshal(payload)
 	if err != nil {
@@ -56,8 +55,14 @@ func (b *Backend) StorePayload(payload interface{}, payloadType string, tr *v1be
 
 	textPayload := base64.StdEncoding.EncodeToString(jsonPayload)
 
-	tr.ObjectMeta.Annotations[PayloadAnnotation] = textPayload
-	if _, err := b.pipelienclientset.TektonV1beta1().TaskRuns(tr.Namespace).Update(tr); err != nil {
+	// Use patch instead of update to prevent race conditions.
+	patchBytes, err := patch.GetAnnotationsPatch(map[string]string{
+		PayloadAnnotation: textPayload,
+	})
+	if err != nil {
+		return err
+	}
+	if _, err := b.pipelienclientset.TektonV1beta1().TaskRuns(tr.Namespace).Patch(tr.Name, types.MergePatchType, patchBytes); err != nil {
 		return err
 	}
 	return nil