diff --git a/pkg/controller/prune.go b/pkg/controller/prune.go index 3b53ad16c..98fa97b77 100644 --- a/pkg/controller/prune.go +++ b/pkg/controller/prune.go @@ -46,8 +46,12 @@ func pruneProhibitedIngress(c *AppGwIngressController, appGw *n.ApplicationGatew // Mutate the list of Ingresses by removing ones that AGIC should not be creating configuration. for idx, ingress := range ingressList { klog.V(5).Infof("Original Ingress[%d] Rules: %+v", idx, ingress.Spec.Rules) - ingressList[idx].Spec.Rules = brownfield.PruneIngressRules(ingress, cbCtx.ProhibitedTargets) - klog.V(5).Infof("Sanitized Ingress[%d] Rules: %+v", idx, ingress.Spec.Rules) + + ingressClone := ingressList[idx].DeepCopy() + ingressClone.Spec.Rules = brownfield.PruneIngressRules(ingress, cbCtx.ProhibitedTargets) + ingressList[idx] = ingressClone + + klog.V(5).Infof("Sanitized Ingress[%d] Rules: %+v", idx, ingressList[idx].Spec.Rules) } return ingressList diff --git a/pkg/controller/prune_test.go b/pkg/controller/prune_test.go index 6c027200f..2eb0f31c6 100644 --- a/pkg/controller/prune_test.go +++ b/pkg/controller/prune_test.go @@ -18,6 +18,7 @@ import ( "github.com/Azure/application-gateway-kubernetes-ingress/pkg/annotations" "github.com/Azure/application-gateway-kubernetes-ingress/pkg/appgw" + "github.com/Azure/application-gateway-kubernetes-ingress/pkg/environment" "github.com/Azure/application-gateway-kubernetes-ingress/pkg/tests" "github.com/Azure/application-gateway-kubernetes-ingress/pkg/tests/fixtures" ) @@ -202,4 +203,62 @@ var _ = Describe("prune function tests", func() { Expect(prunedIngresses).To(ContainElement(ingressValid2)) }) }) + + Context("ensure pruneProhibitedIngress prunes ingress", func() { + env := environment.GetFakeEnv() + env.EnableBrownfieldDeployment = true + ingress := fixtures.GetIngressWithProhibitedTargetConflict() + cbCtx := &appgw.ConfigBuilderContext{ + IngressList: []*networking.Ingress{ + ingress, + }, + ServiceList: []*v1.Service{ + tests.NewServiceFixture(), + }, + ProhibitedTargets: fixtures.GetAzureIngressProhibitedTargets(), + DefaultAddressPoolID: to.StringPtr("xx"), + DefaultHTTPSettingsID: to.StringPtr("yy"), + EnvVariables: env, + } + appGw := fixtures.GetAppGateway() + + validateOldIngress := func(oldIngress *networking.Ingress) { + // should have two rules + Expect(len(oldIngress.Spec.Rules)).To(Equal(2)) + + // should have rule 1 with OldHost as host and no paths + Expect(oldIngress.Spec.Rules[0].Host).To(Equal(tests.OtherHost)) + Expect(len(oldIngress.Spec.Rules[0].HTTP.Paths)).To(Equal(0)) + + // should have rule 2 with Host as host and 2 path rules: /foo /fox + Expect(oldIngress.Spec.Rules[1].Host).To(Equal(tests.Host)) + Expect(len(oldIngress.Spec.Rules[1].HTTP.Paths)).To(Equal(2)) + Expect(oldIngress.Spec.Rules[1].HTTP.Paths[0].Path).To(Equal(fixtures.PathFoo)) + Expect(oldIngress.Spec.Rules[1].HTTP.Paths[1].Path).To(Equal(fixtures.PathFox)) + } + + It("removes the ingress rules without modifying the original ingress", func() { + Expect(len(cbCtx.IngressList)).To(Equal(1)) + + // Get pointer to the old ingress object + oldIngress := cbCtx.IngressList[0] + + // Validate that ingress follows the requirement + validateOldIngress(oldIngress) + + // Prune: test.OtherHost and /fox are prohibited + _ = pruneProhibitedIngress(controller, &appGw, cbCtx, cbCtx.IngressList) + + // Validate old ingress is the same as before + validateOldIngress(oldIngress) + + // Validate new ingress + newIngress := cbCtx.IngressList[0] + + Expect(len(newIngress.Spec.Rules)).To(Equal(1), "should have only 1 rule after pruning") + Expect(len(newIngress.Spec.Rules[0].HTTP.Paths)).To(Equal(1), "Rule should have only 1 path rule left") + Expect(oldIngress.Spec.Rules[1].Host).To(Equal(tests.Host), "Host for that path should be tests.Host") + Expect(oldIngress.Spec.Rules[1].HTTP.Paths[0].Path).To(Equal(fixtures.PathFoo), "Path should /foo; /fox should be removed") + }) + }) }) diff --git a/pkg/tests/fixtures/ingress.go b/pkg/tests/fixtures/ingress.go index 57d80fbc7..89dd2d983 100644 --- a/pkg/tests/fixtures/ingress.go +++ b/pkg/tests/fixtures/ingress.go @@ -60,3 +60,52 @@ func GetIngress() *networking.Ingress { }, } } + +// GetIngressWithProhibitedTargetConflict returns ingress with /foo and /fox as paths +func GetIngressWithProhibitedTargetConflict() *networking.Ingress { + return &networking.Ingress{ + Spec: networking.IngressSpec{ + Rules: []networking.IngressRule{ + { + // Rule with no Paths + Host: tests.OtherHost, + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{}, + }, + }, + { + // Rule with Paths + Host: tests.Host, + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{ + Paths: []networking.HTTPIngressPath{ + { + Path: PathFoo, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: tests.ServiceName, + Port: networking.ServiceBackendPort{ + Number: 80, + }, + }, + }, + }, + { + Path: PathFox, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: tests.ServiceName, + Port: networking.ServiceBackendPort{ + Number: 443, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } +}