diff --git a/pkg/apis/rollouts/validation/validation_references.go b/pkg/apis/rollouts/validation/validation_references.go index c1c152ed55..6aaca21c88 100644 --- a/pkg/apis/rollouts/validation/validation_references.go +++ b/pkg/apis/rollouts/validation/validation_references.go @@ -65,7 +65,7 @@ func ValidateRolloutReferencedResources(rollout *v1alpha1.Rollout, referencedRes allErrs = append(allErrs, ValidateService(service, rollout)...) } for _, template := range referencedResources.AnalysisTemplateWithType { - allErrs = append(allErrs, ValidateAnalysisTemplateWithType(template)...) + allErrs = append(allErrs, ValidateAnalysisTemplateWithType(rollout, template)...) } for _, ingress := range referencedResources.Ingresses { allErrs = append(allErrs, ValidateIngress(rollout, ingress)...) @@ -92,7 +92,7 @@ func ValidateService(svc ServiceWithType, rollout *v1alpha1.Rollout) field.Error return allErrs } -func ValidateAnalysisTemplateWithType(template AnalysisTemplateWithType) field.ErrorList { +func ValidateAnalysisTemplateWithType(rollout *v1alpha1.Rollout, template AnalysisTemplateWithType) field.ErrorList { allErrs := field.ErrorList{} fldPath := GetAnalysisTemplateWithTypeFieldPath(template.TemplateType, template.AnalysisIndex, template.CanaryStepIndex) if fldPath == nil { @@ -121,6 +121,27 @@ func ValidateAnalysisTemplateWithType(template AnalysisTemplateWithType) field.E } } } + } else if template.TemplateType == BackgroundAnalysis && len(templateSpec.Args) > 0 { + for _, arg := range templateSpec.Args { + if arg.Value != nil || arg.ValueFrom != nil { + continue + } + if rollout.Spec.Strategy.Canary == nil || rollout.Spec.Strategy.Canary.Analysis == nil || rollout.Spec.Strategy.Canary.Analysis.Args == nil { + allErrs = append(allErrs, field.Invalid(fldPath, templateName, "missing analysis arguments in rollout spec")) + continue + } + + foundArg := false + for _, rolloutArg := range rollout.Spec.Strategy.Canary.Analysis.Args { + if arg.Name == rolloutArg.Name { + foundArg = true + break + } + } + if !foundArg { + allErrs = append(allErrs, field.Invalid(fldPath, templateName, arg.Name)) + } + } } return allErrs } diff --git a/pkg/apis/rollouts/validation/validation_references_test.go b/pkg/apis/rollouts/validation/validation_references_test.go index 6ea3ce0576..ae41a9177a 100644 --- a/pkg/apis/rollouts/validation/validation_references_test.go +++ b/pkg/apis/rollouts/validation/validation_references_test.go @@ -157,16 +157,18 @@ func TestValidateRolloutReferencedResources(t *testing.T) { func TestValidateAnalysisTemplateWithType(t *testing.T) { t.Run("validate analysisTemplate - success", func(t *testing.T) { + rollout := getRollout() template := getAnalysisTemplateWithType() - allErrs := ValidateAnalysisTemplateWithType(template) + allErrs := ValidateAnalysisTemplateWithType(rollout, template) assert.Empty(t, allErrs) }) t.Run("validate inline analysisTemplate - failure", func(t *testing.T) { + rollout := getRollout() count := intstr.FromInt(0) template := getAnalysisTemplateWithType() template.AnalysisTemplate.Spec.Metrics[0].Count = &count - allErrs := ValidateAnalysisTemplateWithType(template) + allErrs := ValidateAnalysisTemplateWithType(rollout, template) assert.Len(t, allErrs, 1) msg := fmt.Sprintf("AnalysisTemplate %s has metric %s which runs indefinitely. Invalid value for count: %s", "analysis-template-name", "metric-name", count.String()) expectedError := field.Invalid(GetAnalysisTemplateWithTypeFieldPath(template.TemplateType, template.AnalysisIndex, template.CanaryStepIndex), template.AnalysisTemplate.Name, msg) @@ -174,23 +176,92 @@ func TestValidateAnalysisTemplateWithType(t *testing.T) { }) t.Run("validate inline analysisTemplate argument - success", func(t *testing.T) { + rollout := getRollout() template := getAnalysisTemplateWithType() template.AnalysisTemplate.Spec.Args = []v1alpha1.Argument{ { Name: "service-name", }, } - allErrs := ValidateAnalysisTemplateWithType(template) + allErrs := ValidateAnalysisTemplateWithType(rollout, template) + assert.Empty(t, allErrs) + }) + + t.Run("validate background analysisTemplate - failure", func(t *testing.T) { + rollout := getRollout() + template := getAnalysisTemplateWithType() + template.TemplateType = BackgroundAnalysis + template.AnalysisTemplate.Spec.Args = []v1alpha1.Argument{ + { + Name: "service-name", + }, + } + allErrs := ValidateAnalysisTemplateWithType(rollout, template) + assert.NotEmpty(t, allErrs) + + rollout.Spec.Strategy.Canary.Analysis = &v1alpha1.RolloutAnalysisBackground{ + RolloutAnalysis: v1alpha1.RolloutAnalysis{ + Args: []v1alpha1.AnalysisRunArgument{ + { + Name: "a-different-service-name", + }, + }, + }, + } + allErrs = ValidateAnalysisTemplateWithType(rollout, template) + assert.NotEmpty(t, allErrs) + + template.AnalysisTemplate.Spec.Args = append(template.AnalysisTemplate.Spec.Args, v1alpha1.Argument{Name: "second-service-name"}) + allErrs = ValidateAnalysisTemplateWithType(rollout, template) + assert.NotEmpty(t, allErrs) + }) + + // verify background analysis matches the arguments in rollout spec + t.Run("validate background analysisTemplate - success", func(t *testing.T) { + rollout := getRollout() + + template := getAnalysisTemplateWithType() + template.TemplateType = BackgroundAnalysis + allErrs := ValidateAnalysisTemplateWithType(rollout, template) + assert.Empty(t, allErrs) + + // default value should be fine + defaultValue := "value-name" + template.AnalysisTemplate.Spec.Args = []v1alpha1.Argument{ + { + Name: "service-name", + Value: &defaultValue, + }, + } + allErrs = ValidateAnalysisTemplateWithType(rollout, template) + assert.Empty(t, allErrs) + + template.AnalysisTemplate.Spec.Args = []v1alpha1.Argument{ + { + Name: "service-name", + }, + } + rollout.Spec.Strategy.Canary.Analysis = &v1alpha1.RolloutAnalysisBackground{ + RolloutAnalysis: v1alpha1.RolloutAnalysis{ + Args: []v1alpha1.AnalysisRunArgument{ + { + Name: "service-name", + }, + }, + }, + } + allErrs = ValidateAnalysisTemplateWithType(rollout, template) assert.Empty(t, allErrs) }) // verify background analysis does not care about a metric that runs indefinitely t.Run("validate background analysisTemplate - success", func(t *testing.T) { + rollout := getRollout() count := intstr.FromInt(0) template := getAnalysisTemplateWithType() template.TemplateType = BackgroundAnalysis template.AnalysisTemplate.Spec.Metrics[0].Count = &count - allErrs := ValidateAnalysisTemplateWithType(template) + allErrs := ValidateAnalysisTemplateWithType(rollout, template) assert.Empty(t, allErrs) }) }