Allow users/integrators to pass arbitrary keys/values through Result and RecordSummary annotations

docs/watcher/README.md

@@ -36,3 +36,24 @@ precedence):
 
 If no annotation is detected, the Watcher will automatically generate a new
 Result name for the Object.
+
+## Passing arbitrary key/values to Results
+
+Users and/or integrators can pass arbitrary keys/values to Results by adding special annotations to PipelineRuns and TaskRuns:
+
+- `results.tekton.dev/resultAnnotations`: a JSON object (string->string) to be stored into thee `Result.Annotations` field.
+- `results.tekton.dev/recordSummaryAnnotations`: a JSON object (string->string) to be stored into thee `Result.Summary.Annotations` field.
+
+Once the Watcher detects those annotations in the observed object, it passes the keys/values to the respective fields of the underlying Result. Those annotations can be used to store relevant metadata (e.g. the Git commit SHA that triggered a PipelineRun) into Results and may be used later to retrieve the objects from the API server. For instance:
+
+```yaml
+apiVersion: tekton.dev/v1beta1
+kind: PipelineRun
+metadata:
+  generateName: hello-run-
+  annotations:
+    results.tekton.dev/resultAnnotations: |-
+      {"repo": "tektoncd/results", "commit": "1a6b908"}
+    results.tekton.dev/recordSummaryAnnotations: |-
+      {"foo": "bar"}
+```

pkg/watcher/reconciler/annotation/annotation.go

@@ -30,6 +30,14 @@ const (
 
 	Log = "results.tekton.dev/log"
 
+	// Integrators should add this annotation to objects in order to store
+	// arbitrary keys/values into the Result.Annotations field.
+	ResultAnnotations = "results.tekton.dev/resultAnnotations"
+
+	// Integrators should add this annotation to objects in order to store
+	// arbitrary keys/values into the Result.Summary.Annotations field.
+	RecordSummaryAnnotations = "results.tekton.dev/recordSummaryAnnotations"
+
 	// Annotation that signals to the controller that a given child object
 	// (e.g. TaskRun owned by a PipelineRun) is done and up to date in the
 	// API server and therefore, ready to be garbage collected.

pkg/watcher/results/results.go

@@ -16,10 +16,12 @@ package results
 
 import (
 	"context"
-	"go.uber.org/zap"
-	"knative.dev/pkg/logging"
+	"encoding/json"
+	"fmt"
 	"strings"
 
+	"go.uber.org/zap"
+
 	"github.com/google/go-cmp/cmp"
 	"github.com/tektoncd/results/pkg/api/server/v1alpha2/record"
 	"github.com/tektoncd/results/pkg/api/server/v1alpha2/result"
@@ -34,6 +36,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"knative.dev/pkg/apis"
+	"knative.dev/pkg/controller"
+	"knative.dev/pkg/logging"
 )
 
 // Client is a wrapper around a Results client that provides helpful utilities
@@ -116,6 +120,41 @@ func (c *Client) ensureResult(ctx context.Context, o Object, opts ...grpc.CallOp
 		}
 	}
 
+	// Set the Result.Annotations and Result.Summary.Annotations fields if
+	// the object in question contains the required annotations.
+
+	if value, found := o.GetAnnotations()[annotation.ResultAnnotations]; found {
+		if resultAnnotations, err := parseAnnotations(annotation.ResultAnnotations, value); err != nil {
+			return nil, err
+		} else {
+			var annotations map[string]string
+			if curr != nil && len(curr.Annotations) != 0 {
+				copyKeys(resultAnnotations, curr.Annotations)
+				annotations = curr.Annotations
+			} else {
+				annotations = resultAnnotations
+			}
+			res.Annotations = annotations
+		}
+	}
+
+	if topLevel {
+		if value, found := o.GetAnnotations()[annotation.RecordSummaryAnnotations]; found {
+			if recordSummaryAnnotations, err := parseAnnotations(annotation.RecordSummaryAnnotations, value); err != nil {
+				return nil, err
+			} else {
+				var annotations map[string]string
+				if curr != nil && len(curr.Summary.Annotations) != 0 {
+					copyKeys(recordSummaryAnnotations, curr.Summary.Annotations)
+					annotations = curr.Summary.Annotations
+				} else {
+					annotations = recordSummaryAnnotations
+				}
+				res.Summary.Annotations = annotations
+			}
+		}
+	}
+
 	// Regardless of whether the object is a top level record or not,
 	// if the Result doesn't exist yet just create it and return.
 	if status.Code(err) == codes.NotFound {
@@ -151,6 +190,21 @@ func (c *Client) ensureResult(ctx context.Context, o Object, opts ...grpc.CallOp
 	return c.ResultsClient.UpdateResult(ctx, req, opts...)
 }
 
+// parseAnnotations attempts to return the provided value as a map of strings.
+func parseAnnotations(annotationKey, value string) (map[string]string, error) {
+	var annotations map[string]string
+	if err := json.Unmarshal([]byte(value), &annotations); err != nil {
+		return nil, controller.NewPermanentError(fmt.Errorf("error parsing annotation %s: %w", annotationKey, err))
+	}
+	return annotations, nil
+}
+
+func copyKeys(in, out map[string]string) {
+	for key, value := range in {
+		out[key] = value
+	}
+}
+
 func getTimestamp(c *apis.Condition) *timestamppb.Timestamp {
 	if c == nil || c.IsFalse() {
 		return nil

pkg/watcher/results/results_test.go

@@ -344,6 +344,39 @@ func TestEnsureResult_RecordSummaryUpdate(t *testing.T) {
 	}
 }
 
+func TestAnnotations(t *testing.T) {
+	ctx := logtest.TestContextWithLogger(t)
+	client := client(t)
+
+	pipelineRun := &v1beta1.PipelineRun{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "default",
+			Annotations: map[string]string{
+				annotation.ResultAnnotations:        `{"x": "y"}`,
+				annotation.RecordSummaryAnnotations: `{"foo":"bar"}`,
+			},
+			UID: types.UID("1"),
+		},
+	}
+
+	result, err := client.ensureResult(ctx, pipelineRun)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if diff := cmp.Diff(map[string]string{
+		"x": "y",
+	}, result.Annotations); diff != "" {
+		t.Errorf("Result.Annotations: mismatch (-want +got):\n%s", diff)
+	}
+
+	if diff := cmp.Diff(map[string]string{
+		"foo": "bar",
+	}, result.Summary.Annotations); diff != "" {
+		t.Errorf("Result.Summary.Annotations: mismatch (-want +got):\n%s", diff)
+	}
+}
+
 func TestUpsertRecord(t *testing.T) {
 	ctx := context.Background()
 	client := client(t)

test/e2e/e2e_test.go

@@ -19,27 +19,32 @@ package e2e
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
+	"io"
+	"net/http"
+	"strings"
+	"testing"
+
 	"github.com/google/go-cmp/cmp"
 	"github.com/tektoncd/results/test/e2e/client"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/credentials"
-	"google.golang.org/grpc/status"
-	"google.golang.org/protobuf/testing/protocmp"
-	"io"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/transport"
-	"net/http"
-	"strings"
-	"testing"
 
 	resultsv1alpha2 "github.com/tektoncd/results/proto/v1alpha2/results_go_proto"
+	"google.golang.org/grpc/status"
+	"google.golang.org/protobuf/testing/protocmp"
+	"knative.dev/pkg/apis"
+
 	"time"
 
 	"os"
 	"path"
 
+	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	tektonv1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	tektonv1beta1client "github.com/tektoncd/pipeline/pkg/client/clientset/versioned/typed/pipeline/v1beta1"
 	corev1 "k8s.io/api/core/v1"
@@ -248,6 +253,59 @@ func TestPipelineRun(t *testing.T) {
 			t.Errorf("Error getting Record: %v", err)
 		}
 	})
+
+	t.Run("result data consistency", func(t *testing.T) {
+		result, err := gc.GetResult(context.Background(), &resultsv1alpha2.GetResultRequest{
+			Name: resName,
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		t.Run("Result and RecordSummary Annotations were set accordingly", func(t *testing.T) {
+			if diff := cmp.Diff(map[string]string{
+				"repo":   "tektoncd/results",
+				"commit": "1a6b908",
+			}, result.Annotations); diff != "" {
+				t.Errorf("Result.Annotations: mismatch (-want +got):\n%s", diff)
+			}
+
+			if diff := cmp.Diff(map[string]string{
+				"foo": "bar",
+			}, result.Summary.Annotations); diff != "" {
+				t.Errorf("Result.Summary.Annotations: mismatch (-want +got):\n%s", diff)
+			}
+		})
+
+		t.Run("the PipelineRun was archived in its final state", func(t *testing.T) {
+			wantStatus := resultsv1alpha2.RecordSummary_SUCCESS
+			gotStatus := result.Summary.Status
+			if wantStatus != gotStatus {
+				t.Fatalf("Result.Summary.Status: want %v, but got %v", wantStatus, gotStatus)
+			}
+
+			record, err := gc.GetRecord(context.Background(), &resultsv1alpha2.GetRecordRequest{
+				Name: result.Summary.Record,
+			})
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			var pipelineRun v1beta1.PipelineRun
+			if err := json.Unmarshal(record.Data.Value, &pipelineRun); err != nil {
+				t.Fatal(err)
+			}
+
+			if !pipelineRun.IsDone() {
+				t.Fatal("Want PipelineRun to be done, but it isn't")
+			}
+
+			wantReason := v1beta1.PipelineRunReasonSuccessful
+			if gotReason := pipelineRun.Status.GetCondition(apis.ConditionSucceeded).GetReason(); wantReason != v1beta1.PipelineRunReason(gotReason) {
+				t.Fatalf("PipelineRun: want condition reason %s, but got %s", wantReason, gotReason)
+			}
+		})
+	})
 }
 
 func clientConfig(t *testing.T) *rest.Config {

test/e2e/testdata/pipelinerun.yaml

@@ -16,6 +16,11 @@ apiVersion: tekton.dev/v1beta1
 kind: PipelineRun
 metadata:
   name: hello
+  annotations:
+    results.tekton.dev/resultAnnotations: |-
+      {"repo": "tektoncd/results", "commit": "1a6b908"}
+    results.tekton.dev/recordSummaryAnnotations: |-
+      {"foo": "bar"}
 spec:
   pipelineSpec:
     tasks: