Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions pkg/build/buildapihelpers/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import (

const (
// buildPodSuffix is the suffix used to append to a build pod name given a build name
buildPodSuffix = "build"
caConfigMapSuffix = "ca"
buildPodSuffix = "build"
caConfigMapSuffix = "ca"
registryConfConfigMapSuffix = "registry-conf"
)

// GetBuildPodName returns name of the build pod.
Expand All @@ -24,6 +25,12 @@ func GetBuildCAConfigMapName(build *buildv1.Build) string {
return apihelpers.GetConfigMapName(build.Name, caConfigMapSuffix)
}

// GetBuildRegistryConfigMapName returns the name of the ConfigMap containing the build's
// registry configuration.
func GetBuildRegistryConfigMapName(build *buildv1.Build) string {
return apihelpers.GetConfigMapName(build.Name, registryConfConfigMapSuffix)
}

func StrategyType(strategy buildv1.BuildStrategy) string {
switch {
case strategy.DockerStrategy != nil:
Expand Down
69 changes: 57 additions & 12 deletions pkg/build/controller/build/build_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ type BuildController struct {
recorder record.EventRecorder
additionalTrustedCAPath string
additionalTrustedCAData []byte
registryConfData string
}

// BuildControllerParams is the set of parameters needed to
Expand Down Expand Up @@ -957,7 +958,7 @@ func (bc *BuildController) createBuildPod(build *buildv1.Build) (*buildUpdate, e
}
glog.V(4).Infof("Recognised pod %s/%s as belonging to build %s", build.Namespace, buildPod.Name, buildDesc(build))
// Check if the existing pod has the CA ConfigMap properly attached
hasCAMap, err := bc.findBuildCAConfigMap(build, existingPod)
hasCAMap, err := bc.findOwnedConfigMap(existingPod, build.Namespace, buildapihelpers.GetBuildCAConfigMapName(build))
if err != nil {
return update, fmt.Errorf("could not find certificate authority for build: %v", err)
}
Expand All @@ -968,6 +969,17 @@ func (bc *BuildController) createBuildPod(build *buildv1.Build) (*buildUpdate, e
return update, err
}
}
hasRegistryConf, err := bc.findOwnedConfigMap(existingPod, build.Namespace, buildapihelpers.GetBuildRegistryConfigMapName(build))
if err != nil {
return update, fmt.Errorf("could not find registry config for build: %v", err)
}
if !hasRegistryConf {
// Create the registry config ConfigMap to mount the regsitry config to the existing build pod
update, err = bc.createBuildRegistryConfConfigMap(build, existingPod, update)
if err != nil {
return update, err
}
}

} else {
glog.V(4).Infof("Created pod %s/%s for build %s", build.Namespace, buildPod.Name, buildDesc(build))
Expand All @@ -976,6 +988,11 @@ func (bc *BuildController) createBuildPod(build *buildv1.Build) (*buildUpdate, e
if err != nil {
return update, err
}
// Create the registry config ConfigMap to mount the registry configuration into the build pod
update, err = bc.createBuildRegistryConfConfigMap(build, pod, update)
if err != nil {
return nil, err
}
}

update = transitionToPhase(buildv1.BuildPhasePending, "", "")
Expand Down Expand Up @@ -1442,7 +1459,7 @@ func (bc *BuildController) createBuildCAConfigMapSpec(build *buildv1.Build, buil
"service.alpha.openshift.io/inject-cabundle": "true",
},
OwnerReferences: []metav1.OwnerReference{
makeBuildCAOwnerRef(buildPod),
makeBuildPodOwnerRef(buildPod),
},
},
}
Expand All @@ -1467,22 +1484,50 @@ func (bc *BuildController) readBuildCAData() ([]byte, error) {
return pemData, nil
}

// findBuildCAConfigMap finds the ConfigMap containing the certificate authorities for the build.
// The ConfigMap must exist and contain an owner reference to the build pod to be valid.
func (bc *BuildController) findBuildCAConfigMap(build *buildv1.Build, buildPod *corev1.Pod) (bool, error) {
name := buildapihelpers.GetBuildCAConfigMapName(build)
cm, err := bc.configMapClient.ConfigMaps(build.Namespace).Get(name, metav1.GetOptions{})
// findOwnedConfigMap finds the ConfigMap with the given name and namespace, and owned by the provided pod.
func (bc *BuildController) findOwnedConfigMap(owner *corev1.Pod, namespace string, name string) (bool, error) {
cm, err := bc.configMapClient.ConfigMaps(namespace).Get(name, metav1.GetOptions{})
if err != nil && errors.IsNotFound(err) {
return false, nil
} else if err != nil {
return false, err
}
if hasRef := hasBuildCAOwnerRef(buildPod, cm); !hasRef {
return true, fmt.Errorf("build CA configMap %s is not owned by build pod %s", cm.Name, buildPod.Name)
if hasRef := hasBuildPodOwnerRef(owner, cm); !hasRef {
return true, fmt.Errorf("configMap %s/%s is not owned by build pod %s/%s", cm.Namespace, cm.Name, owner.Namespace, owner.Name)
}
return true, nil
}

func (bc *BuildController) createBuildRegistryConfConfigMap(build *buildv1.Build, buildPod *corev1.Pod, update *buildUpdate) (*buildUpdate, error) {
configMapSpec := bc.createBuildRegistryConfigMapSpec(build, buildPod)
configMap, err := bc.configMapClient.ConfigMaps(build.Namespace).Create(configMapSpec)
if err != nil {
bc.recorder.Eventf(build, corev1.EventTypeWarning, "FailedCreate", "Error creating build registry config configMap: %v", err)
update.setReason("CannotCreateRegistryConfConfigMap")
update.setMessage(buildutil.StatusMessageCannotCreateRegistryConfConfigMap)
return update, fmt.Errorf("failed to create build registry config configMap: %v", err)
}
glog.V(4).Infof("Created registry config configMap %s/%s for build %s", build.Namespace, configMap.Name, buildDesc(build))
return update, nil
}

func (bc *BuildController) createBuildRegistryConfigMapSpec(build *buildv1.Build, buildPod *corev1.Pod) *corev1.ConfigMap {
cm := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: buildapihelpers.GetBuildRegistryConfigMapName(build),
OwnerReferences: []metav1.OwnerReference{
makeBuildPodOwnerRef(buildPod),
},
},
}
if len(bc.registryConfData) > 0 {
cm.Data = map[string]string{
buildutil.RegistryConfKey: bc.registryConfData,
}
}
return cm
}

