Refactor Resource result output, and add support for Git resources.

This change builds upon #1406, and logs the exact Git commit used by a Git input
to the ResourceResults field.

Some other cleanups are included:
- Removing custom TerminationMessagePath from the Image resource. The default is fine.
- Test cleanup.
- A new helper to write termination messages from resource containers.

And finally, this adds a new environment variable to the git resource steps, indicating the
name of the resource instance they are running as. We should make this more generic and apply
it to all resource steps as part of the extensiblity refactor.

cmd/git-init/main.go

@@ -18,14 +18,17 @@ package main
 import (
 	"flag"
 
+	v1alpha1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
 	"github.com/tektoncd/pipeline/pkg/git"
+	"github.com/tektoncd/pipeline/pkg/termination"
 	"knative.dev/pkg/logging"
 )
 
 var (
-	url      = flag.String("url", "", "The url of the Git repository to initialize.")
-	revision = flag.String("revision", "", "The Git revision to make the repository HEAD")
-	path     = flag.String("path", "", "Path of directory under which git repository will be copied")
+	url                    = flag.String("url", "", "The url of the Git repository to initialize.")
+	revision               = flag.String("revision", "", "The Git revision to make the repository HEAD")
+	path                   = flag.String("path", "", "Path of directory under which git repository will be copied")
+	terminationMessagePath = flag.String("terminationMessagePath", "/dev/termination-log", "Location of file containing termination message")
 )
 
 func main() {
@@ -36,4 +39,17 @@ func main() {
 	if err := git.Fetch(logger, *revision, *path, *url); err != nil {
 		logger.Fatalf("Error fetching git repository: %s", err)
 	}
+
+	commit, err := git.Commit(logger, *revision, *path)
+	if err != nil {
+		logger.Fatalf("Error parsing commit of git repository: %s", err)
+	}
+	output := []v1alpha1.PipelineResourceResult{
+		{
+			Key:   "commit",
+			Value: commit,
+		},
+	}
+
+	termination.WriteMessage(logger, *terminationMessagePath, output)
 }

cmd/imagedigestexporter/main.go

@@ -19,16 +19,17 @@ package main
 import (
 	"encoding/json"
 	"flag"
-	"log"
-	"os"
+
+	"github.com/tektoncd/pipeline/pkg/termination"
+	"knative.dev/pkg/logging"
 
 	"github.com/google/go-containerregistry/pkg/v1/layout"
 	v1alpha1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
 )
 
 var (
 	images                 = flag.String("images", "", "List of images resources built by task in json format")
-	terminationMessagePath = flag.String("terminationMessagePath", "", "Location of file containing termination message")
+	terminationMessagePath = flag.String("terminationMessagePath", "/dev/termination-log", "Location of file containing termination message")
 )
 
 /* The input of this go program will be a JSON string with all the output PipelineResources of type
@@ -40,10 +41,12 @@ The output is an array of PipelineResourceResult, ex: [{"name":"image","digest":
 */
 func main() {
 	flag.Parse()
+	logger, _ := logging.NewLogger("", "image-digest-exporter")
+	defer logger.Sync()
 
 	imageResources := []*v1alpha1.ImageResource{}
 	if err := json.Unmarshal([]byte(*images), &imageResources); err != nil {
-		log.Fatalf("Error reading images array: %v", err)
+		logger.Fatalf("Error reading images array: %v", err)
 	}
 
 	output := []v1alpha1.PipelineResourceResult{}
@@ -52,12 +55,12 @@ func main() {
 		if err != nil {
 			// if this image doesn't have a builder that supports index.json file,
 			// then it will be skipped
-			log.Printf("ImageResource %s doesn't have an index.json file: %s", imageResource.Name, err)
+			logger.Infof("ImageResource %s doesn't have an index.json file: %s", imageResource.Name, err)
 			continue
 		}
 		digest, err := GetDigest(ii)
 		if err != nil {
-			log.Fatalf("Unexpected error getting image digest for %s: %v", imageResource.Name, err)
+			logger.Fatalf("Unexpected error getting image digest for %s: %v", imageResource.Name, err)
 		}
 		// We need to write both the old Name/Digest style and the new Key/Value styles.
 		output = append(output, v1alpha1.PipelineResourceResult{
@@ -75,22 +78,5 @@ func main() {
 
 	}
 
-	imagesJSON, err := json.Marshal(output)
-	if err != nil {
-		log.Fatalf("Unexpected error converting images to json %v: %v", output, err)
-	}
-	log.Printf("Image digest exporter output: %s ", string(imagesJSON))
-	f, err := os.OpenFile(*terminationMessagePath, os.O_WRONLY|os.O_CREATE, 0666)
-	if err != nil {
-		log.Fatalf("Unexpected error converting images to json %v: %v", output, err)
-	}
-	defer f.Close()
-
-	_, err = f.Write(imagesJSON)
-	if err != nil {
-		log.Fatalf("Unexpected error converting images to json %v: %v", output, err)
-	}
-	if err := f.Sync(); err != nil {
-		log.Fatalf("Unexpected error converting images to json %v: %v", output, err)
-	}
+	termination.WriteMessage(logger, *terminationMessagePath, output)
 }

docs/resources.md

@@ -198,8 +198,7 @@ spec:
 
 When resources are bound inside a TaskRun, they can include extra information in the TaskRun Status.ResourcesResult field.
 This information can be useful for auditing the exact resources used by a TaskRun later.
-Currently the only resource that uses this mechanism is the Image resource, which includes the exact digest
-of an image built by a TaskRun and declared as an output.
+Currently the Image and Git resources use this mechanism.
 
 For an example of what this output looks like:
 
@@ -258,6 +257,17 @@ Params that can be added are the following:
     commit [or branch](#using-a-branch) is used. _If no revision is specified,
     the resource will default to `latest` from `master`._
 
+When used as an input, the Git resource includes the exact commit fetched in the `resourceResults`
+section of the `taskRun`'s status object:
+
+```yaml
+resourceResults:
+- key: commit
+  value: 6ed7aad5e8a36052ee5f6079fc91368e362121f7
+  resourceRef:
+    name: skaffold-git
+```
+
 #### Using a fork
 
 The `Url` parameter can be used to point at any git repository, for example to

pkg/apis/pipeline/v1alpha1/git_resource.go

@@ -109,6 +109,11 @@ func (s *GitResource) GetInputTaskModifier(_ *TaskSpec, path string) (TaskModifi
 			Command:    []string{"/ko-app/git-init"},
 			Args:       args,
 			WorkingDir: WorkspaceDir,
+			// This is used to populate the ResourceResult status.
+			Env: []corev1.EnvVar{{
+				Name:  "TEKTON_RESOURCE_NAME",
+				Value: s.Name,
+			}},
 		},
 	}
 	return &InternalTaskModifier{

pkg/apis/pipeline/v1alpha1/git_resource_test.go

@@ -132,6 +132,7 @@ func Test_GitResource_GetDownloadTaskModifier(t *testing.T) {
 			"/test/test",
 		},
 		WorkingDir: "/workspace",
+		Env:        []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "git-resource"}},
 	}}}
 
 	if diff := cmp.Diff(want, modifier.GetStepsToPrepend()); diff != "" {

pkg/git/git.go

@@ -26,16 +26,21 @@ import (
 	"golang.org/x/xerrors"
 )
 
-func run(logger *zap.SugaredLogger, args ...string) error {
+func run(logger *zap.SugaredLogger, dir string, args ...string) (string, error) {
 	c := exec.Command("git", args...)
 	var output bytes.Buffer
 	c.Stderr = &output
 	c.Stdout = &output
+	// This is the optional working directory. If not set, it defaults to the current
+	// working directory of the process.
+	if dir != "" {
+		c.Dir = dir
+	}
 	if err := c.Run(); err != nil {
 		logger.Errorf("Error running git %v: %v\n%v", args, err, output.String())
-		return err
+		return "", err
 	}
-	return nil
+	return output.String(), nil
 }
 
 // Fetch fetches the specified git repository at the revision into path.
@@ -70,31 +75,39 @@ func Fetch(logger *zap.SugaredLogger, revision, path, url string) error {
 		revision = "master"
 	}
 	if path != "" {
-		if err := run(logger, "init", path); err != nil {
+		if _, err := run(logger, "", "init", path); err != nil {
 			return err
 		}
 		if err := os.Chdir(path); err != nil {
 			return xerrors.Errorf("Failed to change directory with path %s; err: %w", path, err)
 		}
-	} else if err := run(logger, "init"); err != nil {
+	} else if _, err := run(logger, "", "init"); err != nil {
 		return err
 	}
 	trimmedURL := strings.TrimSpace(url)
-	if err := run(logger, "remote", "add", "origin", trimmedURL); err != nil {
+	if _, err := run(logger, "", "remote", "add", "origin", trimmedURL); err != nil {
 		return err
 	}
-	if err := run(logger, "fetch", "--depth=1", "--recurse-submodules=yes", "origin", revision); err != nil {
+	if _, err := run(logger, "", "fetch", "--depth=1", "--recurse-submodules=yes", "origin", revision); err != nil {
 		// Fetch can fail if an old commitid was used so try git pull, performing regardless of error
 		// as no guarantee that the same error is returned by all git servers gitlab, github etc...
-		if err := run(logger, "pull", "--recurse-submodules=yes", "origin"); err != nil {
+		if _, err := run(logger, "", "pull", "--recurse-submodules=yes", "origin"); err != nil {
 			logger.Warnf("Failed to pull origin : %s", err)
 		}
-		if err := run(logger, "checkout", revision); err != nil {
+		if _, err := run(logger, "", "checkout", revision); err != nil {
 			return err
 		}
-	} else if err := run(logger, "reset", "--hard", "FETCH_HEAD"); err != nil {
+	} else if _, err := run(logger, "", "reset", "--hard", "FETCH_HEAD"); err != nil {
 		return err
 	}
 	logger.Infof("Successfully cloned %s @ %s in path %s", trimmedURL, revision, path)
 	return nil
 }
+
+func Commit(logger *zap.SugaredLogger, revision, path string) (string, error) {
+	output, err := run(logger, path, "rev-parse", "HEAD")
+	if err != nil {
+		return "", err
+	}
+	return strings.TrimSuffix(output, "\n"), nil
+}

pkg/reconciler/taskrun/resources/image_exporter.go

@@ -25,7 +25,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 )
 
-const TerminationMessagePath = "/builder/home/image-outputs/termination-log"
+const imageDigestExporterContainerName = "image-digest-exporter"
 
 // AddOutputImageDigestExporter add a step to check the index.json for all output images
 func AddOutputImageDigestExporter(
@@ -82,40 +82,14 @@ func AddOutputImageDigestExporter(
 	return nil
 }
 
-// UpdateTaskRunStatusWithResourceResult if there is an update to the outout image resource, add to taskrun status result
-func UpdateTaskRunStatusWithResourceResult(taskRun *v1alpha1.TaskRun, logContent []byte) error {
-	if err := json.Unmarshal(logContent, &taskRun.Status.ResourcesResult); err != nil {
-		return xerrors.Errorf("Failed to unmarshal output image exporter JSON output: %w", err)
-	}
-	return nil
-}
-
 func imageDigestExporterStep(imageDigestExporterImage string, imagesJSON []byte) v1alpha1.Step {
 	return v1alpha1.Step{Container: corev1.Container{
-		Name:    names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("image-digest-exporter"),
+		Name:    names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(imageDigestExporterContainerName),
 		Image:   imageDigestExporterImage,
 		Command: []string{"/ko-app/imagedigestexporter"},
 		Args: []string{
 			"-images", string(imagesJSON),
-			"-terminationMessagePath", TerminationMessagePath,
 		},
-		TerminationMessagePath:   TerminationMessagePath,
 		TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError,
 	}}
 }
-
-// TaskRunHasOutputImageResource return true if the task has any output resources of type image
-func TaskRunHasOutputImageResource(gr GetResource, taskRun *v1alpha1.TaskRun) bool {
-	if len(taskRun.Spec.Outputs.Resources) > 0 {
-		for _, r := range taskRun.Spec.Outputs.Resources {
-			resource, err := gr(r.ResourceRef.Name)
-			if err != nil {
-				return false
-			}
-			if resource.Spec.Type == v1alpha1.PipelineResourceTypeImage {
-				return true
-			}
-		}
-	}
-	return false
-}