From 7790a33cdbf4b67cd50604e07e1be0a1a151680b Mon Sep 17 00:00:00 2001
From: Aaron Crickenberger <spiffxp@google.com>
Date: Fri, 16 Aug 2019 18:07:15 -0700
Subject: [PATCH] document default prowjob preset behavior

Specifically

- added an example to the prowjob docs
- linked to the way we use it for GOPROXY in k8s-specific job docs
- added unit tests for presets
- since we use it for env vars, added a unit test to confirm env vars
  will conflict
---
 config/jobs/README.md    |  3 +++
 prow/config/jobs_test.go | 52 ++++++++++++++++++++++++++++++++++++++++
 prow/jobs.md             |  7 ++++++
 3 files changed, 62 insertions(+)

diff --git a/config/jobs/README.md b/config/jobs/README.md
index ccf84139b7626..2114e88829939 100644
--- a/config/jobs/README.md
+++ b/config/jobs/README.md
@@ -63,6 +63,8 @@ files here. eg:
   kubernetes e2e tests less susceptible to flakes
 - [`preset-aws-credentials: "true"`] ensures the prowjob has AWS credentials
   for kops tests in a well known location, with an env var pointint to it
+- [the default preset with no labels] is used to set the `GOPROXY` env var
+  for all jobs by default
 
 ## Job Examples
 
@@ -227,3 +229,4 @@ bazel run //experiment/config-forker -- \
 [`preset-service-account: "true"`]: https://github.com/kubernetes/test-infra/blob/f4e6553b27d9ee8b35b2f2e588ea2e18c3fa818b/prow/config.yaml#L467-L483
 [`preset-pull-kubernetes-e2e: "true"`]: https://github.com/kubernetes/test-infra/blob/f4e6553b27d9ee8b35b2f2e588ea2e18c3fa818b/config/jobs/kubernetes/sig-gcp/sig-gcp-gce-config.yaml#L2-L8
 [`preset-aws-credentials: "true"`]: https://github.com/kubernetes/test-infra/blob/f4e6553b27d9ee8b35b2f2e588ea2e18c3fa818b/config/jobs/kubernetes/sig-aws/sig-aws-credentials.yaml#L3-L15
+[the default preset with no labels]: https://github.com/kubernetes/test-infra/blob/551edb4702e262989fe5d162a2c642c3201bf68e/prow/config.yaml#L630
diff --git a/prow/config/jobs_test.go b/prow/config/jobs_test.go
index 025e783dbaa6c..904deb626594b 100644
--- a/prow/config/jobs_test.go
+++ b/prow/config/jobs_test.go
@@ -640,6 +640,19 @@ func TestMergePreset(t *testing.T) {
 			},
 			numEnv: 1,
 		},
+		{
+			name:      "conflicting env",
+			jobLabels: map[string]string{"foo": "bar"},
+			pod:       &coreapi.PodSpec{Containers: []coreapi.Container{coreapi.Container{Env: []coreapi.EnvVar{{Name: "baz"}}}}},
+			buildSpec: &buildapi.BuildSpec{Template: &buildapi.TemplateInstantiationSpec{Env: []coreapi.EnvVar{{Name: "baz"}}}},
+			presets: []Preset{
+				{
+					Labels: map[string]string{"foo": "bar"},
+					Env:    []coreapi.EnvVar{{Name: "baz"}},
+				},
+			},
+			shouldError: true,
+		},
 		{
 			name:      "one vm",
 			jobLabels: map[string]string{"foo": "bar"},
@@ -683,6 +696,45 @@ func TestMergePreset(t *testing.T) {
 			},
 			numVolMounts: 2,
 		},
+		{
+			name:      "default preset only",
+			jobLabels: map[string]string{"foo": "bar"},
+			pod:       &coreapi.PodSpec{Containers: []coreapi.Container{{}}},
+			buildSpec: &buildapi.BuildSpec{},
+			presets: []Preset{
+				{
+					Env: []coreapi.EnvVar{{Name: "baz"}},
+				},
+			},
+			numEnv: 1,
+		},
+		{
+			name:      "default preset and non-matching preset",
+			jobLabels: map[string]string{"foo": "bar"},
+			pod:       &coreapi.PodSpec{Containers: []coreapi.Container{{}}},
+			buildSpec: &buildapi.BuildSpec{},
+			presets: []Preset{
+				{
+					Env: []coreapi.EnvVar{{Name: "baz"}},
+				},
+				{
+					Labels:  map[string]string{"no": "match"},
+					Volumes: []coreapi.Volume{{Name: "qux"}},
+				},
+			},
+			numEnv: 1,
+		},
+		{
+			name:      "default presets can also conflict",
+			jobLabels: map[string]string{"foo": "bar"},
+			pod:       &coreapi.PodSpec{Volumes: []coreapi.Volume{{Name: "baz"}}},
+			presets: []Preset{
+				{
+					Volumes: []coreapi.Volume{{Name: "baz"}},
+				},
+			},
+			shouldError: true,
+		},
 	}
 	for _, tc := range tcs {
 		t.Run(tc.name, func(t *testing.T) {
diff --git a/prow/jobs.md b/prow/jobs.md
index 9dbf65e358d62..aef9f9d6f6477 100644
--- a/prow/jobs.md
+++ b/prow/jobs.md
@@ -99,6 +99,13 @@ presets:
   - name: bar
     mountPath: /etc/bar
     readOnly: true
+- env:                     # a preset with no labels is applied to all jobs
+  - name: BAZ
+    value: qux
+  volumes:
+    # etc...
+  volumeMounts:
+    # etc...
 ```
 
 ## Standard Triggering and Execution Behavior for Jobs