diff --git a/prow/config/config.go b/prow/config/config.go index 3ea727f94620..023474330562 100644 --- a/prow/config/config.go +++ b/prow/config/config.go @@ -1081,7 +1081,7 @@ func validateJobBase(v JobBase, jobType prowapi.ProwJobType, podNamespace string if err := validateAgent(v, podNamespace); err != nil { return err } - if err := validatePodSpec(jobType, v.Spec); err != nil { + if err := validatePodSpec(jobType, v.Decorate, v.Spec); err != nil { return err } if err := ValidatePipelineRunSpec(jobType, v.ExtraRefs, v.PipelineRunSpec); err != nil { @@ -1644,28 +1644,36 @@ func ValidatePipelineRunSpec(jobType prowapi.ProwJobType, extraRefs []prowapi.Re return nil } -func validatePodSpec(jobType prowapi.ProwJobType, spec *v1.PodSpec) error { +func validatePodSpec(jobType prowapi.ProwJobType, decorated bool, spec *v1.PodSpec) error { if spec == nil { return nil } - if len(spec.InitContainers) != 0 { - return errors.New("pod spec may not use init containers") + if len(spec.InitContainers) != 0 && decorated { + return errors.New("pod spec may not use init containers when decorate is true") } - if n := len(spec.Containers); n != 1 { - return fmt.Errorf("pod spec must specify exactly 1 container, found: %d", n) + if n := len(spec.Containers); n != 1 && decorated { + return fmt.Errorf("pod spec must specify exactly 1 container when decorate is true, found: %d", n) + } else if n == 0 { + return fmt.Errorf("pod spec must specify at leaset 1 container, found: %d", n) } - for _, env := range spec.Containers[0].Env { - for _, prowEnv := range downwardapi.EnvForType(jobType) { - if env.Name == prowEnv { - // TODO(fejta): consider allowing this - return fmt.Errorf("env %s is reserved", env.Name) + for _, container := range spec.Containers { + for _, env := range container.Env { + for _, prowEnv := range downwardapi.EnvForType(jobType) { + if env.Name == prowEnv { + // TODO(fejta): consider allowing this + return fmt.Errorf("env %s is reserved", env.Name) + } } } } + if !decorated { + return nil + } + for _, mount := range spec.Containers[0].VolumeMounts { for _, prowMount := range decorate.VolumeMounts() { if mount.Name == prowMount { diff --git a/prow/config/config_test.go b/prow/config/config_test.go index a4e3b2483f15..7d2a22bcc259 100644 --- a/prow/config/config_test.go +++ b/prow/config/config_test.go @@ -542,11 +542,12 @@ func TestValidatePodSpec(t *testing.T) { postEnv := sets.NewString(downwardapi.EnvForType(prowapi.PostsubmitJob)...) preEnv := sets.NewString(downwardapi.EnvForType(prowapi.PresubmitJob)...) cases := []struct { - name string - jobType prowapi.ProwJobType - spec func(s *v1.PodSpec) - noSpec bool - pass bool + name string + decorate bool + jobType prowapi.ProwJobType + spec func(s *v1.PodSpec) + noSpec bool + pass bool }{ { name: "allow nil spec", @@ -558,13 +559,24 @@ func TestValidatePodSpec(t *testing.T) { pass: true, }, { - name: "reject init containers", + name: "reject init containers", + decorate: true, spec: func(s *v1.PodSpec) { s.InitContainers = []v1.Container{ {}, } }, }, + { + name: "allow init containers if not decorated", + decorate: false, + spec: func(s *v1.PodSpec) { + s.InitContainers = []v1.Container{ + {}, + } + }, + pass: true, + }, { name: "reject 0 containers", spec: func(s *v1.PodSpec) { @@ -572,11 +584,20 @@ func TestValidatePodSpec(t *testing.T) { }, }, { - name: "reject 2 containers", + name: "reject 2 containers", + decorate: true, spec: func(s *v1.PodSpec) { s.Containers = append(s.Containers, v1.Container{}) }, }, + { + name: "allow 2 containers if not decorated", + decorate: false, + spec: func(s *v1.PodSpec) { + s.Containers = append(s.Containers, v1.Container{}) + }, + pass: true, + }, { name: "reject reserved presubmit env", jobType: prowapi.PresubmitJob, @@ -620,7 +641,8 @@ func TestValidatePodSpec(t *testing.T) { }, }, { - name: "reject reserved mount name", + name: "reject reserved mount name", + decorate: true, spec: func(s *v1.PodSpec) { s.Containers[0].VolumeMounts = append(s.Containers[0].VolumeMounts, v1.VolumeMount{ Name: decorate.VolumeMounts()[0], @@ -629,7 +651,8 @@ func TestValidatePodSpec(t *testing.T) { }, }, { - name: "reject reserved mount path", + name: "reject reserved mount path", + decorate: true, spec: func(s *v1.PodSpec) { s.Containers[0].VolumeMounts = append(s.Containers[0].VolumeMounts, v1.VolumeMount{ Name: "fun", @@ -638,7 +661,8 @@ func TestValidatePodSpec(t *testing.T) { }, }, { - name: "reject conflicting mount paths (decorate in user)", + name: "reject conflicting mount paths (decorate in user)", + decorate: true, spec: func(s *v1.PodSpec) { s.Containers[0].VolumeMounts = append(s.Containers[0].VolumeMounts, v1.VolumeMount{ Name: "foo", @@ -647,7 +671,8 @@ func TestValidatePodSpec(t *testing.T) { }, }, { - name: "reject conflicting mount paths (user in decorate)", + name: "reject conflicting mount paths (user in decorate)", + decorate: true, spec: func(s *v1.PodSpec) { s.Containers[0].VolumeMounts = append(s.Containers[0].VolumeMounts, v1.VolumeMount{ Name: "foo", @@ -656,7 +681,8 @@ func TestValidatePodSpec(t *testing.T) { }, }, { - name: "reject reserved volume", + name: "reject reserved volume", + decorate: true, spec: func(s *v1.PodSpec) { s.Volumes = append(s.Volumes, v1.Volume{Name: decorate.VolumeMounts()[0]}) }, @@ -681,7 +707,7 @@ func TestValidatePodSpec(t *testing.T) { } else if tc.spec != nil { tc.spec(current) } - switch err := validatePodSpec(jt, current); { + switch err := validatePodSpec(jt, tc.decorate, current); { case err == nil && !tc.pass: t.Error("validation failed to raise an error") case err != nil && tc.pass: