Repository: zalando-incubator/kube-ingress-aws-controller
Pull request #354 (2020-06-02 08:21:46)

Add support for networking.k8s.io/v1beta1
Adds support for listing and updating ingresses via the newer `networking.k8s.io/v1beta1` apiVersion. This also becomes the default but can be reverted back to `extensions/v1beta1` via the flag `--ingress-api-version`. The motivation for making it the default is that it was introduced already in Kubernetes v1.14.

szuecs: @mikkeloscar great!
Please increase the version in https://github.com/zalando-incubator/kube-ingress-aws-controller/blob/master/VERSION and add a note to https://github.com/zalando-incubator/kube-ingress-aws-controller/blob/master/README.md#upgrade. Clusters older than 1.14 have to do something.
mikkeloscar: @szuecs I have update version and README
szuecs: :+1:
mikkeloscar: :+1: 

Patch:
diff --git a/README.md b/README.md
index 5a71eb2..df24c5d 100644
--- a/README.md
+++ b/README.md
@@ -33,7 +33,18 @@ This information is used to manage AWS resources for each ingress objects of the
 
 ## Upgrade
 
-### <0.9.0 to >=v0.9.0
+### <v0.11.0 to >=0.11.0
+
+Version `v0.11.0` changes the default `apiVersion` used for fetching/updating
+ingresses from `extensions/v1beta1` to `networking.k8s.io/v1beta1`. For this to
+work the controller needs to have permissions to `list` `ingresses` and
+`update`, `patch` `ingresses/status` from the `networking.k8s.io` `apiGroup`.
+[See deployment example](deploy/ingress-serviceaccount.yaml). To fallback to
+the old behavior you can set the apiVersion via the `--ingress-api-version`
+flag. Value must be `extensions/v1beta1` or `networking.k8s.io/v1beta1`
+(default).
+
+### <v0.9.0 to >=v0.9.0
 
 Version `v0.9.0` changes the internal flag parsing library to
 [kingpin][kingpin] this means flags are now defined with `--` (two dashes)
diff --git a/VERSION b/VERSION
index 9097bf9..5416288 100644
--- a/VERSION
+++ b/VERSION
@@ -1,1 +1,1 @@
-v0.10
+v0.11
diff --git a/controller.go b/controller.go
index ec3fdea..ca7b8bd 100644
--- a/controller.go
+++ b/controller.go
@@ -68,6 +68,7 @@ var (
 	loadBalancerType              string
 	nlbCrossZone                  bool
 	nlbHTTPEnabled                bool
+	ingressAPIVersion             string
 )
 
 func loadSettings() error {
@@ -130,6 +131,8 @@ func loadSettings() error {
 		Default("false").BoolVar(&nlbCrossZone)
 	kingpin.Flag("nlb-http-enabled", "Enable HTTP (port 80) for Network Load Balancers. By default this is disabled as NLB can't provide HTTP -> HTTPS redirect.").
 		Default("false").BoolVar(&nlbHTTPEnabled)
+	kingpin.Flag("ingress-api-version", "APIversion used for listing/updating ingresses.").
+		Default(kubernetes.IngressAPIVersionNetworking).EnumVar(&ingressAPIVersion, kubernetes.IngressAPIVersionNetworking, kubernetes.IngressAPIVersionExtensions)
 	kingpin.Parse()
 
 	blacklistCertArnMap = make(map[string]bool)
@@ -258,7 +261,7 @@ func main() {
 	}
 
 	log.Debug("kubernetes.NewAdapter")
-	kubeAdapter, err = kubernetes.NewAdapter(kubeConfig, ingressClassFiltersList, awsAdapter.SecurityGroupID(), sslPolicy, loadBalancerType, clusterLocalDomain, disableInstrumentedHttpClient)
+	kubeAdapter, err = kubernetes.NewAdapter(kubeConfig, ingressAPIVersion, ingressClassFiltersList, awsAdapter.SecurityGroupID(), sslPolicy, loadBalancerType, clusterLocalDomain, disableInstrumentedHttpClient)
 	if err != nil {
 		log.Fatal(err)
 	}
diff --git a/deploy/ingress-serviceaccount.yaml b/deploy/ingress-serviceaccount.yaml
index 8d0a25e..1fd640a 100644
--- a/deploy/ingress-serviceaccount.yaml
+++ b/deploy/ingress-serviceaccount.yaml
@@ -10,16 +10,18 @@ kind: ClusterRole
 metadata:
   name: ingress-controller
 rules:
-- apiGroups:
+- apiGroups: # only one of extensions, networking.k8s.io is needed depending on the --ingress-api-version flag
   - extensions
+  - networking.k8s.io
   resources:
   - ingresses
   verbs:
   - get
   - list
   - watch
-- apiGroups:
+- apiGroups: # only one of extensions, networking.k8s.io is needed depending on the --ingress-api-version flag
   - extensions
+  - networking.k8s.io
   resources:
   - ingresses/status
   verbs:
diff --git a/kubernetes/adapter.go b/kubernetes/adapter.go
index 2e6a5f9..ba6cfc1 100644
--- a/kubernetes/adapter.go
+++ b/kubernetes/adapter.go
@@ -12,6 +12,7 @@ import (
 
 type Adapter struct {
 	kubeClient                     client
+	ingressClient                  *ingressClient
 	ingressFilters                 []string
 	ingressDefaultSecurityGroup    string
 	ingressDefaultSSLPolicy        string
@@ -113,7 +114,7 @@ func (c *ConfigMap) String() string {
 }
 
 // NewAdapter creates an Adapter for Kubernetes using a given configuration.
-func NewAdapter(config *Config, ingressClassFilters []string, ingressDefaultSecurityGroup, ingressDefaultSSLPolicy, ingressDefaultLoadBalancerType, clusterLocalDomain string, disableInstrumentedHttpClient bool) (*Adapter, error) {
+func NewAdapter(config *Config, ingressAPIVersion string, ingressClassFilters []string, ingressDefaultSecurityGroup, ingressDefaultSSLPolicy, ingressDefaultLoadBalancerType, clusterLocalDomain string, disableInstrumentedHttpClient bool) (*Adapter, error) {
 	if config == nil || config.BaseURL == "" {
 		return nil, ErrInvalidConfiguration
 	}
@@ -123,6 +124,7 @@ func NewAdapter(config *Config, ingressClassFilters []string, ingressDefaultSecu
 	}
 	return &Adapter{
 		kubeClient:                     c,
+		ingressClient:                  &ingressClient{apiVersion: ingressAPIVersion},
 		ingressFilters:                 ingressClassFilters,
 		ingressDefaultSecurityGroup:    ingressDefaultSecurityGroup,
 		ingressDefaultSSLPolicy:        ingressDefaultSSLPolicy,
@@ -335,7 +337,7 @@ func (a *Adapter) ListResources() ([]*Ingress, error) {
 // object, that for the controller does not matter to be routegroup or
 // ingress..
 func (a *Adapter) ListIngress() ([]*Ingress, error) {
-	il, err := listIngress(a.kubeClient)
+	il, err := a.ingressClient.listIngress(a.kubeClient)
 	if err != nil {
 		return nil, err
 	}
@@ -400,7 +402,7 @@ func (a *Adapter) UpdateIngressLoadBalancer(ingress *Ingress, loadBalancerDNSNam
 	case ingressTypeRouteGroup:
 		return updateRoutegroupLoadBalancer(a.kubeClient, newRouteGroupForKube(ingress), loadBalancerDNSName)
 	case ingressTypeIngress:
-		return updateIngressLoadBalancer(a.kubeClient, newIngressForKube(ingress), loadBalancerDNSName)
+		return a.ingressClient.updateIngressLoadBalancer(a.kubeClient, newIngressForKube(ingress), loadBalancerDNSName)
 	}
 	return fmt.Errorf("Unknown resourceType '%s', failed to update Kubernetes resource", ingress.resourceType)
 }
diff --git a/kubernetes/ingress.go b/kubernetes/ingress.go
index 0f2db85..71ffb13 100644
--- a/kubernetes/ingress.go
+++ b/kubernetes/ingress.go
@@ -61,8 +61,10 @@ type ingressLoadBalancer struct {
 const (
 	// ingressALBIPAddressType is used in external-dns, https://github.com/kubernetes-incubator/external-dns/pull/1079
 	ingressALBIPAddressType           = "alb.ingress.kubernetes.io/ip-address-type"
-	ingressListResource               = "/apis/extensions/v1beta1/ingresses"
-	ingressPatchStatusResource        = "/apis/extensions/v1beta1/namespaces/%s/ingresses/%s/status"
+	IngressAPIVersionExtensions       = "extensions/v1beta1"
+	IngressAPIVersionNetworking       = "networking.k8s.io/v1beta1"
+	ingressListResource               = "/apis/%s/ingresses"
+	ingressPatchStatusResource        = "/apis/%s/namespaces/%s/ingresses/%s/status"
 	ingressCertificateARNAnnotation   = "zalando.org/aws-load-balancer-ssl-cert"
 	ingressSchemeAnnotation           = "zalando.org/aws-load-balancer-scheme"
 	ingressSharedAnnotation           = "zalando.org/aws-load-balancer-shared"
@@ -81,8 +83,12 @@ func getAnnotationsString(annotations map[string]string, key string, defaultValu
 	return defaultValue
 }
 
-func listIngress(c client) (*ingressList, error) {
-	r, err := c.get(ingressListResource)
+type ingressClient struct {
+	apiVersion string
+}
+
+func (ic *ingressClient) listIngress(c client) (*ingressList, error) {
+	r, err := c.get(fmt.Sprintf(ingressListResource, ic.apiVersion))
 	if err != nil {
 		return nil, fmt.Errorf("failed to get ingress list: %v", err)
 	}
@@ -106,7 +112,7 @@ type patchIngressStatus struct {
 	Status ingressStatus `json:"status"`
 }
 
-func updateIngressLoadBalancer(c client, i *ingress, newHostName string) error {
+func (ic *ingressClient) updateIngressLoadBalancer(c client, i *ingress, newHostName string) error {
 	ns, name := i.Metadata.Namespace, i.Metadata.Name
 	for _, ingressLb := range i.Status.LoadBalancer.Ingress {
 		if ingressLb.Hostname == newHostName {
@@ -122,7 +128,7 @@ func updateIngressLoadBalancer(c client, i *ingress, newHostName string) error {
 		},
 	}
 
-	resource := fmt.Sprintf(ingressPatchStatusResource, ns, name)
+	resource := fmt.Sprintf(ingressPatchStatusResource, ic.apiVersion, ns, name)
 	payload, err := json.Marshal(patchStatus)
 	if err != nil {
 		return err


Test patch:
diff --git a/kubernetes/adapter_test.go b/kubernetes/adapter_test.go
index fda7131..ad963fa 100644
--- a/kubernetes/adapter_test.go
+++ b/kubernetes/adapter_test.go
@@ -225,7 +225,7 @@ func TestMappingRoundtrip(tt *testing.T) {
 		},
 	} {
 		tt.Run(tc.msg, func(t *testing.T) {
-			a, err := NewAdapter(testConfig, testIngressFilter, testIngressDefaultSecurityGroup, testSSLPolicy, testLoadBalancerTypeAWS, DefaultClusterLocalDomain, false)
+			a, err := NewAdapter(testConfig, IngressAPIVersionNetworking, testIngressFilter, testIngressDefaultSecurityGroup, testSSLPolicy, testLoadBalancerTypeAWS, DefaultClusterLocalDomain, false)
 			if err != nil {
 				t.Fatalf("cannot create kubernetes adapter: %v", err)
 			}
@@ -271,7 +271,7 @@ func (c *mockClient) get(res string) (io.ReadCloser, error) {
 	switch res {
 	case routegroupListResource:
 		fixture = "testdata/fixture01_rg.json"
-	case ingressListResource:
+	case fmt.Sprintf(ingressListResource, IngressAPIVersionNetworking):
 		fixture = "testdata/fixture01.json"
 	case fmt.Sprintf(configMapResource, "foo-ns", "foo-name"):
 		fixture = "testdata/fixture02.json"
@@ -289,7 +289,7 @@ func (c *mockClient) get(res string) (io.ReadCloser, error) {
 func (c *mockClient) patch(res string, payload []byte) (io.ReadCloser, error) {
 	if !c.broken {
 		switch res {
-		case "/apis/extensions/v1beta1/namespaces/default/ingresses/foo/status":
+		case fmt.Sprintf("/apis/%s/namespaces/default/ingresses/foo/status", IngressAPIVersionNetworking):
 			return ioutil.NopCloser(strings.NewReader(":)")), nil
 		case "/apis/zalando.org/v1/namespaces/default/routegroups/foo/status":
 			return ioutil.NopCloser(strings.NewReader(":)")), nil
@@ -299,7 +299,7 @@ func (c *mockClient) patch(res string, payload []byte) (io.ReadCloser, error) {
 }
 
 func TestListIngress(t *testing.T) {
-	a, _ := NewAdapter(testConfig, testIngressFilter, testIngressDefaultSecurityGroup, testSSLPolicy, testLoadBalancerTypeAWS, DefaultClusterLocalDomain, false)
+	a, _ := NewAdapter(testConfig, IngressAPIVersionNetworking, testIngressFilter, testIngressDefaultSecurityGroup, testSSLPolicy, testLoadBalancerTypeAWS, DefaultClusterLocalDomain, false)
 	client := &mockClient{}
 	a.kubeClient = client
 	ingresses, err := a.ListIngress()
@@ -317,7 +317,7 @@ func TestListIngress(t *testing.T) {
 }
 
 func TestUpdateIngressLoadBalancer(t *testing.T) {
-	a, _ := NewAdapter(testConfig, testIngressFilter, testSecurityGroup, testSSLPolicy, testLoadBalancerTypeAWS, DefaultClusterLocalDomain, false)
+	a, _ := NewAdapter(testConfig, IngressAPIVersionNetworking, testIngressFilter, testSecurityGroup, testSSLPolicy, testLoadBalancerTypeAWS, DefaultClusterLocalDomain, false)
 	client := &mockClient{}
 	a.kubeClient = client
 	ing := &Ingress{
@@ -343,7 +343,7 @@ func TestUpdateIngressLoadBalancer(t *testing.T) {
 }
 
 func TestUpdateRouteGroupLoadBalancer(t *testing.T) {
-	a, _ := NewAdapter(testConfig, testIngressFilter, testSecurityGroup, testSSLPolicy, testLoadBalancerTypeAWS, DefaultClusterLocalDomain, false)
+	a, _ := NewAdapter(testConfig, IngressAPIVersionNetworking, testIngressFilter, testSecurityGroup, testSSLPolicy, testLoadBalancerTypeAWS, DefaultClusterLocalDomain, false)
 	client := &mockClient{}
 	a.kubeClient = client
 	ing := &Ingress{
@@ -379,7 +379,7 @@ func TestBrokenConfig(t *testing.T) {
 		{"broken-cert", &Config{BaseURL: "dontcare", TLSClientConfig: TLSClientConfig{CAFile: "testdata/broken.pem"}}},
 	} {
 		t.Run(fmt.Sprintf("%v", test.cfg), func(t *testing.T) {
-			_, err := NewAdapter(test.cfg, testIngressFilter, testSecurityGroup, testSSLPolicy, testLoadBalancerTypeAWS, DefaultClusterLocalDomain, false)
+			_, err := NewAdapter(test.cfg, IngressAPIVersionNetworking, testIngressFilter, testSecurityGroup, testSSLPolicy, testLoadBalancerTypeAWS, DefaultClusterLocalDomain, false)
 			if err == nil {
 				t.Error("expected an error")
 			}
@@ -388,7 +388,7 @@ func TestBrokenConfig(t *testing.T) {
 }
 
 func TestAdapter_GetConfigMap(t *testing.T) {
-	a, _ := NewAdapter(testConfig, testIngressFilter, testIngressDefaultSecurityGroup, testSSLPolicy, testLoadBalancerTypeAWS, DefaultClusterLocalDomain, false)
+	a, _ := NewAdapter(testConfig, IngressAPIVersionNetworking, testIngressFilter, testIngressDefaultSecurityGroup, testSSLPolicy, testLoadBalancerTypeAWS, DefaultClusterLocalDomain, false)
 	client := &mockClient{}
 	a.kubeClient = client
 
@@ -480,7 +480,7 @@ func TestListIngressFilterClass(t *testing.T) {
 		},
 	} {
 		t.Run(name, func(t *testing.T) {
-			a, _ := NewAdapter(testConfig, test.ingressClassFilters, testIngressDefaultSecurityGroup, testSSLPolicy, testLoadBalancerTypeAWS, DefaultClusterLocalDomain, false)
+			a, _ := NewAdapter(testConfig, IngressAPIVersionNetworking, test.ingressClassFilters, testIngressDefaultSecurityGroup, testSSLPolicy, testLoadBalancerTypeAWS, DefaultClusterLocalDomain, false)
 			client := &mockClient{}
 			a.kubeClient = client
 			ingresses, err := a.ListResources()
diff --git a/kubernetes/ingress_test.go b/kubernetes/ingress_test.go
index c0bd8bd..03dcc60 100644
--- a/kubernetes/ingress_test.go
+++ b/kubernetes/ingress_test.go
@@ -21,12 +21,13 @@ func TestListIngresses(t *testing.T) {
 	}))
 	defer testServer.Close()
 	kubeClient, _ := newSimpleClient(&Config{BaseURL: testServer.URL}, false)
+	ingressClient := &ingressClient{apiVersion: IngressAPIVersionNetworking}
 	want := newList(
 		newIngress("fixture01", nil, "example.org", "fixture01"),
 		newIngress("fixture02", map[string]string{ingressClassAnnotation: "skipper"}, "skipper.example.org", "fixture02"),
 		newIngress("fixture03", map[string]string{ingressClassAnnotation: "other"}, "other.example.org", "fixture03"),
 	)
-	got, err := listIngress(kubeClient)
+	got, err := ingressClient.listIngress(kubeClient)
 	if err != nil {
 		t.Errorf("unexpected error from listIngresses: %v", err)
 	} else {
@@ -52,8 +53,9 @@ func TestListIngressFailureScenarios(t *testing.T) {
 			defer testServer.Close()
 			cfg := &Config{BaseURL: testServer.URL}
 			kubeClient, _ := newSimpleClient(cfg, false)
+			ingressClient := &ingressClient{apiVersion: IngressAPIVersionNetworking}
 
-			_, err := listIngress(kubeClient)
+			_, err := ingressClient.listIngress(kubeClient)
 			if err == nil {
 				t.Error("expected an error but list ingress call succeeded")
 			}
@@ -68,7 +70,7 @@ func TestUpdateIngressLoaBalancer(t *testing.T) {
 		"application/strategic-merge-patch+json": true,
 	}
 	testServer := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
-		if req.URL.Path != fmt.Sprintf(ingressPatchStatusResource, "foo", "bar") {
+		if req.URL.Path != fmt.Sprintf(ingressPatchStatusResource, IngressAPIVersionNetworking, "foo", "bar") {
 			t.Error("unexpected URL path sent by the client", req.URL.Path)
 		}
 		if req.Method != "PATCH" {
@@ -92,6 +94,7 @@ func TestUpdateIngressLoaBalancer(t *testing.T) {
 	defer testServer.Close()
 	cfg := &Config{BaseURL: testServer.URL}
 	kubeClient, _ := newSimpleClient(cfg, false)
+	ingressClient := &ingressClient{apiVersion: IngressAPIVersionNetworking}
 	ing := &ingress{
 		Metadata: kubeItemMetadata{
 			Namespace: "foo",
@@ -99,7 +102,7 @@ func TestUpdateIngressLoaBalancer(t *testing.T) {
 		},
 	}
 
-	if err := updateIngressLoadBalancer(kubeClient, ing, "example.org"); err != nil {
+	if err := ingressClient.updateIngressLoadBalancer(kubeClient, ing, "example.org"); err != nil {
 		t.Error("unexpected result from update call:", err)
 	}
 }
@@ -111,6 +114,7 @@ func TestUpdateIngressFailureScenarios(t *testing.T) {
 	defer testServer.Close()
 	cfg := &Config{BaseURL: testServer.URL}
 	kubeClient, _ := newSimpleClient(cfg, false)
+	ingressClient := &ingressClient{apiVersion: IngressAPIVersionNetworking}
 	for _, test := range []struct {
 		ing *ingress
 	}{
@@ -119,7 +123,7 @@ func TestUpdateIngressFailureScenarios(t *testing.T) {
 	} {
 		arn := getAnnotationsString(test.ing.Metadata.Annotations, ingressCertificateARNAnnotation, "<missing>")
 		t.Run(fmt.Sprintf("%v/%v", test.ing.Status.LoadBalancer.Ingress[0].Hostname, arn), func(t *testing.T) {
-			err := updateIngressLoadBalancer(kubeClient, test.ing, "example.com")
+			err := ingressClient.updateIngressLoadBalancer(kubeClient, test.ing, "example.com")
 			if err == nil {
 				t.Error("expected an error but update ingress call succeeded")
 			}
@@ -147,10 +151,10 @@ func TestAnnotationsFallback(t *testing.T) {
 
 func newList(ingresses ...*ingress) *ingressList {
 	ret := ingressList{
-		APIVersion: "extensions/v1beta1",
+		APIVersion: IngressAPIVersionNetworking,
 		Kind:       "IngressList",
 		Metadata: ingressListMetadata{
-			SelfLink:        "/apis/extensions/v1beta1/ingresses",
+			SelfLink:        fmt.Sprintf("/apis/%s/ingresses", IngressAPIVersionNetworking),
 			ResourceVersion: "42",
 		},
 		Items: ingresses,
@@ -165,7 +169,7 @@ func newIngress(name string, annotations map[string]string, hostname string, arn
 			Namespace:         "default",
 			Annotations:       annotations,
 			ResourceVersion:   "42",
-			SelfLink:          "/apis/extensions/v1beta1/namespaces/default/ingresses/" + name,
+			SelfLink:          fmt.Sprintf("/apis/%s/namespaces/default/ingresses/", IngressAPIVersionNetworking) + name,
 			Generation:        1,
 			UID:               name,
 			CreationTimestamp: time.Date(2016, 11, 29, 14, 53, 42, 0, time.UTC),
diff --git a/kubernetes/testdata/fixture01.json b/kubernetes/testdata/fixture01.json
index ed3ad61..5861912 100644
--- a/kubernetes/testdata/fixture01.json
+++ b/kubernetes/testdata/fixture01.json
@@ -1,8 +1,8 @@
 {
   "kind": "IngressList",
-  "apiVersion": "extensions/v1beta1",
+  "apiVersion": "networking.k8s.io/v1beta1",
   "metadata": {
-    "selfLink": "/apis/extensions/v1beta1/ingresses",
+    "selfLink": "/apis/networking.k8s.io/v1beta1/ingresses",
     "resourceVersion": "42"
   },
   "items": [
@@ -10,7 +10,7 @@
       "metadata": {
         "name": "fixture01",
         "namespace": "default",
-        "selfLink": "/apis/extensions/v1beta1/namespaces/default/ingresses/fixture01",
+        "selfLink": "/apis/networking.k8s.io/v1beta1/namespaces/default/ingresses/fixture01",
         "uid": "fixture01",
         "resourceVersion": "42",
         "generation": 1,
@@ -31,7 +31,7 @@
       "metadata": {
         "name": "fixture02",
         "namespace": "default",
-        "selfLink": "/apis/extensions/v1beta1/namespaces/default/ingresses/fixture02",
+        "selfLink": "/apis/networking.k8s.io/v1beta1/namespaces/default/ingresses/fixture02",
         "uid": "fixture02",
         "resourceVersion": "42",
         "generation": 1,
@@ -53,7 +53,7 @@
       "metadata": {
         "name": "fixture03",
         "namespace": "default",
-        "selfLink": "/apis/extensions/v1beta1/namespaces/default/ingresses/fixture03",
+        "selfLink": "/apis/networking.k8s.io/v1beta1/namespaces/default/ingresses/fixture03",
         "uid": "fixture03",
         "resourceVersion": "42",
         "generation": 1,