diff --git a/pkg/watcher/reconciler/annotation/annotation.go b/pkg/watcher/reconciler/annotation/annotation.go index f61f0a312..91d7e7e93 100644 --- a/pkg/watcher/reconciler/annotation/annotation.go +++ b/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. diff --git a/pkg/watcher/results/results.go b/pkg/watcher/results/results.go index 753b2220f..37531969e 100644 --- a/pkg/watcher/results/results.go +++ b/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,31 @@ 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 annotations, err := parseAnnotations(annotation.ResultAnnotations, value); err != nil { + return nil, err + } else { + if res.Annotations == nil { + res.Annotations = make(map[string]string, len(annotations)) + } + copyKeys(annotations, res.Annotations) + } + } + + if value, found := o.GetAnnotations()[annotation.RecordSummaryAnnotations]; found { + if annotations, err := parseAnnotations(annotation.RecordSummaryAnnotations, value); err != nil { + return nil, err + } else if res.Summary != nil { + if res.Summary.Annotations == nil { + res.Summary.Annotations = make(map[string]string, len(annotations)) + } + copyKeys(annotations, res.Summary.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 +180,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 diff --git a/pkg/watcher/results/results_test.go b/pkg/watcher/results/results_test.go index 35c5e516c..fdf0364d3 100644 --- a/pkg/watcher/results/results_test.go +++ b/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) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 0d4e7c730..7b654ab7a 100644 --- a/test/e2e/e2e_test.go +++ b/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 { diff --git a/test/e2e/testdata/pipelinerun.yaml b/test/e2e/testdata/pipelinerun.yaml index 10e29ec92..dbe4165a3 100644 --- a/test/e2e/testdata/pipelinerun.yaml +++ b/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: