From 8503baa8fe6231d0a923b3e49690af0ec9aa8224 Mon Sep 17 00:00:00 2001 From: Maksim Bezsaznyj Date: Tue, 29 Oct 2024 10:46:58 -0400 Subject: [PATCH] fix(controller): weighted experiment validation should allow delegating to trafficRouter plugins (#3909) Signed-off-by: Maksim Bezsaznyj --- pkg/apis/rollouts/validation/validation.go | 4 ++-- pkg/apis/rollouts/validation/validation_test.go | 13 ++++++++++++- pkg/kubectl-argo-rollouts/cmd/lint/lint_test.go | 2 +- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/pkg/apis/rollouts/validation/validation.go b/pkg/apis/rollouts/validation/validation.go index 6b4ddb45e2..d9c31a37f1 100644 --- a/pkg/apis/rollouts/validation/validation.go +++ b/pkg/apis/rollouts/validation/validation.go @@ -387,8 +387,8 @@ func ValidateRolloutStrategyCanary(rollout *v1alpha1.Rollout, fldPath *field.Pat if template.Weight != nil { if canary.TrafficRouting == nil { allErrs = append(allErrs, field.Invalid(stepFldPath.Child("experiment").Child("templates").Index(tmplIndex).Child("weight"), *canary.Steps[i].Experiment.Templates[tmplIndex].Weight, InvalidCanaryExperimentTemplateWeightWithoutTrafficRouting)) - } else if canary.TrafficRouting.ALB == nil && canary.TrafficRouting.SMI == nil && canary.TrafficRouting.Istio == nil { - allErrs = append(allErrs, field.Invalid(stepFldPath.Child("experiment").Child("templates").Index(tmplIndex).Child("weight"), *canary.Steps[i].Experiment.Templates[tmplIndex].Weight, "Experiment template weight is only available for TrafficRouting with SMI, ALB, and Istio at this time")) + } else if canary.TrafficRouting.ALB == nil && canary.TrafficRouting.SMI == nil && canary.TrafficRouting.Istio == nil && len(canary.TrafficRouting.Plugins) == 0 { + allErrs = append(allErrs, field.Invalid(stepFldPath.Child("experiment").Child("templates").Index(tmplIndex).Child("weight"), *canary.Steps[i].Experiment.Templates[tmplIndex].Weight, "Experiment template weight is only available for TrafficRouting with SMI, ALB, Istio and Plugins at this time")) } } } diff --git a/pkg/apis/rollouts/validation/validation_test.go b/pkg/apis/rollouts/validation/validation_test.go index c9fc9ad923..84fc93b5dd 100644 --- a/pkg/apis/rollouts/validation/validation_test.go +++ b/pkg/apis/rollouts/validation/validation_test.go @@ -17,7 +17,7 @@ import ( ) const ( - errTrafficRoutingWithExperimentSupport = "Experiment template weight is only available for TrafficRouting with SMI, ALB, and Istio at this time" + errTrafficRoutingWithExperimentSupport = "Experiment template weight is only available for TrafficRouting with SMI, ALB, Istio and Plugins at this time" ) func TestValidateRollout(t *testing.T) { @@ -1094,4 +1094,15 @@ func TestCanaryExperimentStepWithWeight(t *testing.T) { assert.Equal(t, 1, len(allErrs)) assert.Equal(t, errTrafficRoutingWithExperimentSupport, allErrs[0].Detail) }) + + t.Run("success - Plugins", func(t *testing.T) { + invalidRo := ro.DeepCopy() + invalidRo.Spec.Strategy.Canary.TrafficRouting = &v1alpha1.RolloutTrafficRouting{ + Plugins: map[string]json.RawMessage{ + "any/plugin": {}, + }, + } + allErrs := ValidateRolloutStrategyCanary(invalidRo, field.NewPath("")) + assert.Equal(t, 0, len(allErrs)) + }) } diff --git a/pkg/kubectl-argo-rollouts/cmd/lint/lint_test.go b/pkg/kubectl-argo-rollouts/cmd/lint/lint_test.go index e9b66fd2ed..6f6d86a610 100644 --- a/pkg/kubectl-argo-rollouts/cmd/lint/lint_test.go +++ b/pkg/kubectl-argo-rollouts/cmd/lint/lint_test.go @@ -85,7 +85,7 @@ func TestLintInvalidRollout(t *testing.T) { }, { filename: "testdata/invalid-nginx-canary.yml", - errmsg: "Error: spec.strategy.steps[1].experiment.templates[0].weight: Invalid value: 20: Experiment template weight is only available for TrafficRouting with SMI, ALB, and Istio at this time\n", + errmsg: "Error: spec.strategy.steps[1].experiment.templates[0].weight: Invalid value: 20: Experiment template weight is only available for TrafficRouting with SMI, ALB, Istio and Plugins at this time\n", }, }