Skip to content

Commit 8bc1e63

Browse files
akshaysnguptaakshaysngupta
authored
fix: clean up path rules added by agic when using prohibited target (#1610)
* clean up path rules added by AGIC * fix e2e * remove crd install * fix test * update timelimit
1 parent 17967aa commit 8bc1e63

File tree

16 files changed

+296
-165
lines changed

16 files changed

+296
-165
lines changed

‎helm/ingress-azure/crds/azureapplicationgatewayrewrite.yaml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ kind: CustomResourceDefinition
33
metadata:
44
name: azureapplicationgatewayrewrites.appgw.ingress.azure.io
55
annotations:
6-
"helm.sh/hook": crd-install
76
"api-approved.kubernetes.io": "https://github.com/Azure/application-gateway-kubernetes-ingress/pull/1272"
87
spec:
98
group: appgw.ingress.azure.io

‎helm/ingress-azure/crds/azureingressprohibitedtarget.yaml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ kind: CustomResourceDefinition
33
metadata:
44
name: azureingressprohibitedtargets.appgw.ingress.k8s.io
55
annotations:
6-
"helm.sh/hook": crd-install
76
"api-approved.kubernetes.io": "https://github.com/Azure/application-gateway-kubernetes-ingress/pull/1272"
87
spec:
98
group: appgw.ingress.k8s.io

‎helm/ingress-azure/templates/NOTES.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ Configuration Details:
3333
- Verbosity level: {{ .Values.verbosityLevel }}
3434
{{- if .Values.appgw }}
3535
{{- if .Values.appgw.shared }}
36-
- Multi-cluster / Shared App Gateway is enabled; Use "kubectl get AzureIngressProhibitedTargets" to view and modify config
36+
- Shared App Gateway is enabled; Use "kubectl get AzureIngressProhibitedTargets" to view and modify config
3737
{{- end }}
3838
{{- end }}
3939
{{- if .Values.armAuth }}

‎helm/ingress-azure/templates/crds.yaml‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
11
{{- if .Values.appgw -}}
22
{{- if .Values.appgw.shared -}}
3-
{{- range $path, $bytes := .Files.Glob "crds/*.yaml" }}
4-
{{ $.Files.Get $path }}
5-
---
6-
{{- end }}
73
{{- $watchNamespace := .Values.kubernetes.watchNamespace -}}
84
{{- if not .Values.appgw.prohibitedTargets }}
95
apiVersion: appgw.ingress.k8s.io/v1

‎pkg/appgw/cleanup.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,29 @@
66
package appgw
77

88
import (
9+
"fmt"
910
"strings"
1011

1112
n "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-03-01/network"
1213
)
1314

15+
// CleanUpPathRulesAddedByAGIC removes path rules that are created by AGIC
16+
func (c *appGwConfigBuilder) CleanUpPathRulesAddedByAGIC() {
17+
pathRuleNamePrefix := fmt.Sprintf("%s%s-", agPrefix, prefixPathRule)
18+
19+
// Remove path rules that are created by AGIC
20+
for _, pathMap := range *c.appGw.URLPathMaps {
21+
var pathRulesAddedManually []n.ApplicationGatewayPathRule
22+
for _, pathRule := range *pathMap.PathRules {
23+
if !strings.HasPrefix(*pathRule.Name, pathRuleNamePrefix) {
24+
pathRulesAddedManually = append(pathRulesAddedManually, pathRule)
25+
}
26+
}
27+
28+
pathMap.PathRules = &pathRulesAddedManually
29+
}
30+
}
31+
1432
// CleanUpUnusedDefaults removes the default backend and default http settings if they are not used by any ingress
1533
func (c *appGwConfigBuilder) CleanUpUnusedDefaults() {
1634
if !c.isPoolUsed(DefaultBackendAddressPoolName) {

‎pkg/appgw/cleanup_test.go‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// -------------------------------------------------------------------------------------------
2+
// Copyright (c) Microsoft Corporation. All rights reserved.
3+
// Licensed under the MIT License. See License.txt in the project root for license information.
4+
// --------------------------------------------------------------------------------------------
5+
6+
package appgw
7+
8+
import (
9+
n "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-03-01/network"
10+
. "github.com/onsi/ginkgo"
11+
. "github.com/onsi/gomega"
12+
)
13+
14+
var _ = Describe("Cleanup", func() {
15+
Context("CleanUpPathRules", func() {
16+
var c *appGwConfigBuilder
17+
agicAddedPathRule := generatePathRuleName("test", "test", 0, 0)
18+
userAddedPathRule := "user-added-path-rule"
19+
20+
BeforeEach(func() {
21+
c = &appGwConfigBuilder{
22+
appGw: n.ApplicationGateway{
23+
ApplicationGatewayPropertiesFormat: &n.ApplicationGatewayPropertiesFormat{
24+
URLPathMaps: &[]n.ApplicationGatewayURLPathMap{
25+
{
26+
ApplicationGatewayURLPathMapPropertiesFormat: &n.ApplicationGatewayURLPathMapPropertiesFormat{
27+
PathRules: &[]n.ApplicationGatewayPathRule{
28+
{
29+
Name: &agicAddedPathRule,
30+
},
31+
{
32+
Name: &userAddedPathRule,
33+
},
34+
},
35+
},
36+
},
37+
},
38+
},
39+
},
40+
}
41+
})
42+
43+
It("should remove path rules that are created by AGIC", func() {
44+
c.CleanUpPathRulesAddedByAGIC()
45+
46+
Expect(*c.appGw.URLPathMaps).To(HaveLen(1))
47+
48+
pathRule := *(*c.appGw.URLPathMaps)[0].PathRules
49+
Expect(pathRule).To(HaveLen(1))
50+
Expect(*pathRule[0].Name).To(Equal(userAddedPathRule))
51+
})
52+
})
53+
})

‎pkg/appgw/requestroutingrules.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ func (c *appGwConfigBuilder) RequestRoutingRules(cbCtx *ConfigBuilderContext) er
3030
requestRoutingRules, pathMaps := c.getRules(cbCtx)
3131

3232
if cbCtx.EnvVariables.EnableBrownfieldDeployment {
33+
c.CleanUpPathRulesAddedByAGIC()
34+
3335
rCtx := brownfield.NewExistingResources(c.appGw, cbCtx.ProhibitedTargets, nil)
3436
{
3537
// PathMaps we obtained from App Gateway - we segment them into ones AGIC is and is not allowed to change.

‎scripts/e2e/cmd/runner/environment.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ const (
2323
// AppGwNameVarName is the name of the applicationGatewayName
2424
AppGwNameVarName = "applicationGatewayName"
2525

26+
// PublicIPAddressNameVarName is the name of the publicIPAddressName
27+
PublicIPAddressNameVarName = "publicIPAddressName"
28+
2629
// KubeConfigVarName is the name of the KUBECONFIG
2730
KubeConfigVarName = "KUBECONFIG"
2831

@@ -41,6 +44,7 @@ type EnvVariables struct {
4144
SubscriptionID string
4245
ResourceGroupName string
4346
AppGwName string
47+
PublicIPAddressName string
4448
SubResourceNamePrefix string
4549
KubeConfigFilePath string
4650
ObjectID string
@@ -53,6 +57,7 @@ func GetEnv() *EnvVariables {
5357
SubscriptionID: os.Getenv(SubscriptionIDVarName),
5458
ResourceGroupName: os.Getenv(ResourceGroupNameVarName),
5559
AppGwName: os.Getenv(AppGwNameVarName),
60+
PublicIPAddressName: os.Getenv(PublicIPAddressNameVarName),
5661
SubResourceNamePrefix: os.Getenv(SubResourceNamePrefixVarName),
5762
KubeConfigFilePath: GetEnvironmentVariable(KubeConfigVarName, "~/.kube/config", nil),
5863
ObjectID: os.Getenv(ObjectIDVarName),

‎scripts/e2e/cmd/runner/helper.go‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,35 @@ func getApplicationGatewaysClient() (*n.ApplicationGatewaysClient, error) {
121121
return &client, nil
122122
}
123123

124+
func getPublicIPAddressesClient() (*n.PublicIPAddressesClient, error) {
125+
env := GetEnv()
126+
127+
settings, err := auth.GetSettingsFromEnvironment()
128+
if err != nil {
129+
return nil, err
130+
}
131+
132+
client := n.NewPublicIPAddressesClientWithBaseURI(settings.Environment.ResourceManagerEndpoint, GetEnv().SubscriptionID)
133+
var authorizer autorest.Authorizer
134+
if env.AzureAuthLocation != "" {
135+
// https://docs.microsoft.com/en-us/azure/developer/go/azure-sdk-authorization#use-file-based-authentication
136+
authorizer, err = auth.NewAuthorizerFromFile(n.DefaultBaseURI)
137+
} else {
138+
authorizer, err = settings.GetAuthorizer()
139+
}
140+
if err != nil {
141+
return nil, err
142+
}
143+
144+
client.Authorizer = authorizer
145+
err = client.AddToUserAgent(UserAgent)
146+
if err != nil {
147+
return nil, err
148+
}
149+
150+
return &client, nil
151+
}
152+
124153
func getRoleAssignmentsClient() (*a.RoleAssignmentsClient, error) {
125154
env := GetEnv()
126155

@@ -842,6 +871,29 @@ func getGateway() (*n.ApplicationGateway, error) {
842871
return &gateway, nil
843872
}
844873

874+
func getPublicIPAddress() (*n.PublicIPAddress, error) {
875+
env := GetEnv()
876+
877+
klog.Info("preparing public ip client")
878+
client, err := getPublicIPAddressesClient()
879+
if err != nil {
880+
return nil, err
881+
}
882+
883+
publicIP, err := client.Get(
884+
context.TODO(),
885+
env.ResourceGroupName,
886+
env.PublicIPAddressName,
887+
"",
888+
)
889+
890+
if err != nil {
891+
return nil, err
892+
}
893+
894+
return &publicIP, nil
895+
}
896+
845897
func supportsNetworkingV1IngressPackage(client clientset.Interface) bool {
846898
version119, _ := version.ParseGeneric("v1.19.0")
847899

‎scripts/e2e/cmd/runner/networking-v1-lfu_one_namespace_one_ingress_test.go‎

Lines changed: 28 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,8 @@ package runner
1111
import (
1212
"context"
1313
"fmt"
14-
"strings"
1514
"time"
1615

17-
"github.com/Azure/go-autorest/autorest/to"
1816
. "github.com/onsi/ginkgo"
1917
. "github.com/onsi/gomega"
2018

@@ -43,33 +41,8 @@ var _ = Describe("networking-v1-LFU", func() {
4341
cleanUp(clientset)
4442
})
4543

46-
It("[prohibited-target-test] prohibited service should be available to be accessed", func() {
47-
// get ip address for 1 ingress
48-
klog.Info("Getting public IP from blacklisted Ingress...")
49-
publicIP, _ := getPublicIP(clientset, "test-brownfield-ns")
50-
Expect(publicIP).ToNot(Equal(""))
51-
52-
//prohibited service will be kept by agic
53-
url_blacklist := fmt.Sprintf("http://%s/blacklist", publicIP)
54-
_, err = makeGetRequest(url_blacklist, "brownfield-blacklist-ns.host", 200, true)
55-
Expect(err).To(BeNil())
56-
57-
//delete namespaces for blacklist testing
58-
deleteOptions := metav1.DeleteOptions{
59-
GracePeriodSeconds: to.Int64Ptr(0),
60-
}
61-
62-
klog.Info("Delete namespaces test-brownfield-ns after blacklist testing...")
63-
err = clientset.CoreV1().Namespaces().Delete(context.TODO(), "test-brownfield-ns", deleteOptions)
64-
Expect(err).To(BeNil())
65-
})
66-
67-
It("[sub-resource-prefix] should be use the sub-resource-prefix to prefix sub-resources", func() {
68-
env := GetEnv()
69-
klog.Infof("'subResourceNamePrefix': %s", env.SubResourceNamePrefix)
70-
Expect(env.SubResourceNamePrefix).ToNot(Equal(""), "Please make sure that environment variable 'subResourceNamePrefix' is set")
71-
72-
namespaceName := "e2e-sub-resource-prefix"
44+
It("[prohibited-target-test] prohibited target should be available to be accessed", func() {
45+
namespaceName := "e2e-prohibited-target"
7346
ns := &v1.Namespace{
7447
ObjectMeta: metav1.ObjectMeta{
7548
Name: namespaceName,
@@ -79,26 +52,38 @@ var _ = Describe("networking-v1-LFU", func() {
7952
_, err = clientset.CoreV1().Namespaces().Create(context.TODO(), ns, metav1.CreateOptions{})
8053
Expect(err).To(BeNil())
8154

82-
SSLE2ERedirectYamlPath := "testdata/networking-v1/one-namespace-one-ingress/ssl-e2e-redirect/app.yaml"
83-
klog.Info("Applying yaml: ", SSLE2ERedirectYamlPath)
84-
err = applyYaml(clientset, crdClient, namespaceName, SSLE2ERedirectYamlPath)
55+
appYamlPath := "testdata/networking-v1/one-namespace-one-ingress/prohibited-target/app.yaml"
56+
klog.Info("Applying yaml: ", appYamlPath)
57+
err = applyYaml(clientset, crdClient, namespaceName, appYamlPath)
8558
Expect(err).To(BeNil())
8659
time.Sleep(30 * time.Second)
8760

88-
gateway, err := getGateway()
61+
// get ip address for 1 ingress
62+
klog.Info("Getting public IP of the app gateway")
63+
ip, err := getPublicIPAddress()
64+
Expect(err).To(BeNil())
65+
66+
publicIP := *ip.IPAddress
67+
klog.Infof("Public IP: %s", publicIP)
68+
69+
protectedPath := fmt.Sprintf("http://%s/landing/", publicIP)
70+
_, err = makeGetRequest(protectedPath, "www.microsoft.com", 302, true)
8971
Expect(err).To(BeNil())
9072

91-
prefixUsed := false
92-
for _, listener := range *gateway.HTTPListeners {
93-
klog.Infof("checking listener %s for %s", *listener.Name, env.SubResourceNamePrefix)
94-
if strings.HasPrefix(*listener.Name, env.SubResourceNamePrefix) {
95-
klog.Infof("found %s that uses the prefix", *listener.Name)
96-
prefixUsed = true
97-
break
98-
}
99-
}
73+
ingressPath := fmt.Sprintf("http://%s/aspnet", publicIP)
74+
_, err = makeGetRequest(ingressPath, "www.microsoft.com", 200, true)
75+
Expect(err).To(BeNil())
10076

101-
Expect(prefixUsed).To(BeTrue(), "%s wasn't used for naming the sub-resource of app gateway. Currently, this check looks at HTTP listener only", env.SubResourceNamePrefix)
77+
klog.Info("Deleting yaml: ", appYamlPath)
78+
err = deleteYaml(clientset, crdClient, namespaceName, appYamlPath)
79+
Expect(err).To(BeNil())
80+
time.Sleep(30 * time.Second)
81+
82+
_, err = makeGetRequest(protectedPath, "www.microsoft.com", 302, true)
83+
Expect(err).To(BeNil())
84+
85+
_, err = makeGetRequest(ingressPath, "www.microsoft.com", 502, true)
86+
Expect(err).To(BeNil())
10287
})
10388

10489
AfterEach(func() {

0 commit comments

Comments
 (0)