diff --git a/pkg/deploy/api/test/ok.go b/pkg/deploy/api/test/ok.go index a0d2d8d3a22d..5864b4001ac1 100644 --- a/pkg/deploy/api/test/ok.go +++ b/pkg/deploy/api/test/ok.go @@ -160,6 +160,18 @@ func OkPodTemplate() *kapi.PodTemplateSpec { } } +func OkPodTemplateMissingImage(missing ...string) *kapi.PodTemplateSpec { + set := sets.NewString(missing...) + template := OkPodTemplate() + for i, c := range template.Spec.Containers { + if set.Has(c.Name) { + // rememeber that slices use copies, so have to ref array entry explicitly + template.Spec.Containers[i].Image = "" + } + } + return template +} + func OkConfigChangeTrigger() deployapi.DeploymentTriggerPolicy { return deployapi.DeploymentTriggerPolicy{ Type: deployapi.DeploymentTriggerOnConfigChange, diff --git a/pkg/deploy/api/validation/validation.go b/pkg/deploy/api/validation/validation.go index 40b3464dbe1f..e00f67795784 100644 --- a/pkg/deploy/api/validation/validation.go +++ b/pkg/deploy/api/validation/validation.go @@ -39,6 +39,9 @@ func ValidateDeploymentConfigSpec(spec deployapi.DeploymentConfigSpec) field.Err if spec.Template == nil { allErrs = append(allErrs, field.Required(specPath.Child("template"), "")) } else { + originalContainerImageNames := getContainerImageNames(spec.Template) + defer setContainerImageNames(spec.Template, originalContainerImageNames) + handleEmptyImageReferences(spec.Template, spec.Triggers) allErrs = append(allErrs, validation.ValidatePodTemplateSpec(spec.Template, specPath.Child("template"))...) } if spec.Replicas < 0 { @@ -50,6 +53,63 @@ func ValidateDeploymentConfigSpec(spec deployapi.DeploymentConfigSpec) field.Err return allErrs } +func getContainerImageNames(template *kapi.PodTemplateSpec) []string { + originalContainerImageNames := make([]string, len(template.Spec.Containers)) + for i := range template.Spec.Containers { + originalContainerImageNames[i] = template.Spec.Containers[i].Image + } + return originalContainerImageNames +} + +func setContainerImageNames(template *kapi.PodTemplateSpec, originalNames []string) { + for i := range template.Spec.Containers { + template.Spec.Containers[i].Image = originalNames[i] + } +} + +func handleEmptyImageReferences(template *kapi.PodTemplateSpec, triggers []deployapi.DeploymentTriggerPolicy) { + // if we have both an ICT defined and an empty Template->PodSpec->Container->Image field, we are going + // to modify this method's local copy (a pointer was NOT used for the parameter) by setting the field to a non-empty value to + // work around the k8s validation as our ICT will supply the image field value + containerEmptyImageInICT := make(map[string]bool) + for _, container := range template.Spec.Containers { + if len(container.Image) == 0 { + containerEmptyImageInICT[container.Name] = false + } + } + + if len(containerEmptyImageInICT) == 0 { + return + } + + needToChangeImageField := false + for _, trigger := range triggers { + // note, the validateTrigger call above will add an error if ImageChangeParams is nil, but + // we can still fall down this path so account for it being nil + if trigger.Type != deployapi.DeploymentTriggerOnImageChange || trigger.ImageChangeParams == nil { + continue + } + + for _, container := range trigger.ImageChangeParams.ContainerNames { + if _, ok := containerEmptyImageInICT[container]; ok { + needToChangeImageField = true + containerEmptyImageInICT[container] = true + } + } + } + + if needToChangeImageField { + for i, container := range template.Spec.Containers { + // only update containers listed in the ict + match, ok := containerEmptyImageInICT[container.Name] + if match && ok { + template.Spec.Containers[i].Image = "unset" + } + } + } + +} + func ValidateDeploymentConfigStatus(status deployapi.DeploymentConfigStatus) field.ErrorList { allErrs := field.ErrorList{} statusPath := field.NewPath("status") diff --git a/pkg/deploy/api/validation/validation_test.go b/pkg/deploy/api/validation/validation_test.go index cc4adc280d8f..4037c5d92b3d 100644 --- a/pkg/deploy/api/validation/validation_test.go +++ b/pkg/deploy/api/validation/validation_test.go @@ -79,12 +79,50 @@ func TestValidateDeploymentConfigOK(t *testing.T) { } } +func TestValidateDeploymentConfigICTMissingImage(t *testing.T) { + dc := &api.DeploymentConfig{ + ObjectMeta: kapi.ObjectMeta{Name: "foo", Namespace: "bar"}, + Spec: api.DeploymentConfigSpec{ + Replicas: 1, + Triggers: []api.DeploymentTriggerPolicy{test.OkImageChangeTrigger()}, + Selector: test.OkSelector(), + Strategy: test.OkStrategy(), + Template: test.OkPodTemplateMissingImage("container1"), + }, + } + errs := ValidateDeploymentConfig(dc) + + if len(errs) > 0 { + t.Errorf("Unexpected non-empty error list: %+v", errs) + } + + for _, c := range dc.Spec.Template.Spec.Containers { + if c.Image == "unset" { + t.Errorf("%s image field still has validation fake out value of %s", c.Name, c.Image) + } + } +} + func TestValidateDeploymentConfigMissingFields(t *testing.T) { errorCases := map[string]struct { DeploymentConfig api.DeploymentConfig ErrorType field.ErrorType Field string }{ + "empty container field": { + api.DeploymentConfig{ + ObjectMeta: kapi.ObjectMeta{Name: "foo", Namespace: "bar"}, + Spec: api.DeploymentConfigSpec{ + Replicas: 1, + Triggers: []api.DeploymentTriggerPolicy{test.OkConfigChangeTrigger()}, + Selector: test.OkSelector(), + Strategy: test.OkStrategy(), + Template: test.OkPodTemplateMissingImage("container1"), + }, + }, + field.ErrorTypeRequired, + "spec.template.spec.containers[0].image", + }, "missing name": { api.DeploymentConfig{ ObjectMeta: kapi.ObjectMeta{Name: "", Namespace: "bar"},