diff --git a/ingress/alb.go b/ingress/alb.go index 3fdff50932..d697839537 100644 --- a/ingress/alb.go +++ b/ingress/alb.go @@ -18,7 +18,8 @@ import ( func (c *Controller) syncALBIngress(ingress *ingressutil.Ingress, rollouts []*v1alpha1.Rollout) error { ctx := context.TODO() annotations := ingress.GetAnnotations() - managedActions, err := ingressutil.NewManagedALBActions(annotations[ingressutil.ManagedActionsAnnotation]) + managedAnnotationsKey := ingressutil.ManagedAnnotations + managedActions, err := ingressutil.NewManagedALBAnnotations(annotations[managedAnnotationsKey]) if err != nil { return nil } @@ -35,31 +36,38 @@ func (c *Controller) syncALBIngress(ingress *ingressutil.Ingress, rollouts []*v1 for roName := range managedActions { if _, ok := actionHasExistingRollout[roName]; !ok { modified = true - actionKey := managedActions[roName] + actionKeys := managedActions[roName] delete(managedActions, roName) - resetALBAction, err := getResetALBActionStr(ingress, actionKey) - if err != nil { - log.WithField(logutil.RolloutKey, roName). - WithField(logutil.IngressKey, ingress.GetName()). - WithField(logutil.NamespaceKey, ingress.GetNamespace()). - Error(err) - return nil + for _, actionKey := range actionKeys { + if !strings.Contains(actionKey, ingressutil.ALBActionPrefix) { + continue + } + resetALBAction, err := getResetALBActionStr(ingress, actionKey) + if err != nil { + log.WithField(logutil.RolloutKey, roName). + WithField(logutil.IngressKey, ingress.GetName()). + WithField(logutil.NamespaceKey, ingress.GetNamespace()). + Error(err) + return nil + } + annotations := newIngress.GetAnnotations() + annotations[actionKey] = resetALBAction + newIngress.SetAnnotations(annotations) } - annotations := newIngress.GetAnnotations() - annotations[actionKey] = resetALBAction - newIngress.SetAnnotations(annotations) } } if !modified { return nil } - newManagedStr := managedActions.String() newAnnotations := newIngress.GetAnnotations() - newAnnotations[ingressutil.ManagedActionsAnnotation] = newManagedStr - newIngress.SetAnnotations(newAnnotations) - if newManagedStr == "" { - delete(newIngress.GetAnnotations(), ingressutil.ManagedActionsAnnotation) + if len(managedActions) == 0 { + delete(newAnnotations, managedAnnotationsKey) + } else { + newAnnotations[managedAnnotationsKey] = managedActions.String() } + // delete leftovers from old implementation ManagedActionsAnnotation + delete(newAnnotations, ingressutil.ManagedActionsAnnotation) + newIngress.SetAnnotations(newAnnotations) _, err = c.ingressWrapper.Update(ctx, ingress.GetNamespace(), newIngress) return err } diff --git a/ingress/alb_test.go b/ingress/alb_test.go index d3b9a8bfe2..79a1c915a1 100644 --- a/ingress/alb_test.go +++ b/ingress/alb_test.go @@ -60,7 +60,10 @@ func albActionAnnotation(stable string) string { func newALBIngress(name string, port int, serviceName string, rollout string, includeStickyConfig bool) *extensionsv1beta1.Ingress { canaryService := fmt.Sprintf("%s-canary", serviceName) albActionKey := albActionAnnotation(serviceName) - managedBy := fmt.Sprintf("%s:%s", rollout, albActionKey) + albConditionKey := fmt.Sprintf("%s%s%s", ingressutil.ALBIngressAnnotation, ingressutil.ALBConditionPrefix, serviceName) + managedBy := ingressutil.ManagedALBAnnotations{ + rollout: ingressutil.ManagedALBAnnotation{albActionKey, albConditionKey}, + } action := fmt.Sprintf(actionTemplate, serviceName, port, canaryService, port) if includeStickyConfig { action = fmt.Sprintf(actionTemplateWithStickyConfig, serviceName, port, canaryService, port) @@ -70,9 +73,9 @@ func newALBIngress(name string, port int, serviceName string, rollout string, in Name: name, Namespace: metav1.NamespaceDefault, Annotations: map[string]string{ - "kubernetes.io/ingress.class": "alb", - albActionKey: action, - ingressutil.ManagedActionsAnnotation: managedBy, + "kubernetes.io/ingress.class": "alb", + albActionKey: action, + ingressutil.ManagedAnnotations: managedBy.String(), }, }, Spec: extensionsv1beta1.IngressSpec{ @@ -123,7 +126,7 @@ func rollout(name, service, ingress string) *v1alpha1.Rollout { func TestInvalidManagedALBActions(t *testing.T) { rollout := rollout("rollout", "stable-service", "test-ingress") ing := newALBIngress("test-ingress", 80, "stable-service", rollout.Name, false) - ing.Annotations[ingressutil.ManagedActionsAnnotation] = "invalid-managed-by" + ing.Annotations[ingressutil.ManagedAnnotations] = "invalid-managed-by" ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, rollout) @@ -147,7 +150,7 @@ func TestInvalidPreviousALBActionAnnotationValue(t *testing.T) { func TestInvalidPreviousALBActionAnnotationKey(t *testing.T) { ing := newALBIngress("test-ingress", 80, "stable-service", "also-not-existing-rollout", false) - ing.Annotations[ingressutil.ManagedActionsAnnotation] = "invalid-action-key" + ing.Annotations[ingressutil.ManagedAnnotations] = "invalid-action-key" ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, nil) err := ctrl.syncIngress("default/test-ingress") @@ -199,7 +202,7 @@ func TestALBIngressResetAction(t *testing.T) { panic(err) } annotations := acc.GetAnnotations() - assert.NotContains(t, annotations, ingressutil.ManagedActionsAnnotation) + assert.NotContains(t, annotations, ingressutil.ManagedAnnotations) expectedAction := `{"Type":"forward","ForwardConfig":{"TargetGroups":[{"ServiceName":"stable-service","ServicePort":"80","Weight":100}]}}` assert.Equal(t, expectedAction, annotations[albActionAnnotation("stable-service")]) } @@ -223,7 +226,7 @@ func TestALBIngressResetActionWithStickyConfig(t *testing.T) { panic(err) } annotations := acc.GetAnnotations() - assert.NotContains(t, annotations, ingressutil.ManagedActionsAnnotation) + assert.NotContains(t, annotations, ingressutil.ManagedAnnotations) expectedAction := `{"Type":"forward","ForwardConfig":{"TargetGroups":[{"ServiceName":"stable-service","ServicePort":"80","Weight":100}],"TargetGroupStickinessConfig":{"Enabled":true,"DurationSeconds":300}}}` assert.Equal(t, expectedAction, annotations[albActionAnnotation("stable-service")]) } diff --git a/pkg/apis/rollouts/validation/validation.go b/pkg/apis/rollouts/validation/validation.go index de07fa9581..8b3f8ee52c 100644 --- a/pkg/apis/rollouts/validation/validation.go +++ b/pkg/apis/rollouts/validation/validation.go @@ -33,14 +33,18 @@ const ( InvalidCanaryExperimentTemplateWeightWithoutTrafficRouting = "Experiment template weight cannot be set unless TrafficRouting is enabled" // InvalidSetCanaryScaleTrafficPolicy indicates that TrafficRouting, required for SetCanaryScale, is missing InvalidSetCanaryScaleTrafficPolicy = "SetCanaryScale requires TrafficRouting to be set" - // InvalidSetHeaderRoutingTrafficPolicy indicates that TrafficRouting, required for SetCanaryScale, is missing - InvalidSetHeaderRoutingTrafficPolicy = "SetHeaderRoute requires TrafficRouting, supports Istio only" + // InvalidSetHeaderRoutingTrafficPolicy indicates that TrafficRouting required for SetHeaderRouting, is missing + InvalidSetHeaderRoutingTrafficPolicy = "SetHeaderRoute requires TrafficRouting, supports Istio and ALB" // InvalidSetMirrorRouteTrafficPolicy indicates that TrafficRouting, required for SetCanaryScale, is missing InvalidSetMirrorRouteTrafficPolicy = "SetMirrorRoute requires TrafficRouting, supports Istio only" - // InvalidStringMatchMultipleValuePolicy indicates that SetCanaryScale, has multiple values set - InvalidStringMatchMultipleValuePolicy = "StringMatch match value must have exactly one of the following: exact, regex, prefix" - // InvalidStringMatchMissedValuePolicy indicates that SetCanaryScale, has multiple values set + // InvalidStringMatchMultipleValuePolicy indicates that SetHeaderRouting match, has multiple values set + InvalidStringMatchMultipleValuePolicy = "StringMatch match has multiple values, but it must have exactly one of the following: exact, regex, prefix" + // InvalidStringMatchMissedValuePolicy indicates that SetHeaderRouting, has multiple values set InvalidStringMatchMissedValuePolicy = "StringMatch value missed, match value must have one of the following: exact, regex, prefix" + // InvalidSetHeaderRoutingALBValueMissing indicates that SetHeaderRouting using with ALB missed the 'exact' value + InvalidSetHeaderRoutingALBValueMissing = "SetHeaderRouting match value missed. ALB supports 'exact' value only" + // InvalidSetHeaderRoutingALBValuePolicy indicates that SetHeaderRouting using with ALB missed the 'exact' value + InvalidSetHeaderRoutingALBValuePolicy = "SetHeaderRouting match value invalid. ALB supports 'exact' value only" // InvalidDurationMessage indicates the Duration value needs to be greater than 0 InvalidDurationMessage = "Duration needs to be greater than 0" // InvalidMaxSurgeMaxUnavailable indicates both maxSurge and MaxUnavailable can not be set to zero @@ -305,13 +309,17 @@ func ValidateRolloutStrategyCanary(rollout *v1alpha1.Rollout, fldPath *field.Pat if step.SetHeaderRoute != nil { trafficRouting := rollout.Spec.Strategy.Canary.TrafficRouting - if trafficRouting == nil || trafficRouting.Istio == nil { + if trafficRouting == nil || (trafficRouting.Istio == nil && trafficRouting.ALB == nil) { allErrs = append(allErrs, field.Invalid(stepFldPath.Child("setHeaderRoute"), step.SetHeaderRoute, InvalidSetHeaderRoutingTrafficPolicy)) - } - if step.SetHeaderRoute.Match != nil && len(step.SetHeaderRoute.Match) > 0 { + } else if step.SetHeaderRoute.Match != nil && len(step.SetHeaderRoute.Match) > 0 { for j, match := range step.SetHeaderRoute.Match { - matchFld := stepFldPath.Child("setHeaderRoute").Child("match").Index(j) - allErrs = append(allErrs, hasMultipleMatchValues(match.HeaderValue, matchFld)...) + if trafficRouting.ALB != nil { + matchFld := stepFldPath.Child("setHeaderRoute").Child("match").Index(j) + allErrs = append(allErrs, hasALBInvalidValues(match.HeaderValue, matchFld)...) + } else { + matchFld := stepFldPath.Child("setHeaderRoute").Child("match").Index(j) + allErrs = append(allErrs, hasMultipleMatchValues(match.HeaderValue, matchFld)...) + } } } } @@ -473,6 +481,22 @@ func hasMultipleStepsType(s v1alpha1.CanaryStep, fldPath *field.Path) field.Erro return allErrs } +func hasALBInvalidValues(match *v1alpha1.StringMatch, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if match == nil { + e := field.Invalid(fldPath, match, InvalidStringMatchMissedValuePolicy) + allErrs = append(allErrs, e) + return allErrs + } + if match.Exact == "" { + return append(allErrs, field.Invalid(fldPath, match, InvalidSetHeaderRoutingALBValueMissing)) + } + if match.Regex != "" || match.Prefix != "" { + return append(allErrs, field.Invalid(fldPath, match, InvalidSetHeaderRoutingALBValuePolicy)) + } + return allErrs +} + func hasMultipleMatchValues(match *v1alpha1.StringMatch, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} diff --git a/pkg/apis/rollouts/validation/validation_test.go b/pkg/apis/rollouts/validation/validation_test.go index 8888ac29ac..0ec01bf6e3 100644 --- a/pkg/apis/rollouts/validation/validation_test.go +++ b/pkg/apis/rollouts/validation/validation_test.go @@ -281,21 +281,15 @@ func TestValidateRolloutStrategyAntiAffinity(t *testing.T) { assert.Equal(t, InvalidAntiAffinityWeightMessage, allErrs[0].Detail) } -func TestValidateRolloutStrategyCanarySetHeaderRouteIstio(t *testing.T) { +func TestValidateRolloutStrategyCanarySetHeaderRoute(t *testing.T) { ro := &v1alpha1.Rollout{} ro.Spec.Strategy.Canary = &v1alpha1.CanaryStrategy{ CanaryService: "canary", StableService: "stable", - TrafficRouting: &v1alpha1.RolloutTrafficRouting{ - Istio: &v1alpha1.IstioTrafficRouting{ - VirtualService: &v1alpha1.IstioVirtualService{Name: "virtual-service"}, - }, - }, } t.Run("using SetHeaderRoute step without the traffic routing", func(t *testing.T) { invalidRo := ro.DeepCopy() - invalidRo.Spec.Strategy.Canary.TrafficRouting = nil invalidRo.Spec.Strategy.Canary.Steps = []v1alpha1.CanaryStep{{ SetHeaderRoute: &v1alpha1.SetHeaderRoute{ Match: []v1alpha1.HeaderRoutingMatch{ @@ -309,6 +303,19 @@ func TestValidateRolloutStrategyCanarySetHeaderRouteIstio(t *testing.T) { allErrs := ValidateRolloutStrategyCanary(invalidRo, field.NewPath("")) assert.Equal(t, InvalidSetHeaderRoutingTrafficPolicy, allErrs[0].Detail) }) +} + +func TestValidateRolloutStrategyCanarySetHeaderRouteIstio(t *testing.T) { + ro := &v1alpha1.Rollout{} + ro.Spec.Strategy.Canary = &v1alpha1.CanaryStrategy{ + CanaryService: "canary", + StableService: "stable", + TrafficRouting: &v1alpha1.RolloutTrafficRouting{ + Istio: &v1alpha1.IstioTrafficRouting{ + VirtualService: &v1alpha1.IstioVirtualService{Name: "virtual-service"}, + }, + }, + } t.Run("using SetHeaderRoute step with multiple values", func(t *testing.T) { invalidRo := ro.DeepCopy() @@ -364,6 +371,53 @@ func TestValidateRolloutStrategyCanarySetHeaderRouteIstio(t *testing.T) { }) } +func TestValidateRolloutStrategyCanarySetHeaderRoutingALB(t *testing.T) { + ro := &v1alpha1.Rollout{} + ro.Spec.Strategy.Canary = &v1alpha1.CanaryStrategy{ + CanaryService: "canary", + StableService: "stable", + TrafficRouting: &v1alpha1.RolloutTrafficRouting{ + ALB: &v1alpha1.ALBTrafficRouting{ + RootService: "action_name", + }, + }, + } + + t.Run("using SetHeaderRouting step with multiple values", func(t *testing.T) { + invalidRo := ro.DeepCopy() + invalidRo.Spec.Strategy.Canary.Steps = []v1alpha1.CanaryStep{{ + SetHeaderRoute: &v1alpha1.SetHeaderRoute{ + Match: []v1alpha1.HeaderRoutingMatch{ + { + HeaderName: "agent", + HeaderValue: &v1alpha1.StringMatch{ + Exact: "chrome", + Regex: "chrome(.*)", + }, + }, + }, + }, + }} + allErrs := ValidateRolloutStrategyCanary(invalidRo, field.NewPath("")) + assert.Equal(t, InvalidSetHeaderRoutingALBValuePolicy, allErrs[0].Detail) + }) + + t.Run("using SetHeaderRouting step with missed values", func(t *testing.T) { + invalidRo := ro.DeepCopy() + invalidRo.Spec.Strategy.Canary.Steps = []v1alpha1.CanaryStep{{ + SetHeaderRoute: &v1alpha1.SetHeaderRoute{ + Match: []v1alpha1.HeaderRoutingMatch{ + { + HeaderName: "agent", + }, + }, + }, + }} + allErrs := ValidateRolloutStrategyCanary(invalidRo, field.NewPath("")) + assert.Equal(t, InvalidSetHeaderRoutingALBValueMissing, allErrs[0].Detail) + }) +} + func TestValidateRolloutStrategyCanarySetMirrorRouteIstio(t *testing.T) { ro := &v1alpha1.Rollout{} ro.Spec.Strategy.Canary = &v1alpha1.CanaryStrategy{ diff --git a/rollout/trafficrouting/alb/alb.go b/rollout/trafficrouting/alb/alb.go index 62d1a465d4..1d53547524 100644 --- a/rollout/trafficrouting/alb/alb.go +++ b/rollout/trafficrouting/alb/alb.go @@ -27,7 +27,8 @@ import ( const ( // Type holds this controller type - Type = "ALB" + Type = "ALB" + HeaderRouteName = "argo-rollouts-header-based-route" ) // ReconcilerConfig describes static configuration data for the ALB Ingress reconciler @@ -116,6 +117,48 @@ func (r *Reconciler) SetWeight(desiredWeight int32, additionalDestinations ...v1 } func (r *Reconciler) SetHeaderRoute(headerRouting *v1alpha1.SetHeaderRoute) error { + if headerRouting == nil { + return nil + } + ctx := context.TODO() + rollout := r.cfg.Rollout + ingressName := rollout.Spec.Strategy.Canary.TrafficRouting.ALB.Ingress + action := HeaderRouteName + port := rollout.Spec.Strategy.Canary.TrafficRouting.ALB.ServicePort + + ingress, err := r.cfg.IngressWrapper.GetCached(rollout.Namespace, ingressName) + if err != nil { + return err + } + + desiredAnnotations, err := getDesiredHeaderAnnotations(ingress, rollout, port, headerRouting) + if err != nil { + return err + } + desiredIngress := ingressutil.NewIngressWithSpecAndAnnotations(ingress, desiredAnnotations) + hasRule := ingressutil.HasRuleWithService(ingress, action) + if hasRule && (headerRouting == nil || headerRouting.Match == nil) { + desiredIngress.RemovePathByServiceName(action) + } + if !hasRule && headerRouting != nil && headerRouting.Match != nil { + desiredIngress.CreateAnnotationBasedPath(action) + } + patch, modified, err := ingressutil.BuildIngressPatch(ingress.Mode(), ingress, desiredIngress, ingressutil.WithAnnotations(), ingressutil.WithSpec()) + if err != nil { + return nil + } + if !modified { + r.log.Info("no changes to the ALB Ingress for header routing") + return nil + } + r.log.WithField("patch", string(patch)).Debug("applying ALB Ingress patch") + r.cfg.Recorder.Eventf(rollout, record.EventOptions{EventReason: "PatchingALBIngress"}, "Updating Ingress `%s` to headerRouting '%d'", ingressName, headerRouting) + + _, err = r.cfg.IngressWrapper.Patch(ctx, ingress.GetNamespace(), ingress.GetName(), types.MergePatchType, patch, metav1.PatchOptions{}) + if err != nil { + r.log.WithField("err", err.Error()).Error("error patching alb ingress") + return fmt.Errorf("error patching alb ingress `%s`: %v", ingressName, err) + } return nil } @@ -294,13 +337,111 @@ func getDesiredAnnotations(current *ingressutil.Ingress, r *v1alpha1.Rollout, po return nil, err } desired[key] = value - m, err := ingressutil.NewManagedALBActions(desired[ingressutil.ManagedActionsAnnotation]) + return addManagedAnnotation(desired, r.Name, key) +} + +func getDesiredHeaderAnnotations(current *ingressutil.Ingress, r *v1alpha1.Rollout, port int32, headerRouting *v1alpha1.SetHeaderRouting) (map[string]string, error) { + desired := current.DeepCopy().GetAnnotations() + actionName := HeaderRouteName + actionKey := ingressutil.ALBHeaderBasedActionAnnotationKey(r, actionName) + conditionKey := ingressutil.ALBHeaderBasedConditionAnnotationKey(r, actionName) + actionValue, err := getTrafficForwardActionString(r, port) if err != nil { return nil, err } - m[r.Name] = key - desired[ingressutil.ManagedActionsAnnotation] = m.String() - return desired, nil + if headerRouting == nil || headerRouting.Match == nil { + delete(desired, actionKey) + delete(desired, conditionKey) + } else { + conditionValue, err := getTrafficForwardConditionString(headerRouting) + if err != nil { + return nil, err + } + desired[actionKey] = actionValue + desired[conditionKey] = conditionValue + } + + return addManagedAnnotation(desired, r.Name, actionKey, conditionKey) +} + +func addManagedAnnotation(annotations map[string]string, rolloutName string, annotationKeys ...string) (map[string]string, error) { + managedAnnotationsKey := ingressutil.ManagedAnnotations + m, err := ingressutil.NewManagedALBAnnotations(annotations[managedAnnotationsKey]) + if err != nil { + return nil, err + } + for _, annotationKey := range annotationKeys { + if m[rolloutName] == nil { + m[rolloutName] = ingressutil.ManagedALBAnnotation{} + } + if !hasValue(m[rolloutName], annotationKey) { + m[rolloutName] = append(m[rolloutName], annotationKey) + } + } + annotations[managedAnnotationsKey] = m.String() + return annotations, nil +} + +func hasValue(array []string, key string) bool { + for _, item := range array { + if item == key { + return true + } + } + return false +} + +func getTrafficForwardActionString(r *v1alpha1.Rollout, port int32) (string, error) { + _, canaryService := trafficrouting.GetStableAndCanaryServices(r) + portStr := strconv.Itoa(int(port)) + weight := int64(100) + targetGroups := make([]ingressutil.ALBTargetGroup, 0) + // create target group for canary + targetGroups = append(targetGroups, ingressutil.ALBTargetGroup{ + ServiceName: canaryService, + ServicePort: portStr, + Weight: pointer.Int64Ptr(weight), + }) + + action := ingressutil.ALBAction{ + Type: "forward", + ForwardConfig: ingressutil.ALBForwardConfig{ + TargetGroups: targetGroups, + }, + } + + var stickinessConfig = r.Spec.Strategy.Canary.TrafficRouting.ALB.StickinessConfig + if stickinessConfig != nil && stickinessConfig.Enabled { + // AWS API valid range + // https://docs.aws.amazon.com/elasticloadbalancing/latest/APIReference/API_TargetGroupStickinessConfig.html + if stickinessConfig.DurationSeconds < 1 || stickinessConfig.DurationSeconds > 604800 { + return "", fmt.Errorf("TargetGroupStickinessConfig's duration must be between 1 and 604800 seconds (7 days)!") + } + newStickyConfig := ingressutil.ALBTargetGroupStickinessConfig{ + Enabled: true, + DurationSeconds: stickinessConfig.DurationSeconds, + } + action.ForwardConfig.TargetGroupStickinessConfig = &newStickyConfig + } + + bytes := jsonutil.MustMarshal(action) + return string(bytes), nil +} + +func getTrafficForwardConditionString(headerRouting *v1alpha1.SetHeaderRouting) (string, error) { + var res []ingressutil.ALBCondition + for _, match := range headerRouting.Match { + condition := ingressutil.ALBCondition{ + Field: "http-header", + HttpHeaderConfig: ingressutil.HttpHeaderConfig{ + HttpHeaderName: match.HeaderName, + Values: []string{match.HeaderValue.Exact}, + }, + } + res = append(res, condition) + } + bytes := jsonutil.MustMarshal(res) + return string(bytes), nil } // UpdateHash informs a traffic routing reconciler about new canary/stable pod hashes diff --git a/rollout/trafficrouting/alb/alb_test.go b/rollout/trafficrouting/alb/alb_test.go index 7e26f6f16e..06ab014f4b 100644 --- a/rollout/trafficrouting/alb/alb_test.go +++ b/rollout/trafficrouting/alb/alb_test.go @@ -101,7 +101,9 @@ func albActionAnnotation(stable string) string { } func ingress(name, stableSvc, canarySvc, actionService string, port, weight int32, managedBy string, includeStickyConfig bool) *extensionsv1beta1.Ingress { - managedByValue := fmt.Sprintf("%s:%s", managedBy, albActionAnnotation(actionService)) + managedByValue := ingressutil.ManagedALBAnnotations{ + managedBy: ingressutil.ManagedALBAnnotation{albActionAnnotation(actionService)}, + } action := fmt.Sprintf(actionTemplate, canarySvc, port, weight, stableSvc, port, 100-weight) if includeStickyConfig { action = fmt.Sprintf(actionTemplateWithStickyConfig, canarySvc, port, weight, stableSvc, port, 100-weight) @@ -117,8 +119,8 @@ func ingress(name, stableSvc, canarySvc, actionService string, port, weight int3 Name: name, Namespace: metav1.NamespaceDefault, Annotations: map[string]string{ - albActionAnnotation(actionService): string(jsonutil.MustMarshal(a)), - ingressutil.ManagedActionsAnnotation: managedByValue, + albActionAnnotation(actionService): string(jsonutil.MustMarshal(a)), + ingressutil.ManagedAnnotations: managedByValue.String(), }, }, Spec: extensionsv1beta1.IngressSpec{ @@ -156,6 +158,13 @@ func TestType(t *testing.T) { assert.NoError(t, err) } +func TestAddManagedAnnotation(t *testing.T) { + annotations, _ := addManagedAnnotation(map[string]string{}, "argo-rollouts", "alb.ingress.kubernetes.io/actions.action1", "alb.ingress.kubernetes.io/conditions.action1") + assert.Equal(t, annotations[ingressutil.ManagedAnnotations], "{\"argo-rollouts\":[\"alb.ingress.kubernetes.io/actions.action1\",\"alb.ingress.kubernetes.io/conditions.action1\"]}") + _, err := addManagedAnnotation(map[string]string{ingressutil.ManagedAnnotations: "invalid, non-json value"}, "some-rollout") + assert.Error(t, err) +} + func TestIngressNotFound(t *testing.T) { ro := fakeRollout("stable-service", "canary-service", nil, "stable-ingress", 443) client := fake.NewSimpleClientset() @@ -225,7 +234,7 @@ func TestNoChanges(t *testing.T) { func TestErrorOnInvalidManagedBy(t *testing.T) { ro := fakeRollout(STABLE_SVC, CANARY_SVC, nil, "ingress", 443) i := ingress("ingress", STABLE_SVC, CANARY_SVC, STABLE_SVC, 443, 5, ro.Name, false) - i.Annotations[ingressutil.ManagedActionsAnnotation] = "test" + i.Annotations[ingressutil.ManagedAnnotations] = "test" client := fake.NewSimpleClientset(i) k8sI := kubeinformers.NewSharedInformerFactory(client, 0) k8sI.Extensions().V1beta1().Ingresses().Informer().GetIndexer().Add(i) diff --git a/test/e2e/aws_test.go b/test/e2e/aws_test.go index 5525d4650d..95e07f31a7 100644 --- a/test/e2e/aws_test.go +++ b/test/e2e/aws_test.go @@ -12,7 +12,9 @@ import ( "github.com/stretchr/testify/suite" "github.com/tj/assert" + "k8s.io/utils/pointer" + "github.com/argoproj/argo-rollouts/rollout/trafficrouting/istio" "github.com/argoproj/argo-rollouts/test/fixtures" ingress2 "github.com/argoproj/argo-rollouts/utils/ingress" ) @@ -171,3 +173,74 @@ func (s *AWSSuite) TestALBExperimentStepNoSetWeight() { Then(). Assert(assertWeights(s, "alb-rollout-canary", "alb-rollout-stable", 0, 100)) } + +func (s *AWSSuite) TestAlbHeaderRoute() { + s.Given(). + RolloutObjects("@header-routing/alb-header-route.yaml"). + When(). + ApplyManifests(). + WaitForRolloutStatus("Healthy"). + Then(). + Assert(func(t *fixtures.Then) { + assertAlbActionDoesNotExist(t, s, istio.HeaderRouteName) + assertAlbActionServiceWeight(t, s, "action1", "canary-service", 0) + assertAlbActionServiceWeight(t, s, "action1", "stable-service", 100) + }). + When(). + UpdateSpec(). + WaitForRolloutStatus("Paused"). + Sleep(1 * time.Second). + Then(). + Assert(func(t *fixtures.Then) { + assertAlbActionDoesNotExist(t, s, istio.HeaderRouteName) + assertAlbActionServiceWeight(t, s, "action1", "canary-service", 20) + assertAlbActionServiceWeight(t, s, "action1", "stable-service", 80) + }). + When(). + PromoteRollout(). + WaitForRolloutStatus("Paused"). + Sleep(1 * time.Second). + Then(). + Assert(func(t *fixtures.Then) { + assertAlbActionServiceWeight(t, s, istio.HeaderRouteName, "canary-service", 100) + assertAlbActionServiceWeight(t, s, "action1", "canary-service", 20) + assertAlbActionServiceWeight(t, s, "action1", "stable-service", 80) + }). + When(). + PromoteRollout(). + WaitForRolloutStatus("Paused"). + Sleep(1 * time.Second). + Then(). + Assert(func(t *fixtures.Then) { + assertAlbActionDoesNotExist(t, s, istio.HeaderRouteName) + }) +} + +func assertAlbActionServiceWeight(t *fixtures.Then, s *AWSSuite, actionName, serviceName string, expectedWeight int64) { + ingress := t.GetALBIngress() + key := "alb.ingress.kubernetes.io/actions." + actionName + actionStr, ok := ingress.Annotations[key] + assert.True(s.T(), ok, "Annotation for action was not found: %s", key) + + var albAction ingress2.ALBAction + err := json.Unmarshal([]byte(actionStr), &albAction) + if err != nil { + panic(err) + } + + found := false + for _, group := range albAction.ForwardConfig.TargetGroups { + if group.ServiceName == serviceName { + assert.Equal(s.T(), pointer.Int64(expectedWeight), group.Weight) + found = true + } + } + assert.True(s.T(), found, "Service %s was not found", serviceName) +} + +func assertAlbActionDoesNotExist(t *fixtures.Then, s *AWSSuite, actionName string) { + ingress := t.GetALBIngress() + key := "alb.ingress.kubernetes.io/actions." + actionName + _, ok := ingress.Annotations[key] + assert.False(s.T(), ok, "Annotation for action should not exist: %s", key) +} diff --git a/test/e2e/header-routing/alb-header-route.yaml b/test/e2e/header-routing/alb-header-route.yaml new file mode 100644 index 0000000000..0dbe27e193 --- /dev/null +++ b/test/e2e/header-routing/alb-header-route.yaml @@ -0,0 +1,103 @@ +apiVersion: v1 +kind: Service +metadata: + name: canary-service +spec: + type: NodePort + ports: + - port: 8080 + targetPort: 8080 + protocol: TCP + name: http + selector: + app: alb-rollout +--- +apiVersion: v1 +kind: Service +metadata: + name: stable-service +spec: + type: NodePort + ports: + - port: 8080 + targetPort: 8080 + protocol: TCP + name: http + selector: + app: alb-rollout +--- +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: alb-rollout-ingress + annotations: + alb.ingress.kubernetes.io/security-groups: 'iks-intuit-cidr-ingress-tcp-443' + alb.ingress.kubernetes.io/certificate-arn: arn:aws:acm:us-east-2:795188202216:certificate/27d920c5-a8a6-4210-9f31-bd4a2d439039 + alb.ingress.kubernetes.io/load-balancer-attributes: access_logs.s3.enabled=false + alb.ingress.kubernetes.io/ssl-policy: ELBSecurityPolicy-TLS-1-2-2017-01 + kubernetes.io/ingress.class: aws-alb + alb.ingress.kubernetes.io/load-balancer-name: rollouts-sample + alb.ingress.kubernetes.io/target-type: ip + alb.ingress.kubernetes.io/healthcheck-protocol: HTTP + alb.ingress.kubernetes.io/healthcheck-port: traffic-port + alb.ingress.kubernetes.io/healthcheck-path: /color + alb.ingress.kubernetes.io/backend-protocol: HTTP + alb.ingress.kubernetes.io/listen-ports: '[{"HTTPS": 443}]' + alb.ingress.kubernetes.io/ssl-redirect: '443' + alb.ingress.kubernetes.io/scheme: internet-facing + alb.ingress.kubernetes.io/subnets: IngressSubnetAz1, IngressSubnetAz2, IngressSubnetAz3 +spec: + rules: + - http: + paths: + - path: /* + pathType: ImplementationSpecific + backend: + service: + name: action1 + port: + name: use-annotation + +--- +apiVersion: argoproj.io/v1alpha1 +kind: Rollout +metadata: + name: rollouts-demo +spec: + replicas: 5 + selector: + matchLabels: + app: alb-rollout + template: + metadata: + labels: + app: alb-rollout + spec: + containers: + - name: alb-rollout + image: "argoproj/rollouts-demo:yellow" + ports: + - name: http + containerPort: 8080 + protocol: TCP + strategy: + canary: + scaleDownDelaySeconds: 5 + stableService: stable-service + canaryService: canary-service + trafficRouting: + alb: + ingress: alb-rollout-ingress + rootService: action1 + servicePort: 8080 + steps: + - setWeight: 20 + - pause: {} + - setHeaderRouting: + match: + - headerName: Custom-Header + headerValue: + exact: Mozilla* + - pause: {} + - setHeaderRouting: {} + - pause: {} diff --git a/utils/defaults/defaults.go b/utils/defaults/defaults.go index c38a044c25..9b18b16db1 100644 --- a/utils/defaults/defaults.go +++ b/utils/defaults/defaults.go @@ -85,6 +85,14 @@ func init() { } } +func GetStringOrDefault(value, defaultValue string) string { + if value == "" { + return defaultValue + } else { + return value + } +} + // GetReplicasOrDefault returns the deferenced number of replicas or the default number func GetReplicasOrDefault(replicas *int32) int32 { if replicas == nil { diff --git a/utils/defaults/defaults_test.go b/utils/defaults/defaults_test.go index d099cc0343..2162f1be82 100644 --- a/utils/defaults/defaults_test.go +++ b/utils/defaults/defaults_test.go @@ -12,6 +12,11 @@ import ( "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" ) +func TestGetStringOrDefault(t *testing.T) { + assert.Equal(t, "some value", GetStringOrDefault("some value", "default value")) + assert.Equal(t, "default value", GetStringOrDefault("", "default value")) +} + func TestGetReplicasOrDefault(t *testing.T) { replicas := int32(2) assert.Equal(t, replicas, GetReplicasOrDefault(&replicas)) diff --git a/utils/ingress/ingress.go b/utils/ingress/ingress.go index 42dc61ca20..81991ee87d 100644 --- a/utils/ingress/ingress.go +++ b/utils/ingress/ingress.go @@ -1,6 +1,7 @@ package ingress import ( + json2 "encoding/json" "errors" "fmt" "strconv" @@ -11,7 +12,9 @@ import ( "k8s.io/client-go/discovery" "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" + "github.com/argoproj/argo-rollouts/utils/defaults" "github.com/argoproj/argo-rollouts/utils/diff" + "github.com/argoproj/argo-rollouts/utils/json" ) const ( @@ -19,10 +22,14 @@ const ( CanaryIngressSuffix = "-canary" // ManagedActionsAnnotation holds list of ALB actions that are managed by rollouts ManagedActionsAnnotation = "rollouts.argoproj.io/managed-alb-actions" + // ManagedAnnotations holds list of ALB annotations that are managed by rollouts supports multiple annotations + ManagedAnnotations = "rollouts.argoproj.io/managed-alb-annotations" //ALBIngressAnnotation is the prefix annotation that is used by the ALB Ingress controller to configure an ALB ALBIngressAnnotation = "alb.ingress.kubernetes.io" // ALBActionPrefix the prefix to specific actions within an ALB ingress. ALBActionPrefix = "/actions." + // ALBConditionPrefix the prefix to specific conditions within an ALB ingress. + ALBConditionPrefix = "/conditions." ) // ALBAction describes an ALB action that configure the behavior of an ALB. This struct is marshaled into a string @@ -32,6 +39,19 @@ type ALBAction struct { ForwardConfig ALBForwardConfig `json:"ForwardConfig"` } +// ALBCondition describes an ALB action condition that configure the behavior of an ALB. This struct is marshaled into a string +// that is added to the Ingress's annotations. +type ALBCondition struct { + Field string `json:"field"` + HttpHeaderConfig HttpHeaderConfig `json:"httpHeaderConfig"` +} + +// HttpHeaderConfig describes header config for the ALB action condition +type HttpHeaderConfig struct { + HttpHeaderName string `json:"httpHeaderName"` + Values []string `json:"values"` +} + // ALBForwardConfig describes a list of target groups that the ALB should route traffic towards type ALBForwardConfig struct { TargetGroups []ALBTargetGroup `json:"TargetGroups"` @@ -142,6 +162,26 @@ func hasLegacyIngressRuleWithService(ingress *extensionsv1beta1.Ingress, svc str // ManagedALBActions a mapping of Rollout names to the ALB action that the Rollout manages type ManagedALBActions map[string]string +type ManagedALBAnnotations map[string]ManagedALBAnnotation + +type ManagedALBAnnotation []string + +// String outputs a string of all the managed ALB annotations that is stored in the Ingress's annotations +func (m ManagedALBAnnotations) String() string { + return string(json.MustMarshal(m)) +} + +func NewManagedALBAnnotations(json string) (ManagedALBAnnotations, error) { + res := ManagedALBAnnotations{} + if json == "" { + return res, nil + } + if err := json2.Unmarshal([]byte(json), &res); err != nil { + return nil, err + } + return res, nil +} + // String outputs a string of all the managed ALB actions that is stored in the Ingress's annotations func (m ManagedALBActions) String() string { str := "" @@ -173,15 +213,23 @@ func NewManagedALBActions(annotation string) (ManagedALBActions, error) { // ALBActionAnnotationKey returns the annotation key for a specific action func ALBActionAnnotationKey(r *v1alpha1.Rollout) string { - prefix := ALBIngressAnnotation - if r.Spec.Strategy.Canary.TrafficRouting.ALB.AnnotationPrefix != "" { - prefix = r.Spec.Strategy.Canary.TrafficRouting.ALB.AnnotationPrefix - } - actionService := r.Spec.Strategy.Canary.StableService - if r.Spec.Strategy.Canary.TrafficRouting.ALB.RootService != "" { - actionService = r.Spec.Strategy.Canary.TrafficRouting.ALB.RootService - } - return fmt.Sprintf("%s%s%s", prefix, ALBActionPrefix, actionService) + actionService := defaults.GetStringOrDefault(r.Spec.Strategy.Canary.TrafficRouting.ALB.RootService, r.Spec.Strategy.Canary.StableService) + return albIngressKubernetesIoKey(r, ALBActionPrefix, actionService) +} + +// ALBHeaderBasedActionAnnotationKey returns the annotation key for a specific action +func ALBHeaderBasedActionAnnotationKey(r *v1alpha1.Rollout, action string) string { + return albIngressKubernetesIoKey(r, ALBActionPrefix, action) +} + +// ALBHeaderBasedConditionAnnotationKey returns the annotation key for a specific condition +func ALBHeaderBasedConditionAnnotationKey(r *v1alpha1.Rollout, action string) string { + return albIngressKubernetesIoKey(r, ALBConditionPrefix, action) +} + +func albIngressKubernetesIoKey(r *v1alpha1.Rollout, action, service string) string { + prefix := defaults.GetStringOrDefault(r.Spec.Strategy.Canary.TrafficRouting.ALB.AnnotationPrefix, ALBIngressAnnotation) + return fmt.Sprintf("%s%s%s", prefix, action, service) } type patchConfig struct { diff --git a/utils/ingress/wrapper.go b/utils/ingress/wrapper.go index 1d28c8021d..a691e280c4 100644 --- a/utils/ingress/wrapper.go +++ b/utils/ingress/wrapper.go @@ -10,6 +10,7 @@ import ( v1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" types "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/informers" extensionsv1beta1 "k8s.io/client-go/informers/extensions/v1beta1" networkingv1 "k8s.io/client-go/informers/networking/v1" @@ -68,6 +69,29 @@ func NewIngressWithAnnotations(mode IngressMode, annotations map[string]string) } } +func NewIngressWithSpecAndAnnotations(ingress *Ingress, annotations map[string]string) *Ingress { + switch ingress.mode { + case IngressModeNetworking: + i := &v1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: annotations, + }, + Spec: *ingress.ingress.Spec.DeepCopy(), + } + return NewIngress(i) + case IngressModeExtensions: + i := &v1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: annotations, + }, + Spec: *ingress.legacyIngress.Spec.DeepCopy(), + } + return NewLegacyIngress(i) + default: + return nil + } +} + func (i *Ingress) GetExtensionsIngress() (*v1beta1.Ingress, error) { if i.legacyIngress == nil { return nil, errors.New("extensions Ingress is nil in this wrapper") @@ -128,6 +152,85 @@ func (i *Ingress) SetAnnotations(annotations map[string]string) { } } +func (i *Ingress) CreateAnnotationBasedPath(actionName string) { + i.mux.Lock() + defer i.mux.Unlock() + if HasRuleWithService(i, actionName) { + return + } + switch i.mode { + case IngressModeNetworking: + t := v1.PathTypeImplementationSpecific + p := v1.HTTPIngressPath{ + Path: "/*", + PathType: &t, + Backend: v1.IngressBackend{ + Service: &v1.IngressServiceBackend{ + Name: actionName, + Port: v1.ServiceBackendPort{ + Name: "use-annotation", + }, + }, + }, + } + for _, rule := range i.ingress.Spec.Rules { + rule.HTTP.Paths = append(rule.HTTP.Paths[:1], rule.HTTP.Paths[0:]...) + rule.HTTP.Paths[0] = p + } + case IngressModeExtensions: + t := v1beta1.PathTypeImplementationSpecific + p := v1beta1.HTTPIngressPath{ + Path: "/*", + PathType: &t, + Backend: v1beta1.IngressBackend{ + ServiceName: actionName, + ServicePort: intstr.FromString("use-annotation"), + }, + } + for _, rule := range i.legacyIngress.Spec.Rules { + rule.HTTP.Paths = append(rule.HTTP.Paths[:1], rule.HTTP.Paths[0:]...) + rule.HTTP.Paths[0] = p + } + } +} + +func (i *Ingress) RemovePathByServiceName(actionName string) { + i.mux.Lock() + defer i.mux.Unlock() + switch i.mode { + case IngressModeNetworking: + for _, rule := range i.ingress.Spec.Rules { + if j := indexPathByService(rule, actionName); j != -1 { + rule.HTTP.Paths = append(rule.HTTP.Paths[:j], rule.HTTP.Paths[j+1:]...) + } + } + case IngressModeExtensions: + for _, rule := range i.legacyIngress.Spec.Rules { + if j := indexLegacyPathByService(rule, actionName); j != -1 { + rule.HTTP.Paths = append(rule.HTTP.Paths[:j], rule.HTTP.Paths[j+1:]...) + } + } + } +} + +func indexPathByService(rule v1.IngressRule, name string) int { + for i, path := range rule.HTTP.Paths { + if path.Backend.Service.Name == name { + return i + } + } + return -1 +} + +func indexLegacyPathByService(rule v1beta1.IngressRule, name string) int { + for i, path := range rule.HTTP.Paths { + if path.Backend.ServiceName == name { + return i + } + } + return -1 +} + func (i *Ingress) DeepCopy() *Ingress { switch i.mode { case IngressModeNetworking: diff --git a/utils/ingress/wrapper_test.go b/utils/ingress/wrapper_test.go index eae08b4828..38d395e3ec 100644 --- a/utils/ingress/wrapper_test.go +++ b/utils/ingress/wrapper_test.go @@ -4,7 +4,6 @@ import ( "context" "testing" - "github.com/argoproj/argo-rollouts/utils/ingress" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" "k8s.io/api/extensions/v1beta1" @@ -14,6 +13,9 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" kubeinformers "k8s.io/client-go/informers" k8sfake "k8s.io/client-go/kubernetes/fake" + "k8s.io/utils/pointer" + + "github.com/argoproj/argo-rollouts/utils/ingress" ) func TestNewIngressWithAnnotations(t *testing.T) { @@ -68,6 +70,57 @@ func TestNewIngressWithAnnotations(t *testing.T) { }) } +func TestNewIngressWithSpecAndAnnotations(t *testing.T) { + annotations := make(map[string]string) + annotations["some.annotation.key1"] = "some.annotation.value1" + annotations["some.annotation.key2"] = "some.annotation.value2" + getAnnotations := func() map[string]string { + annotations := make(map[string]string) + annotations["some.annotation.key1"] = "some.annotation.value1" + annotations["some.annotation.key2"] = "some.annotation.value2" + return annotations + } + t.Run("will instantiate an Ingress wrapped with an annotated networkingv1.Ingress", func(t *testing.T) { + ing := networkingIngress() + + // given + t.Parallel() + + // when + i := ingress.NewIngressWithSpecAndAnnotations(ing, getAnnotations()) + + // then + assert.NotNil(t, i) + a := i.GetAnnotations() + assert.Equal(t, 2, len(a)) + a["extra-annotation-key"] = "extra-annotation-value" + i.SetAnnotations(a) + assert.Equal(t, 3, len(a)) + actualIngress, _ := i.GetNetworkingIngress() + expectedIngress, _ := ing.GetNetworkingIngress() + assert.Equal(t, expectedIngress.Spec, actualIngress.Spec) + }) + t.Run("will instantiate an Ingress wrapped with an annotated extensions/v1beta1.Ingress", func(t *testing.T) { + ing := extensionsIngress() + // given + t.Parallel() + + // when + i := ingress.NewIngressWithSpecAndAnnotations(ing, getAnnotations()) + + // then + assert.NotNil(t, i) + a := i.GetAnnotations() + assert.Equal(t, 2, len(a)) + a["extra-annotation-key"] = "extra-annotation-value" + i.SetAnnotations(a) + assert.Equal(t, 3, len(a)) + actualIngress, _ := i.GetExtensionsIngress() + expectedIngress, _ := ing.GetExtensionsIngress() + assert.Equal(t, expectedIngress.Spec, actualIngress.Spec) + }) +} + func TestGetExtensionsIngress(t *testing.T) { extensionsIngress := &v1beta1.Ingress{} t.Run("will get extensions ingress successfully", func(t *testing.T) { @@ -188,6 +241,74 @@ func TestGetObjectMeta(t *testing.T) { }) } +func TestCreateAnnotationBasedPath(t *testing.T) { + t.Run("v1 ingress, create path", func(t *testing.T) { + ing := networkingIngress() + ni, _ := ing.GetNetworkingIngress() + + assert.Equal(t, 1, len(ni.Spec.Rules[0].HTTP.Paths)) + ing.CreateAnnotationBasedPath("test-route") + assert.Equal(t, 2, len(ni.Spec.Rules[0].HTTP.Paths)) + }) + t.Run("v1 ingress, create existing path", func(t *testing.T) { + ing := networkingIngress() + ni, _ := ing.GetNetworkingIngress() + + ing.CreateAnnotationBasedPath("v1backend") + assert.Equal(t, 1, len(ni.Spec.Rules[0].HTTP.Paths)) + }) + t.Run("v1beta1 ingress, create path", func(t *testing.T) { + ing := extensionsIngress() + ni, _ := ing.GetExtensionsIngress() + + assert.Equal(t, 1, len(ni.Spec.Rules[0].HTTP.Paths)) + ing.CreateAnnotationBasedPath("test-route") + assert.Equal(t, 2, len(ni.Spec.Rules[0].HTTP.Paths)) + }) + t.Run("v1beta1 ingress, create existing path", func(t *testing.T) { + ing := extensionsIngress() + ni, _ := ing.GetExtensionsIngress() + + ing.CreateAnnotationBasedPath("v1beta1backend") + assert.Equal(t, 1, len(ni.Spec.Rules[0].HTTP.Paths)) + }) +} + +func TestRemoveAnnotationBasedPath(t *testing.T) { + t.Run("v1 ingress, remove path", func(t *testing.T) { + ing := networkingIngress() + ni, _ := ing.GetNetworkingIngress() + + assert.Equal(t, 1, len(ni.Spec.Rules[0].HTTP.Paths)) + ing.RemovePathByServiceName("v1backend") + assert.Equal(t, 0, len(ni.Spec.Rules[0].HTTP.Paths)) + }) + t.Run("v1 ingress, remove non existing path", func(t *testing.T) { + ing := networkingIngress() + ni, _ := ing.GetNetworkingIngress() + + assert.Equal(t, 1, len(ni.Spec.Rules[0].HTTP.Paths)) + ing.RemovePathByServiceName("non-exsisting-route") + assert.Equal(t, 1, len(ni.Spec.Rules[0].HTTP.Paths)) + }) + t.Run("v1beta1 ingress, remove path", func(t *testing.T) { + ing := extensionsIngress() + ni, _ := ing.GetExtensionsIngress() + + assert.Equal(t, 1, len(ni.Spec.Rules[0].HTTP.Paths)) + ing.RemovePathByServiceName("v1beta1backend") + assert.Equal(t, 0, len(ni.Spec.Rules[0].HTTP.Paths)) + }) + t.Run("v1beta1 ingress, remove non existing path", func(t *testing.T) { + ing := extensionsIngress() + ni, _ := ing.GetExtensionsIngress() + + assert.Equal(t, 1, len(ni.Spec.Rules[0].HTTP.Paths)) + ing.RemovePathByServiceName("non-exsisting-route") + assert.Equal(t, 1, len(ni.Spec.Rules[0].HTTP.Paths)) + }) +} + func TestDeepCopy(t *testing.T) { t.Run("will deepcopy ingress wrapped with networking.Ingress", func(t *testing.T) { // given @@ -736,3 +857,63 @@ func getExtensionsIngress() *v1beta1.Ingress { }, } } + +func networkingIngress() *ingress.Ingress { + pathType := v1.PathTypeImplementationSpecific + res := v1.Ingress{ + Spec: v1.IngressSpec{ + IngressClassName: pointer.String("v1ingress"), + Rules: []v1.IngressRule{ + { + Host: "v1host", + IngressRuleValue: v1.IngressRuleValue{ + HTTP: &v1.HTTPIngressRuleValue{ + Paths: []v1.HTTPIngressPath{ + { + Backend: v1.IngressBackend{ + Service: &v1.IngressServiceBackend{ + Name: "v1backend", + Port: v1.ServiceBackendPort{Name: "use-annotation"}, + }, + }, + Path: "/*", + PathType: &pathType, + }, + }, + }, + }, + }, + }, + }, + } + return ingress.NewIngress(&res) +} + +func extensionsIngress() *ingress.Ingress { + pathType := v1beta1.PathTypeImplementationSpecific + res := v1beta1.Ingress{ + Spec: v1beta1.IngressSpec{ + IngressClassName: pointer.String("v1beta1ingress"), + Rules: []v1beta1.IngressRule{ + { + Host: "v1beta1host", + IngressRuleValue: v1beta1.IngressRuleValue{ + HTTP: &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + { + Backend: v1beta1.IngressBackend{ + ServiceName: "v1beta1backend", + ServicePort: intstr.FromString("use-annotation"), + }, + Path: "/*", + PathType: &pathType, + }, + }, + }, + }, + }, + }, + }, + } + return ingress.NewLegacyIngress(&res) +}