fix: Workload rollout spec is invalid if template is not empty

- Check lables and annotations of the template for workloadRef
- new e2e test

Signed-off-by: Hui Kang <[email protected]>

pkg/apis/rollouts/v1alpha1/types.go

@@ -83,6 +83,16 @@ func (s *RolloutSpec) SetResolvedTemplate(template corev1.PodTemplateSpec) {
 	s.Template = template
 }
 
+func (s *RolloutSpec) EmptyTemplate() bool {
+	if len(s.Template.Labels) > 0 {
+		return false
+	}
+	if len(s.Template.Annotations) > 0 {
+		return false
+	}
+	return true
+}
+
 func (s *RolloutSpec) MarshalJSON() ([]byte, error) {
 	type Alias RolloutSpec
 

pkg/apis/rollouts/validation/validation.go

@@ -83,6 +83,12 @@ func ValidateRolloutSpec(rollout *v1alpha1.Rollout, fldPath *field.Path) field.E
 		}
 	}
 
+	// WorkloadRef and template can not be set at the same time
+	if spec.WorkloadRef != nil && !spec.EmptyTemplate() {
+		allErrs = append(allErrs, field.InternalError(fldPath.Child("template"),
+			fmt.Errorf("template must be empty for workload reference rollout")))
+	}
+
 	selector, err := metav1.LabelSelectorAsSelector(spec.Selector)
 	if err != nil {
 		allErrs = append(allErrs, field.Invalid(fldPath.Child("selector"), spec.Selector, "invalid label selector"))

pkg/apis/rollouts/validation/validation_test.go

@@ -312,3 +312,37 @@ func TestCanaryScaleDownDelaySeconds(t *testing.T) {
 	})
 
 }
+
+func TestWorkloadRefWithTemplate(t *testing.T) {
+	selector := &metav1.LabelSelector{
+		MatchLabels: map[string]string{"key": "value"},
+	}
+	ro := &v1alpha1.Rollout{
+		Spec: v1alpha1.RolloutSpec{
+			WorkloadRef: &v1alpha1.ObjectRef{
+				Name:       "my-deployment",
+				Kind:       "Deployment",
+				APIVersion: "apps/v1",
+			},
+			Selector: selector,
+			Strategy: v1alpha1.RolloutStrategy{
+				Canary: &v1alpha1.CanaryStrategy{
+					StableService: "stable",
+					CanaryService: "canary",
+				},
+			},
+			Template: corev1.PodTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: selector.MatchLabels,
+				},
+			},
+		},
+	}
+	t.Run("workload reference with template", func(t *testing.T) {
+		ro := ro.DeepCopy()
+		allErrs := ValidateRollout(ro)
+		assert.Equal(t, 2, len(allErrs))
+		assert.EqualError(t, allErrs[0], "spec.template: Internal error: template must be empty for workload reference rollout")
+		assert.EqualError(t, allErrs[1], "spec.template.spec.containers: Required value")
+	})
+}

test/e2e/functional_test.go

@@ -1063,6 +1063,60 @@ spec:
 		ExpectActiveRevision("2")
 }
 
+func (s *FunctionalSuite) TestWorkloadRefTemplate() {
+	s.Given().
+		RolloutObjects(`
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  labels:
+    app.kubernetes.io/instance: rollout-canary
+  name: rollout-ref-deployment
+spec:
+  replicas: 0
+  selector:
+    matchLabels:
+      app: rollout-ref-deployment
+  template:
+    metadata:
+      labels:
+        app: rollout-ref-deployment
+    spec:
+      containers:
+        - name: rollouts-demo
+          image: argoproj/rollouts-demo:green
+---
+apiVersion: argoproj.io/v1alpha1
+kind: Rollout
+metadata:
+  name: rollout-ref-deployment
+spec:
+  replicas: 1
+  workloadRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    name: rollout-ref-deployment
+  selector:
+    matchLabels:
+      app: rollout-demo-deploy
+  template:
+    metadata:
+      labels:
+        app: rollout-ref-deployment
+  strategy:
+    blueGreen: 
+      activeService: rollout-bluegreen-active
+`).
+		When().
+		ApplyManifests().
+		WaitForRolloutStatus("Degraded").
+		Then().
+		ExpectRollout("error due to workload ref and template", func(r *v1alpha1.Rollout) bool {
+			return len(r.Status.Conditions) == 1 && strings.Contains(r.Status.Conditions[0].Message, "template must be empty for workload reference rollout")
+		})
+}
+
 func (s *FunctionalSuite) TestWorkloadRef() {
 	s.Given().
 		RolloutObjects(`