fix: Populate .spec.template with default values before Rollout Validation

pkg/apis/rollouts/validation/validation.go

@@ -5,6 +5,9 @@ import (
 	"fmt"
 	"strconv"
 
+	corev1 "k8s.io/api/core/v1"
+	corev1defaults "k8s.io/kubernetes/pkg/apis/core/v1"
+
 	"github.com/argoproj/argo-rollouts/utils/defaults"
 	"k8s.io/apimachinery/pkg/util/intstr"
 
@@ -17,7 +20,6 @@ import (
 	apivalidation "k8s.io/kubernetes/pkg/apis/core/validation"
 
 	"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
-	//"github.com/argoproj/argo-rollouts/utils/defaults"
 )
 
 const (
@@ -77,7 +79,17 @@ func ValidateRolloutSpec(rollout *v1alpha1.Rollout, fldPath *field.Path) field.E
 	if err != nil {
 		allErrs = append(allErrs, field.Invalid(fldPath.Child("selector"), spec.Selector, "invalid label selector"))
 	} else {
-		data, structConvertErr := json.Marshal(&spec.Template)
+		// The upstream K8s validation we are using expects the default values of a PodSpec to be set otherwise throwing a validation error.
+		// However, the Rollout does not need to have them set since the ReplicaSet it creates will have the default values set.
+		// As a result, the controller sets the default values before validation to prevent the validation errors due to the lack of these default fields. See #576 for more info.
+		podTemplate := corev1.PodTemplate{
+			Template: *spec.Template.DeepCopy(),
+		}
+		corev1defaults.SetObjectDefaults_PodTemplate(&podTemplate)
+		templateCoreV1 := podTemplate.Template
+		// ValidatePodTemplateSpecForReplicaSet function requires PodTemplateSpec from "k8s.io/api/core".
+		// We must cast spec.Template from "k8s.io/api/core/v1" to "k8s.io/api/core" in order to use ValidatePodTemplateSpecForReplicaSet.
+		data, structConvertErr := json.Marshal(&templateCoreV1)
 		if structConvertErr != nil {
 			allErrs = append(allErrs, field.InternalError(fldPath.Child("template"), structConvertErr))
 			return allErrs

pkg/apis/rollouts/validation/validation_test.go

@@ -4,15 +4,13 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
+	"github.com/argoproj/argo-rollouts/utils/defaults"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/apimachinery/pkg/util/validation/field"
-	corev1defaults "k8s.io/kubernetes/pkg/apis/core/v1"
-
-	"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
-	"github.com/argoproj/argo-rollouts/utils/defaults"
 )
 
 func TestValidateRollout(t *testing.T) {
@@ -39,11 +37,6 @@ func TestValidateRollout(t *testing.T) {
 			},
 		},
 	}
-	podTemplate := corev1.PodTemplate{
-		Template: ro.Spec.Template,
-	}
-	corev1defaults.SetObjectDefaults_PodTemplate(&podTemplate)
-	ro.Spec.Template = podTemplate.Template
 
 	t.Run("missing selector", func(t *testing.T) {
 		invalidRo := ro.DeepCopy()

rollout/controller_test.go

@@ -115,14 +115,10 @@ func newRollout(name string, replicas int, revisionHistoryLimit *int32, selector
 				Spec: corev1.PodSpec{
 					Containers: []corev1.Container{
 						{
-							Name:                     "container-name",
-							Image:                    "foo/bar",
-							ImagePullPolicy:          "Always",
-							TerminationMessagePolicy: "FallbackToLogsOnError",
+							Name:  "container-name",
+							Image: "foo/bar",
 						},
 					},
-					DNSPolicy:     "ClusterFirst",
-					RestartPolicy: "Always",
 				},
 			},
 			RevisionHistoryLimit: revisionHistoryLimit,
@@ -1060,7 +1056,7 @@ func TestPodTemplateHashEquivalence(t *testing.T) {
 	var err error
 	// NOTE: This test will fail on every k8s library upgrade.
 	// To fix it, update expectedReplicaSetName to match the new hash.
-	expectedReplicaSetName := "guestbook-7df8bcd895"
+	expectedReplicaSetName := "guestbook-898c8c6bd"
 
 	r1 := newBlueGreenRollout("guestbook", 1, nil, "active", "")
 	r1Resources := `
@@ -1169,7 +1165,7 @@ func TestComputeHashChangeTolerationBlueGreen(t *testing.T) {
 	// this should only update observedGeneration and nothing else
 	// NOTE: This test will fail on every k8s library upgrade.
 	// To fix it, update expectedPatch to match the new hash.
-	expectedPatch := `{"status":{"observedGeneration":"6687587ff6"}}`
+	expectedPatch := `{"status":{"observedGeneration":"6979d9866d"}}`
 	patch := f.getPatchedRollout(patchIndex)
 	assert.Equal(t, expectedPatch, patch)
 }
@@ -1214,7 +1210,7 @@ func TestComputeHashChangeTolerationCanary(t *testing.T) {
 	// this should only update observedGeneration and nothing else
 	// NOTE: This test will fail on every k8s library upgrade.
 	// To fix it, update expectedPatch to match the new hash.
-	expectedPatch := `{"status":{"observedGeneration":"75945ffcb"}}`
+	expectedPatch := `{"status":{"observedGeneration":"7c59bcf464"}}`
 	patch := f.getPatchedRollout(patchIndex)
 	assert.Equal(t, expectedPatch, patch)
 }