Skip to content

Commit

Permalink
bypass k8s container image validation when ict's defined (allow for e…
Browse files Browse the repository at this point in the history
…mpty string)
  • Loading branch information
gabemontero committed Jun 14, 2016
1 parent d09e92f commit 6049db4
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 0 deletions.
12 changes: 12 additions & 0 deletions pkg/deploy/api/test/ok.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
60 changes: 60 additions & 0 deletions pkg/deploy/api/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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")
Expand Down
38 changes: 38 additions & 0 deletions pkg/deploy/api/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down

0 comments on commit 6049db4

Please sign in to comment.