// isBuildPod returns true if the given pod is a build pod
func isBuildPod(pod *corev1.Pod) bool {
return len(getBuildName(pod)) > 0
Expand Down Expand Up @@ -1604,7 +1649,7 @@ func getBuildName(pod metav1.Object) string {
return pod.GetAnnotations()[buildutil.BuildAnnotation]
}

func makeBuildCAOwnerRef(buildPod *corev1.Pod) metav1.OwnerReference {
func makeBuildPodOwnerRef(buildPod *corev1.Pod) metav1.OwnerReference {
return metav1.OwnerReference{
APIVersion: "v1",
Kind: "Pod",
Expand All @@ -1613,8 +1658,8 @@ func makeBuildCAOwnerRef(buildPod *corev1.Pod) metav1.OwnerReference {
}
}

func hasBuildCAOwnerRef(buildPod *corev1.Pod, caMap *corev1.ConfigMap) bool {
ref := makeBuildCAOwnerRef(buildPod)
func hasBuildPodOwnerRef(buildPod *corev1.Pod, caMap *corev1.ConfigMap) bool {
ref := makeBuildPodOwnerRef(buildPod)
for _, owner := range caMap.OwnerReferences {
if reflect.DeepEqual(ref, owner) {
return true
Expand Down
75 changes: 66 additions & 9 deletions pkg/build/controller/build/build_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,20 @@ import (
)

const (
dummyCA = `
---- BEGIN CERTIFICATE ----
dummyCA = `---- BEGIN CERTIFICATE ----
VEhJUyBJUyBBIEJBRCBDRVJUSUZJQ0FURQo=
---- END CERTIFICATE ----
`

dummyRegistryConf = `registries:
- registry.redhat.io
- quay.io
- docker.io
insecure_registries:
- my.registry.corp.com
block_registries:
- all
`
)

// TestHandleBuild is the main test for build updates through the controller
Expand Down Expand Up @@ -359,12 +368,19 @@ func TestHandleBuild(t *testing.T) {
newPod.UID = types.UID(uuid.New().String())
return true, newPod, nil
})
configMapCreated := false
caConfigMapCreated := false
registryConfigMapCreated := false
kubeClient.(*fake.Clientset).PrependReactor("create", "configmaps",
func(action clientgotesting.Action) (bool, runtime.Object, error) {
configMapCreated = true
newConfigMap := mockBuildCAConfigMap(tc.build, newPod)
return true, newConfigMap, nil
if !caConfigMapCreated {
caConfigMapCreated = true
return true, mockBuildCAConfigMap(tc.build, newPod), nil
}
if !registryConfigMapCreated {
registryConfigMapCreated = true
return true, mockBuilRegistryConfigMap(tc.build, newPod), nil
}
return false, nil, nil
})
bc := newFakeBuildController(buildClient, nil, kubeClient, nil)
defer bc.stop()
Expand All @@ -390,8 +406,11 @@ func TestHandleBuild(t *testing.T) {
if tc.expectPodCreated != podCreated {
t.Errorf("%s: pod created. expected: %v, actual: %v", tc.name, tc.expectPodCreated, podCreated)
}
if tc.expectConfigMapCreated != configMapCreated {
t.Errorf("%s: configMap created. expected: %v, actual:%v", tc.name, tc.expectConfigMapCreated, configMapCreated)
if tc.expectConfigMapCreated != caConfigMapCreated {
t.Errorf("%s: ca configMap created. expected: %v, actual: %v", tc.name, tc.expectConfigMapCreated, caConfigMapCreated)
}
if tc.expectConfigMapCreated != registryConfigMapCreated {
t.Errorf("%s: registry configMap created. expected: %v, actual: %v", tc.name, tc.expectConfigMapCreated, registryConfigMapCreated)
}
if tc.expectUpdate != nil {
if patchedBuild == nil {
Expand Down Expand Up @@ -1158,7 +1177,7 @@ func TestCreateBuildCAConfigMap(t *testing.T) {
if caMap == nil {
t.Error("certificate authority configMap was not created")
}
if !hasBuildCAOwnerRef(pod, caMap) {
if !hasBuildPodOwnerRef(pod, caMap) {
t.Error("build CA configMap is missing owner ref to the build pod")
}
injectAnnotation := "service.alpha.openshift.io/inject-cabundle"
Expand Down Expand Up @@ -1202,6 +1221,27 @@ func TestReadBuildCAData(t *testing.T) {

}

func TestCreateBuildRegistryConfConfigMap(t *testing.T) {
bc := newFakeBuildController(nil, nil, nil, nil)
bc.registryConfData = dummyRegistryConf
defer bc.stop()
build := dockerStrategy(mockBuild(buildv1.BuildPhaseNew, buildv1.BuildOutput{}))
pod := mockBuildPod(build)
caMap := bc.createBuildRegistryConfigMapSpec(build, pod)
if caMap == nil {
t.Error("registry config configMap was not created")
}
if !hasBuildPodOwnerRef(pod, caMap) {
t.Error("registry conf configMap is missing owner ref to the build pod")
}
if _, hasConf := caMap.Data[buildutil.RegistryConfKey]; !hasConf {
t.Errorf("expected registry conf configMap to have key %s", buildutil.RegistryConfKey)
}
if caMap.Data[buildutil.RegistryConfKey] != dummyRegistryConf {
t.Errorf("expected registry conf configMap.%s to contain\n%s\ngot:\n%s", buildutil.RegistryConfKey, dummyCA, caMap.Data[buildutil.RegistryConfKey])
}
}

func mockBuild(phase buildv1.BuildPhase, output buildv1.BuildOutput) *buildv1.Build {
return &buildv1.Build{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -1582,3 +1622,20 @@ func mockBuildCAConfigMap(build *buildv1.Build, pod *corev1.Pod) *corev1.ConfigM
}
return cm
}

func mockBuilRegistryConfigMap(build *buildv1.Build, pod *corev1.Pod) *corev1.ConfigMap {
cm := &corev1.ConfigMap{}
cm.Name = buildapihelpers.GetBuildRegistryConfigMapName(build)
cm.Namespace = build.Namespace
if pod != nil {
pod.OwnerReferences = []metav1.OwnerReference{
{
APIVersion: "v1",
Kind: "Pod",
Name: pod.Name,
UID: pod.UID,
},
}
}
return cm
}
1 change: 1 addition & 0 deletions pkg/build/controller/strategy/custom.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ func (bs *CustomBuildStrategy) CreateBuildPod(build *buildv1.Build, includeAddit
setupAdditionalSecrets(pod, &pod.Spec.Containers[0], build.Spec.Strategy.CustomStrategy.Secrets)
setupContainersConfigs(pod, &pod.Spec.Containers[0])
setupBuildCAs(build, pod, includeAdditionalCA)
setupRegistries(build, pod)
setupContainersStorage(pod, &pod.Spec.Containers[0]) // for unprivileged builds
return pod, nil
}
10 changes: 6 additions & 4 deletions pkg/build/controller/strategy/custom_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,18 @@ func TestCustomCreateBuildPod(t *testing.T) {
// additional secrets
// build-system-configmap
// certificate authorities
// registry config
// container storage
if len(container.VolumeMounts) != 7 {
t.Fatalf("Expected 7 volumes in container, got %d", len(container.VolumeMounts))
if len(container.VolumeMounts) != 8 {
t.Fatalf("Expected 8 volumes in container, got %d", len(container.VolumeMounts))
}
expectedMounts := []string{"/var/run/docker.sock",
DockerPushSecretMountPath,
sourceSecretMountPath,
"secret",
ConfigMapBuildSystemConfigsMountPath,
ConfigMapCertsMountPath,
ConfigMapRegistryConfMountPath,
"/var/lib/containers/storage",
}
for i, expected := range expectedMounts {
Expand All @@ -94,8 +96,8 @@ func TestCustomCreateBuildPod(t *testing.T) {
if !kapihelper.Semantic.DeepEqual(container.Resources, build.Spec.Resources) {
t.Fatalf("Expected actual=expected, %v != %v", container.Resources, build.Spec.Resources)
}
if len(actual.Spec.Volumes) != 7 {
t.Fatalf("Expected 7 volumes in Build pod, got %d", len(actual.Spec.Volumes))
if len(actual.Spec.Volumes) != 8 {
t.Fatalf("Expected 8 volumes in Build pod, got %d", len(actual.Spec.Volumes))
}
buildJSON, _ := runtime.Encode(customBuildEncodingCodecFactory.LegacyCodec(buildv1.GroupVersion), build)
errorCases := map[int][]string{
Expand Down
1 change: 1 addition & 0 deletions pkg/build/controller/strategy/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ func (bs *DockerBuildStrategy) CreateBuildPod(build *buildv1.Build, includeAddit
setupInputConfigMaps(pod, &pod.Spec.Containers[0], build.Spec.Source.ConfigMaps)
setupContainersConfigs(pod, &pod.Spec.Containers[0])
setupBuildCAs(build, pod, includeAdditionalCA)
setupRegistries(build, pod)
setupContainersStorage(pod, &pod.Spec.Containers[0]) // for unprivileged builds
// setupContainersNodeStorage(pod, &pod.Spec.Containers[0]) // for privileged builds
return pod, nil
Expand Down
10 changes: 6 additions & 4 deletions pkg/build/controller/strategy/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,10 @@ func TestDockerCreateBuildPod(t *testing.T) {
// inputconfigmap
// build-system-config
// certificate authorities
// registry config
// container storage
if len(container.VolumeMounts) != 8 {
t.Fatalf("Expected 8 volumes in container, got %d", len(container.VolumeMounts))
if len(container.VolumeMounts) != 9 {
t.Fatalf("Expected 9 volumes in container, got %d", len(container.VolumeMounts))
}
if *actual.Spec.ActiveDeadlineSeconds != 60 {
t.Errorf("Expected ActiveDeadlineSeconds 60, got %d", *actual.Spec.ActiveDeadlineSeconds)
Expand All @@ -84,6 +85,7 @@ func TestDockerCreateBuildPod(t *testing.T) {
filepath.Join(ConfigMapBuildSourceBaseMountPath, "build-config"),
ConfigMapBuildSystemConfigsMountPath,
ConfigMapCertsMountPath,
ConfigMapRegistryConfMountPath,
"/var/lib/containers/storage",
}
for i, expected := range expectedMounts {
Expand All @@ -92,8 +94,8 @@ func TestDockerCreateBuildPod(t *testing.T) {
}
}
// build pod has an extra volume: the git clone source secret
if len(actual.Spec.Volumes) != 9 {
t.Fatalf("Expected 9 volumes in Build pod, got %d", len(actual.Spec.Volumes))
if len(actual.Spec.Volumes) != 10 {
t.Fatalf("Expected 10 volumes in Build pod, got %d", len(actual.Spec.Volumes))
}
if !kapihelper.Semantic.DeepEqual(container.Resources, build.Spec.Resources) {
t.Fatalf("Expected actual=expected, %v != %v", container.Resources, build.Spec.Resources)
Expand Down
1 change: 1 addition & 0 deletions pkg/build/controller/strategy/sti.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ func (bs *SourceBuildStrategy) CreateBuildPod(build *buildv1.Build, includeAddit
setupInputConfigMaps(pod, &pod.Spec.Containers[0], build.Spec.Source.ConfigMaps)
setupContainersConfigs(pod, &pod.Spec.Containers[0])
setupBuildCAs(build, pod, includeAdditionalCA)
setupRegistries(build, pod)
setupContainersStorage(pod, &pod.Spec.Containers[0]) // for unprivileged builds
// setupContainersNodeStorage(pod, &pod.Spec.Containers[0]) // for privileged builds
return pod, nil
Expand Down
10 changes: 6 additions & 4 deletions pkg/build/controller/strategy/sti_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,10 @@ func testSTICreateBuildPod(t *testing.T, rootAllowed bool) {
// inputconfigmap
// build-system-configmap
// certificate authorities
// registry conf
// container storage
if len(container.VolumeMounts) != 8 {
t.Fatalf("Expected 8 volumes in container, got %d %v", len(container.VolumeMounts), container.VolumeMounts)
if len(container.VolumeMounts) != 9 {
t.Fatalf("Expected 9 volumes in container, got %d %v", len(container.VolumeMounts), container.VolumeMounts)
}
expectedMounts := []string{buildutil.BuildWorkDirMount,
DockerPushSecretMountPath,
Expand All @@ -121,6 +122,7 @@ func testSTICreateBuildPod(t *testing.T, rootAllowed bool) {
filepath.Join(ConfigMapBuildSourceBaseMountPath, "configmap"),
ConfigMapBuildSystemConfigsMountPath,
ConfigMapCertsMountPath,
ConfigMapRegistryConfMountPath,
"/var/lib/containers/storage",
}
for i, expected := range expectedMounts {
Expand All @@ -129,8 +131,8 @@ func testSTICreateBuildPod(t *testing.T, rootAllowed bool) {
}
}
// build pod has an extra volume: the git clone source secret
if len(actual.Spec.Volumes) != 9 {
t.Fatalf("Expected 9 volumes in Build pod, got %d", len(actual.Spec.Volumes))
if len(actual.Spec.Volumes) != 10 {
t.Fatalf("Expected 10 volumes in Build pod, got %d", len(actual.Spec.Volumes))
}
if *actual.Spec.ActiveDeadlineSeconds != 60 {
t.Errorf("Expected ActiveDeadlineSeconds 60, got %d", *actual.Spec.ActiveDeadlineSeconds)
Expand Down
Loading