diff --git a/cmd/sidecarlogresults/main.go b/cmd/sidecarlogresults/main.go index c66ce1133e0..b63d3c4feb3 100644 --- a/cmd/sidecarlogresults/main.go +++ b/cmd/sidecarlogresults/main.go @@ -32,14 +32,19 @@ func main() { var resultsDir string var resultNames string var stepResultsStr string + var stepNames string + flag.StringVar(&resultsDir, "results-dir", pipeline.DefaultResultPath, "Path to the results directory. Default is /tekton/results") flag.StringVar(&resultNames, "result-names", "", "comma separated result names to expect from the steps running in the pod. eg. foo,bar,baz") flag.StringVar(&stepResultsStr, "step-results", "", "json containing a map of step Name as key and list of result Names. eg. {\"stepName\":[\"foo\",\"bar\",\"baz\"]}") + flag.StringVar(&stepNames, "step-names", "", "comma separated step names. eg. foo,bar,baz") flag.Parse() - if resultNames == "" { - log.Fatal("result-names were not provided") + + var expectedResults []string + // strings.Split returns [""] instead of [] for empty string, we don't want pass [""] to other methods. + if len(resultNames) > 0 { + expectedResults = strings.Split(resultNames, ",") } - expectedResults := strings.Split(resultNames, ",") expectedStepResults := map[string][]string{} if err := json.Unmarshal([]byte(stepResultsStr), &expectedStepResults); err != nil { log.Fatal(err) @@ -48,4 +53,13 @@ func main() { if err != nil { log.Fatal(err) } + + var names []string + if len(stepNames) > 0 { + names = strings.Split(stepNames, ",") + } + err = sidecarlogresults.LookForArtifacts(os.Stdout, names, pod.RunDir) + if err != nil { + log.Fatal(err) + } } diff --git a/examples/v1/taskruns/alpha/produce-consume-artifacts.yaml b/examples/v1/taskruns/alpha/produce-consume-artifacts.yaml index e270b1b85ff..0079fe930cf 100644 --- a/examples/v1/taskruns/alpha/produce-consume-artifacts.yaml +++ b/examples/v1/taskruns/alpha/produce-consume-artifacts.yaml @@ -17,7 +17,7 @@ spec: "name":"input-artifacts", "values":[ { - "uri":"git:jjjsss", + "uri":"pkg:example.github.com/inputs", "digest":{ "sha256":"b35cacccfdb1e24dc497d15d553891345fd155713ffe647c281c583269eaaae0" } @@ -30,7 +30,7 @@ spec: "name":"image", "values":[ { - "uri":"pkg:balba", + "uri":"pkg:github/package-url/purl-spec@244fd47e07d1004f0aed9c", "digest":{ "sha256":"df85b9e3983fe2ce20ef76ad675ecf435cc99fc9350adc54fa230bae8c32ce48", "sha1":"95588b8f34c31eb7d62c92aaa4e6506639b06ef2" diff --git a/internal/sidecarlogresults/sidecarlogresults.go b/internal/sidecarlogresults/sidecarlogresults.go index e9ce11b86db..72f961de894 100644 --- a/internal/sidecarlogresults/sidecarlogresults.go +++ b/internal/sidecarlogresults/sidecarlogresults.go @@ -28,6 +28,8 @@ import ( "strings" "github.com/tektoncd/pipeline/pkg/apis/config" + "github.com/tektoncd/pipeline/pkg/apis/pipeline" + v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/result" "golang.org/x/sync/errgroup" corev1 "k8s.io/api/core/v1" @@ -36,12 +38,15 @@ import ( // ErrSizeExceeded indicates that the result exceeded its maximum allowed size var ErrSizeExceeded = errors.New("results size exceeds configured limit") +var stepDir = pipeline.StepsDir type SidecarLogResultType string const ( - taskResultType SidecarLogResultType = "task" - stepResultType SidecarLogResultType = "step" + taskResultType SidecarLogResultType = "task" + stepResultType SidecarLogResultType = "step" + + stepArtifactType SidecarLogResultType = "stepArtifact" sidecarResultNameSeparator string = "." ) @@ -197,6 +202,37 @@ func LookForResults(w io.Writer, runDir string, resultsDir string, resultNames [ return nil } +// LookForArtifacts searches for and processes artifacts within a specified run directory. +// It looks for "provenance.json" files within the "artifacts" subdirectory of each named step. +// If the provenance file exists, the function extracts artifact information, formats it into a +// JSON string, and encodes it for output alongside relevant metadata (step name, artifact type). +func LookForArtifacts(w io.Writer, names []string, runDir string) error { + if err := waitForStepsToFinish(runDir); err != nil { + return err + } + + for _, name := range names { + p := filepath.Join(stepDir, name, "artifacts", "provenance.json") + if exist, err := fileExists(p); err != nil { + return err + } else if !exist { + continue + } + subRes, err := extractArtifactsFromFile(p) + if err != nil { + return err + } + values, err := json.Marshal(&subRes) + if err != nil { + return err + } + if err := encode(w, SidecarLogResult{Name: name, Value: string(values), Type: stepArtifactType}); err != nil { + return err + } + } + return nil +} + // GetResultsFromSidecarLogs extracts results from the logs of the results sidecar func GetResultsFromSidecarLogs(ctx context.Context, clientset kubernetes.Interface, namespace string, name string, container string, podPhase corev1.PodPhase) ([]result.RunResult, error) { sidecarLogResults := []result.RunResult{} @@ -250,6 +286,8 @@ func parseResults(resultBytes []byte, maxResultLimit int) (result.RunResult, err resultType = result.TaskRunResultType case stepResultType: resultType = result.StepResultType + case stepArtifactType: + resultType = result.StepArtifactsResultType default: return result.RunResult{}, fmt.Errorf("invalid sidecar result type %v. Must be %v or %v", res.Type, taskResultType, stepResultType) } @@ -260,3 +298,19 @@ func parseResults(resultBytes []byte, maxResultLimit int) (result.RunResult, err } return runResult, nil } + +func parseArtifacts(fileContent []byte) (v1.Artifacts, error) { + var as v1.Artifacts + if err := json.Unmarshal(fileContent, &as); err != nil { + return as, fmt.Errorf("invalid artifacts : %w", err) + } + return as, nil +} + +func extractArtifactsFromFile(filename string) (v1.Artifacts, error) { + b, err := os.ReadFile(filename) + if err != nil { + return v1.Artifacts{}, fmt.Errorf("error reading the results file %w", err) + } + return parseArtifacts(b) +} diff --git a/internal/sidecarlogresults/sidecarlogresults_test.go b/internal/sidecarlogresults/sidecarlogresults_test.go index 2f6a3de8dad..ff81e1aca1a 100644 --- a/internal/sidecarlogresults/sidecarlogresults_test.go +++ b/internal/sidecarlogresults/sidecarlogresults_test.go @@ -29,10 +29,10 @@ import ( "testing" "github.com/google/go-cmp/cmp" + v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/result" "github.com/tektoncd/pipeline/test/diff" corev1 "k8s.io/api/core/v1" - v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" fakekubeclientset "k8s.io/client-go/kubernetes/fake" ) @@ -397,7 +397,7 @@ func TestParseResults_Failure(t *testing.T) { func TestGetResultsFromSidecarLogs(t *testing.T) { for _, c := range []struct { desc string - podPhase v1.PodPhase + podPhase corev1.PodPhase wantError bool }{{ desc: "pod pending to start", @@ -411,7 +411,7 @@ func TestGetResultsFromSidecarLogs(t *testing.T) { t.Run(c.desc, func(t *testing.T) { ctx := context.Background() clientset := fakekubeclientset.NewSimpleClientset() - pod := &v1.Pod{ + pod := &corev1.Pod{ TypeMeta: metav1.TypeMeta{ Kind: "Pod", APIVersion: "v1", @@ -420,15 +420,15 @@ func TestGetResultsFromSidecarLogs(t *testing.T) { Name: "pod", Namespace: "foo", }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ { Name: "container", Image: "image", }, }, }, - Status: v1.PodStatus{ + Status: corev1.PodStatus{ Phase: c.podPhase, }, } @@ -474,6 +474,165 @@ func TestExtractStepAndResultFromSidecarResultName_Error(t *testing.T) { } } +func TestLookForArtifacts(t *testing.T) { + base := basicArtifacts() + var modified = base.DeepCopy() + modified.Outputs[0].Name = "tests" + type Arg struct { + stepName string + artifacts *v1.Artifacts + customContent []byte + } + tests := []struct { + desc string + wantErr bool + args []Arg + expected []SidecarLogResult + }{ + { + desc: "one step produces artifacts, read success", + args: []Arg{{stepName: "first", artifacts: &base}}, + expected: []SidecarLogResult{{ + Name: "first", + Type: stepArtifactType, + Value: mustJSON(&base), + }}, + }, { + desc: "two step produce artifacts, read success", + args: []Arg{{stepName: "first", artifacts: &base}, {stepName: "second", artifacts: modified}}, + expected: []SidecarLogResult{{ + Name: "first", + Type: stepArtifactType, + Value: mustJSON(&base), + }, { + Name: "second", + Type: stepArtifactType, + Value: mustJSON(modified), + }}, + }, + { + desc: "one step produces artifacts, one step does not, read success", + args: []Arg{{stepName: "first", artifacts: &base}, {stepName: "second"}}, + expected: []SidecarLogResult{{ + Name: "first", + Type: stepArtifactType, + Value: mustJSON(&base), + }}, + }, + { + desc: "two step produces, one read success, one not, error out and result is not empty.", + args: []Arg{{stepName: "first", artifacts: &base}, {stepName: "second", artifacts: modified, customContent: []byte("this is to break json")}}, + expected: []SidecarLogResult{{ + Name: "first", + Type: stepArtifactType, + Value: mustJSON(&base), + }}, + wantErr: true, + }, + { + desc: "two step produces, first read fails, error out and result is empty.", + args: []Arg{{stepName: "first", artifacts: modified, customContent: []byte("this is to break json")}, {stepName: "second", artifacts: &base}}, + expected: []SidecarLogResult{}, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.desc, func(t *testing.T) { + dir := t.TempDir() + curStepDir := stepDir + stepDir = dir + t.Cleanup(func() { + stepDir = curStepDir + }) + + var names []string + for _, arg := range tc.args { + names = append(names, arg.stepName) + if err := os.MkdirAll(filepath.Join(dir, arg.stepName, "artifacts"), os.ModePerm); err != nil { + t.Errorf("failed to create artifacts folder, err: %v", err) + } + if _, err := os.Create(filepath.Join(dir, arg.stepName, "out")); err != nil { + t.Errorf("failed to file, err: %v", err) + } + if arg.artifacts != nil { + if err := writeArtifacts(filepath.Join(dir, arg.stepName, "artifacts", "provenance.json"), arg.artifacts); err != nil { + t.Errorf("failed to write artifacts to provenance.json, err: %v", err) + } + } + if arg.customContent != nil { + if err := os.WriteFile(filepath.Join(dir, arg.stepName, "artifacts", "provenance.json"), arg.customContent, os.ModePerm); err != nil { + t.Errorf("failed to write customContent to provenance.json, err: %v", err) + } + } + } + var buf bytes.Buffer + err := LookForArtifacts(&buf, names, dir) + if (err != nil) != tc.wantErr { + t.Errorf("error checking failed, wantErr: %v, got: %v", tc.wantErr, err) + } + want := "" + for _, logResult := range tc.expected { + want += mustJSON(logResult) + "\n" + } + got := buf.String() + + if d := cmp.Diff(want, got); d != "" { + t.Errorf(diff.PrintWantGot(d)) + } + }) + } +} + +func writeArtifacts(path string, artifacts *v1.Artifacts) error { + f, err := os.Create(path) + if err != nil { + return err + } + defer f.Close() + res := json.NewEncoder(f).Encode(artifacts) + return res +} + +func basicArtifacts() v1.Artifacts { + data := `{ + "inputs":[ + { + "name":"inputs", + "values":[ + { + "uri":"pkg:example.github.com/inputs", + "digest":{ + "sha256":"b35cacccfdb1e24dc497d15d553891345fd155713ffe647c281c583269eaaae0" + } + } + ] + } + ], + "outputs":[ + { + "name":"image", + "values":[ + { + "uri":"pkg:github/package-url/purl-spec@244fd47e07d1004f0aed9c", + "digest":{ + "sha256":"df85b9e3983fe2ce20ef76ad675ecf435cc99fc9350adc54fa230bae8c32ce48", + "sha1":"95588b8f34c31eb7d62c92aaa4e6506639b06ef2" + } + } + ] + } + ] + } +` + var ars v1.Artifacts + err := json.Unmarshal([]byte(data), &ars) + if err != nil { + panic(err) + } + return ars +} + func createStepResult(t *testing.T, dir, stepName, resultName, resultValue string) { t.Helper() resultDir := filepath.Join(dir, stepName, "results") @@ -507,3 +666,11 @@ func createRun(t *testing.T, dir string, causeErr bool) { t.Fatal(err) } } + +func mustJSON(data any) string { + marshal, err := json.Marshal(data) + if err != nil { + panic(err) + } + return string(marshal) +} diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go index 95708574409..6f35adb18f1 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -30,6 +30,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/validate" "github.com/tektoncd/pipeline/pkg/internal/resultref" + "github.com/tektoncd/pipeline/pkg/pod" "github.com/tektoncd/pipeline/pkg/substitution" "golang.org/x/exp/slices" @@ -312,7 +313,7 @@ func errorIfStepResultReferenceinField(value, fieldName string) (errs *apis.Fiel } func stepArtifactReferenceExists(src string) bool { - return len(artifactref.StepArtifactRegex.FindAllStringSubmatch(src, -1)) > 0 || strings.Contains(src, "$(step.artifacts.path)") + return len(artifactref.StepArtifactRegex.FindAllStringSubmatch(src, -1)) > 0 || strings.Contains(src, "$("+pod.StepArtifactPathPattern+")") } func errorIfStepArtifactReferencedInField(value, fieldName string) (errs *apis.FieldError) { if stepArtifactReferenceExists(value) { diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go index 9ef0db6909a..43ca36fdcaa 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -31,6 +31,7 @@ import ( v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/apis/validate" "github.com/tektoncd/pipeline/pkg/internal/resultref" + "github.com/tektoncd/pipeline/pkg/pod" "github.com/tektoncd/pipeline/pkg/substitution" "golang.org/x/exp/slices" @@ -301,7 +302,7 @@ func errorIfStepResultReferenceinField(value, fieldName string) (errs *apis.Fiel } func stepArtifactReferenceExists(src string) bool { - return len(artifactref.StepArtifactRegex.FindAllStringSubmatch(src, -1)) > 0 || strings.Contains(src, "$(step.artifacts.path)") + return len(artifactref.StepArtifactRegex.FindAllStringSubmatch(src, -1)) > 0 || strings.Contains(src, "$("+pod.StepArtifactPathPattern+")") } func errorIfStepArtifactReferencedInField(value, fieldName string) (errs *apis.FieldError) { diff --git a/pkg/entrypoint/entrypointer.go b/pkg/entrypoint/entrypointer.go index 8ae4c1f2cfd..07c41791ecf 100644 --- a/pkg/entrypoint/entrypointer.go +++ b/pkg/entrypoint/entrypointer.go @@ -300,7 +300,7 @@ func readArtifacts(fp string) ([]result.RunResult, error) { if err != nil { return nil, err } - return []result.RunResult{{Key: fp, Value: string(file), ResultType: result.ArtifactsResultType}}, nil + return []result.RunResult{{Key: fp, Value: string(file), ResultType: result.StepResultType}}, nil } func (e Entrypointer) readResultsFromDisk(ctx context.Context, resultDir string, resultType result.ResultType) error { diff --git a/pkg/pod/entrypoint.go b/pkg/pod/entrypoint.go index 5997131e5bd..3addcd1df00 100644 --- a/pkg/pod/entrypoint.go +++ b/pkg/pod/entrypoint.go @@ -126,7 +126,7 @@ var ( // Additionally, Step timeouts are added as entrypoint flag. func orderContainers(ctx context.Context, commonExtraEntrypointArgs []string, steps []corev1.Container, taskSpec *v1.TaskSpec, breakpointConfig *v1.TaskRunDebug, waitForReadyAnnotation, enableKeepPodOnCancel bool) ([]corev1.Container, error) { if len(steps) == 0 { - return nil, errors.New("No steps specified") + return nil, errors.New("no steps specified") } for i, s := range steps { diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index 9b117e50b8b..45400939175 100644 --- a/pkg/pod/pod.go +++ b/pkg/pod/pod.go @@ -73,6 +73,8 @@ const ( // TerminationReasonCancelled indicates a step was cancelled. TerminationReasonCancelled = "Cancelled" + + StepArtifactPathPattern = "step.artifacts.path" ) // These are effectively const, but Go doesn't have such an annotation. @@ -201,15 +203,18 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1.TaskRun, taskSpec v1.Ta tasklevel.ApplyTaskLevelComputeResources(steps, taskRun.Spec.ComputeResources) } windows := usesWindows(taskRun) - if sidecarLogsResultsEnabled && taskSpec.Results != nil { - // create a results sidecar - resultsSidecar, err := createResultsSidecar(taskSpec, b.Images.SidecarLogResultsImage, setSecurityContext, windows) - if err != nil { - return nil, err + if sidecarLogsResultsEnabled { + if taskSpec.Results != nil || artifactsPathReferenced(steps) { + // create a results sidecar + resultsSidecar, err := createResultsSidecar(taskSpec, b.Images.SidecarLogResultsImage, setSecurityContext, windows) + if err != nil { + return nil, err + } + taskSpec.Sidecars = append(taskSpec.Sidecars, resultsSidecar) + commonExtraEntrypointArgs = append(commonExtraEntrypointArgs, "-result_from", config.ResultExtractionMethodSidecarLogs) } - taskSpec.Sidecars = append(taskSpec.Sidecars, resultsSidecar) - commonExtraEntrypointArgs = append(commonExtraEntrypointArgs, "-result_from", config.ResultExtractionMethodSidecarLogs) } + sidecars, err := v1.MergeSidecarsWithSpecs(taskSpec.Sidecars, taskRun.Spec.SidecarSpecs) if err != nil { return nil, err @@ -339,25 +344,30 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1.TaskRun, taskSpec v1.Ta stepContainers[i].VolumeMounts = vms } - if sidecarLogsResultsEnabled && taskSpec.Results != nil { + if sidecarLogsResultsEnabled { // Mount implicit volumes onto sidecarContainers // so that they can access /tekton/results and /tekton/run. - for i, s := range sidecarContainers { - for j := 0; j < len(stepContainers); j++ { - s.VolumeMounts = append(s.VolumeMounts, runMount(j, true)) - } - requestedVolumeMounts := map[string]bool{} - for _, vm := range s.VolumeMounts { - requestedVolumeMounts[filepath.Clean(vm.MountPath)] = true - } - var toAdd []corev1.VolumeMount - for _, imp := range volumeMounts { - if !requestedVolumeMounts[filepath.Clean(imp.MountPath)] { - toAdd = append(toAdd, imp) + if taskSpec.Results != nil || artifactsPathReferenced(steps) { + for i, s := range sidecarContainers { + if s.Name != pipeline.ReservedResultsSidecarName { + continue + } + for j := 0; j < len(stepContainers); j++ { + s.VolumeMounts = append(s.VolumeMounts, runMount(j, true)) } + requestedVolumeMounts := map[string]bool{} + for _, vm := range s.VolumeMounts { + requestedVolumeMounts[filepath.Clean(vm.MountPath)] = true + } + var toAdd []corev1.VolumeMount + for _, imp := range volumeMounts { + if !requestedVolumeMounts[filepath.Clean(imp.MountPath)] { + toAdd = append(toAdd, imp) + } + } + vms := append(s.VolumeMounts, toAdd...) //nolint:gocritic + sidecarContainers[i].VolumeMounts = vms } - vms := append(s.VolumeMounts, toAdd...) //nolint:gocritic - sidecarContainers[i].VolumeMounts = vms } } @@ -687,8 +697,19 @@ func createResultsSidecar(taskSpec v1.TaskSpec, image string, setSecurityContext for _, r := range taskSpec.Results { names = append(names, r.Name) } + + stepNames := make([]string, 0, len(taskSpec.Steps)) + var artifactProducerSteps []string + for i, s := range taskSpec.Steps { + stepName := StepName(s.Name, i) + stepNames = append(stepNames, stepName) + if artifactPathReferencedInStep(s) { + artifactProducerSteps = append(artifactProducerSteps, GetContainerName(s.Name)) + } + } + resultsStr := strings.Join(names, ",") - command := []string{"/ko-app/sidecarlogresults", "-results-dir", pipeline.DefaultResultPath, "-result-names", resultsStr} + command := []string{"/ko-app/sidecarlogresults", "-results-dir", pipeline.DefaultResultPath, "-result-names", resultsStr, "-step-names", strings.Join(artifactProducerSteps, ",")} // create a map of container Name to step results stepResults := map[string][]string{} @@ -734,3 +755,39 @@ func usesWindows(tr *v1.TaskRun) bool { osSelector := tr.Spec.PodTemplate.NodeSelector[osSelectorLabel] return osSelector == "windows" } + +func artifactsPathReferenced(steps []v1.Step) bool { + for _, step := range steps { + if artifactPathReferencedInStep(step) { + return true + } + } + return false +} + +func artifactPathReferencedInStep(step v1.Step) bool { + // `$(step.artifacts.path)` in taskRun.Spec.TaskSpec.Steps and `taskSpec.steps` are substituted when building the pod while when setting status for taskRun + // neither of them is substituted, so we need two forms to check if artifactsPath is referenced in steps. + unresolvedPath := "$(" + StepArtifactPathPattern + ")" + + path := filepath.Join(pipeline.StepsDir, GetContainerName(step.Name), "artifacts", "provenance.json") + if strings.Contains(step.Script, path) || strings.Contains(step.Script, unresolvedPath) { + return true + } + for _, arg := range step.Args { + if strings.Contains(arg, path) || strings.Contains(arg, unresolvedPath) { + return true + } + } + for _, c := range step.Command { + if strings.Contains(c, path) || strings.Contains(c, unresolvedPath) { + return true + } + } + for _, e := range step.Env { + if strings.Contains(e.Value, path) || strings.Contains(e.Value, unresolvedPath) { + return true + } + } + return false +} diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index 0ff4e0f0154..2917333a4f1 100644 --- a/pkg/pod/pod_test.go +++ b/pkg/pod/pod_test.go @@ -1937,7 +1937,7 @@ _EOF_ }, }, { - desc: "sidecar logs enabled", + desc: "sidecar logs enabled, artifacts not enabled", featureFlags: map[string]string{"results-from": "sidecar-logs"}, ts: v1.TaskSpec{ Results: []v1.TaskResult{{ @@ -1991,6 +1991,8 @@ _EOF_ "/tekton/results", "-result-names", "foo", + "-step-names", + "", "-step-results", "{}", }, @@ -2010,7 +2012,7 @@ _EOF_ }, }, { - desc: "sidecar logs enabled with step results", + desc: "sidecar logs enabled with step results, artifacts not enabled", featureFlags: map[string]string{"results-from": "sidecar-logs"}, ts: v1.TaskSpec{ Results: []v1.TaskResult{{ @@ -2070,6 +2072,8 @@ _EOF_ "/tekton/results", "-result-names", "foo", + "-step-names", + "", "-step-results", "{\"step-name\":[\"step-foo\"]}", }, @@ -2089,7 +2093,7 @@ _EOF_ }, }, { - desc: "sidecar logs enabled with security context", + desc: "sidecar logs enabled and artifacts not enabled, set security context is true", featureFlags: map[string]string{"results-from": "sidecar-logs", "set-security-context": "true"}, ts: v1.TaskSpec{ Results: []v1.TaskResult{{ @@ -2143,6 +2147,249 @@ _EOF_ "/tekton/results", "-result-names", "foo", + "-step-names", + "", + "-step-results", + "{}", + }, + Resources: corev1.ResourceRequirements{ + Requests: nil, + }, + VolumeMounts: append([]corev1.VolumeMount{ + {Name: "tekton-internal-bin", ReadOnly: true, MountPath: "/tekton/bin"}, + {Name: "tekton-internal-run-0", ReadOnly: true, MountPath: "/tekton/run/0"}, + }, implicitVolumeMounts...), + SecurityContext: linuxSecurityContext, + }}, + Volumes: append(implicitVolumes, binVolume, runVolume(0), downwardVolume, corev1.Volume{ + Name: "tekton-creds-init-home-0", + VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}}, + }), + ActiveDeadlineSeconds: &defaultActiveDeadlineSeconds, + }, + }, + { + desc: "sidecar logs enabled and artifacts referenced", + featureFlags: map[string]string{"results-from": "sidecar-logs", "enable-artifacts": "true"}, + ts: v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "foo", + Type: v1.ResultsTypeString, + }}, + Steps: []v1.Step{{ + Name: "name", + Image: "image", + Command: []string{"echo", "aaa", ">>>", "/tekton/steps/step-name/artifacts/provenance.json"}, // avoid entrypoint lookup. + }}, + }, + want: &corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyNever, + InitContainers: []corev1.Container{ + entrypointInitContainer(images.EntrypointImage, []v1.Step{{Name: "name"}}, false /* setSecurityContext */, false /* windows */), + }, + Containers: []corev1.Container{{ + Name: "step-name", + Image: "image", + Command: []string{"/tekton/bin/entrypoint"}, + Args: []string{ + "-wait_file", + "/tekton/downward/ready", + "-wait_file_content", + "-post_file", + "/tekton/run/0/out", + "-termination_path", + "/tekton/termination", + "-step_metadata_dir", + "/tekton/run/0/status", + "-result_from", + "sidecar-logs", + "-results", + "foo", + "-entrypoint", + "echo", + "--", + "aaa", + ">>>", + "/tekton/steps/step-name/artifacts/provenance.json", + }, + VolumeMounts: append([]corev1.VolumeMount{binROMount, runMount(0, false), downwardMount, { + Name: "tekton-creds-init-home-0", + MountPath: "/tekton/creds", + }}, implicitVolumeMounts...), + TerminationMessagePath: "/tekton/termination", + }, { + Name: pipeline.ReservedResultsSidecarContainerName, + Image: "", + Command: []string{ + "/ko-app/sidecarlogresults", + "-results-dir", + "/tekton/results", + "-result-names", + "foo", + "-step-names", + "step-name", + "-step-results", + "{}", + }, + Resources: corev1.ResourceRequirements{ + Requests: nil, + }, + VolumeMounts: append([]corev1.VolumeMount{ + {Name: "tekton-internal-bin", ReadOnly: true, MountPath: "/tekton/bin"}, + {Name: "tekton-internal-run-0", ReadOnly: true, MountPath: "/tekton/run/0"}, + }, implicitVolumeMounts...), + }}, + Volumes: append(implicitVolumes, binVolume, runVolume(0), downwardVolume, corev1.Volume{ + Name: "tekton-creds-init-home-0", + VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}}, + }), + ActiveDeadlineSeconds: &defaultActiveDeadlineSeconds, + }, + }, + { + desc: "sidecar logs enabled with step results, artifacts referenced", + featureFlags: map[string]string{"results-from": "sidecar-logs", "enable-artifacts": "true"}, + ts: v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "foo", + Type: v1.ResultsTypeString, + }}, + Steps: []v1.Step{{ + Name: "name", + Results: []v1.StepResult{{ + Name: "step-foo", + Type: v1.ResultsTypeString, + }}, + Image: "image", + Command: []string{"echo", "aaa", ">>>", "/tekton/steps/step-name/artifacts/provenance.json"}, // + }}, + }, + want: &corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyNever, + InitContainers: []corev1.Container{ + entrypointInitContainer(images.EntrypointImage, []v1.Step{{Name: "name"}}, false /* setSecurityContext */, false /* windows */), + }, + Containers: []corev1.Container{{ + Name: "step-name", + Image: "image", + Command: []string{"/tekton/bin/entrypoint"}, + Args: []string{ + "-wait_file", + "/tekton/downward/ready", + "-wait_file_content", + "-post_file", + "/tekton/run/0/out", + "-termination_path", + "/tekton/termination", + "-step_metadata_dir", + "/tekton/run/0/status", + "-result_from", + "sidecar-logs", + "-step_results", + "step-foo", + "-results", + "foo", + "-entrypoint", + "echo", + "--", + "aaa", + ">>>", + "/tekton/steps/step-name/artifacts/provenance.json", + }, + VolumeMounts: append([]corev1.VolumeMount{binROMount, runMount(0, false), downwardMount, { + Name: "tekton-creds-init-home-0", + MountPath: "/tekton/creds", + }}, implicitVolumeMounts...), + TerminationMessagePath: "/tekton/termination", + }, { + Name: pipeline.ReservedResultsSidecarContainerName, + Image: "", + Command: []string{ + "/ko-app/sidecarlogresults", + "-results-dir", + "/tekton/results", + "-result-names", + "foo", + "-step-names", + "step-name", + "-step-results", + "{\"step-name\":[\"step-foo\"]}", + }, + Resources: corev1.ResourceRequirements{ + Requests: nil, + }, + VolumeMounts: append([]corev1.VolumeMount{ + {Name: "tekton-internal-bin", ReadOnly: true, MountPath: "/tekton/bin"}, + {Name: "tekton-internal-run-0", ReadOnly: true, MountPath: "/tekton/run/0"}, + }, implicitVolumeMounts...), + }}, + Volumes: append(implicitVolumes, binVolume, runVolume(0), downwardVolume, corev1.Volume{ + Name: "tekton-creds-init-home-0", + VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}}, + }), + ActiveDeadlineSeconds: &defaultActiveDeadlineSeconds, + }, + }, + { + desc: "sidecar logs enabled, artifacts referenced and security context set ", + featureFlags: map[string]string{"results-from": "sidecar-logs", "set-security-context": "true", "enable-artifacts": "true"}, + ts: v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "foo", + Type: v1.ResultsTypeString, + }}, + Steps: []v1.Step{{ + Name: "name", + Image: "image", + Command: []string{"echo", "aaa", ">>>", "/tekton/steps/step-name/artifacts/provenance.json"}, + }}, + }, + want: &corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyNever, + InitContainers: []corev1.Container{ + entrypointInitContainer(images.EntrypointImage, []v1.Step{{Name: "name"}}, true /* setSecurityContext */, false /* windows */), + }, + Containers: []corev1.Container{{ + Name: "step-name", + Image: "image", + Command: []string{"/tekton/bin/entrypoint"}, + Args: []string{ + "-wait_file", + "/tekton/downward/ready", + "-wait_file_content", + "-post_file", + "/tekton/run/0/out", + "-termination_path", + "/tekton/termination", + "-step_metadata_dir", + "/tekton/run/0/status", + "-result_from", + "sidecar-logs", + "-results", + "foo", + "-entrypoint", + "echo", + "--", + "aaa", + ">>>", + "/tekton/steps/step-name/artifacts/provenance.json", + }, + VolumeMounts: append([]corev1.VolumeMount{binROMount, runMount(0, false), downwardMount, { + Name: "tekton-creds-init-home-0", + MountPath: "/tekton/creds", + }}, implicitVolumeMounts...), + TerminationMessagePath: "/tekton/termination", + }, { + Name: pipeline.ReservedResultsSidecarContainerName, + Image: "", + Command: []string{ + "/ko-app/sidecarlogresults", + "-results-dir", + "/tekton/results", + "-result-names", + "foo", + "-step-names", + "step-name", "-step-results", "{}", }, @@ -3626,3 +3873,128 @@ func TestUpdateResourceRequirements(t *testing.T) { }) } } + +func Test_artifactsPathReferenced(t *testing.T) { + tests := []struct { + name string + steps []v1.Step + want bool + }{ + { + name: "No Steps", + steps: []v1.Step{}, + want: false, + }, + { + name: "No Reference", + steps: []v1.Step{ + { + Name: "name", + Script: "echo hello", + Command: []string{"echo", "hello"}, + }, + }, + want: false, + }, + { + name: "Reference in Script", + steps: []v1.Step{ + { + Name: "name", + Script: "echo aaa >> /tekton/steps/step-name/artifacts/provenance.json", + }, + }, + want: true, + }, + { + name: "Reference in Args", + steps: []v1.Step{ + { + Name: "name", + Command: []string{"cat"}, + Args: []string{"/tekton/steps/step-name/artifacts/provenance.json"}, + }, + }, + want: true, + }, + { + name: "Reference in Command", + steps: []v1.Step{ + { + Name: "name", + Command: []string{"cat", "/tekton/steps/step-name/artifacts/provenance.json"}, + }, + }, + want: true, + }, + { + name: "Reference in Env", + steps: []v1.Step{ + { + Name: "name", + Env: []corev1.EnvVar{ + { + Name: "MY_VAR", + Value: "/tekton/steps/step-name/artifacts/provenance.json", + }, + }, + }, + }, + want: true, + }, + { + name: "Unresolved reference in Script", + steps: []v1.Step{ + { + Name: "name", + Script: "echo aaa >> $(step.artifacts.path)", + }, + }, + want: true, + }, + { + name: "Unresolved reference in Args", + steps: []v1.Step{ + { + Name: "name", + Command: []string{"cat"}, + Args: []string{"$(step.artifacts.path)"}, + }, + }, + want: true, + }, + { + name: "Unresolved reference in Command", + steps: []v1.Step{ + { + Name: "name", + Command: []string{"cat", "$(step.artifacts.path)"}, + }, + }, + want: true, + }, + { + name: "Unresolved reference in Env", + steps: []v1.Step{ + { + Name: "name", + Env: []corev1.EnvVar{ + { + Name: "MY_VAR", + Value: "$(step.artifacts.path)", + }, + }, + }, + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := artifactsPathReferenced(tt.steps) + if d := cmp.Diff(tt.want, got); d != "" { + t.Errorf("Diff %s", diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/pod/status.go b/pkg/pod/status.go index 0b034e11fda..d67fbe230a8 100644 --- a/pkg/pod/status.go +++ b/pkg/pod/status.go @@ -222,14 +222,19 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL // Extract results from sidecar logs sidecarLogsResultsEnabled := config.FromContextOrDefaults(ctx).FeatureFlags.ResultExtractionMethod == config.ResultExtractionMethodSidecarLogs + // temporary solution to check if artifacts sidecar created in taskRun as we don't have the api for users to declare if a step/task is producing artifacts yet + artifactsSidecarCreated := artifactsPathReferenced(ts.Steps) sidecarLogResults := []result.RunResult{} - if sidecarLogsResultsEnabled && tr.Status.TaskSpec.Results != nil { + + if sidecarLogsResultsEnabled { // extraction of results from sidecar logs - slr, err := sidecarlogresults.GetResultsFromSidecarLogs(ctx, kubeclient, tr.Namespace, tr.Status.PodName, pipeline.ReservedResultsSidecarContainerName, podPhase) - if err != nil { - merr = multierror.Append(merr, err) + if tr.Status.TaskSpec.Results != nil || artifactsSidecarCreated { + slr, err := sidecarlogresults.GetResultsFromSidecarLogs(ctx, kubeclient, tr.Namespace, tr.Status.PodName, pipeline.ReservedResultsSidecarContainerName, podPhase) + if err != nil { + merr = multierror.Append(merr, err) + } + sidecarLogResults = append(sidecarLogResults, slr...) } - sidecarLogResults = append(sidecarLogResults, slr...) } // Populate Task results from sidecar logs taskResultsFromSidecarLogs := getTaskResultsFromSidecarLogs(sidecarLogResults) @@ -270,10 +275,20 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL // Set TaskResults from StepResults trs.Results = append(trs.Results, createTaskResultsFromStepResults(stepRunRes, neededStepResults)...) } + var as v1.Artifacts + + err = setStepArtifactsValueFromSidecarLogs(sidecarLogResults, s.Name, &as) + if err != nil { + logger.Errorf("Failed to set artifacts value from sidecar logs: %v", err) + merr = multierror.Append(merr, err) + } + + if err != nil { + merr = multierror.Append(merr, err) + } // Parse termination messages terminationReason := "" - var as v1.Artifacts if state.Terminated != nil && len(state.Terminated.Message) != 0 { msg := state.Terminated.Message @@ -283,7 +298,7 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL merr = multierror.Append(merr, err) } else { for _, r := range results { - if r.ResultType == result.ArtifactsResultType { + if r.ResultType == result.StepArtifactsResultType { if err := json.Unmarshal([]byte(r.Value), &as); err != nil { logger.Errorf("result value could not be parsed as Artifacts: %v", err) merr = multierror.Append(merr, err) @@ -343,6 +358,15 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL return merr } +func setStepArtifactsValueFromSidecarLogs(results []result.RunResult, name string, artifacts *v1.Artifacts) error { + for _, r := range results { + if r.Key == name && r.ResultType == result.StepArtifactsResultType { + return json.Unmarshal([]byte(r.Value), artifacts) + } + } + return nil +} + func setTaskRunStatusBasedOnSidecarStatus(sidecarStatuses []corev1.ContainerStatus, trs *v1.TaskRunStatus) { for _, s := range sidecarStatuses { trs.Sidecars = append(trs.Sidecars, v1.SidecarState{ @@ -448,7 +472,7 @@ func filterResults(results []result.RunResult, specResults []v1.TaskResult, step } taskRunStepResults = append(taskRunStepResults, taskRunStepResult) filteredResults = append(filteredResults, r) - case result.ArtifactsResultType: + case result.StepArtifactsResultType: filteredResults = append(filteredResults, r) continue case result.InternalTektonResultType: diff --git a/pkg/pod/status_test.go b/pkg/pod/status_test.go index 93796e0759c..44ac4df717c 100644 --- a/pkg/pod/status_test.go +++ b/pkg/pod/status_test.go @@ -116,36 +116,87 @@ func TestSetTaskRunStatusBasedOnStepStatus(t *testing.T) { func TestSetTaskRunStatusBasedOnStepStatus_sidecar_logs(t *testing.T) { for _, c := range []struct { - desc string - maxResultSize int - wantErr error + desc string + maxResultSize int + wantErr error + enableArtifacts bool + tr v1.TaskRun }{{ - desc: "test result with sidecar logs too large", + desc: "test result with sidecar logs too large", + tr: v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task-run", + Namespace: "foo", + }, + Status: v1.TaskRunStatus{ + TaskRunStatusFields: v1.TaskRunStatusFields{ + TaskSpec: &v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "result1", + }}, + }, + PodName: "task-run-pod", + }, + }, + }, maxResultSize: 1, wantErr: sidecarlogresults.ErrSizeExceeded, }, { - desc: "test result with sidecar logs bad format", + desc: "test result with sidecar logs bad format", + tr: v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task-run", + Namespace: "foo", + }, + Status: v1.TaskRunStatus{ + TaskRunStatusFields: v1.TaskRunStatusFields{ + TaskSpec: &v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "result1", + }}, + }, + PodName: "task-run-pod", + }, + }, + }, maxResultSize: 4096, wantErr: fmt.Errorf("%s", "invalid result \"\": invalid character 'k' in literal false (expecting 'l')"), - }} { - t.Run(c.desc, func(t *testing.T) { - tr := v1.TaskRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "task-run", - Namespace: "foo", + }, { + desc: "test artifact with sidecar logs too large", + tr: v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task-run", + Namespace: "foo", + }, + Status: v1.TaskRunStatus{ + TaskRunStatusFields: v1.TaskRunStatusFields{ + TaskSpec: &v1.TaskSpec{}, + PodName: "task-run-pod", }, - Status: v1.TaskRunStatus{ - TaskRunStatusFields: v1.TaskRunStatusFields{ - TaskSpec: &v1.TaskSpec{ - Results: []v1.TaskResult{{ - Name: "result1", - }}, - }, - PodName: "task-run-pod", - }, + }, + }, + maxResultSize: 1, + wantErr: sidecarlogresults.ErrSizeExceeded, + enableArtifacts: true, + }, { + desc: "test artifact with sidecar logs bad format", + tr: v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task-run", + Namespace: "foo", + }, + Status: v1.TaskRunStatus{ + TaskRunStatusFields: v1.TaskRunStatusFields{ + TaskSpec: &v1.TaskSpec{}, + PodName: "task-run-pod", }, - } - + }, + }, + maxResultSize: 4096, + wantErr: fmt.Errorf("%s", "invalid result \"\": invalid character 'k' in literal false (expecting 'l')"), + enableArtifacts: true, + }} { + t.Run(c.desc, func(t *testing.T) { logger, _ := logging.NewLogger("", "status") kubeclient := fakek8s.NewSimpleClientset() pod := &corev1.Pod{ @@ -173,15 +224,21 @@ func TestSetTaskRunStatusBasedOnStepStatus_sidecar_logs(t *testing.T) { if err != nil { t.Errorf("Error occurred while creating pod %s: %s", pod.Name, err.Error()) } + featureFlags := &config.FeatureFlags{ + ResultExtractionMethod: config.ResultExtractionMethodSidecarLogs, + MaxResultSize: c.maxResultSize, + } + ts := &v1.TaskSpec{} + if c.enableArtifacts { + featureFlags.EnableArtifacts = true + ts.Steps = []v1.Step{{Name: "name", Script: `echo aaa >> /tekton/steps/step-name/artifacts/provenance.json`}} + } ctx := config.ToContext(context.Background(), &config.Config{ - FeatureFlags: &config.FeatureFlags{ - ResultExtractionMethod: config.ResultExtractionMethodSidecarLogs, - MaxResultSize: c.maxResultSize, - }, + FeatureFlags: featureFlags, }) var wantErr *multierror.Error wantErr = multierror.Append(wantErr, c.wantErr) - merr := setTaskRunStatusBasedOnStepStatus(ctx, logger, []corev1.ContainerStatus{{}}, &tr, pod.Status.Phase, kubeclient, &v1.TaskSpec{}) + merr := setTaskRunStatusBasedOnStepStatus(ctx, logger, []corev1.ContainerStatus{{}}, &c.tr, pod.Status.Phase, kubeclient, ts) if d := cmp.Diff(wantErr.Error(), merr.Error()); d != "" { t.Errorf("Got unexpected error %s", diff.PrintWantGot(d)) diff --git a/pkg/reconciler/taskrun/resources/apply.go b/pkg/reconciler/taskrun/resources/apply.go index b54a618dba1..cbcfc42b2ad 100644 --- a/pkg/reconciler/taskrun/resources/apply.go +++ b/pkg/reconciler/taskrun/resources/apply.go @@ -411,7 +411,7 @@ func ApplyArtifacts(spec *v1.TaskSpec) *v1.TaskSpec { func getStepArtifactReplacements(step v1.Step, idx int) map[string]string { stringReplacements := map[string]string{} stepName := pod.StepName(step.Name, idx) - stringReplacements["step.artifacts.path"] = filepath.Join(pipeline.StepsDir, stepName, "artifacts", "provenance.json") + stringReplacements[pod.StepArtifactPathPattern] = filepath.Join(pipeline.StepsDir, stepName, "artifacts", "provenance.json") return stringReplacements } diff --git a/pkg/result/result.go b/pkg/result/result.go index e3d66b596ff..ab52c6c8275 100644 --- a/pkg/result/result.go +++ b/pkg/result/result.go @@ -36,8 +36,8 @@ const ( // StepResultType default step result value StepResultType ResultType = 4 - // ArtifactsResultType default artifacts result value - ArtifactsResultType ResultType = 5 + // StepArtifactsResultType default artifacts result value + StepArtifactsResultType ResultType = 5 ) // RunResult is used to write key/value pairs to TaskRun pod termination messages. @@ -91,8 +91,8 @@ func (r *ResultType) UnmarshalJSON(data []byte) error { *r = TaskRunResultType case "InternalTektonResult": *r = InternalTektonResultType - case "ArtifactsResult": - *r = ArtifactsResultType + case "StepArtifactsResult": + *r = StepArtifactsResultType default: *r = UnknownResultType } diff --git a/pkg/result/result_test.go b/pkg/result/result_test.go index 5b96acb2041..4ab14598f35 100644 --- a/pkg/result/result_test.go +++ b/pkg/result/result_test.go @@ -44,8 +44,8 @@ func TestRunResult_UnmarshalJSON(t *testing.T) { pr: RunResult{Key: "resultName", Value: "", ResultType: InternalTektonResultType}, }, { name: "type defined as string - ArtifactsResult", - data: "{\"key\":\"resultName\",\"value\":\"\", \"type\": \"ArtifactsResult\"}", - pr: RunResult{Key: "resultName", Value: "", ResultType: ArtifactsResultType}, + data: "{\"key\":\"resultName\",\"value\":\"\", \"type\": \"StepArtifactsResult\"}", + pr: RunResult{Key: "resultName", Value: "", ResultType: StepArtifactsResultType}, }, { name: "type defined as int", data: "{\"key\":\"resultName\",\"value\":\"\", \"type\": 1}", diff --git a/test/artifacts_test.go b/test/artifacts_test.go index 39ca286b3aa..5680594c34f 100644 --- a/test/artifacts_test.go +++ b/test/artifacts_test.go @@ -40,43 +40,57 @@ var ( } ) -func TestSurfaceArtifactsThroughTerminationMessage(t *testing.T) { - featureFlags := getFeatureFlagsBaseOnAPIFlag(t) - checkFlagsEnabled := requireAllGates(requireEnableStepArtifactsGate) +func TestSurfaceArtifacts(t *testing.T) { + tests := []struct { + desc string + resultExtractionMethod string + }{ + { + desc: "surface artifacts through termination message", + resultExtractionMethod: config.ResultExtractionMethodTerminationMessage}, + { + desc: "surface artifacts through sidecar logs", + resultExtractionMethod: config.ResultExtractionMethodSidecarLogs}, + } - ctx := context.Background() - ctx, cancel := context.WithCancel(ctx) - defer cancel() - c, namespace := setup(ctx, t) - checkFlagsEnabled(ctx, t, c, "") - previous := featureFlags.ResultExtractionMethod - updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ - "results-from": config.ResultExtractionMethodTerminationMessage, - }) + for _, tc := range tests { + t.Run(tc.desc, func(t *testing.T) { + featureFlags := getFeatureFlagsBaseOnAPIFlag(t) + checkFlagsEnabled := requireAllGates(requireEnableStepArtifactsGate) - knativetest.CleanupOnInterrupt(func() { - updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ - "results-from": previous, - }) - tearDown(ctx, t, c, namespace) - }, t.Logf) - defer func() { - updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ - "results-from": previous, - }) - tearDown(ctx, t, c, namespace) - }() + ctx := context.Background() + ctx, cancel := context.WithCancel(ctx) + defer cancel() + c, namespace := setup(ctx, t) + checkFlagsEnabled(ctx, t, c, "") + previous := featureFlags.ResultExtractionMethod + updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ + "results-from": tc.resultExtractionMethod, + }) - taskRunName := helpers.ObjectNameForTest(t) + knativetest.CleanupOnInterrupt(func() { + updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ + "results-from": previous, + }) + tearDown(ctx, t, c, namespace) + }, t.Logf) + defer func() { + updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ + "results-from": previous, + }) + tearDown(ctx, t, c, namespace) + }() - fqImageName := getTestImage(busyboxImage) + taskRunName := helpers.ObjectNameForTest(t) - t.Logf("Creating Task and TaskRun in namespace %s", namespace) - task := simpleArtifactProducerTask(t, namespace, fqImageName) - if _, err := c.V1TaskClient.Create(ctx, task, metav1.CreateOptions{}); err != nil { - t.Fatalf("Failed to create Task: %s", err) - } - taskRun := parse.MustParseV1TaskRun(t, fmt.Sprintf(` + fqImageName := getTestImage(busyboxImage) + + t.Logf("Creating Task and TaskRun in namespace %s", namespace) + task := simpleArtifactProducerTask(t, namespace, fqImageName) + if _, err := c.V1TaskClient.Create(ctx, task, metav1.CreateOptions{}); err != nil { + t.Fatalf("Failed to create Task: %s", err) + } + taskRun := parse.MustParseV1TaskRun(t, fmt.Sprintf(` metadata: name: %s namespace: %s @@ -84,31 +98,33 @@ spec: taskRef: name: %s `, taskRunName, namespace, task.Name)) - if _, err := c.V1TaskRunClient.Create(ctx, taskRun, metav1.CreateOptions{}); err != nil { - t.Fatalf("Failed to create TaskRun: %s", err) - } + if _, err := c.V1TaskRunClient.Create(ctx, taskRun, metav1.CreateOptions{}); err != nil { + t.Fatalf("Failed to create TaskRun: %s", err) + } - if err := WaitForTaskRunState(ctx, c, taskRunName, TaskRunSucceed(taskRunName), "TaskRunSucceed", v1Version); err != nil { - t.Errorf("Error waiting for TaskRun to finish: %s", err) - } + if err := WaitForTaskRunState(ctx, c, taskRunName, TaskRunSucceed(taskRunName), "TaskRunSucceed", v1Version); err != nil { + t.Errorf("Error waiting for TaskRun to finish: %s", err) + } - taskrun, err := c.V1TaskRunClient.Get(ctx, taskRunName, metav1.GetOptions{}) - if err != nil { - t.Fatalf("Couldn't get expected TaskRun %s: %s", taskRunName, err) - } - if d := cmp.Diff([]v1.TaskRunStepArtifact{{Name: "input-artifacts", - Values: []v1.ArtifactValue{{Digest: map[v1.Algorithm]string{"sha256": "b35cacccfdb1e24dc497d15d553891345fd155713ffe647c281c583269eaaae0"}, - Uri: "git:jjjsss", - }}, - }}, taskrun.Status.Steps[0].Inputs); d != "" { - t.Fatalf(`The expected stepState Inputs does not match created taskrun stepState Inputs. Here is the diff: %v`, d) - } - if d := cmp.Diff([]v1.TaskRunStepArtifact{{Name: "build-result", - Values: []v1.ArtifactValue{{Digest: map[v1.Algorithm]string{"sha1": "95588b8f34c31eb7d62c92aaa4e6506639b06ef2", "sha256": "df85b9e3983fe2ce20ef76ad675ecf435cc99fc9350adc54fa230bae8c32ce48"}, - Uri: "pkg:balba", - }}, - }}, taskrun.Status.Steps[0].Outputs); d != "" { - t.Fatalf(`The expected stepState Outputs does not match created taskrun stepState Outputs. Here is the diff: %v`, d) + taskrun, err := c.V1TaskRunClient.Get(ctx, taskRunName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Couldn't get expected TaskRun %s: %s", taskRunName, err) + } + if d := cmp.Diff([]v1.TaskRunStepArtifact{{Name: "source", + Values: []v1.ArtifactValue{{Digest: map[v1.Algorithm]string{"sha256": "b35cacccfdb1e24dc497d15d553891345fd155713ffe647c281c583269eaaae0"}, + Uri: "git:jjjsss", + }}, + }}, taskrun.Status.Steps[0].Inputs); d != "" { + t.Fatalf(`The expected stepState Inputs does not match created taskrun stepState Inputs. Here is the diff: %v`, d) + } + if d := cmp.Diff([]v1.TaskRunStepArtifact{{Name: "image", + Values: []v1.ArtifactValue{{Digest: map[v1.Algorithm]string{"sha1": "95588b8f34c31eb7d62c92aaa4e6506639b06ef2", "sha256": "df85b9e3983fe2ce20ef76ad675ecf435cc99fc9350adc54fa230bae8c32ce48"}, + Uri: "pkg:balba", + }}, + }}, taskrun.Status.Steps[0].Outputs); d != "" { + t.Fatalf(`The expected stepState Outputs does not match created taskrun stepState Outputs. Here is the diff: %v`, d) + } + }) } } @@ -185,53 +201,67 @@ spec: } func TestConsumeArtifacts(t *testing.T) { - featureFlags := getFeatureFlagsBaseOnAPIFlag(t) - checkFlagsEnabled := requireAllGates(map[string]string{ - "enable-artifacts": "true", - "enable-step-actions": "true", - }) + tests := []struct { + desc string + resultExtractionMethod string + }{ + { + desc: "surface artifacts through termination message", + resultExtractionMethod: config.ResultExtractionMethodTerminationMessage}, + { + desc: "surface artifacts through sidecar logs", + resultExtractionMethod: config.ResultExtractionMethodSidecarLogs}, + } - ctx := context.Background() - ctx, cancel := context.WithCancel(ctx) - defer cancel() - c, namespace := setup(ctx, t) - checkFlagsEnabled(ctx, t, c, "") - previous := featureFlags.ResultExtractionMethod - updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ - "results-from": config.ResultExtractionMethodTerminationMessage, - }) + for _, tc := range tests { + t.Run(tc.desc, func(t *testing.T) { + featureFlags := getFeatureFlagsBaseOnAPIFlag(t) + checkFlagsEnabled := requireAllGates(map[string]string{ + "enable-artifacts": "true", + "enable-step-actions": "true", + }) - knativetest.CleanupOnInterrupt(func() { - updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ - "results-from": previous, - }) - tearDown(ctx, t, c, namespace) - }, t.Logf) + ctx := context.Background() + ctx, cancel := context.WithCancel(ctx) + defer cancel() + c, namespace := setup(ctx, t) + checkFlagsEnabled(ctx, t, c, "") + previous := featureFlags.ResultExtractionMethod + updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ + "results-from": tc.resultExtractionMethod, + }) - defer func() { - updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ - "results-from": previous, - }) - tearDown(ctx, t, c, namespace) - }() - taskRunName := helpers.ObjectNameForTest(t) + knativetest.CleanupOnInterrupt(func() { + updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ + "results-from": previous, + }) + tearDown(ctx, t, c, namespace) + }, t.Logf) - fqImageName := getTestImage(busyboxImage) + defer func() { + updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ + "results-from": previous, + }) + tearDown(ctx, t, c, namespace) + }() + taskRunName := helpers.ObjectNameForTest(t) - t.Logf("Creating Task and TaskRun in namespace %s", namespace) - task := simpleArtifactProducerTask(t, namespace, fqImageName) - task.Spec.Steps = append(task.Spec.Steps, - v1.Step{Name: "consume-outputs", Image: fqImageName, - Command: []string{"sh", "-c", "echo -n $(steps.hello.outputs) >> $(step.results.result1.path)"}, - Results: []v1.StepResult{{Name: "result1", Type: v1.ResultsTypeString}}}, - v1.Step{Name: "consume-inputs", Image: fqImageName, - Command: []string{"sh", "-c", "echo -n $(steps.hello.inputs) >> $(step.results.result2.path)"}, - Results: []v1.StepResult{{Name: "result2", Type: v1.ResultsTypeString}}}, - ) - if _, err := c.V1TaskClient.Create(ctx, task, metav1.CreateOptions{}); err != nil { - t.Fatalf("Failed to create Task: %s", err) - } - taskRun := parse.MustParseV1TaskRun(t, fmt.Sprintf(` + fqImageName := getTestImage(busyboxImage) + + t.Logf("Creating Task and TaskRun in namespace %s", namespace) + task := simpleArtifactProducerTask(t, namespace, fqImageName) + task.Spec.Steps = append(task.Spec.Steps, + v1.Step{Name: "consume-outputs", Image: fqImageName, + Command: []string{"sh", "-c", "echo -n $(steps.hello.outputs) >> $(step.results.result1.path)"}, + Results: []v1.StepResult{{Name: "result1", Type: v1.ResultsTypeString}}}, + v1.Step{Name: "consume-inputs", Image: fqImageName, + Command: []string{"sh", "-c", "echo -n $(steps.hello.inputs) >> $(step.results.result2.path)"}, + Results: []v1.StepResult{{Name: "result2", Type: v1.ResultsTypeString}}}, + ) + if _, err := c.V1TaskClient.Create(ctx, task, metav1.CreateOptions{}); err != nil { + t.Fatalf("Failed to create Task: %s", err) + } + taskRun := parse.MustParseV1TaskRun(t, fmt.Sprintf(` metadata: name: %s namespace: %s @@ -239,27 +269,127 @@ spec: taskRef: name: %s `, taskRunName, namespace, task.Name)) - if _, err := c.V1TaskRunClient.Create(ctx, taskRun, metav1.CreateOptions{}); err != nil { - t.Fatalf("Failed to create TaskRun: %s", err) - } + if _, err := c.V1TaskRunClient.Create(ctx, taskRun, metav1.CreateOptions{}); err != nil { + t.Fatalf("Failed to create TaskRun: %s", err) + } - if err := WaitForTaskRunState(ctx, c, taskRunName, TaskRunSucceed(taskRunName), "TaskRunSucceed", v1Version); err != nil { - t.Errorf("Error waiting for TaskRun to finish: %s", err) - } + if err := WaitForTaskRunState(ctx, c, taskRunName, TaskRunSucceed(taskRunName), "TaskRunSucceed", v1Version); err != nil { + t.Errorf("Error waiting for TaskRun to finish: %s", err) + } - taskrun, err := c.V1TaskRunClient.Get(ctx, taskRunName, metav1.GetOptions{}) - if err != nil { - t.Fatalf("Couldn't get expected TaskRun %s: %s", taskRunName, err) + taskrun, err := c.V1TaskRunClient.Get(ctx, taskRunName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Couldn't get expected TaskRun %s: %s", taskRunName, err) + } + wantOut := `[{digest:{sha1:95588b8f34c31eb7d62c92aaa4e6506639b06ef2,sha256:df85b9e3983fe2ce20ef76ad675ecf435cc99fc9350adc54fa230bae8c32ce48},uri:pkg:balba}]` + gotOut := taskrun.Status.Steps[1].Results[0].Value.StringVal + if d := cmp.Diff(wantOut, gotOut); d != "" { + t.Fatalf(`The expected artifact outputs consumption result doesnot match expected. Here is the diff: %v`, d) + } + wantIn := `[{digest:{sha256:b35cacccfdb1e24dc497d15d553891345fd155713ffe647c281c583269eaaae0},uri:git:jjjsss}]` + gotIn := taskrun.Status.Steps[2].Results[0].Value.StringVal + if d := cmp.Diff(wantIn, gotIn); d != "" { + t.Fatalf(`The expected artifact Inputs consumption result doesnot match expected. Here is the diff: %v`, d) + } + }) } - wantOut := `[{digest:{sha1:95588b8f34c31eb7d62c92aaa4e6506639b06ef2,sha256:df85b9e3983fe2ce20ef76ad675ecf435cc99fc9350adc54fa230bae8c32ce48},uri:pkg:balba}]` - gotOut := taskrun.Status.Steps[1].Results[0].Value.StringVal - if d := cmp.Diff(wantOut, gotOut); d != "" { - t.Fatalf(`The expected artifact outputs consumption result doesnot match expected. Here is the diff: %v`, d) +} + +func TestStepProduceResultsAndArtifacts(t *testing.T) { + tests := []struct { + desc string + resultExtractionMethod string + }{ + { + desc: "surface artifacts through termination message", + resultExtractionMethod: config.ResultExtractionMethodTerminationMessage}, + { + desc: "surface artifacts through sidecar logs", + resultExtractionMethod: config.ResultExtractionMethodSidecarLogs}, } - wantIn := `[{digest:{sha256:b35cacccfdb1e24dc497d15d553891345fd155713ffe647c281c583269eaaae0},uri:git:jjjsss}]` - gotIn := taskrun.Status.Steps[2].Results[0].Value.StringVal - if d := cmp.Diff(wantIn, gotIn); d != "" { - t.Fatalf(`The expected artifact Inputs consumption result doesnot match expected. Here is the diff: %v`, d) + + for _, tc := range tests { + t.Run(tc.desc, func(t *testing.T) { + featureFlags := getFeatureFlagsBaseOnAPIFlag(t) + checkFlagsEnabled := requireAllGates(map[string]string{ + "enable-artifacts": "true", + "enable-step-actions": "true", + }) + + ctx := context.Background() + ctx, cancel := context.WithCancel(ctx) + defer cancel() + c, namespace := setup(ctx, t) + checkFlagsEnabled(ctx, t, c, "") + previous := featureFlags.ResultExtractionMethod + updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ + "results-from": tc.resultExtractionMethod, + }) + + knativetest.CleanupOnInterrupt(func() { + updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ + "results-from": previous, + }) + tearDown(ctx, t, c, namespace) + }, t.Logf) + + defer func() { + updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ + "results-from": previous, + }) + tearDown(ctx, t, c, namespace) + }() + taskRunName := helpers.ObjectNameForTest(t) + + fqImageName := getTestImage(busyboxImage) + + t.Logf("Creating Task and TaskRun in namespace %s", namespace) + task := produceResultsAndArtifactsTask(t, namespace, fqImageName) + + if _, err := c.V1TaskClient.Create(ctx, task, metav1.CreateOptions{}); err != nil { + t.Fatalf("Failed to create Task: %s", err) + } + taskRun := parse.MustParseV1TaskRun(t, fmt.Sprintf(` +metadata: + name: %s + namespace: %s +spec: + taskRef: + name: %s +`, taskRunName, namespace, task.Name)) + if _, err := c.V1TaskRunClient.Create(ctx, taskRun, metav1.CreateOptions{}); err != nil { + t.Fatalf("Failed to create TaskRun: %s", err) + } + + if err := WaitForTaskRunState(ctx, c, taskRunName, TaskRunSucceed(taskRunName), "TaskRunSucceed", v1Version); err != nil { + t.Errorf("Error waiting for TaskRun to finish: %s", err) + } + + taskrun, err := c.V1TaskRunClient.Get(ctx, taskRunName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Couldn't get expected TaskRun %s: %s", taskRunName, err) + } + if d := cmp.Diff([]v1.TaskRunStepArtifact{{Name: "source", + Values: []v1.ArtifactValue{{Digest: map[v1.Algorithm]string{"sha256": "b35cacccfdb1e24dc497d15d553891345fd155713ffe647c281c583269eaaae0"}, + Uri: "git:jjjsss", + }}, + }}, taskrun.Status.Steps[0].Inputs); d != "" { + t.Fatalf(`The expected stepState Inputs does not match created taskrun stepState Inputs. Here is the diff: %v`, d) + } + if d := cmp.Diff([]v1.TaskRunStepArtifact{{Name: "image", + Values: []v1.ArtifactValue{{Digest: map[v1.Algorithm]string{"sha1": "95588b8f34c31eb7d62c92aaa4e6506639b06ef2", "sha256": "df85b9e3983fe2ce20ef76ad675ecf435cc99fc9350adc54fa230bae8c32ce48"}, + Uri: "pkg:balba", + }}, + }}, taskrun.Status.Steps[0].Outputs); d != "" { + t.Fatalf(`The expected stepState Outputs does not match created taskrun stepState Outputs. Here is the diff: %v`, d) + } + + wantResult := `result1Value` + gotResult := taskrun.Status.Steps[0].Results[0].Value.StringVal + if d := cmp.Diff(wantResult, gotResult); d != "" { + t.Fatalf(`The expected artifact outputs consumption result doesnot match expected. Here is the diff: %v`, d) + } + }) } } @@ -281,7 +411,7 @@ spec: { "inputs":[ { - "name":"input-artifacts", + "name":"source", "values":[ { "uri":"git:jjjsss", @@ -294,7 +424,58 @@ spec: ], "outputs":[ { - "name":"build-result", + "name":"image", + "values":[ + { + "uri":"pkg:balba", + "digest":{ + "sha256":"df85b9e3983fe2ce20ef76ad675ecf435cc99fc9350adc54fa230bae8c32ce48", + "sha1":"95588b8f34c31eb7d62c92aaa4e6506639b06ef2" + } + } + ] + } + ] + } + EOF +`, helpers.ObjectNameForTest(t), namespace, fqImageName)) + return task +} + +func produceResultsAndArtifactsTask(t *testing.T, namespace string, fqImageName string) *v1.Task { + t.Helper() + task := parse.MustParseV1Task(t, fmt.Sprintf(` +metadata: + name: %s + namespace: %s +spec: + steps: + - name: hello + image: %s + command: ['/bin/sh'] + results: + - name: result1 + args: + - "-c" + - | + cat > $(step.artifacts.path) << EOF + { + "inputs":[ + { + "name":"source", + "values":[ + { + "uri":"git:jjjsss", + "digest":{ + "sha256":"b35cacccfdb1e24dc497d15d553891345fd155713ffe647c281c583269eaaae0" + } + } + ] + } + ], + "outputs":[ + { + "name":"image", "values":[ { "uri":"pkg:balba", @@ -308,6 +489,7 @@ spec: ] } EOF + echo -n result1Value >> $(step.results.result1.path) `, helpers.ObjectNameForTest(t), namespace, fqImageName)) return task }