From 726d5690660232f4500dbe82cb67552f9e62bd81 Mon Sep 17 00:00:00 2001 From: Matthew McNew Date: Wed, 20 Oct 2021 16:10:27 -0500 Subject: [PATCH 1/5] Consolidate arguments to BuildPod into singular BuildContext --- pkg/apis/build/v1alpha2/build_pod.go | 56 +++-- pkg/apis/build/v1alpha2/build_pod_test.go | 257 +++++++++++++--------- pkg/buildpod/generator.go | 8 +- pkg/buildpod/generator_test.go | 54 +++-- 4 files changed, 214 insertions(+), 161 deletions(-) diff --git a/pkg/apis/build/v1alpha2/build_pod.go b/pkg/apis/build/v1alpha2/build_pod.go index 56e61f96a..26dff326c 100644 --- a/pkg/apis/build/v1alpha2/build_pod.go +++ b/pkg/apis/build/v1alpha2/build_pod.go @@ -77,6 +77,16 @@ func (bpi *BuildPodImages) completion(os string) string { } } +type BuildContext struct { + BuildPodBuilderConfig BuildPodBuilderConfig + Secrets []corev1.Secret + Bindings []ServiceBinding +} + +func (c BuildContext) os() string { + return c.BuildPodBuilderConfig.OS +} + type BuildPodBuilderConfig struct { StackID string RunImage string @@ -148,14 +158,14 @@ var ( type stepModifier func(corev1.Container) corev1.Container -func (b *Build) BuildPod(images BuildPodImages, secrets []corev1.Secret, config BuildPodBuilderConfig, bindings []ServiceBinding) (*corev1.Pod, error) { - platformAPI, err := config.highestSupportedPlatformAPI(b) +func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*corev1.Pod, error) { + platformAPI, err := buildContext.BuildPodBuilderConfig.highestSupportedPlatformAPI(b) if err != nil { return nil, err } - if b.rebasable(config.StackID) { - return b.rebasePod(secrets, images, config) + if b.rebasable(buildContext.BuildPodBuilderConfig.StackID) { + return b.rebasePod(buildContext.Secrets, images, buildContext.BuildPodBuilderConfig) } ref, err := name.ParseReference(b.Tag()) @@ -169,11 +179,11 @@ func (b *Build) BuildPod(images BuildPodImages, secrets []corev1.Secret, config return nil, err } - secretVolumes, secretVolumeMounts, secretArgs := b.setupSecretVolumesAndArgs(secrets, gitAndDockerSecrets) - cosignVolumes, _, _ := b.setupSecretVolumesAndArgs(secrets, cosignSecrets) + secretVolumes, secretVolumeMounts, secretArgs := b.setupSecretVolumesAndArgs(buildContext.Secrets, gitAndDockerSecrets) + cosignVolumes, _, _ := b.setupSecretVolumesAndArgs(buildContext.Secrets, cosignSecrets) secretVolumes = append(secretVolumes, cosignVolumes...) - bindingVolumes, bindingVolumeMounts, err := setupBindingVolumesAndMounts(bindings) + bindingVolumes, bindingVolumeMounts, err := setupBindingVolumesAndMounts(buildContext.Bindings) if err != nil { return nil, err } @@ -190,7 +200,7 @@ func (b *Build) BuildPod(images BuildPodImages, secrets []corev1.Secret, config var exporterCacheArgs []string var cacheVolumes []corev1.VolumeMount - if (!b.Spec.NeedVolumeCache() && !b.Spec.NeedRegistryCache()) || config.OS == "windows" { + if (!b.Spec.NeedVolumeCache() && !b.Spec.NeedRegistryCache()) || buildContext.os() == "windows" { genericCacheArgs = nil } else if b.Spec.NeedRegistryCache() { useCacheFromLastBuild := (b.Spec.LastBuild != nil && b.Spec.LastBuild.Cache.Image != "") @@ -219,16 +229,16 @@ func (b *Build) BuildPod(images BuildPodImages, secrets []corev1.Secret, config // If the build fails, don't restart it. RestartPolicy: corev1.RestartPolicyNever, Containers: steps(func(step func(corev1.Container, ...stepModifier)) { - step(b.completionContainer(secrets, images.completion(config.OS)), - ifWindows(config.OS, addNetworkWaitLauncherVolume(), useNetworkWaitLauncher(dnsProbeHost))...) + step(b.completionContainer(buildContext.Secrets, images.completion(buildContext.os())), + ifWindows(buildContext.os(), addNetworkWaitLauncherVolume(), useNetworkWaitLauncher(dnsProbeHost))...) }), - SecurityContext: podSecurityContext(config), + SecurityContext: podSecurityContext(buildContext.BuildPodBuilderConfig), InitContainers: steps(func(step func(corev1.Container, ...stepModifier)) { step( corev1.Container{ - Name: "prepare", - Image: images.buildInit(config.OS), - Args: secretArgs, + Name: "prepare", + Image: images.buildInit(buildContext.os()), + Args: secretArgs, Resources: b.Spec.Resources, Env: append( b.Spec.Source.Source().BuildEnvVars(), @@ -250,7 +260,7 @@ func (b *Build) BuildPod(images BuildPodImages, secrets []corev1.Secret, config }, corev1.EnvVar{ Name: "RUN_IMAGE", - Value: config.RunImage, + Value: buildContext.BuildPodBuilderConfig.RunImage, }, corev1.EnvVar{ Name: "DNS_PROBE_HOSTNAME", @@ -273,7 +283,7 @@ func (b *Build) BuildPod(images BuildPodImages, secrets []corev1.Secret, config projectMetadataVolume, ), }, - ifWindows(config.OS, addNetworkWaitLauncherVolume())..., + ifWindows(buildContext.os(), addNetworkWaitLauncherVolume())..., ) step( corev1.Container{ @@ -299,7 +309,7 @@ func (b *Build) BuildPod(images BuildPodImages, secrets []corev1.Secret, config }, }, }, - ifWindows(config.OS, addNetworkWaitLauncherVolume(), useNetworkWaitLauncher(dnsProbeHost))..., + ifWindows(buildContext.os(), addNetworkWaitLauncherVolume(), useNetworkWaitLauncher(dnsProbeHost))..., ) step( corev1.Container{ @@ -334,7 +344,7 @@ func (b *Build) BuildPod(images BuildPodImages, secrets []corev1.Secret, config }, ImagePullPolicy: corev1.PullIfNotPresent, }, - ifWindows(config.OS, + ifWindows(buildContext.os(), addNetworkWaitLauncherVolume(), useNetworkWaitLauncher(dnsProbeHost), userprofileHomeEnv(), @@ -362,7 +372,7 @@ func (b *Build) BuildPod(images BuildPodImages, secrets []corev1.Secret, config }, ImagePullPolicy: corev1.PullIfNotPresent, }, - ifWindows(config.OS, addNetworkWaitLauncherVolume(), useNetworkWaitLauncher(dnsProbeHost))..., + ifWindows(buildContext.os(), addNetworkWaitLauncherVolume(), useNetworkWaitLauncher(dnsProbeHost))..., ) step( corev1.Container{ @@ -390,7 +400,7 @@ func (b *Build) BuildPod(images BuildPodImages, secrets []corev1.Secret, config serviceBindingRootEnv, }, }, - ifWindows(config.OS, addNetworkWaitLauncherVolume(), useNetworkWaitLauncher(dnsProbeHost))..., + ifWindows(buildContext.os(), addNetworkWaitLauncherVolume(), useNetworkWaitLauncher(dnsProbeHost))..., ) step( corev1.Container{ @@ -438,7 +448,7 @@ func (b *Build) BuildPod(images BuildPodImages, secrets []corev1.Secret, config }, ImagePullPolicy: corev1.PullIfNotPresent, }, - ifWindows(config.OS, + ifWindows(buildContext.os(), addNetworkWaitLauncherVolume(), useNetworkWaitLauncher(dnsProbeHost), userprofileHomeEnv(), @@ -446,13 +456,13 @@ func (b *Build) BuildPod(images BuildPodImages, secrets []corev1.Secret, config ) }), ServiceAccountName: b.Spec.ServiceAccountName, - NodeSelector: b.nodeSelector(config.OS), + NodeSelector: b.nodeSelector(buildContext.os()), Tolerations: b.Spec.Tolerations, Affinity: b.Spec.Affinity, RuntimeClassName: b.Spec.RuntimeClassName, SchedulerName: b.Spec.SchedulerName, Volumes: append(append( - append(secretVolumes, b.cacheVolume(config.OS)...), + append(secretVolumes, b.cacheVolume(buildContext.os())...), corev1.Volume{ Name: layersDirName, VolumeSource: corev1.VolumeSource{ diff --git a/pkg/apis/build/v1alpha2/build_pod_test.go b/pkg/apis/build/v1alpha2/build_pod_test.go index bab52186a..b0c261d57 100644 --- a/pkg/apis/build/v1alpha2/build_pod_test.go +++ b/pkg/apis/build/v1alpha2/build_pod_test.go @@ -258,18 +258,22 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { CompletionWindowsImage: "completion/image/windows:image", } - buildPodBuilderConfig := buildapi.BuildPodBuilderConfig{ - StackID: "com.builder.stack.io", - RunImage: "builderregistry.io/run", - Uid: 2000, - Gid: 3000, - PlatformAPIs: []string{"0.2", "0.3", "0.4", "0.5", "0.6"}, - OS: "linux", + buildContext := buildapi.BuildContext{ + BuildPodBuilderConfig: buildapi.BuildPodBuilderConfig{ + StackID: "com.builder.stack.io", + RunImage: "builderregistry.io/run", + Uid: 2000, + Gid: 3000, + PlatformAPIs: []string{"0.2", "0.3", "0.4", "0.5", "0.6"}, + OS: "linux", + }, + Secrets: secrets, + Bindings: serviceBindings, } when("BuildPod", func() { it("creates a pod with a builder owner reference and build labels and annotations", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, pod.ObjectMeta, metav1.ObjectMeta{ @@ -290,14 +294,14 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("creates a pod with a correct service account", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, serviceAccount, pod.Spec.ServiceAccountName) }) it("sets the pod tolerations and affinity from the build and merges the os node selector", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, map[string]string{"kubernetes.io/os": "linux", "foo": "bar"}, pod.Spec.NodeSelector) @@ -308,23 +312,44 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { it("handles a nil node selector", func() { build.Spec.NodeSelector = nil - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, map[string]string{"kubernetes.io/os": "linux"}, pod.Spec.NodeSelector) }) it("configures the pod security context to match the builder config user and group", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) - assert.Equal(t, buildPodBuilderConfig.Uid, *pod.Spec.SecurityContext.RunAsUser) - assert.Equal(t, buildPodBuilderConfig.Gid, *pod.Spec.SecurityContext.RunAsGroup) - assert.Equal(t, buildPodBuilderConfig.Gid, *pod.Spec.SecurityContext.FSGroup) + assert.Equal(t, buildapi.BuildPodBuilderConfig{ + StackID: "com.builder.stack.io", + RunImage: "builderregistry.io/run", + Uid: 2000, + Gid: 3000, + PlatformAPIs: []string{"0.2", "0.3", "0.4", "0.5", "0.6"}, + OS: "linux", + }.Uid, *pod.Spec.SecurityContext.RunAsUser) + assert.Equal(t, buildapi.BuildPodBuilderConfig{ + StackID: "com.builder.stack.io", + RunImage: "builderregistry.io/run", + Uid: 2000, + Gid: 3000, + PlatformAPIs: []string{"0.2", "0.3", "0.4", "0.5", "0.6"}, + OS: "linux", + }.Gid, *pod.Spec.SecurityContext.RunAsGroup) + assert.Equal(t, buildapi.BuildPodBuilderConfig{ + StackID: "com.builder.stack.io", + RunImage: "builderregistry.io/run", + Uid: 2000, + Gid: 3000, + PlatformAPIs: []string{"0.2", "0.3", "0.4", "0.5", "0.6"}, + OS: "linux", + }.Gid, *pod.Spec.SecurityContext.FSGroup) }) it("creates init containers with all the build steps", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) var names []string @@ -345,7 +370,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { it("configures the workspace volume with a subPath", func() { build.Spec.Source.SubPath = "some/path" - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) vol := volumeMountFromContainer(t, pod.Spec.InitContainers, "prepare", "workspace-dir") @@ -360,7 +385,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures the services", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Contains(t, @@ -401,7 +426,8 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures the v1alpha1bindings", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, v1alpha1ServiceBindings) + buildContext.Bindings = v1alpha1ServiceBindings + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Contains(t, @@ -444,7 +470,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures prepare with docker and git credentials", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, pod.Spec.InitContainers[0].Name, "prepare") @@ -483,7 +509,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures prepare with the build configuration", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, pod.Spec.InitContainers[0].Name, "prepare") @@ -529,7 +555,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures the prepare step for git source", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, "prepare", pod.Spec.InitContainers[0].Name) @@ -552,7 +578,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { build.Spec.Source.Blob = &corev1alpha1.Blob{ URL: "https://some-blobstore.example.com/some-blob", } - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, "prepare", pod.Spec.InitContainers[0].Name) @@ -570,7 +596,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { build.Spec.Source.Registry = &corev1alpha1.Registry{ Image: "some-registry.io/some-image", } - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, "prepare", pod.Spec.InitContainers[0].Name) @@ -598,7 +624,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { {Name: "registry-secret"}, }, } - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, "prepare", pod.Spec.InitContainers[0].Name) @@ -628,7 +654,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures detect step", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, pod.Spec.InitContainers[1].Name, "detect") @@ -644,7 +670,8 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures detect step with cnb bindings", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, v1alpha1ServiceBindings) + buildContext.Bindings = v1alpha1ServiceBindings + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, pod.Spec.InitContainers[1].Name, "detect") @@ -660,7 +687,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures analyze step", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, pod.Spec.InitContainers[2].Name, "analyze") @@ -684,7 +711,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { it("configures analyze step with the current tag if no previous build", func() { build.Spec.LastBuild = nil - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, pod.Spec.InitContainers[2].Name, "analyze") @@ -707,14 +734,14 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { it("configures analyze step with the current tag if previous build is corrupted", func() { build.Spec.LastBuild = &buildapi.LastBuild{} - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Contains(t, pod.Spec.InitContainers[2].Args, build.Tag()) }) it("configures restore step", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, pod.Spec.InitContainers[3].Name, "restore") @@ -734,7 +761,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures build step", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, pod.Spec.InitContainers[4].Name, "build") @@ -750,7 +777,8 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures the build step with v1alpha1bindings", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, v1alpha1ServiceBindings) + buildContext.Bindings = v1alpha1ServiceBindings + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, pod.Spec.InitContainers[4].Name, "build") @@ -766,7 +794,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures export step", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, pod.Spec.InitContainers[5].Name, "export") @@ -794,7 +822,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures export step with non-web default process", func() { build.Spec.DefaultProcess = "sys-info" - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, pod.Spec.InitContainers[5].Name, "export") @@ -823,7 +851,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures the builder image in all lifecycle steps", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) for _, container := range pod.Spec.InitContainers { @@ -844,7 +872,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures the completion container with resources", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) completionContainer := pod.Spec.Containers[0] @@ -852,7 +880,8 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("creates a pod with reusable cache when name is provided", func() { - pod, err := build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + buildContext.Secrets = nil + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, corev1.Volume{ @@ -864,33 +893,33 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) when("registry cache is requested (first build)", func() { - podWithVolumeCache, _ := build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + podWithVolumeCache, _ := build.BuildPod(config, buildContext) build.Spec.Cache.Volume = nil build.Spec.Cache.Registry = &buildapi.RegistryCache{Tag: "test-cache-image"} it("creates a pod without cache volume", func() { - podWithImageCache, err := build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + podWithImageCache, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Len(t, podWithImageCache.Spec.Volumes, len(podWithVolumeCache.Spec.Volumes)-1) }) it("does not add the cache to analyze container", func() { - podWithImageCache, err := build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + podWithImageCache, err := build.BuildPod(config, buildContext) require.NoError(t, err) analyzeContainer := podWithImageCache.Spec.InitContainers[2] assert.NotContains(t, analyzeContainer.Args, "-cache-image=test-cache-image") }) it("does not add the cache to restore container", func() { - podWithImageCache, err := build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + podWithImageCache, err := build.BuildPod(config, buildContext) require.NoError(t, err) restoreContainer := podWithImageCache.Spec.InitContainers[3] assert.NotContains(t, restoreContainer.Args, "-cache-image=test-cache-image") }) it("adds the cache to export container", func() { - podWithImageCache, err := build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + podWithImageCache, err := build.BuildPod(config, buildContext) require.NoError(t, err) exportContainer := podWithImageCache.Spec.InitContainers[5] @@ -899,7 +928,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) when("registry cache is requested (second build)", func() { - podWithVolumeCache, _ := build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + podWithVolumeCache, _ := build.BuildPod(config, buildContext) build.Spec.Cache.Volume = nil build.Spec.Cache.Registry = &buildapi.RegistryCache{Tag: "test-cache-image"} build.Spec.LastBuild = &buildapi.LastBuild{ @@ -909,28 +938,28 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { } it("creates a pod without cache volume", func() { - podWithImageCache, err := build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + podWithImageCache, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Len(t, podWithImageCache.Spec.Volumes, len(podWithVolumeCache.Spec.Volumes)-1) }) it("adds the cache to analyze container", func() { - podWithImageCache, err := build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + podWithImageCache, err := build.BuildPod(config, buildContext) require.NoError(t, err) analyzeContainer := podWithImageCache.Spec.InitContainers[2] assert.Contains(t, analyzeContainer.Args, "-cache-image=test-cache-image@sha") }) it("adds the cache to restore container", func() { - podWithImageCache, err := build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + podWithImageCache, err := build.BuildPod(config, buildContext) require.NoError(t, err) restoreContainer := podWithImageCache.Spec.InitContainers[3] assert.Contains(t, restoreContainer.Args, "-cache-image=test-cache-image@sha") }) it("adds the cache to export container", func() { - podWithImageCache, err := build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + podWithImageCache, err := build.BuildPod(config, buildContext) require.NoError(t, err) exportContainer := podWithImageCache.Spec.InitContainers[5] @@ -944,21 +973,21 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { build.Spec.Cache.Registry = &buildapi.RegistryCache{Tag: ""} it("does not add the cache to analyze container", func() { - pod, err = build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + pod, err = build.BuildPod(config, buildContext) require.NoError(t, err) analyzeContainer := pod.Spec.InitContainers[2] assert.NotContains(t, analyzeContainer.Args, "-cache-image") }) it("does not add the cache to restore container", func() { - pod, err = build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + pod, err = build.BuildPod(config, buildContext) require.NoError(t, err) restoreContainer := pod.Spec.InitContainers[3] assert.NotContains(t, restoreContainer.Args, "-cache-image") }) it("does not add the cache to export container", func() { - pod, err = build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + pod, err = build.BuildPod(config, buildContext) require.NoError(t, err) exportContainer := pod.Spec.InitContainers[5] @@ -968,11 +997,11 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { when("Cache is nil", func() { buildCopy := build.DeepCopy() - podWithCache, _ := buildCopy.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + podWithCache, _ := buildCopy.BuildPod(config, buildContext) buildCopy.Spec.Cache = nil it("creates a pod without cache volume", func() { - pod, err := buildCopy.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + pod, err := buildCopy.BuildPod(config, buildContext) require.NoError(t, err) assert.Len(t, pod.Spec.Volumes, len(podWithCache.Spec.Volumes)-1) @@ -980,18 +1009,18 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) when("CacheName is empty", func() { - podWithCache, _ := build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + podWithCache, _ := build.BuildPod(config, buildContext) build.Spec.Cache.Volume = &buildapi.BuildPersistentVolumeCache{ClaimName: ""} it("creates a pod without cache volume", func() { - pod, err := build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Len(t, pod.Spec.Volumes, len(podWithCache.Spec.Volumes)-1) }) it("does not add the cache to analyze container", func() { - pod, err := build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) analyzeContainer := pod.Spec.InitContainers[2] @@ -1000,7 +1029,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("does not add the cache to restore container", func() { - pod, err := build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) restoreContainer := pod.Spec.InitContainers[3] @@ -1009,7 +1038,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("does not add the cache to exporter container", func() { - pod, err := build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) exportContainer := pod.Spec.InitContainers[5] @@ -1019,7 +1048,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("attach volumes for secrets", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assertSecretPresent(t, pod, "git-secret-1") @@ -1031,7 +1060,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("attach image pull secrets to pod", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) require.Len(t, pod.Spec.ImagePullSecrets, 1) @@ -1039,7 +1068,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("mounts volumes for bindings", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) require.Len(t, pod.Spec.ImagePullSecrets, 1) @@ -1047,10 +1076,10 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) when("only 0.3 platform api is supported", func() { - buildPodBuilderConfig.PlatformAPIs = []string{"0.3", "0.2"} + buildContext.BuildPodBuilderConfig.PlatformAPIs = []string{"0.3", "0.2"} it("exports without a report and without default process type", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Contains(t, pod.Spec.InitContainers[5].Env, corev1.EnvVar{Name: "CNB_PLATFORM_API", Value: "0.3"}) @@ -1069,10 +1098,10 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) when("no supported platform apis are available", func() { - buildPodBuilderConfig.PlatformAPIs = []string{"0.2", "0.7"} + buildContext.BuildPodBuilderConfig.PlatformAPIs = []string{"0.2", "0.7"} it("returns an error", func() { - _, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + _, err := build.BuildPod(config, buildContext) require.EqualError(t, err, "unsupported builder platform API versions: 0.2,0.7") }) }) @@ -1085,7 +1114,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { } it("creates a pod just to rebase", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, pod.ObjectMeta, metav1.ObjectMeta{ @@ -1219,7 +1248,8 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { when("cosign secrets are present on the build", func() { it("skips invalid secrets", func() { - pod, err := build.BuildPod(config, append(secrets, cosignInvalidSecrets...), buildPodBuilderConfig, serviceBindings) + buildContext.Secrets = append(secrets, cosignInvalidSecrets...) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.NotNil(t, pod.Spec.Containers[0]) @@ -1236,7 +1266,8 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("sets up the completion image to use cosign secrets", func() { - pod, err := build.BuildPod(config, append(secrets, cosignValidSecrets...), buildPodBuilderConfig, serviceBindings) + buildContext.Secrets = append(secrets, cosignValidSecrets...) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) validSecrets := []string{ @@ -1276,7 +1307,8 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { {Name: "customAnnotationKey", Value: "customAnnotationValue"}, }, } - pod, err := build.BuildPod(config, append(secrets, cosignValidSecrets...), buildPodBuilderConfig, serviceBindings) + buildContext.Secrets = append(secrets, cosignValidSecrets...) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) require.Equal(t, @@ -1307,7 +1339,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }, } - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) require.Equal(t, []string{ @@ -1353,14 +1385,14 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { } it("errs if platformApi does not support report.toml", func() { - buildPodBuilderConfig.PlatformAPIs = []string{"0.3", "0.2"} + buildContext.BuildPodBuilderConfig.PlatformAPIs = []string{"0.3", "0.2"} - _, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + _, err := build.BuildPod(config, buildContext) require.EqualError(t, err, "unsupported builder platform API versions: 0.3,0.2") }) it("sets up the completion image to sign the image", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, "/cnb/process/web", pod.Spec.Containers[0].Command[0]) @@ -1410,7 +1442,8 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("skips invalid secrets", func() { - pod, err := build.BuildPod(config, append(secrets, cosignInvalidSecrets...), buildPodBuilderConfig, serviceBindings) + buildContext.Secrets = append(secrets, cosignInvalidSecrets...) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, "/cnb/process/web", pod.Spec.Containers[0].Command[0]) @@ -1425,7 +1458,8 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("sets up the completion image to use cosign secrets", func() { - pod, err := build.BuildPod(config, append(secrets, cosignValidSecrets...), buildPodBuilderConfig, serviceBindings) + buildContext.Secrets = append(secrets, cosignValidSecrets...) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, "/cnb/process/web", pod.Spec.Containers[0].Command[0]) @@ -1488,7 +1522,8 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { {Name: "customAnnotationKey", Value: "customAnnotationValue"}, }, } - pod, err := build.BuildPod(config, append(secrets, cosignValidSecrets...), buildPodBuilderConfig, serviceBindings) + buildContext.Secrets = append(secrets, cosignValidSecrets...) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) require.Equal(t, @@ -1510,9 +1545,9 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("errs if platformApi does not support report.toml", func() { - buildPodBuilderConfig.PlatformAPIs = []string{"0.3", "0.2"} + buildContext.BuildPodBuilderConfig.PlatformAPIs = []string{"0.3", "0.2"} - _, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + _, err := build.BuildPod(config, buildContext) require.EqualError(t, err, "unsupported builder platform API versions: 0.3,0.2") }) }) @@ -1520,7 +1555,8 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { when("cosign secrets are present on the build", func() { it("skips invalid secrets", func() { - pod, err := build.BuildPod(config, append(secrets, cosignInvalidSecrets...), buildPodBuilderConfig, serviceBindings) + buildContext.Secrets = append(secrets, cosignInvalidSecrets...) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.NotNil(t, pod.Spec.Containers[0]) @@ -1537,7 +1573,8 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("sets up the completion image to use cosign secrets", func() { - pod, err := build.BuildPod(config, append(secrets, cosignValidSecrets...), buildPodBuilderConfig, serviceBindings) + buildContext.Secrets = append(secrets, cosignValidSecrets...) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) validSecrets := []string{ @@ -1577,7 +1614,8 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { {Name: "customAnnotationKey", Value: "customAnnotationValue"}, }, } - pod, err := build.BuildPod(config, append(secrets, cosignValidSecrets...), buildPodBuilderConfig, serviceBindings) + buildContext.Secrets = append(secrets, cosignValidSecrets...) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) require.Equal(t, @@ -1609,14 +1647,14 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { } it("errs if platformApi does not support report.toml", func() { - buildPodBuilderConfig.PlatformAPIs = []string{"0.3", "0.2"} + buildContext.BuildPodBuilderConfig.PlatformAPIs = []string{"0.3", "0.2"} - _, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + _, err := build.BuildPod(config, buildContext) require.EqualError(t, err, "unsupported builder platform API versions: 0.3,0.2") }) it("sets up the completion image to sign the image", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, "/cnb/process/web", pod.Spec.Containers[0].Command[0]) @@ -1666,7 +1704,8 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("skips invalid secrets", func() { - pod, err := build.BuildPod(config, append(secrets, cosignInvalidSecrets...), buildPodBuilderConfig, serviceBindings) + buildContext.Secrets = append(secrets, cosignInvalidSecrets...) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, "/cnb/process/web", pod.Spec.Containers[0].Command[0]) @@ -1681,7 +1720,8 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("sets up the completion image to use cosign secrets", func() { - pod, err := build.BuildPod(config, append(secrets, cosignValidSecrets...), buildPodBuilderConfig, serviceBindings) + buildContext.Secrets = append(secrets, cosignValidSecrets...) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, "/cnb/process/web", pod.Spec.Containers[0].Command[0]) @@ -1744,7 +1784,8 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { {Name: "customAnnotationKey", Value: "customAnnotationValue"}, }, } - pod, err := build.BuildPod(config, append(secrets, cosignValidSecrets...), buildPodBuilderConfig, serviceBindings) + buildContext.Secrets = append(secrets, cosignValidSecrets...) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) require.Equal(t, @@ -1766,15 +1807,15 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("errs if platformApi does not support report.toml", func() { - buildPodBuilderConfig.PlatformAPIs = []string{"0.3", "0.2"} + buildContext.BuildPodBuilderConfig.PlatformAPIs = []string{"0.3", "0.2"} - _, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + _, err := build.BuildPod(config, buildContext) require.EqualError(t, err, "unsupported builder platform API versions: 0.3,0.2") }) }) it("creates the pod container correctly", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) require.Len(t, pod.Spec.Containers, 1) @@ -1782,31 +1823,31 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) when("builder is windows", func() { - buildPodBuilderConfig.OS = "windows" + buildContext.BuildPodBuilderConfig.OS = "windows" it("errs if platformApi does not support windows", func() { - buildPodBuilderConfig.PlatformAPIs = []string{"0.3", "0.2"} + buildContext.BuildPodBuilderConfig.PlatformAPIs = []string{"0.3", "0.2"} - _, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + _, err := build.BuildPod(config, buildContext) require.EqualError(t, err, "unsupported builder platform API versions: 0.3,0.2") }) it("uses windows node selector", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Equal(t, map[string]string{"kubernetes.io/os": "windows", "foo": "bar"}, pod.Spec.NodeSelector) }) it("removes the spec securityContext", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Nil(t, pod.Spec.SecurityContext) }) it("configures prepare for windows build init", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) prepareContainer := pod.Spec.InitContainers[0] @@ -1839,7 +1880,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures detect step for windows", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) detectContainer := pod.Spec.InitContainers[1] @@ -1870,7 +1911,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures analyze step", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) analyzeContainer := pod.Spec.InitContainers[2] @@ -1908,7 +1949,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures restore step", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) restoreContainer := pod.Spec.InitContainers[3] @@ -1938,7 +1979,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures build step", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) buildContainer := pod.Spec.InitContainers[4] @@ -1970,7 +2011,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures export step", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) exportContainer := pod.Spec.InitContainers[5] @@ -2014,7 +2055,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { URL: "some-notary-server", }} - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) completionContainer := pod.Spec.Containers[0] @@ -2049,7 +2090,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures the completion container on windows", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) completionContainer := pod.Spec.Containers[0] @@ -2063,12 +2104,12 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("does not use cache on windows", func() { - buildPodBuilderConfigLinux := buildPodBuilderConfig.DeepCopy() - buildPodBuilderConfigLinux.OS = "linux" - podWithCache, _ := build.BuildPod(config, nil, *buildPodBuilderConfigLinux, serviceBindings) - build.Spec.Cache.Volume.ClaimName = "non-empty" + buildContext.BuildPodBuilderConfig.OS = "linux" + podWithCache, _ := build.BuildPod(config, buildContext) - pod, err := build.BuildPod(config, nil, buildPodBuilderConfig, serviceBindings) + build.Spec.Cache.Volume.ClaimName = "non-empty" + buildContext.BuildPodBuilderConfig.OS = "windows" + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) assert.Len(t, pod.Spec.Volumes, len(podWithCache.Spec.Volumes)-1) diff --git a/pkg/buildpod/generator.go b/pkg/buildpod/generator.go index b07be12e3..6ca1df98d 100644 --- a/pkg/buildpod/generator.go +++ b/pkg/buildpod/generator.go @@ -50,7 +50,7 @@ type BuildPodable interface { CnbBindings() corev1alpha1.CNBBindings Services() buildapi.Services - BuildPod(buildapi.BuildPodImages, []corev1.Secret, buildapi.BuildPodBuilderConfig, []buildapi.ServiceBinding) (*corev1.Pod, error) + BuildPod(buildapi.BuildPodImages, buildapi.BuildContext) (*corev1.Pod, error) } func (g *Generator) Generate(ctx context.Context, build BuildPodable) (*v1.Pod, error) { @@ -69,7 +69,11 @@ func (g *Generator) Generate(ctx context.Context, build BuildPodable) (*v1.Pod, return nil, err } - return build.BuildPod(g.BuildPodConfig, secrets, buildPodBuilderConfig, bindings) + return build.BuildPod(g.BuildPodConfig, buildapi.BuildContext{ + BuildPodBuilderConfig: buildPodBuilderConfig, + Secrets: secrets, + Bindings: bindings, + }) } func (g *Generator) fetchServiceBindings(ctx context.Context, build BuildPodable) ([]buildapi.ServiceBinding, error) { diff --git a/pkg/buildpod/generator_test.go b/pkg/buildpod/generator_test.go index ea1119149..eb79e0ca9 100644 --- a/pkg/buildpod/generator_test.go +++ b/pkg/buildpod/generator_test.go @@ -226,19 +226,21 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) { assert.Equal(t, []buildPodCall{{ BuildPodImages: buildPodConfig, - Secrets: []corev1.Secret{ - *gitSecret, - *dockerSecret, - }, - BuildPodBuilderConfig: buildapi.BuildPodBuilderConfig{ - StackID: "some.stack.id", - RunImage: "some-registry.io/run-image", - Uid: 1234, - Gid: 5678, - PlatformAPIs: []string{"0.4", "0.5", "0.6"}, - OS: "linux", + BuildContext: buildapi.BuildContext{ + Secrets: []corev1.Secret{ + *gitSecret, + *dockerSecret, + }, + BuildPodBuilderConfig: buildapi.BuildPodBuilderConfig{ + StackID: "some.stack.id", + RunImage: "some-registry.io/run-image", + Uid: 1234, + Gid: 5678, + PlatformAPIs: []string{"0.4", "0.5", "0.6"}, + OS: "linux", + }, + Bindings: []buildapi.ServiceBinding{}, }, - Bindings: []buildapi.ServiceBinding{}, }}, build.buildPodCalls) }) @@ -261,7 +263,7 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) { assert.NotNil(t, pod) assert.Len(t, build.buildPodCalls, 1) - assert.Len(t, build.buildPodCalls[0].Secrets, 2) + assert.Len(t, build.buildPodCalls[0].BuildContext.Secrets, 2) }) it("passes in k8s service bindings if present", func() { @@ -290,8 +292,8 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) { }, } - assert.Len(t, build.buildPodCalls[0].Bindings, 1) - assert.Equal(t, expectedBindings, build.buildPodCalls[0].Bindings) + assert.Len(t, build.buildPodCalls[0].BuildContext.Bindings, 1) + assert.Equal(t, expectedBindings, build.buildPodCalls[0].BuildContext.Bindings) }) it("passes in v1alpha1 service bindings if present", func() { @@ -321,8 +323,8 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) { }, } - assert.Len(t, build.buildPodCalls[0].Bindings, 1) - assert.Equal(t, expectedBindings, build.buildPodCalls[0].Bindings) + assert.Len(t, build.buildPodCalls[0].BuildContext.Bindings, 1) + assert.Equal(t, expectedBindings, build.buildPodCalls[0].BuildContext.Bindings) }) it("rejects a build with a cnb binding secret that is attached to a service account", func() { @@ -395,8 +397,8 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) { }, } - assert.Len(t, build.buildPodCalls[0].Bindings, 1) - assert.Equal(t, expectedBindings, build.buildPodCalls[0].Bindings) + assert.Len(t, build.buildPodCalls[0].BuildContext.Bindings, 1) + assert.Equal(t, expectedBindings, build.buildPodCalls[0].BuildContext.Bindings) }) }) } @@ -417,10 +419,8 @@ type testBuildPodable struct { } type buildPodCall struct { - BuildPodImages buildapi.BuildPodImages - Secrets []corev1.Secret - BuildPodBuilderConfig buildapi.BuildPodBuilderConfig - Bindings []buildapi.ServiceBinding + BuildPodImages buildapi.BuildPodImages + BuildContext buildapi.BuildContext } func (tb *testBuildPodable) GetName() string { @@ -439,12 +439,10 @@ func (tb *testBuildPodable) BuilderSpec() corev1alpha1.BuildBuilderSpec { return tb.buildBuilderSpec } -func (tb *testBuildPodable) BuildPod(images buildapi.BuildPodImages, secrets []corev1.Secret, config buildapi.BuildPodBuilderConfig, bindings []buildapi.ServiceBinding) (*corev1.Pod, error) { +func (tb *testBuildPodable) BuildPod(images buildapi.BuildPodImages, buildContext buildapi.BuildContext) (*corev1.Pod, error) { tb.buildPodCalls = append(tb.buildPodCalls, buildPodCall{ - BuildPodImages: images, - Secrets: secrets, - BuildPodBuilderConfig: config, - Bindings: bindings, + BuildPodImages: images, + BuildContext: buildContext, }) return &corev1.Pod{}, nil } From 56f3e9b5d91708935f4f0ea5d7684c5eb20a96a0 Mon Sep 17 00:00:00 2001 From: Matthew McNew Date: Thu, 21 Oct 2021 13:50:51 -0500 Subject: [PATCH 2/5] Mount all ServiceAccount imagePullSecrets to allow builds to read the run image - The build's ServiceAccount's imagePullSecrets will be used to pull the builder image but, are unavailable to read the run image which is likely in the same registry. - This also supports multiple builder image pull secrets configured directly on the build resource. - Mounts the imagePullSecrets inside the rebase build pod. --- cmd/build-init/main.go | 29 ++-- cmd/rebase/main.go | 12 +- pkg/apis/build/v1alpha2/build_pod.go | 180 ++++++++++++---------- pkg/apis/build/v1alpha2/build_pod_test.go | 163 ++++++++++++-------- pkg/buildpod/generator.go | 21 ++- pkg/buildpod/generator_test.go | 35 ++++- 6 files changed, 276 insertions(+), 164 deletions(-) diff --git a/cmd/build-init/main.go b/cmd/build-init/main.go index 2fcbf53c1..0e093bca6 100644 --- a/cmd/build-init/main.go +++ b/cmd/build-init/main.go @@ -43,6 +43,7 @@ var ( dockerCredentials flaghelpers.CredentialsFlags dockerCfgCredentials flaghelpers.CredentialsFlags dockerConfigCredentials flaghelpers.CredentialsFlags + imagePullSecrets flaghelpers.CredentialsFlags ) func init() { @@ -51,6 +52,7 @@ func init() { flag.Var(&dockerCredentials, "basic-docker", "Basic authentication for docker of the form 'secretname=git.domain.com'") flag.Var(&dockerCfgCredentials, "dockercfg", "Docker Cfg credentials in the form of the path to the credential") flag.Var(&dockerConfigCredentials, "dockerconfig", "Docker Config JSON credentials in the form of the path to the credential") + flag.Var(&imagePullSecrets, "imagepull", "Builder Image pull credentials in the form of the path to the credential") } const ( @@ -59,7 +61,6 @@ const ( platformDir = "/platform" buildSecretsDir = "/var/build-secrets" imagePullSecretsDir = "/imagePullSecrets" - builderPullSecretsDir = "/builderPullSecrets" projectMetadataDir = "/projectMetadata" networkWaitLauncherDir = "/networkWait" networkWaitLauncherBinary = "network-wait-launcher.exe" @@ -108,6 +109,20 @@ func main() { logger.Fatal(errors.Wrapf(err, "Error verifying write access to %q", *imageTag)) } + for _, c := range imagePullSecrets { + credPath := filepath.Join(buildSecretsDir, c) + + imagePullCreds, err := dockercreds.ParseDockerPullSecrets(credPath) + if err != nil { + logger.Fatal(err) + } + + creds, err = creds.Append(imagePullCreds) + if err != nil { + logger.Fatalf("error appending image pull creds %s", err) + } + } + err = dockercreds.VerifyReadAccess(creds, *runImage) if err != nil { logger.Fatal(errors.Wrapf(err, "Error verifying read access to run image %q", *runImage)) @@ -128,17 +143,7 @@ func main() { logger.Fatalf("error setting up platform env vars %s", err) } - builderCreds, err := dockercreds.ParseDockerPullSecrets(builderPullSecretsDir) - if err != nil { - logger.Fatal(err) - } - - dockerCreds, err := creds.Append(builderCreds) - if err != nil { - logger.Fatalf("error appending builder creds %s", err) - } - - err = dockerCreds.Save(path.Join(secretsHome, ".docker", "config.json")) + err = creds.Save(path.Join(secretsHome, ".docker", "config.json")) if err != nil { logger.Fatalf("error writing docker creds %s", err) } diff --git a/cmd/rebase/main.go b/cmd/rebase/main.go index 596b2c609..5cae286c3 100644 --- a/cmd/rebase/main.go +++ b/cmd/rebase/main.go @@ -32,12 +32,14 @@ var ( dockerCredentials flaghelpers.CredentialsFlags dockerCfgCredentials flaghelpers.CredentialsFlags dockerConfigCredentials flaghelpers.CredentialsFlags + imagePullSecrets flaghelpers.CredentialsFlags ) func init() { flag.Var(&dockerCredentials, "basic-docker", "Basic authentication for docker of the form 'secretname=git.domain.com'") flag.Var(&dockerCfgCredentials, "dockercfg", "Docker Cfg credentials in the form of the path to the credential") flag.Var(&dockerConfigCredentials, "dockerconfig", "Docker Config JSON credentials in the form of the path to the credential") + flag.Var(&imagePullSecrets, "imagepull", "Builder Image pull credentials in the form of the path to the credential") } func main() { @@ -62,7 +64,7 @@ func rebase(tags []string, logger *log.Logger) error { return cmd.FailErrCode(err, cmd.CodeInvalidArgs) } - for _, c := range append(dockerCfgCredentials, dockerConfigCredentials...) { + for _, c := range combine(dockerCfgCredentials, dockerConfigCredentials, imagePullSecrets) { credPath := filepath.Join(buildSecretsDir, c) dockerCfgCreds, err := dockercreds.ParseDockerPullSecrets(credPath) @@ -118,3 +120,11 @@ func rebase(tags []string, logger *log.Logger) error { return ioutil.WriteFile(*reportFilePath, buf.Bytes(), 0777) } + +func combine(credentials ...[]string) []string { + var combinded []string + for _, creds := range credentials { + combinded = append(combinded, creds...) + } + return combinded +} diff --git a/pkg/apis/build/v1alpha2/build_pod.go b/pkg/apis/build/v1alpha2/build_pod.go index 26dff326c..59d606326 100644 --- a/pkg/apis/build/v1alpha2/build_pod.go +++ b/pkg/apis/build/v1alpha2/build_pod.go @@ -28,13 +28,12 @@ const ( COSIGNSecretDataCosignPassword = "cosign.password" k8sOSLabel = "kubernetes.io/os" - cacheDirName = "cache-dir" - layersDirName = "layers-dir" - platformDir = "platform-dir" - homeDir = "home-dir" - workspaceDir = "workspace-dir" - imagePullSecretsDirName = "image-pull-secrets-dir" - builderPullSecretsDirName = "builder-pull-secrets-dir" + cacheDirName = "cache-dir" + layersDirName = "layers-dir" + platformDir = "platform-dir" + homeDir = "home-dir" + workspaceDir = "workspace-dir" + imagePullSecretsDirName = "image-pull-secrets-dir" notaryDirName = "notary-dir" reportDirName = "report-dir" @@ -81,6 +80,7 @@ type BuildContext struct { BuildPodBuilderConfig BuildPodBuilderConfig Secrets []corev1.Secret Bindings []ServiceBinding + ImagePullSecrets []corev1.LocalObjectReference } func (c BuildContext) os() string { @@ -130,11 +130,6 @@ var ( MountPath: "/imagePullSecrets", ReadOnly: true, } - builderPullSecretsVolume = corev1.VolumeMount{ - Name: builderPullSecretsDirName, - MountPath: "/builderPullSecrets", - ReadOnly: true, - } notaryV1Volume = corev1.VolumeMount{ Name: notaryDirName, MountPath: "/var/notary/v1", @@ -165,7 +160,7 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor } if b.rebasable(buildContext.BuildPodBuilderConfig.StackID) { - return b.rebasePod(buildContext.Secrets, images, buildContext.BuildPodBuilderConfig) + return b.rebasePod(buildContext, images) } ref, err := name.ParseReference(b.Tag()) @@ -188,6 +183,8 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor return nil, err } + imagePullVolumes, imagePullVolumeMounts, imagePullArgs := b.setupImagePullVolumes(buildContext.ImagePullSecrets) + builderImage := b.Spec.Builder.Image workspaceVolume := corev1.VolumeMount{ @@ -236,9 +233,9 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor InitContainers: steps(func(step func(corev1.Container, ...stepModifier)) { step( corev1.Container{ - Name: "prepare", - Image: images.buildInit(buildContext.os()), - Args: secretArgs, + Name: "prepare", + Image: images.buildInit(buildContext.os()), + Args: append(secretArgs, imagePullArgs...), Resources: b.Spec.Resources, Env: append( b.Spec.Source.Source().BuildEnvVars(), @@ -274,8 +271,7 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor ImagePullPolicy: corev1.PullIfNotPresent, WorkingDir: "/workspace", VolumeMounts: append( - secretVolumeMounts, - builderPullSecretsVolume, + append(secretVolumeMounts, imagePullVolumeMounts...), imagePullSecretsVolume, platformVolume, sourceVolume, @@ -461,48 +457,51 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor Affinity: b.Spec.Affinity, RuntimeClassName: b.Spec.RuntimeClassName, SchedulerName: b.Spec.SchedulerName, - Volumes: append(append( - append(secretVolumes, b.cacheVolume(buildContext.os())...), - corev1.Volume{ - Name: layersDirName, - VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, + Volumes: volumes( + secretVolumes, + imagePullVolumes, + b.cacheVolume(buildContext.os()), + []corev1.Volume{ + { + Name: layersDirName, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, }, - }, - corev1.Volume{ - Name: homeDir, - VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, + { + Name: homeDir, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, }, - }, - corev1.Volume{ - Name: workspaceDir, - VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, + { + Name: workspaceDir, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, }, - }, - corev1.Volume{ - Name: platformDir, - VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, + { + Name: platformDir, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, }, - }, - corev1.Volume{ - Name: reportDirName, - VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, + { + Name: reportDirName, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, }, - }, - corev1.Volume{ - Name: networkWaitLauncherDir, - VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, + { + Name: networkWaitLauncherDir, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, }, + b.Spec.Source.Source().ImagePullSecretsVolume(imagePullSecretsDirName), + b.notarySecretVolume(), }, - b.Spec.Source.Source().ImagePullSecretsVolume(imagePullSecretsDirName), - builderSecretVolume(b.Spec.Builder), - b.notarySecretVolume(), - ), bindingVolumes...), + bindingVolumes), ImagePullSecrets: b.Spec.Builder.ImagePullSecrets, }, }, nil @@ -583,11 +582,13 @@ func (b *Build) notarySecretVolume() corev1.Volume { } } -func (b *Build) rebasePod(secrets []corev1.Secret, images BuildPodImages, config BuildPodBuilderConfig) (*corev1.Pod, error) { - secretVolumes, secretVolumeMounts, secretArgs := b.setupSecretVolumesAndArgs(secrets, dockerSecrets) - cosignVolumes, _, _ := b.setupSecretVolumesAndArgs(secrets, cosignSecrets) +func (b *Build) rebasePod(buildContext BuildContext, images BuildPodImages) (*corev1.Pod, error) { + secretVolumes, secretVolumeMounts, secretArgs := b.setupSecretVolumesAndArgs(buildContext.Secrets, dockerSecrets) + cosignVolumes, _, _ := b.setupSecretVolumesAndArgs(buildContext.Secrets, cosignSecrets) secretVolumes = append(secretVolumes, cosignVolumes...) + imagePullVolumes, imagePullVolumeMounts, imagePullArgs := b.setupImagePullVolumes(buildContext.ImagePullSecrets) + return &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: b.PodName(), @@ -608,7 +609,7 @@ func (b *Build) rebasePod(secrets []corev1.Secret, images BuildPodImages, config RuntimeClassName: b.Spec.RuntimeClassName, SchedulerName: b.Spec.SchedulerName, Volumes: append( - secretVolumes, + append(secretVolumes, imagePullVolumes...), corev1.Volume{ Name: reportDirName, VolumeSource: corev1.VolumeSource{ @@ -619,7 +620,7 @@ func (b *Build) rebasePod(secrets []corev1.Secret, images BuildPodImages, config ), RestartPolicy: corev1.RestartPolicyNever, Containers: steps(func(step func(corev1.Container, ...stepModifier)) { - step(b.completionContainer(secrets, images.CompletionImage)) + step(b.completionContainer(buildContext.Secrets, images.CompletionImage)) }), InitContainers: []corev1.Container{ { @@ -628,13 +629,14 @@ func (b *Build) rebasePod(secrets []corev1.Secret, images BuildPodImages, config Resources: b.Spec.Resources, Args: args(a( "--run-image", - config.RunImage, + buildContext.BuildPodBuilderConfig.RunImage, "--last-built-image", b.Spec.LastBuild.Image, "--report", "/var/report/report.toml", ), secretArgs, + imagePullArgs, b.Spec.Tags, ), Env: []corev1.EnvVar{ @@ -645,7 +647,9 @@ func (b *Build) rebasePod(secrets []corev1.Secret, images BuildPodImages, config }, ImagePullPolicy: corev1.PullIfNotPresent, WorkingDir: "/workspace", - VolumeMounts: append(secretVolumeMounts, reportVolume), + VolumeMounts: append( + append(secretVolumeMounts, imagePullVolumeMounts...), + reportVolume), }, }, }, @@ -732,6 +736,34 @@ func (b *Build) setupSecretVolumesAndArgs(secrets []corev1.Secret, filter func(s return volumes, volumeMounts, args } +func (b *Build) setupImagePullVolumes(secrets []corev1.LocalObjectReference) ([]corev1.Volume, []corev1.VolumeMount, []string) { + var ( + volumes []corev1.Volume + volumeMounts []corev1.VolumeMount + args []string + ) + for _, secret := range append(secrets, b.Spec.Builder.ImagePullSecrets...) { + args = append(args, fmt.Sprintf("-imagepull=%s", secret.Name)) + volumeName := fmt.Sprintf(SecretTemplateName, secret.Name) + + volumes = append(volumes, corev1.Volume{ + Name: volumeName, + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: secret.Name, + }, + }, + }) + + volumeMounts = append(volumeMounts, corev1.VolumeMount{ + Name: volumeName, + MountPath: fmt.Sprintf(DefaultSecretPathName, secret.Name), + }) + } + + return volumes, volumeMounts, args +} + var ( highestSupportedPlatformVersion = semver.MustParse("0.6") lowestSupportedPlatformVersion = semver.MustParse("0.3") @@ -771,26 +803,6 @@ func (b Build) nodeSelector(os string) map[string]string { return b.Spec.NodeSelector } -func builderSecretVolume(bbs corev1alpha1.BuildBuilderSpec) corev1.Volume { - if len(bbs.ImagePullSecrets) > 0 { - return corev1.Volume{ - Name: builderPullSecretsDirName, - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: bbs.ImagePullSecrets[0].Name, - }, - }, - } - } else { - return corev1.Volume{ - Name: builderPullSecretsDirName, - VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, - }, - } - } -} - func setupBindingVolumesAndMounts(bindings []ServiceBinding) ([]corev1.Volume, []corev1.VolumeMount, error) { volumes := make([]corev1.Volume, 0, len(bindings)) volumeMounts := make([]corev1.VolumeMount, 0, len(bindings)) @@ -877,6 +889,14 @@ func a(args ...string) []string { return args } +func volumes(volumes ...[]corev1.Volume) []corev1.Volume { + var combined []corev1.Volume + for _, v := range volumes { + combined = append(combined, v...) + } + return combined +} + func steps(f func(step func(corev1.Container, ...stepModifier))) []corev1.Container { containers := make([]corev1.Container, 0, 7) diff --git a/pkg/apis/build/v1alpha2/build_pod_test.go b/pkg/apis/build/v1alpha2/build_pod_test.go index b0c261d57..8402d7f5a 100644 --- a/pkg/apis/build/v1alpha2/build_pod_test.go +++ b/pkg/apis/build/v1alpha2/build_pod_test.go @@ -44,7 +44,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { builderImageRef := corev1alpha1.BuildBuilderSpec{ Image: builderImage, ImagePullSecrets: []corev1.LocalObjectReference{ - {Name: "some-image-secret"}, + {Name: "builder-pull-secret"}, }} build := &buildapi.Build{ @@ -269,6 +269,14 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }, Secrets: secrets, Bindings: serviceBindings, + ImagePullSecrets: []corev1.LocalObjectReference{ + { + Name: "image-pull-1", + }, + { + Name: "image-pull-2", + }, + }, } when("BuildPod", func() { @@ -469,7 +477,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { } }) - it("configures prepare with docker and git credentials", func() { + it("configures prepare with docker and git credentials and image pull secrets", func() { pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) @@ -481,31 +489,49 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { "-basic-docker=docker-secret-1=acr.io", "-dockerconfig=docker-secret-2", "-dockercfg=docker-secret-3", + "-imagepull=image-pull-1", + "-imagepull=image-pull-2", + "-imagepull=builder-pull-secret", }, pod.Spec.InitContainers[0].Args) - assert.Contains(t, + assert.Subset(t, pod.Spec.InitContainers[0].VolumeMounts, - corev1.VolumeMount{ - Name: "secret-volume-git-secret-1", - MountPath: "/var/build-secrets/git-secret-1", - }, - corev1.VolumeMount{ - Name: "secret-volume-git-secret-2", - MountPath: "/var/build-secrets/git-secret-2", - }, - corev1.VolumeMount{ - Name: "secret-volume-docker-secret-1", - MountPath: "/var/build-secrets/docker-secret-1", - }, - corev1.VolumeMount{ - Name: "secret-volume-docker-secret-2", - MountPath: "/var/build-secrets/docker-secret-2", - }, - corev1.VolumeMount{ - Name: "secret-volume-docker-secret-3", - MountPath: "/var/build-secrets/docker-secret-3", - }, + []corev1.VolumeMount{ + { + Name: "secret-volume-git-secret-1", + MountPath: "/var/build-secrets/git-secret-1", + }, + { + Name: "secret-volume-git-secret-2", + MountPath: "/var/build-secrets/git-secret-2", + }, + { + Name: "secret-volume-docker-secret-1", + MountPath: "/var/build-secrets/docker-secret-1", + }, + { + Name: "secret-volume-docker-secret-2", + MountPath: "/var/build-secrets/docker-secret-2", + }, + { + Name: "secret-volume-docker-secret-3", + MountPath: "/var/build-secrets/docker-secret-3", + }, + + { + Name: "secret-volume-image-pull-1", + MountPath: "/var/build-secrets/image-pull-1", + }, + { + Name: "secret-volume-image-pull-2", + MountPath: "/var/build-secrets/image-pull-2", + }, + { + Name: "secret-volume-builder-pull-secret", + MountPath: "/var/build-secrets/builder-pull-secret", + }}, ) + }) it("configures prepare with the build configuration", func() { @@ -542,11 +568,6 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { Name: "home-dir", MountPath: "/builder/home", }, - { - Name: "builder-pull-secrets-dir", - MountPath: "/builderPullSecrets", - ReadOnly: true, - }, { Name: "layers-dir", MountPath: "/projectMetadata", @@ -590,31 +611,6 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) }) - it("configures prepare with the registry source and empty imagePullSecrets when not provided", func() { - build.Spec.Source.Git = nil - build.Spec.Source.Blob = nil - build.Spec.Source.Registry = &corev1alpha1.Registry{ - Image: "some-registry.io/some-image", - } - pod, err := build.BuildPod(config, buildContext) - require.NoError(t, err) - - assert.Equal(t, "prepare", pod.Spec.InitContainers[0].Name) - assert.Contains(t, pod.Spec.InitContainers[0].VolumeMounts, - corev1.VolumeMount{ - Name: "image-pull-secrets-dir", - MountPath: "/imagePullSecrets", - ReadOnly: true, - }) - assert.NotNil(t, *pod.Spec.Volumes[7].EmptyDir) - assert.Equal(t, config.BuildInitImage, pod.Spec.InitContainers[0].Image) - assert.Contains(t, pod.Spec.InitContainers[0].Env, - corev1.EnvVar{ - Name: "REGISTRY_IMAGE", - Value: "some-registry.io/some-image", - }) - }) - it("configures prepare with the registry source and a secret volume when is imagePullSecrets provided", func() { build.Spec.Source.Git = nil build.Spec.Source.Blob = nil @@ -862,7 +858,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) it("configures the init containers with resources", func() { - pod, err := build.BuildPod(config, secrets, buildPodBuilderConfig, serviceBindings) + pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) initContainers := pod.Spec.InitContainers @@ -884,12 +880,12 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { pod, err := build.BuildPod(config, buildContext) require.NoError(t, err) - assert.Equal(t, corev1.Volume{ + assert.Contains(t, pod.Spec.Volumes, corev1.Volume{ Name: "cache-dir", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: "some-cache-name"}, }, - }, pod.Spec.Volumes[0]) + }) }) when("registry cache is requested (first build)", func() { @@ -1056,6 +1052,9 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { assertSecretPresent(t, pod, "docker-secret-1") assertSecretPresent(t, pod, "docker-secret-2") assertSecretPresent(t, pod, "docker-secret-3") + assertSecretPresent(t, pod, "image-pull-1") + assertSecretPresent(t, pod, "image-pull-2") + assertSecretPresent(t, pod, "builder-pull-secret") assertSecretNotPresent(t, pod, "random-secret-1") }) @@ -1064,15 +1063,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { require.NoError(t, err) require.Len(t, pod.Spec.ImagePullSecrets, 1) - assert.Equal(t, corev1.LocalObjectReference{Name: "some-image-secret"}, pod.Spec.ImagePullSecrets[0]) - }) - - it("mounts volumes for bindings", func() { - pod, err := build.BuildPod(config, buildContext) - require.NoError(t, err) - - require.Len(t, pod.Spec.ImagePullSecrets, 1) - assert.Equal(t, corev1.LocalObjectReference{Name: "some-image-secret"}, pod.Spec.ImagePullSecrets[0]) + assert.Equal(t, corev1.LocalObjectReference{Name: "builder-pull-secret"}, pod.Spec.ImagePullSecrets[0]) }) when("only 0.3 platform api is supported", func() { @@ -1168,6 +1159,30 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }, }, }, + { + Name: "secret-volume-image-pull-1", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "image-pull-1", + }, + }, + }, + { + Name: "secret-volume-image-pull-2", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "image-pull-2", + }, + }, + }, + { + Name: "secret-volume-builder-pull-secret", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "builder-pull-secret", + }, + }, + }, { Name: "report-dir", VolumeSource: corev1.VolumeSource{ @@ -1213,6 +1228,9 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { "-basic-docker=docker-secret-1=acr.io", "-dockerconfig=docker-secret-2", "-dockercfg=docker-secret-3", + "-imagepull=image-pull-1", + "-imagepull=image-pull-2", + "-imagepull=builder-pull-secret", "someimage/name", "someimage/name:tag2", "someimage/name:tag3", }, Env: []corev1.EnvVar{ @@ -1236,6 +1254,18 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { Name: "secret-volume-docker-secret-3", MountPath: "/var/build-secrets/docker-secret-3", }, + { + Name: "secret-volume-image-pull-1", + MountPath: "/var/build-secrets/image-pull-1", + }, + { + Name: "secret-volume-image-pull-2", + MountPath: "/var/build-secrets/image-pull-2", + }, + { + Name: "secret-volume-builder-pull-secret", + MountPath: "/var/build-secrets/builder-pull-secret", + }, { Name: "report-dir", MountPath: "/var/report", @@ -1859,6 +1889,9 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { "-basic-docker=docker-secret-1=acr.io", "-dockerconfig=docker-secret-2", "-dockercfg=docker-secret-3", + "-imagepull=image-pull-1", + "-imagepull=image-pull-2", + "-imagepull=builder-pull-secret", }, prepareContainer.Args) assert.Subset(t, pod.Spec.Volumes, []corev1.Volume{ diff --git a/pkg/buildpod/generator.go b/pkg/buildpod/generator.go index 6ca1df98d..0bb516f34 100644 --- a/pkg/buildpod/generator.go +++ b/pkg/buildpod/generator.go @@ -59,7 +59,7 @@ func (g *Generator) Generate(ctx context.Context, build BuildPodable) (*v1.Pod, return nil, err } - secrets, err := g.fetchBuildSecrets(ctx, build) + secrets, imagePullSecrets, err := g.fetchBuildSecrets(ctx, build) if err != nil { return nil, err } @@ -73,6 +73,7 @@ func (g *Generator) Generate(ctx context.Context, build BuildPodable) (*v1.Pod, BuildPodBuilderConfig: buildPodBuilderConfig, Secrets: secrets, Bindings: bindings, + ImagePullSecrets: imagePullSecrets, }) } @@ -162,24 +163,34 @@ func (g *Generator) fetchServiceAccounts(ctx context.Context, build BuildPodable return serviceAccounts.Items, nil } -func (g *Generator) fetchBuildSecrets(ctx context.Context, build BuildPodable) ([]corev1.Secret, error) { +func (g *Generator) fetchBuildSecrets(ctx context.Context, build BuildPodable) ([]corev1.Secret, []corev1.LocalObjectReference, error) { var secrets []corev1.Secret var secretSet = map[string]struct{}{} serviceAccount, err := g.K8sClient.CoreV1().ServiceAccounts(build.GetNamespace()).Get(ctx, build.ServiceAccount(), metav1.GetOptions{}) if err != nil { - return nil, err + return nil, nil, err } for _, secretRef := range serviceAccount.Secrets { secret, err := g.K8sClient.CoreV1().Secrets(build.GetNamespace()).Get(ctx, secretRef.Name, metav1.GetOptions{}) if err != nil { - return nil, err + return nil, nil, err } if _, ok := secretSet[secret.Name]; !ok { secrets = append(secrets, *secret) secretSet[secret.Name] = struct{}{} } } - return secrets, nil + + var imagePullSecrets []corev1.LocalObjectReference + for _, secretRef := range serviceAccount.ImagePullSecrets { + if _, ok := secretSet[secretRef.Name]; !ok { + imagePullSecrets = append(imagePullSecrets, secretRef) + secretSet[secretRef.Name] = struct{}{} + } + + } + + return secrets, imagePullSecrets, nil } func (g *Generator) fetchBuilderConfig(ctx context.Context, build BuildPodable) (buildapi.BuildPodBuilderConfig, error) { diff --git a/pkg/buildpod/generator_test.go b/pkg/buildpod/generator_test.go index eb79e0ca9..29f8d1596 100644 --- a/pkg/buildpod/generator_test.go +++ b/pkg/buildpod/generator_test.go @@ -119,6 +119,14 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) { Name: "docker-secret-1", }, }, + ImagePullSecrets: []corev1.LocalObjectReference{ + { + Name: "image-pull-1", + }, + { + Name: "image-pull-2", + }, + }, } builderPullSecrets := []v1.LocalObjectReference{ @@ -240,6 +248,14 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) { OS: "linux", }, Bindings: []buildapi.ServiceBinding{}, + ImagePullSecrets: []corev1.LocalObjectReference{ + { + Name: "image-pull-1", + }, + { + Name: "image-pull-2", + }, + }, }, }}, build.buildPodCalls) }) @@ -256,6 +272,9 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) { serviceAccount.Secrets = append(serviceAccount.Secrets, corev1.ObjectReference{Name: "docker-secret-1"}) serviceAccount.Secrets = append(serviceAccount.Secrets, corev1.ObjectReference{Name: "docker-secret-1"}) + serviceAccount.ImagePullSecrets = append(serviceAccount.ImagePullSecrets, corev1.LocalObjectReference{Name: "docker-secret-1"}) + serviceAccount.ImagePullSecrets = append(serviceAccount.ImagePullSecrets, corev1.LocalObjectReference{Name: "image-pull-duplicate-1"}) + serviceAccount.ImagePullSecrets = append(serviceAccount.ImagePullSecrets, corev1.LocalObjectReference{Name: "image-pull-duplicate-1"}) fakeK8sClient.CoreV1().ServiceAccounts(namespace).Update(context.TODO(), serviceAccount, metav1.UpdateOptions{}) pod, err := generator.Generate(context.TODO(), build) @@ -263,7 +282,21 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) { assert.NotNil(t, pod) assert.Len(t, build.buildPodCalls, 1) - assert.Len(t, build.buildPodCalls[0].BuildContext.Secrets, 2) + assert.Equal(t, build.buildPodCalls[0].BuildContext.Secrets, []corev1.Secret{ + *gitSecret, + *dockerSecret, + }) + assert.Equal(t, build.buildPodCalls[0].BuildContext.ImagePullSecrets, []corev1.LocalObjectReference{ + { + Name: "image-pull-1", + }, + { + Name: "image-pull-2", + }, + { + Name: "image-pull-duplicate-1", + }, + }) }) it("passes in k8s service bindings if present", func() { From f13cc2d74ee660f982f9bb66570f9d694aea3f4b Mon Sep 17 00:00:00 2001 From: Matthew McNew Date: Thu, 21 Oct 2021 16:03:59 -0500 Subject: [PATCH 3/5] Use descriptive name for secrets used to pull registry source images --- cmd/build-init/main.go | 20 ++++++++++---------- pkg/apis/build/v1alpha2/build_pod.go | 22 +++++++++++----------- pkg/apis/build/v1alpha2/build_pod_test.go | 6 +++--- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/cmd/build-init/main.go b/cmd/build-init/main.go index 0e093bca6..5e35e7a57 100644 --- a/cmd/build-init/main.go +++ b/cmd/build-init/main.go @@ -56,14 +56,14 @@ func init() { } const ( - secretsHome = "/builder/home" - appDir = "/workspace" - platformDir = "/platform" - buildSecretsDir = "/var/build-secrets" - imagePullSecretsDir = "/imagePullSecrets" - projectMetadataDir = "/projectMetadata" - networkWaitLauncherDir = "/networkWait" - networkWaitLauncherBinary = "network-wait-launcher.exe" + secretsHome = "/builder/home" + appDir = "/workspace" + platformDir = "/platform" + buildSecretsDir = "/var/build-secrets" + registrySourcePullSecretsDir = "/registrySourcePullSecrets" + projectMetadataDir = "/projectMetadata" + networkWaitLauncherDir = "/networkWait" + networkWaitLauncherBinary = "network-wait-launcher.exe" ) func main() { @@ -190,7 +190,7 @@ func fetchSource(logger *log.Logger, serviceAccountCreds dockercreds.DockerCreds } return fetcher.Fetch(appDir, *blobURL) case *registryImage != "": - imagePullSecrets, err := dockercreds.ParseDockerPullSecrets(imagePullSecretsDir) + registrySourcePullSecrets, err := dockercreds.ParseDockerPullSecrets(registrySourcePullSecretsDir) if err != nil { return err } @@ -198,7 +198,7 @@ func fetchSource(logger *log.Logger, serviceAccountCreds dockercreds.DockerCreds fetcher := registry.Fetcher{ Logger: logger, Client: ®istry.Client{}, - Keychain: authn.NewMultiKeychain(imagePullSecrets, serviceAccountCreds), + Keychain: authn.NewMultiKeychain(registrySourcePullSecrets, serviceAccountCreds), } return fetcher.Fetch(appDir, *registryImage) default: diff --git a/pkg/apis/build/v1alpha2/build_pod.go b/pkg/apis/build/v1alpha2/build_pod.go index 59d606326..1d4a519d4 100644 --- a/pkg/apis/build/v1alpha2/build_pod.go +++ b/pkg/apis/build/v1alpha2/build_pod.go @@ -28,12 +28,12 @@ const ( COSIGNSecretDataCosignPassword = "cosign.password" k8sOSLabel = "kubernetes.io/os" - cacheDirName = "cache-dir" - layersDirName = "layers-dir" - platformDir = "platform-dir" - homeDir = "home-dir" - workspaceDir = "workspace-dir" - imagePullSecretsDirName = "image-pull-secrets-dir" + cacheDirName = "cache-dir" + layersDirName = "layers-dir" + platformDir = "platform-dir" + homeDir = "home-dir" + workspaceDir = "workspace-dir" + registrySourcePullSecretsDir = "registry-source-pull-secrets-dir" notaryDirName = "notary-dir" reportDirName = "report-dir" @@ -125,9 +125,9 @@ var ( Name: "HOME", Value: "/builder/home", } - imagePullSecretsVolume = corev1.VolumeMount{ - Name: imagePullSecretsDirName, - MountPath: "/imagePullSecrets", + registrySourcePullSecretsVolume = corev1.VolumeMount{ + Name: registrySourcePullSecretsDir, + MountPath: "/registrySourcePullSecrets", ReadOnly: true, } notaryV1Volume = corev1.VolumeMount{ @@ -272,7 +272,7 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor WorkingDir: "/workspace", VolumeMounts: append( append(secretVolumeMounts, imagePullVolumeMounts...), - imagePullSecretsVolume, + registrySourcePullSecretsVolume, platformVolume, sourceVolume, homeVolume, @@ -498,7 +498,7 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor EmptyDir: &corev1.EmptyDirVolumeSource{}, }, }, - b.Spec.Source.Source().ImagePullSecretsVolume(imagePullSecretsDirName), + b.Spec.Source.Source().ImagePullSecretsVolume(registrySourcePullSecretsDir), b.notarySecretVolume(), }, bindingVolumes), diff --git a/pkg/apis/build/v1alpha2/build_pod_test.go b/pkg/apis/build/v1alpha2/build_pod_test.go index 8402d7f5a..0e6eb72ac 100644 --- a/pkg/apis/build/v1alpha2/build_pod_test.go +++ b/pkg/apis/build/v1alpha2/build_pod_test.go @@ -626,14 +626,14 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { assert.Equal(t, "prepare", pod.Spec.InitContainers[0].Name) assert.Contains(t, pod.Spec.InitContainers[0].VolumeMounts, corev1.VolumeMount{ - Name: "image-pull-secrets-dir", - MountPath: "/imagePullSecrets", + Name: "registry-source-pull-secrets-dir", + MountPath: "/registrySourcePullSecrets", ReadOnly: true, }) match := 0 for _, v := range pod.Spec.Volumes { - if v.Name == "image-pull-secrets-dir" { + if v.Name == "registry-source-pull-secrets-dir" { require.NotNil(t, v.Secret) assert.Equal(t, "registry-secret", v.Secret.SecretName) match++ From 4aba25f178975a2de1106bccc2d2975856241b34 Mon Sep 17 00:00:00 2001 From: Matthew McNew Date: Thu, 21 Oct 2021 22:49:24 -0500 Subject: [PATCH 4/5] Simplify the construction of the completion container - Always pass in all docker/git/cosign flags --- cmd/completion/main.go | 5 + pkg/apis/build/v1alpha2/build_pod.go | 262 +++++++++++-------- pkg/apis/build/v1alpha2/build_pod_test.go | 299 +++++++++++----------- 3 files changed, 312 insertions(+), 254 deletions(-) diff --git a/cmd/completion/main.go b/cmd/completion/main.go index cd41c1bac..47bb1c528 100644 --- a/cmd/completion/main.go +++ b/cmd/completion/main.go @@ -34,6 +34,8 @@ var ( cosignAnnotations flaghelpers.CredentialsFlags cosignRepositories flaghelpers.CredentialsFlags cosignDockerMediaTypes flaghelpers.CredentialsFlags + basicGitCredentials flaghelpers.CredentialsFlags + sshGitCredentials flaghelpers.CredentialsFlags logger *log.Logger ) @@ -42,6 +44,9 @@ func init() { flag.Var(&dockerCredentials, "basic-docker", "Basic authentication for docker of the form 'secretname=git.domain.com'") flag.Var(&dockerCfgCredentials, "dockercfg", "Docker Cfg credentials in the form of the path to the credential") flag.Var(&dockerConfigCredentials, "dockerconfig", "Docker Config JSON credentials in the form of the path to the credential") + flag.Var(&basicGitCredentials, "basic-git", "Basic authentication for git of the form 'secretname=git.domain.com'") + flag.Var(&sshGitCredentials, "ssh-git", "SSH authentication for git of the form 'secretname=git.domain.com'") + flag.Var(&cosignAnnotations, "cosign-annotations", "Cosign custom signing annotations") flag.Var(&cosignRepositories, "cosign-repositories", "Cosign signing repository of the form 'secretname=registry.example.com/project'") flag.Var(&cosignDockerMediaTypes, "cosign-docker-media-types", "Cosign signing with legacy docker media types of the form 'secretname=1'") diff --git a/pkg/apis/build/v1alpha2/build_pod.go b/pkg/apis/build/v1alpha2/build_pod.go index 1d4a519d4..cdb931c66 100644 --- a/pkg/apis/build/v1alpha2/build_pod.go +++ b/pkg/apis/build/v1alpha2/build_pod.go @@ -19,6 +19,7 @@ import ( const ( SecretTemplateName = "secret-volume-%s" DefaultSecretPathName = "/var/build-secrets/%s" + CosignDefaultSecretPathName = "/var/build-secrets/cosign/%s" BuildLabel = "kpack.io/build" DOCKERSecretAnnotationPrefix = "kpack.io/docker" GITSecretAnnotationPrefix = "kpack.io/git" @@ -175,18 +176,14 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor } secretVolumes, secretVolumeMounts, secretArgs := b.setupSecretVolumesAndArgs(buildContext.Secrets, gitAndDockerSecrets) - cosignVolumes, _, _ := b.setupSecretVolumesAndArgs(buildContext.Secrets, cosignSecrets) - secretVolumes = append(secretVolumes, cosignVolumes...) + cosignVolumes, cosignVolumeMounts, cosignSecretArgs := b.setupCosignVolumes(buildContext.Secrets) + imagePullVolumes, imagePullVolumeMounts, imagePullArgs := b.setupImagePullVolumes(buildContext.ImagePullSecrets) bindingVolumes, bindingVolumeMounts, err := setupBindingVolumesAndMounts(buildContext.Bindings) if err != nil { return nil, err } - imagePullVolumes, imagePullVolumeMounts, imagePullArgs := b.setupImagePullVolumes(buildContext.ImagePullSecrets) - - builderImage := b.Spec.Builder.Image - workspaceVolume := corev1.VolumeMount{ Name: sourceVolume.Name, MountPath: sourceVolume.MountPath, @@ -226,8 +223,27 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor // If the build fails, don't restart it. RestartPolicy: corev1.RestartPolicyNever, Containers: steps(func(step func(corev1.Container, ...stepModifier)) { - step(b.completionContainer(buildContext.Secrets, images.completion(buildContext.os())), - ifWindows(buildContext.os(), addNetworkWaitLauncherVolume(), useNetworkWaitLauncher(dnsProbeHost))...) + step(corev1.Container{ + Name: "completion", + Image: images.completion(buildContext.os()), + Command: []string{"/cnb/process/web"}, + Args: args( + b.notaryArgs(), + secretArgs, + cosignSecretArgs, + b.cosignArgs(), + ), + Resources: b.Spec.Resources, + VolumeMounts: volumeMounts( + secretVolumeMounts, + cosignVolumeMounts, + []corev1.VolumeMount{ + reportVolume, + notaryV1Volume, + }, + ), + ImagePullPolicy: corev1.PullIfNotPresent, + }, ifWindows(buildContext.os(), addNetworkWaitLauncherVolume(), useNetworkWaitLauncher(dnsProbeHost))...) }), SecurityContext: podSecurityContext(buildContext.BuildPodBuilderConfig), InitContainers: steps(func(step func(corev1.Container, ...stepModifier)) { @@ -270,13 +286,16 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor ), ImagePullPolicy: corev1.PullIfNotPresent, WorkingDir: "/workspace", - VolumeMounts: append( - append(secretVolumeMounts, imagePullVolumeMounts...), - registrySourcePullSecretsVolume, - platformVolume, - sourceVolume, - homeVolume, - projectMetadataVolume, + VolumeMounts: volumeMounts( + secretVolumeMounts, + imagePullVolumeMounts, + []corev1.VolumeMount{ + registrySourcePullSecretsVolume, + platformVolume, + sourceVolume, + homeVolume, + projectMetadataVolume, + }, ), }, ifWindows(buildContext.os(), addNetworkWaitLauncherVolume())..., @@ -284,7 +303,7 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor step( corev1.Container{ Name: "detect", - Image: builderImage, + Image: b.Spec.Builder.Image, Command: []string{"/cnb/lifecycle/detector"}, Resources: b.Spec.Resources, Args: []string{ @@ -292,11 +311,11 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor "-group=/layers/group.toml", "-plan=/layers/plan.toml", }, - VolumeMounts: append([]corev1.VolumeMount{ + VolumeMounts: volumeMounts([]corev1.VolumeMount{ layersVolume, platformVolume, workspaceVolume, - }, bindingVolumeMounts...), + }, bindingVolumeMounts), ImagePullPolicy: corev1.PullIfNotPresent, Env: []corev1.EnvVar{ { @@ -310,7 +329,7 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor step( corev1.Container{ Name: "analyze", - Image: builderImage, + Image: b.Spec.Builder.Image, Command: []string{"/cnb/lifecycle/analyzer"}, Resources: b.Spec.Resources, Args: args([]string{ @@ -325,11 +344,11 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor return []string{b.Tag()} }(), ), - VolumeMounts: append([]corev1.VolumeMount{ + VolumeMounts: volumeMounts([]corev1.VolumeMount{ layersVolume, workspaceVolume, homeVolume, - }, cacheVolumes...), + }, cacheVolumes), Env: []corev1.EnvVar{ homeEnv, { @@ -349,17 +368,17 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor step( corev1.Container{ Name: "restore", - Image: builderImage, + Image: b.Spec.Builder.Image, Command: []string{"/cnb/lifecycle/restorer"}, Resources: b.Spec.Resources, Args: args([]string{ "-group=/layers/group.toml", "-layers=/layers", }, genericCacheArgs), - VolumeMounts: append([]corev1.VolumeMount{ + VolumeMounts: volumeMounts([]corev1.VolumeMount{ layersVolume, homeVolume, - }, cacheVolumes...), + }, cacheVolumes), Env: []corev1.EnvVar{ { Name: platformAPIEnvVar, @@ -373,7 +392,7 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor step( corev1.Container{ Name: "build", - Image: builderImage, + Image: b.Spec.Builder.Image, Command: []string{"/cnb/lifecycle/builder"}, Resources: b.Spec.Resources, Args: []string{ @@ -382,11 +401,11 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor "-group=/layers/group.toml", "-plan=/layers/plan.toml", }, - VolumeMounts: append([]corev1.VolumeMount{ + VolumeMounts: volumeMounts([]corev1.VolumeMount{ layersVolume, platformVolume, workspaceVolume, - }, bindingVolumeMounts...), + }, bindingVolumeMounts), ImagePullPolicy: corev1.PullIfNotPresent, Env: []corev1.EnvVar{ { @@ -401,7 +420,7 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor step( corev1.Container{ Name: "export", - Image: builderImage, + Image: b.Spec.Builder.Image, Command: []string{"/cnb/lifecycle/exporter"}, Resources: b.Spec.Resources, Args: args([]string{ @@ -429,12 +448,12 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor }(), b.Spec.Tags), - VolumeMounts: append([]corev1.VolumeMount{ + VolumeMounts: volumeMounts([]corev1.VolumeMount{ layersVolume, workspaceVolume, homeVolume, reportVolume, - }, cacheVolumes...), + }, cacheVolumes), Env: []corev1.EnvVar{ homeEnv, { @@ -459,6 +478,7 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor SchedulerName: b.Spec.SchedulerName, Volumes: volumes( secretVolumes, + cosignVolumes, imagePullVolumes, b.cacheVolume(buildContext.os()), []corev1.Volume{ @@ -582,10 +602,30 @@ func (b *Build) notarySecretVolume() corev1.Volume { } } +func (b *Build) notaryArgs() []string { + if b.NotaryV1Config() == nil { + return nil + } + return []string{"-notary-v1-url=" + b.NotaryV1Config().URL} +} + +func (b *Build) cosignArgs() []string { + args := []string{ + fmt.Sprintf("-cosign-annotations=buildTimestamp=%s", b.ObjectMeta.CreationTimestamp.Format("20060102.150405")), + fmt.Sprintf("-cosign-annotations=buildNumber=%s", b.Labels[BuildNumberLabel]), + } + + if b.Spec.Cosign != nil && b.Spec.Cosign.Annotations != nil { + for _, annotation := range b.Spec.Cosign.Annotations { + args = append(args, fmt.Sprintf("-cosign-annotations=%s=%s", annotation.Name, annotation.Value)) + } + } + return args +} + func (b *Build) rebasePod(buildContext BuildContext, images BuildPodImages) (*corev1.Pod, error) { secretVolumes, secretVolumeMounts, secretArgs := b.setupSecretVolumesAndArgs(buildContext.Secrets, dockerSecrets) - cosignVolumes, _, _ := b.setupSecretVolumesAndArgs(buildContext.Secrets, cosignSecrets) - secretVolumes = append(secretVolumes, cosignVolumes...) + cosignVolumes, cosignVolumeMounts, cosignSecretArgs := b.setupCosignVolumes(buildContext.Secrets) imagePullVolumes, imagePullVolumeMounts, imagePullArgs := b.setupImagePullVolumes(buildContext.ImagePullSecrets) @@ -608,20 +648,44 @@ func (b *Build) rebasePod(buildContext BuildContext, images BuildPodImages) (*co Affinity: b.Spec.Affinity, RuntimeClassName: b.Spec.RuntimeClassName, SchedulerName: b.Spec.SchedulerName, - Volumes: append( - append(secretVolumes, imagePullVolumes...), - corev1.Volume{ - Name: reportDirName, - VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, + Volumes: volumes( + secretVolumes, + cosignVolumes, + imagePullVolumes, + []corev1.Volume{ + { + Name: reportDirName, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, }, + b.notarySecretVolume(), }, - b.notarySecretVolume(), ), RestartPolicy: corev1.RestartPolicyNever, - Containers: steps(func(step func(corev1.Container, ...stepModifier)) { - step(b.completionContainer(buildContext.Secrets, images.CompletionImage)) - }), + Containers: []corev1.Container{ + { + Name: "completion", + Image: images.completion(buildContext.os()), + Command: []string{"/cnb/process/web"}, + Args: args( + b.notaryArgs(), + secretArgs, + b.cosignArgs(), + cosignSecretArgs, + ), + Resources: b.Spec.Resources, + VolumeMounts: volumeMounts( + secretVolumeMounts, + cosignVolumeMounts, + []corev1.VolumeMount{ + reportVolume, + notaryV1Volume, + }, + ), + ImagePullPolicy: corev1.PullIfNotPresent, + }, + }, InitContainers: []corev1.Container{ { Name: "rebase", @@ -647,9 +711,13 @@ func (b *Build) rebasePod(buildContext BuildContext, images BuildPodImages) (*co }, ImagePullPolicy: corev1.PullIfNotPresent, WorkingDir: "/workspace", - VolumeMounts: append( - append(secretVolumeMounts, imagePullVolumeMounts...), - reportVolume), + VolumeMounts: volumeMounts( + secretVolumeMounts, + imagePullVolumeMounts, + []corev1.VolumeMount{ + reportVolume, + }, + ), }, }, }, @@ -678,10 +746,6 @@ func dockerSecrets(secret corev1.Secret) bool { return secret.Annotations[DOCKERSecretAnnotationPrefix] != "" || secret.Type == corev1.SecretTypeDockercfg || secret.Type == corev1.SecretTypeDockerConfigJson } -func cosignSecrets(secret corev1.Secret) bool { - return string(secret.Data[COSIGNSecretDataCosignKey]) != "" -} - func (b *Build) setupSecretVolumesAndArgs(secrets []corev1.Secret, filter func(secret corev1.Secret) bool) ([]corev1.Volume, []corev1.VolumeMount, []string) { var ( volumes []corev1.Volume @@ -689,8 +753,6 @@ func (b *Build) setupSecretVolumesAndArgs(secrets []corev1.Secret, filter func(s args []string ) for _, secret := range secrets { - secretPathName := DefaultSecretPathName - switch { case !filter(secret): continue @@ -707,10 +769,6 @@ func (b *Build) setupSecretVolumesAndArgs(secrets []corev1.Secret, filter func(s case secret.Type == corev1.SecretTypeSSHAuth: annotatedUrl := secret.Annotations[GITSecretAnnotationPrefix] args = append(args, fmt.Sprintf("-ssh-%s=%s=%s", "git", secret.Name, annotatedUrl)) - case string(secret.Data[COSIGNSecretDataCosignKey]) != "": - cosignArgs := cosignSecretArgs(secret) - args = append(args, cosignArgs...) - secretPathName = fmt.Sprintf(DefaultSecretPathName, "cosign/%s") default: //ignoring secret continue @@ -729,7 +787,7 @@ func (b *Build) setupSecretVolumesAndArgs(secrets []corev1.Secret, filter func(s volumeMounts = append(volumeMounts, corev1.VolumeMount{ Name: volumeName, - MountPath: fmt.Sprintf(secretPathName, secret.Name), + MountPath: fmt.Sprintf(DefaultSecretPathName, secret.Name), }) } @@ -764,6 +822,40 @@ func (b *Build) setupImagePullVolumes(secrets []corev1.LocalObjectReference) ([] return volumes, volumeMounts, args } +func (b *Build) setupCosignVolumes(secrets []corev1.Secret) ([]corev1.Volume, []corev1.VolumeMount, []string) { + var ( + volumes []corev1.Volume + volumeMounts []corev1.VolumeMount + args []string + ) + for _, secret := range secrets { + if string(secret.Data[COSIGNSecretDataCosignKey]) == "" { + continue + } + + cosignArgs := cosignSecretArgs(secret) + args = append(args, cosignArgs...) + + volumeName := fmt.Sprintf(SecretTemplateName, secret.Name) + + volumes = append(volumes, corev1.Volume{ + Name: volumeName, + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: secret.Name, + }, + }, + }) + + volumeMounts = append(volumeMounts, corev1.VolumeMount{ + Name: volumeName, + MountPath: fmt.Sprintf(CosignDefaultSecretPathName, secret.Name), + }) + } + + return volumes, volumeMounts, args +} + var ( highestSupportedPlatformVersion = semver.MustParse("0.6") lowestSupportedPlatformVersion = semver.MustParse("0.3") @@ -897,6 +989,14 @@ func volumes(volumes ...[]corev1.Volume) []corev1.Volume { return combined } +func volumeMounts(volumes ...[]corev1.VolumeMount) []corev1.VolumeMount { + var combined []corev1.VolumeMount + for _, v := range volumes { + combined = append(combined, v...) + } + return combined +} + func steps(f func(step func(corev1.Container, ...stepModifier))) []corev1.Container { containers := make([]corev1.Container, 0, 7) @@ -909,56 +1009,8 @@ func steps(f func(step func(corev1.Container, ...stepModifier))) []corev1.Contai return containers } -func (b *Build) completionContainer(secrets []corev1.Secret, completionImage string) corev1.Container { - volumeMounts := []corev1.VolumeMount{ - reportVolume, - } - args := []string{} - - _, cosignVolumeMounts, cosignArgs := b.setupSecretVolumesAndArgs(secrets, cosignSecrets) - hasCosign := len(cosignVolumeMounts) > 0 - - if b.NotaryV1Config() != nil { - volumeMounts = append(volumeMounts, notaryV1Volume) - args = append(args, "-notary-v1-url="+b.NotaryV1Config().URL) - } - - if b.NotaryV1Config() != nil || hasCosign { - _, secretVolumeMounts, secretArgs := b.setupSecretVolumesAndArgs(secrets, dockerSecrets) - volumeMounts = append(volumeMounts, secretVolumeMounts...) - args = append(args, secretArgs...) - } - - if hasCosign { - volumeMounts = append(volumeMounts, cosignVolumeMounts...) - args = append( - args, - "-cosign-annotations=buildTimestamp="+b.ObjectMeta.CreationTimestamp.Format("20060102.150405"), - "-cosign-annotations=buildNumber="+b.Labels[BuildNumberLabel], - ) - - if b.Spec.Cosign != nil && b.Spec.Cosign.Annotations != nil { - for _, annotation := range b.Spec.Cosign.Annotations { - args = append(args, fmt.Sprintf("-cosign-annotations=%s=%s", annotation.Name, annotation.Value)) - } - } - - args = append(args, cosignArgs...) - } - - return corev1.Container{ - Name: "completion", - Image: completionImage, - Command: []string{"/cnb/process/web"}, - Args: args, - Resources: b.Spec.Resources, - VolumeMounts: volumeMounts, - ImagePullPolicy: corev1.PullIfNotPresent, - } -} - func cosignSecretArgs(secret corev1.Secret) []string { - cosignArgs := make([]string, 0) + var cosignArgs []string if cosignRepository := secret.ObjectMeta.Annotations[COSIGNRespositoryAnnotationPrefix]; cosignRepository != "" { cosignArgs = append(cosignArgs, fmt.Sprintf("-cosign-repositories=%s=%s", secret.Name, cosignRepository)) } diff --git a/pkg/apis/build/v1alpha2/build_pod_test.go b/pkg/apis/build/v1alpha2/build_pod_test.go index 0e6eb72ac..393af1aaa 100644 --- a/pkg/apis/build/v1alpha2/build_pod_test.go +++ b/pkg/apis/build/v1alpha2/build_pod_test.go @@ -1126,154 +1126,136 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }, }) - require.Equal(t, corev1.PodSpec{ - ServiceAccountName: build.Spec.ServiceAccountName, - NodeSelector: map[string]string{ - "kubernetes.io/os": "linux", - "foo": "bar", - }, - Tolerations: build.Spec.Tolerations, - Affinity: build.Spec.Affinity, - Volumes: []corev1.Volume{ - { - Name: "secret-volume-docker-secret-1", - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: "docker-secret-1", - }, + require.Equal(t, build.Spec.ServiceAccountName, pod.Spec.ServiceAccountName) + require.Equal(t, build.Spec.Tolerations, pod.Spec.Tolerations) + require.Equal(t, build.Spec.Affinity, pod.Spec.Affinity) + require.Equal(t, build.Spec.NodeSelector, map[string]string{ + "kubernetes.io/os": "linux", + "foo": "bar", + }) + require.Equal(t, pod.Spec.Volumes, []corev1.Volume{ + { + Name: "secret-volume-docker-secret-1", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "docker-secret-1", }, }, - { - Name: "secret-volume-docker-secret-2", - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: "docker-secret-2", - }, + }, + { + Name: "secret-volume-docker-secret-2", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "docker-secret-2", }, }, - { - Name: "secret-volume-docker-secret-3", - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: "docker-secret-3", - }, + }, + { + Name: "secret-volume-docker-secret-3", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "docker-secret-3", }, }, - { - Name: "secret-volume-image-pull-1", - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: "image-pull-1", - }, + }, + { + Name: "secret-volume-image-pull-1", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "image-pull-1", }, }, - { - Name: "secret-volume-image-pull-2", - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: "image-pull-2", - }, + }, + { + Name: "secret-volume-image-pull-2", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "image-pull-2", }, }, - { - Name: "secret-volume-builder-pull-secret", - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: "builder-pull-secret", - }, + }, + { + Name: "secret-volume-builder-pull-secret", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "builder-pull-secret", }, }, - { - Name: "report-dir", - VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, - }, + }, + { + Name: "report-dir", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, }, - { - Name: "notary-dir", - VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, - }, + }, + { + Name: "notary-dir", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, }, }, - RestartPolicy: corev1.RestartPolicyNever, - Containers: []corev1.Container{ - { - Name: "completion", - Image: config.CompletionImage, - Command: []string{"/cnb/process/web"}, - Args: []string{}, - ImagePullPolicy: corev1.PullIfNotPresent, - Resources: build.Spec.Resources, - VolumeMounts: []corev1.VolumeMount{ - { - Name: "report-dir", - MountPath: "/var/report", - }, + }) + + require.Equal(t, []corev1.Container{ + { + Name: "rebase", + Image: config.RebaseImage, + Resources: build.Spec.Resources, + Args: []string{ + "--run-image", + "builderregistry.io/run", + "--last-built-image", + build.Spec.LastBuild.Image, + "--report", + "/var/report/report.toml", + "-basic-docker=docker-secret-1=acr.io", + "-dockerconfig=docker-secret-2", + "-dockercfg=docker-secret-3", + "-imagepull=image-pull-1", + "-imagepull=image-pull-2", + "-imagepull=builder-pull-secret", + "someimage/name", "someimage/name:tag2", "someimage/name:tag3", + }, + Env: []corev1.EnvVar{ + { + Name: "BUILD_CHANGES", + Value: "some-stack-change", }, }, - }, - InitContainers: []corev1.Container{ - { - Name: "rebase", - Image: config.RebaseImage, - Resources: build.Spec.Resources, - Args: []string{ - "--run-image", - "builderregistry.io/run", - "--last-built-image", - build.Spec.LastBuild.Image, - "--report", - "/var/report/report.toml", - "-basic-docker=docker-secret-1=acr.io", - "-dockerconfig=docker-secret-2", - "-dockercfg=docker-secret-3", - "-imagepull=image-pull-1", - "-imagepull=image-pull-2", - "-imagepull=builder-pull-secret", - "someimage/name", "someimage/name:tag2", "someimage/name:tag3", + ImagePullPolicy: corev1.PullIfNotPresent, + WorkingDir: "/workspace", + VolumeMounts: []corev1.VolumeMount{ + { + Name: "secret-volume-docker-secret-1", + MountPath: "/var/build-secrets/docker-secret-1", }, - Env: []corev1.EnvVar{ - { - Name: "BUILD_CHANGES", - Value: "some-stack-change", - }, + { + Name: "secret-volume-docker-secret-2", + MountPath: "/var/build-secrets/docker-secret-2", }, - ImagePullPolicy: corev1.PullIfNotPresent, - WorkingDir: "/workspace", - VolumeMounts: []corev1.VolumeMount{ - { - Name: "secret-volume-docker-secret-1", - MountPath: "/var/build-secrets/docker-secret-1", - }, - { - Name: "secret-volume-docker-secret-2", - MountPath: "/var/build-secrets/docker-secret-2", - }, - { - Name: "secret-volume-docker-secret-3", - MountPath: "/var/build-secrets/docker-secret-3", - }, - { - Name: "secret-volume-image-pull-1", - MountPath: "/var/build-secrets/image-pull-1", - }, - { - Name: "secret-volume-image-pull-2", - MountPath: "/var/build-secrets/image-pull-2", - }, - { - Name: "secret-volume-builder-pull-secret", - MountPath: "/var/build-secrets/builder-pull-secret", - }, - { - Name: "report-dir", - MountPath: "/var/report", - }, + { + Name: "secret-volume-docker-secret-3", + MountPath: "/var/build-secrets/docker-secret-3", + }, + { + Name: "secret-volume-image-pull-1", + MountPath: "/var/build-secrets/image-pull-1", + }, + { + Name: "secret-volume-image-pull-2", + MountPath: "/var/build-secrets/image-pull-2", + }, + { + Name: "secret-volume-builder-pull-secret", + MountPath: "/var/build-secrets/builder-pull-secret", + }, + { + Name: "report-dir", + MountPath: "/var/report", }, }, }, - }, pod.Spec) + }, pod.Spec.InitContainers) }) when("cosign secrets are present on the build", func() { @@ -1377,6 +1359,8 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { "-basic-docker=docker-secret-1=acr.io", "-dockerconfig=docker-secret-2", "-dockercfg=docker-secret-3", + "-cosign-annotations=buildTimestamp=19440606.133000", + "-cosign-annotations=buildNumber=12", }, pod.Spec.Containers[0].Args, ) @@ -1427,14 +1411,14 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { assert.Equal(t, "/cnb/process/web", pod.Spec.Containers[0].Command[0]) - require.Equal(t, + require.Subset(t, + pod.Spec.Containers[0].Args, []string{ "-notary-v1-url=some-notary-url", "-basic-docker=docker-secret-1=acr.io", "-dockerconfig=docker-secret-2", "-dockercfg=docker-secret-3", }, - pod.Spec.Containers[0].Args, ) require.Contains(t, pod.Spec.Containers[0].VolumeMounts, corev1.VolumeMount{ @@ -1460,16 +1444,14 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { }) when("cosign secrets and a notary config are present on the build", func() { - it.Before(func() { - build.Spec.Notary = &corev1alpha1.NotaryConfig{ - V1: &corev1alpha1.NotaryV1Config{ - URL: "some-notary-url", - SecretRef: corev1alpha1.NotarySecretRef{ - Name: "some-notary-secret", - }, + build.Spec.Notary = &corev1alpha1.NotaryConfig{ + V1: &corev1alpha1.NotaryV1Config{ + URL: "some-notary-url", + SecretRef: corev1alpha1.NotarySecretRef{ + Name: "some-notary-secret", }, - } - }) + }, + } it("skips invalid secrets", func() { buildContext.Secrets = append(secrets, cosignInvalidSecrets...) @@ -1624,15 +1606,17 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { require.Equal(t, []string{ + "-basic-git=git-secret-1=https://github.com", + "-ssh-git=git-secret-2=https://bitbucket.com", "-basic-docker=docker-secret-1=acr.io", "-dockerconfig=docker-secret-2", "-dockercfg=docker-secret-3", - "-cosign-annotations=buildTimestamp=19440606.133000", - "-cosign-annotations=buildNumber=12", "-cosign-repositories=cosign-secret-1=testRepository.com/fake-project-1", "-cosign-docker-media-types=cosign-secret-1=1", "-cosign-repositories=cosign-secret-no-password-1=testRepository.com/fake-project-2", "-cosign-docker-media-types=cosign-secret-no-password-2=1", + "-cosign-annotations=buildTimestamp=19440606.133000", + "-cosign-annotations=buildNumber=12", }, pod.Spec.Containers[0].Args, ) @@ -1650,16 +1634,18 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { require.Equal(t, []string{ + "-basic-git=git-secret-1=https://github.com", + "-ssh-git=git-secret-2=https://bitbucket.com", "-basic-docker=docker-secret-1=acr.io", "-dockerconfig=docker-secret-2", "-dockercfg=docker-secret-3", - "-cosign-annotations=buildTimestamp=19440606.133000", - "-cosign-annotations=buildNumber=12", - "-cosign-annotations=customAnnotationKey=customAnnotationValue", "-cosign-repositories=cosign-secret-1=testRepository.com/fake-project-1", "-cosign-docker-media-types=cosign-secret-1=1", "-cosign-repositories=cosign-secret-no-password-1=testRepository.com/fake-project-2", "-cosign-docker-media-types=cosign-secret-no-password-2=1", + "-cosign-annotations=buildTimestamp=19440606.133000", + "-cosign-annotations=buildNumber=12", + "-cosign-annotations=customAnnotationKey=customAnnotationValue", }, pod.Spec.Containers[0].Args, ) @@ -1689,14 +1675,14 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { assert.Equal(t, "/cnb/process/web", pod.Spec.Containers[0].Command[0]) - require.Equal(t, + require.Subset(t, + pod.Spec.Containers[0].Args, []string{ "-notary-v1-url=some-notary-url", "-basic-docker=docker-secret-1=acr.io", "-dockerconfig=docker-secret-2", "-dockercfg=docker-secret-3", }, - pod.Spec.Containers[0].Args, ) require.Contains(t, pod.Spec.Containers[0].VolumeMounts, corev1.VolumeMount{ @@ -1794,15 +1780,17 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { require.Equal(t, []string{ "-notary-v1-url=some-notary-url", + "-basic-git=git-secret-1=https://github.com", + "-ssh-git=git-secret-2=https://bitbucket.com", "-basic-docker=docker-secret-1=acr.io", "-dockerconfig=docker-secret-2", "-dockercfg=docker-secret-3", - "-cosign-annotations=buildTimestamp=19440606.133000", - "-cosign-annotations=buildNumber=12", "-cosign-repositories=cosign-secret-1=testRepository.com/fake-project-1", "-cosign-docker-media-types=cosign-secret-1=1", "-cosign-repositories=cosign-secret-no-password-1=testRepository.com/fake-project-2", "-cosign-docker-media-types=cosign-secret-no-password-2=1", + "-cosign-annotations=buildTimestamp=19440606.133000", + "-cosign-annotations=buildNumber=12", }, pod.Spec.Containers[0].Args, ) @@ -1821,16 +1809,18 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { require.Equal(t, []string{ "-notary-v1-url=some-notary-url", + "-basic-git=git-secret-1=https://github.com", + "-ssh-git=git-secret-2=https://bitbucket.com", "-basic-docker=docker-secret-1=acr.io", "-dockerconfig=docker-secret-2", "-dockercfg=docker-secret-3", - "-cosign-annotations=buildTimestamp=19440606.133000", - "-cosign-annotations=buildNumber=12", - "-cosign-annotations=customAnnotationKey=customAnnotationValue", "-cosign-repositories=cosign-secret-1=testRepository.com/fake-project-1", "-cosign-docker-media-types=cosign-secret-1=1", "-cosign-repositories=cosign-secret-no-password-1=testRepository.com/fake-project-2", "-cosign-docker-media-types=cosign-secret-no-password-2=1", + "-cosign-annotations=buildTimestamp=19440606.133000", + "-cosign-annotations=buildNumber=12", + "-cosign-annotations=customAnnotationKey=customAnnotationValue", }, pod.Spec.Containers[0].Args, ) @@ -2100,9 +2090,13 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { "--", "/cnb/process/web", "-notary-v1-url=some-notary-server", + "-basic-git=git-secret-1=https://github.com", + "-ssh-git=git-secret-2=https://bitbucket.com", "-basic-docker=docker-secret-1=acr.io", "-dockerconfig=docker-secret-2", "-dockercfg=docker-secret-3", + "-cosign-annotations=buildTimestamp=19440606.133000", + "-cosign-annotations=buildNumber=12", }, completionContainer.Args) assert.Equal(t, "/networkWait/network-wait-launcher", completionContainer.Command[0]) @@ -2133,6 +2127,13 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { dnsProbeHost, "--", "/cnb/process/web", + "-basic-git=git-secret-1=https://github.com", + "-ssh-git=git-secret-2=https://bitbucket.com", + "-basic-docker=docker-secret-1=acr.io", + "-dockerconfig=docker-secret-2", + "-dockercfg=docker-secret-3", + "-cosign-annotations=buildTimestamp=19440606.133000", + "-cosign-annotations=buildNumber=12", }, completionContainer.Args) }) From d2816c91c879c097a684c0462eb8b4467374b067 Mon Sep 17 00:00:00 2001 From: Matthew McNew Date: Fri, 22 Oct 2021 09:22:25 -0500 Subject: [PATCH 5/5] Deduplicate builder imagePullSecrets from serviceAccountImagePullSecrets --- pkg/apis/build/v1alpha2/build_pod.go | 17 ++++++++++++++++- pkg/apis/build/v1alpha2/build_pod_test.go | 15 +++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/pkg/apis/build/v1alpha2/build_pod.go b/pkg/apis/build/v1alpha2/build_pod.go index cdb931c66..4da45ed48 100644 --- a/pkg/apis/build/v1alpha2/build_pod.go +++ b/pkg/apis/build/v1alpha2/build_pod.go @@ -800,7 +800,7 @@ func (b *Build) setupImagePullVolumes(secrets []corev1.LocalObjectReference) ([] volumeMounts []corev1.VolumeMount args []string ) - for _, secret := range append(secrets, b.Spec.Builder.ImagePullSecrets...) { + for _, secret := range deduplicate(secrets, b.Spec.Builder.ImagePullSecrets) { args = append(args, fmt.Sprintf("-imagepull=%s", secret.Name)) volumeName := fmt.Sprintf(SecretTemplateName, secret.Name) @@ -997,6 +997,21 @@ func volumeMounts(volumes ...[]corev1.VolumeMount) []corev1.VolumeMount { return combined } +func deduplicate(lists ...[]corev1.LocalObjectReference) []corev1.LocalObjectReference { + names := map[string]struct{}{} + var deduplicated []corev1.LocalObjectReference + + for _, list := range lists { + for _, entry := range list { + if _, ok := names[entry.Name]; !ok { + deduplicated = append(deduplicated, entry) + } + names[entry.Name] = struct{}{} + } + } + return deduplicated +} + func steps(f func(step func(corev1.Container, ...stepModifier))) []corev1.Container { containers := make([]corev1.Container, 0, 7) diff --git a/pkg/apis/build/v1alpha2/build_pod_test.go b/pkg/apis/build/v1alpha2/build_pod_test.go index 393af1aaa..61889088e 100644 --- a/pkg/apis/build/v1alpha2/build_pod_test.go +++ b/pkg/apis/build/v1alpha2/build_pod_test.go @@ -1058,6 +1058,21 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { assertSecretNotPresent(t, pod, "random-secret-1") }) + it("deduplicates builder imagepullSecrets from service account image pull secrets", func() { + buildContext.ImagePullSecrets = []corev1.LocalObjectReference{{Name: "duplicated-secret"}} + build.Spec.Builder.ImagePullSecrets = []corev1.LocalObjectReference{{Name: "duplicated-secret"}} + + pod, err := build.BuildPod(config, buildContext) + require.NoError(t, err) + + volumeNames := map[string]struct{}{} + for _, v := range pod.Spec.Volumes { + volumeNames[v.Name] = struct{}{} + } + + require.Len(t, pod.Spec.Volumes, len(volumeNames)) + }) + it("attach image pull secrets to pod", func() { pod, err := build.BuildPod(config, buildContext) require.NoError(t, err)