Skip to content

Commit

Permalink
Allow users/integrators to pass arbitrary keys/values through Result and
Browse files Browse the repository at this point in the history
RecordSummary annotations.
  • Loading branch information
alan-ghelardi committed Apr 2, 2023
1 parent 3ae05ce commit db319d5
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 2 deletions.
8 changes: 8 additions & 0 deletions pkg/watcher/reconciler/annotation/annotation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
36 changes: 34 additions & 2 deletions pkg/watcher/results/results.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -116,6 +120,25 @@ 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 {
res.Annotations = annotations
}
}

if value, found := o.GetAnnotations()[annotation.RecordSummaryAnnotations]; found {
if annotations, err := parseAnnotations(annotation.RecordSummaryAnnotations, value); err != nil {
return nil, err
} else {
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 {
Expand Down Expand Up @@ -151,6 +174,15 @@ 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 getTimestamp(c *apis.Condition) *timestamppb.Timestamp {
if c == nil || c.IsFalse() {
return nil
Expand Down
33 changes: 33 additions & 0 deletions pkg/watcher/results/results_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
59 changes: 59 additions & 0 deletions test/e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package e2e

import (
"context"
"encoding/json"
"errors"
"io/ioutil"
"strings"
Expand All @@ -29,6 +30,7 @@ import (
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/testing/protocmp"
"knative.dev/pkg/apis"

"time"

Expand Down Expand Up @@ -142,6 +144,8 @@ func TestPipelineRun(t *testing.T) {
t.Fatalf("Create: %v", err)
}

var resultID string

t.Run("result annotations", func(t *testing.T) {
// Wait for Result ID to show up.
if err := wait.PollImmediate(1*time.Second, 1*time.Minute, func() (done bool, err error) {
Expand All @@ -152,6 +156,7 @@ func TestPipelineRun(t *testing.T) {
}
if r, ok := pr.GetAnnotations()["results.tekton.dev/result"]; ok {
t.Logf("Found Result: %s", r)
resultID = r
return true, nil
}
return false, nil
Expand All @@ -172,6 +177,60 @@ func TestPipelineRun(t *testing.T) {
t.Fatalf("error waiting PipelineRun to be deleted: %v", err)
}
})

t.Run("result data consistency", func(t *testing.T) {
client := newResultsClient(t, allNamespacesReadAccessPath)
result, err := client.GetResult(context.Background(), &resultsv1alpha2.GetResultRequest{
Name: resultID,
})
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 := client.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 {
Expand Down
5 changes: 5 additions & 0 deletions test/e2e/testdata/pipelinerun.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit db319d5

Please sign in to comment.