Merge pull request #154 from adambkaplan/build-mount-cas

Bug 1895053: Instruct builds to optionally mount trusted CAs

go.mod

@@ -16,7 +16,7 @@ require (
 	github.com/grpc-ecosystem/go-grpc-middleware v1.1.0 // indirect
 	github.com/hashicorp/golang-lru v0.5.1
 	github.com/mtrmac/gpgme v0.1.2 // indirect
-	github.com/openshift/api v0.0.0-20201209094822-71d5d69227c6
+	github.com/openshift/api v0.0.0-20210222200543-70b287c0a0a5
 	github.com/openshift/build-machinery-go v0.0.0-20200917070002-f171684f77ab
 	github.com/openshift/client-go v0.0.0-20200722173614-5a1b0aaeff15
 	github.com/openshift/library-go v0.0.0-20200731134909-dbf343342338

go.sum

@@ -507,6 +507,8 @@ github.com/openshift/api v0.0.0-20200723134351-89de68875e7c h1:qsj/GaQ1sdT584yIc
 github.com/openshift/api v0.0.0-20200723134351-89de68875e7c/go.mod h1:IXsT3F4NjLtRzfnQvwU+g/oPWpoNsVV5vd5aaOMO8eU=
 github.com/openshift/api v0.0.0-20201209094822-71d5d69227c6 h1:6ItRryeAYG5U9KL9HtotKXrHoboiKoz1IgKAWrPKttE=
 github.com/openshift/api v0.0.0-20201209094822-71d5d69227c6/go.mod h1:aqU5Cq+kqKKPbDMqxo9FojgDeSpNJI7iuskjXjtojDg=
+github.com/openshift/api v0.0.0-20210222200543-70b287c0a0a5 h1:PwdrcuVguOBKjA6ZFHKzL+DnvSeF6cXkliyEJKBm7D0=
+github.com/openshift/api v0.0.0-20210222200543-70b287c0a0a5/go.mod h1:aqU5Cq+kqKKPbDMqxo9FojgDeSpNJI7iuskjXjtojDg=
 github.com/openshift/build-machinery-go v0.0.0-20200211121458-5e3d6e570160/go.mod h1:1CkcsT3aVebzRBzVTSbiKSkJMsC/CASqxesfqEMfJEc=
 github.com/openshift/build-machinery-go v0.0.0-20200713135615-1f43d26dccc7 h1:iP7TOaN+tEVNUQ0ODEbN1ukjLz918lsIt7Czf8giWlM=
 github.com/openshift/build-machinery-go v0.0.0-20200713135615-1f43d26dccc7/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE=

pkg/build/controller/build/build_controller.go

@@ -1943,6 +1943,7 @@ func (bc *BuildController) createBuildSystemConfigMapSpec(build *buildv1.Build,
 		},
 		Data: make(map[string]string),
 	}
+
 	registryConf := bc.registryConfTOML()
 	if len(registryConf) > 0 {
 		cm.Data[buildv1.RegistryConfKey] = registryConf

pkg/build/controller/build/build_controller_test.go

@@ -74,6 +74,11 @@ const (
 	block_registries:
 	- all
 	`
+
+	defaultMountsConf = "/run/secrets:/run/secrets\n"
+	proxyMountsConf   = `/run/secrets:/run/secrets
+/etc/pki/ca-trust:/etc/pki/ca-trust
+`
 )
 
 // registryCAConfigMap is created by the openshift-controller-manager-operator, serving as a placeholder
@@ -1936,27 +1941,59 @@ func (e *errorConfigMapLister) ConfigMaps(namespace string) v1lister.ConfigMapNa
 	return e
 }
 
-func TestCreateBuildRegistryConfConfigMap(t *testing.T) {
-	bc := newFakeBuildController(nil, nil, nil, nil, nil)
-	bc.setRegistryConfTOML(dummyRegistryConf)
-	defer bc.stop()
-	build := dockerStrategy(mockBuild(buildv1.BuildPhaseNew, buildv1.BuildOutput{}))
-	pod := mockBuildPod(build)
-	caMap := bc.createBuildSystemConfigMapSpec(build, pod)
-	if caMap == nil {
-		t.Error("build system config configMap was not created")
-	}
-	if !hasBuildPodOwnerRef(pod, caMap) {
-		t.Error("build system config configMap is missing owner ref to the build pod")
-	}
-	if _, hasConf := caMap.Data[buildv1.RegistryConfKey]; !hasConf {
-		t.Errorf("expected build system config configMap to have key %s", buildv1.RegistryConfKey)
+func TestCreateBuildSystemConfigMapSpec(t *testing.T) {
+	cases := []struct {
+		name                string
+		includeRegistryConf bool
+		mountProxyCA        bool
+	}{
+		{
+			name: "default",
+		},
+		{
+			name:                "override registries.conf",
+			includeRegistryConf: true,
+		},
+		{
+			name:         "mountProxyCA",
+			mountProxyCA: true,
+		},
 	}
-	if caMap.Data[buildv1.RegistryConfKey] != dummyRegistryConf {
-		t.Errorf("expected build system config configMap.%s to contain\n%s\ngot:\n%s",
-			buildv1.RegistryConfKey,
-			dummyCA,
-			caMap.Data[buildv1.RegistryConfKey])
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			bc := newFakeBuildController(nil, nil, nil, nil, nil)
+			if tc.includeRegistryConf {
+				bc.setRegistryConfTOML(dummyRegistryConf)
+			}
+			defer bc.stop()
+			build := dockerStrategy(mockBuild(buildv1.BuildPhaseNew, buildv1.BuildOutput{}))
+			if tc.mountProxyCA {
+				build.Spec.MountTrustedCA = &tc.mountProxyCA
+			}
+			pod := mockBuildPod(build)
+			caMap := bc.createBuildSystemConfigMapSpec(build, pod)
+			if caMap == nil {
+				t.Error("build system config configMap was not created")
+			}
+			if !hasBuildPodOwnerRef(pod, caMap) {
+				t.Error("build system config configMap is missing owner ref to the build pod")
+			}
+
+			registriesConfData, hasConf := caMap.Data[buildv1.RegistryConfKey]
+			if tc.includeRegistryConf {
+				if !hasConf {
+					t.Errorf("expected build system config configMap to have key %s", buildv1.RegistryConfKey)
+				}
+				if registriesConfData != dummyRegistryConf {
+					t.Errorf("expected build system config configMap.%s to contain\n%s\ngot:\n%s",
+						buildv1.RegistryConfKey,
+						dummyCA,
+						caMap.Data[buildv1.RegistryConfKey])
+				}
+			} else if len(registriesConfData) > 0 {
+				t.Errorf("expected no content for registries.conf, got %s", registriesConfData)
+			}
+		})
 	}
 }
 

pkg/build/controller/strategy/custom.go

@@ -61,6 +61,7 @@ func (bs *CustomBuildStrategy) CreateBuildPod(build *buildv1.Build, additionalCA
 	if build.Spec.Source.Git != nil {
 		addSourceEnvVars(build.Spec.Source, &containerEnv)
 	}
+	addTrustedCAMountEnvVar(build.Spec.MountTrustedCA, &containerEnv)
 
 	if build.Spec.Output.To != nil {
 		addOutputEnvVars(build.Spec.Output.To, &containerEnv)

pkg/build/controller/strategy/custom_test.go

@@ -102,7 +102,15 @@ func TestCustomCreateBuildPod(t *testing.T) {
 		0: {"BUILD", string(buildJSON)},
 		1: {"LANG", "en_US.utf8"},
 	}
-	standardEnv := []string{"SOURCE_REPOSITORY", "SOURCE_URI", "SOURCE_CONTEXT_DIR", "SOURCE_REF", "OUTPUT_IMAGE", "OUTPUT_REGISTRY"}
+	standardEnv := []string{
+		"SOURCE_REPOSITORY",
+		"SOURCE_URI",
+		"SOURCE_CONTEXT_DIR",
+		"SOURCE_REF",
+		"OUTPUT_IMAGE",
+		"OUTPUT_REGISTRY",
+		"BUILD_MOUNT_ETC_PKI_CATRUST",
+	}
 	for index, exp := range errorCases {
 		if e := container.Env[index]; e.Name != exp[0] || e.Value != exp[1] {
 			t.Errorf("Expected %s:%s, got %s:%s!\n", exp[0], exp[1], e.Name, e.Value)
@@ -190,6 +198,7 @@ func TestCustomBuildLongName(t *testing.T) {
 
 func mockCustomBuild(forcePull, emptySource bool) *buildv1.Build {
 	timeout := int64(60)
+	mountCA := true
 	src := buildv1.BuildSource{}
 	if !emptySource {
 		src = buildv1.BuildSource{
@@ -250,6 +259,7 @@ func mockCustomBuild(forcePull, emptySource bool) *buildv1.Build {
 				},
 				CompletionDeadlineSeconds: &timeout,
 				NodeSelector:              nodeSelector,
+				MountTrustedCA:            &mountCA,
 			},
 		},
 		Status: buildv1.BuildStatus{

pkg/build/controller/strategy/docker.go

@@ -47,6 +47,7 @@ func (bs *DockerBuildStrategy) CreateBuildPod(build *buildv1.Build, additionalCA
 	}
 
 	addSourceEnvVars(build.Spec.Source, &containerEnv)
+	addTrustedCAMountEnvVar(build.Spec.MountTrustedCA, &containerEnv)
 
 	if len(strategy.Env) > 0 {
 		buildutil.MergeTrustedEnvWithoutDuplicates(strategy.Env, &containerEnv, true)

pkg/build/controller/strategy/docker_test.go

@@ -67,6 +67,7 @@ func TestDockerCreateBuildPod(t *testing.T) {
 		"BUILD_STORAGE_CONF_PATH":     "",
 		"BUILD_STORAGE_DRIVER":        "",
 		"BUILD_BLOBCACHE_DIR":         "",
+		"BUILD_MOUNT_ETC_PKI_CATRUST": "",
 	}
 	gotKeys := map[string]string{}
 	for _, k := range container.Env {
@@ -168,6 +169,7 @@ func TestDockerBuildLongName(t *testing.T) {
 
 func mockDockerBuild() *buildv1.Build {
 	timeout := int64(60)
+	mountCA := true
 	return &buildv1.Build{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "dockerBuild",
@@ -228,6 +230,7 @@ func mockDockerBuild() *buildv1.Build {
 				},
 				CompletionDeadlineSeconds: &timeout,
 				NodeSelector:              nodeSelector,
+				MountTrustedCA:            &mountCA,
 			},
 		},
 		Status: buildv1.BuildStatus{

pkg/build/controller/strategy/sti.go

@@ -44,6 +44,7 @@ func (bs *SourceBuildStrategy) CreateBuildPod(build *buildv1.Build, additionalCA
 	}
 
 	addSourceEnvVars(build.Spec.Source, &containerEnv)
+	addTrustedCAMountEnvVar(build.Spec.MountTrustedCA, &containerEnv)
 
 	strategy := build.Spec.Strategy.SourceStrategy
 	if len(strategy.Env) > 0 {

pkg/build/controller/strategy/sti_test.go

@@ -103,6 +103,7 @@ func testSTICreateBuildPod(t *testing.T, rootAllowed bool) {
 		"BUILD_STORAGE_CONF_PATH":     "",
 		"BUILD_STORAGE_DRIVER":        "",
 		"BUILD_BLOBCACHE_DIR":         "",
+		"BUILD_MOUNT_ETC_PKI_CATRUST": "",
 	}
 	if !rootAllowed {
 		expectedKeys["ALLOWED_UIDS"] = ""
@@ -228,6 +229,7 @@ func TestS2IBuildLongName(t *testing.T) {
 
 func mockSTIBuild() *buildv1.Build {
 	timeout := int64(60)
+	mountCA := true
 	return &buildv1.Build{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "stiBuild",
@@ -293,6 +295,7 @@ func mockSTIBuild() *buildv1.Build {
 				},
 				CompletionDeadlineSeconds: &timeout,
 				NodeSelector:              nodeSelector,
+				MountTrustedCA:            &mountCA,
 			},
 		},
 		Status: buildv1.BuildStatus{

pkg/build/controller/strategy/util.go

@@ -289,6 +289,14 @@ func addOutputEnvVars(buildOutput *corev1.ObjectReference, output *[]corev1.EnvV
 	return nil
 }
 
+// addTrustedCAMountEnvVar sets the BUILD_MOUNT_ETC_PKI_CATRUST environment variable if the build
+// pod needs the CA trust bundle (`/etc/pki/ca-trust`) mounted into build processes.
+func addTrustedCAMountEnvVar(mountTrustedCA *bool, envVars *[]corev1.EnvVar) {
+	if mountTrustedCA != nil {
+		*envVars = append(*envVars, corev1.EnvVar{Name: "BUILD_MOUNT_ETC_PKI_CATRUST", Value: strconv.FormatBool(*mountTrustedCA)})
+	}
+}
+
 // setupActiveDeadline sets up the Pod activeDeadlineSeconds field
 func setupActiveDeadline(pod *corev1.Pod, build *buildv1.Build) *corev1.Pod {
 	if build.Spec.CompletionDeadlineSeconds != nil {
@@ -356,7 +364,8 @@ func copyEnvVarSlice(in []corev1.EnvVar) []corev1.EnvVar {
 }
 
 // setupContainersConfigs sets up volumes for mounting the node's configuration which governs which
-// registries it knows about, whether or not they should be accessed with TLS, and signature policies.
+// registries it knows about, whether or not they should be accessed with TLS, signature policies,
+// and default bind mounts for buildah.
 func setupContainersConfigs(build *buildv1.Build, pod *corev1.Pod) {
 	const volumeName = "build-system-configs"
 	const configDir = ConfigMapBuildSystemConfigsMountPath