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 8, 2016
1 parent de2a8bc commit 2faa1aa
Show file tree
Hide file tree
Showing 3 changed files with 102 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
52 changes: 52 additions & 0 deletions pkg/deploy/api/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ func ValidateDeploymentConfigSpec(spec deployapi.DeploymentConfigSpec) field.Err
if spec.Template == nil {
allErrs = append(allErrs, field.Required(specPath.Child("template"), ""))
} else {
changedContainers := handleEmptyImageReferences(spec.Template, spec.Triggers)
allErrs = append(allErrs, validation.ValidatePodTemplateSpec(spec.Template, specPath.Child("template"))...)
resetEmptyImageReferences(changedContainers)
}
if spec.Replicas < 0 {
allErrs = append(allErrs, field.Invalid(specPath.Child("replicas"), spec.Replicas, "replicas cannot be negative"))
Expand All @@ -50,6 +52,56 @@ func ValidateDeploymentConfigSpec(spec deployapi.DeploymentConfigSpec) field.Err
return allErrs
}

func resetEmptyImageReferences(changedContainers []*kapi.Container) {
if changedContainers != nil && len(changedContainers) > 0 {
for _, container := range changedContainers {
container.Image = ""
}
}
}

func handleEmptyImageReferences(template *kapi.PodTemplateSpec, triggers []deployapi.DeploymentTriggerPolicy) []*kapi.Container {
// 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
}
}
needToChangeImageField := false
if len(containerEmptyImageInICT) > 0 {
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 {
needToChangeImageField = true
containerEmptyImageInICT[container] = true
}
}
}

if needToChangeImageField {
changedContainers := make([]*kapi.Container, 0)
for i, container := range template.Spec.Containers {
// only update containers listed in the icp
match, ok := containerEmptyImageInICT[container.Name]
if match && ok {
changedContainers = append(changedContainers, &(template.Spec.Containers[i]))
template.Spec.Containers[i].Image = "unset"
}
}
return changedContainers
}

return nil
}

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 2faa1aa

Please sign in to comment.