fix: lint subcommand for workload ref rollout (argoproj#1328)

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

Author: huikang

pkg/apis/rollouts/validation/validation.go

@@ -71,13 +71,15 @@ func ValidateRolloutSpec(rollout *v1alpha1.Rollout, fldPath *field.Path) field.E
 	replicas := defaults.GetReplicasOrDefault(spec.Replicas)
 	allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(replicas), fldPath.Child("replicas"))...)
 
-	if spec.Selector == nil {
-		message := fmt.Sprintf(MissingFieldMessage, ".spec.selector")
-		allErrs = append(allErrs, field.Required(fldPath.Child("selector"), message))
-	} else {
-		allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(spec.Selector, fldPath.Child("selector"))...)
-		if len(spec.Selector.MatchLabels)+len(spec.Selector.MatchExpressions) == 0 {
-			allErrs = append(allErrs, field.Invalid(fldPath.Child("selector"), spec.Selector, "empty selector is invalid for deployment"))
+	if spec.WorkloadRef == nil {
+		if spec.Selector == nil {
+			message := fmt.Sprintf(MissingFieldMessage, ".spec.selector")
+			allErrs = append(allErrs, field.Required(fldPath.Child("selector"), message))
+		} else {
+			allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(spec.Selector, fldPath.Child("selector"))...)
+			if len(spec.Selector.MatchLabels)+len(spec.Selector.MatchExpressions) == 0 {
+				allErrs = append(allErrs, field.Invalid(fldPath.Child("selector"), spec.Selector, "empty selector is invalid for deployment"))
+			}
 		}
 	}
 
@@ -119,9 +121,12 @@ func ValidateRolloutSpec(rollout *v1alpha1.Rollout, fldPath *field.Path) field.E
 			AllowMultipleHugePageResources: true,
 			AllowDownwardAPIHugePages:      true,
 		}
-		allErrs = append(allErrs, validation.ValidatePodTemplateSpecForReplicaSet(&template, selector, replicas, fldPath.Child("template"), opts)...)
-	}
 
+		// Skip validating empty template for rollout resolved from ref
+		if rollout.Spec.TemplateResolvedFromRef || spec.WorkloadRef == nil {
+			allErrs = append(allErrs, validation.ValidatePodTemplateSpecForReplicaSet(&template, selector, replicas, fldPath.Child("template"), opts)...)
+		}
+	}
 	allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(spec.MinReadySeconds), fldPath.Child("minReadySeconds"))...)
 
 	revisionHistoryLimit := defaults.GetRevisionHistoryLimitOrDefault(rollout)

pkg/apis/rollouts/validation/validation_test.go

@@ -47,6 +47,13 @@ func TestValidateRollout(t *testing.T) {
 		assert.Equal(t, message, allErrs[0].Detail)
 	})
 
+	t.Run("empty selector", func(t *testing.T) {
+		invalidRo := ro.DeepCopy()
+		invalidRo.Spec.Selector = &metav1.LabelSelector{}
+		allErrs := ValidateRollout(invalidRo)
+		assert.Equal(t, "empty selector is invalid for deployment", allErrs[0].Detail)
+	})
+
 	t.Run("invalid progressDeadlineSeconds", func(t *testing.T) {
 		invalidRo := ro.DeepCopy()
 		invalidRo.Spec.MinReadySeconds = defaults.GetProgressDeadlineSecondsOrDefault(invalidRo) + 1
@@ -341,8 +348,20 @@ func TestWorkloadRefWithTemplate(t *testing.T) {
 	t.Run("workload reference with template", func(t *testing.T) {
 		ro := ro.DeepCopy()
 		allErrs := ValidateRollout(ro)
-		assert.Equal(t, 2, len(allErrs))
+		assert.Equal(t, 1, 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")
+	})
+	t.Run("valid workload reference with selector", func(t *testing.T) {
+		ro := ro.DeepCopy()
+		ro.Spec.Template = corev1.PodTemplateSpec{}
+		allErrs := ValidateRollout(ro)
+		assert.Equal(t, 0, len(allErrs))
+	})
+	t.Run("valid workload reference without selector", func(t *testing.T) {
+		ro := ro.DeepCopy()
+		ro.Spec.Selector = nil
+		ro.Spec.Template = corev1.PodTemplateSpec{}
+		allErrs := ValidateRollout(ro)
+		assert.Equal(t, 0, len(allErrs))
 	})
 }

pkg/kubectl-argo-rollouts/cmd/lint/lint_test.go

@@ -15,7 +15,7 @@ func TestLintValidRollout(t *testing.T) {
 	cmd := NewCmdLint(o)
 	cmd.PersistentPreRunE = o.PersistentPreRunE
 
-	for _, filename := range []string{"testdata/valid.yml", "testdata/valid-with-another-empty-object.yml"} {
+	for _, filename := range []string{"testdata/valid.yml", "testdata/valid-workload-ref.yaml", "testdata/valid-with-another-empty-object.yml"} {
 		cmd.SetArgs([]string{"-f", filename})
 		err := cmd.Execute()
 		assert.NoError(t, err)

pkg/kubectl-argo-rollouts/cmd/lint/testdata/valid-workload-ref.yaml

@@ -0,0 +1,15 @@
+apiVersion: argoproj.io/v1alpha1               # Create a rollout resource
+kind: Rollout
+metadata:
+  name: rollout-ref-deployment
+spec:
+  replicas: 5
+  workloadRef:                                 # Reference an existing Deployment using workloadRef field
+    apiVersion: apps/v1
+    kind: Deployment
+    name: rollout-ref-deployment
+  strategy:
+    canary:
+      steps:
+        - setWeight: 20
+        - pause: {duration: 10s}