Remove FabricGateway support

Initial support zalando-incubator#474 was implemented as a part of an effort to make FabricGateway a Skipper-native route format. Since then the approach has changed and there is a plan to use a new controller that would translate FabricGateway into equivalent RouteGroup. Since RouteGroups are natively supported by Skipper and Ingress Controller there is no need to keep track of the FabricGateway loadbalancer name in the status. Signed-off-by: Alexander Yastrebov <[email protected]>
jhuntwork · Jul 25, 2022 · 268c4b2 · 268c4b2
1 parent 27d524a
commit 268c4b2
Show file tree

Hide file tree

Showing 11 changed files with 9 additions and 509 deletions.
diff --git a/README.md b/README.md
@@ -33,7 +33,6 @@ This information is used to manage AWS resources for each ingress objects of the
 - Support for AWS WAF and WAFv2
 - Support for AWS CNI pod direct access
 - Support for Kubernetes CRD [RouteGroup](https://opensource.zalando.com/skipper/kubernetes/routegroups/)
-- Support for Kubernetes CRD [FabricGateway](https://opensource.zalando.com/skipper/kubernetes/fabricgateways/)
 
 ## Upgrade
 

diff --git a/deploy/ingress-serviceaccount.yaml b/deploy/ingress-serviceaccount.yaml
@@ -39,15 +39,13 @@ rules:
   - zalando.org
   resources:
   - routegroups
-  - fabricgateways
   verbs:
   - get
   - list
 - apiGroups:
   - zalando.org
   resources:
   - routegroups/status
-  - fabricgateways/status
   verbs:
   - patch
   - update

diff --git a/deploy/skipper-serviceaccount.yaml b/deploy/skipper-serviceaccount.yaml
@@ -22,7 +22,6 @@ rules:
   - zalando.org
   resources:
   - routegroups
-  - fabricgateways
   verbs:
   - get
   - list

diff --git a/kubernetes/adapter.go b/kubernetes/adapter.go
@@ -23,15 +23,13 @@ type Adapter struct {
 	ingressDefaultLoadBalancerType string
 	clusterLocalDomain             string
 	routeGroupSupport              bool
-	fabricSupport                  bool
 }
 
 type IngressType string
 
 const (
-	TypeIngress       IngressType = "ingress"
-	TypeRouteGroup    IngressType = "routegroup"
-	TypeFabricGateway IngressType = "fabricgateway"
+	TypeIngress    IngressType = "ingress"
+	TypeRouteGroup IngressType = "routegroup"
 )
 
 const (
@@ -126,7 +124,6 @@ func NewAdapter(config *Config, ingressAPIVersion string, ingressClassFilters []
 		ingressDefaultLoadBalancerType: loadBalancerTypesAWSToIngress[ingressDefaultLoadBalancerType],
 		clusterLocalDomain:             clusterLocalDomain,
 		routeGroupSupport:              true,
-		fabricSupport:                  true,
 	}, nil
 }
 
@@ -168,34 +165,6 @@ func (a *Adapter) newIngressFromRouteGroup(rg *routegroup) (*Ingress, error) {
 	return a.newIngress(TypeRouteGroup, rg.Metadata, host, hostnames)
 }
 
-func (a *Adapter) newIngressFromFabric(fg *fabric) (*Ingress, error) {
-	var host string
-	var hostnames []string
-	for _, lb := range fg.Status.LoadBalancer.Fabric {
-		if lb.Hostname != "" {
-			host = lb.Hostname
-			break
-		}
-	}
-
-	switch {
-	case len(fg.Spec.Service) > 0:
-		for _, svc := range fg.Spec.Service {
-			if svc.Host != "" && (a.clusterLocalDomain == "" || !strings.HasSuffix(svc.Host, a.clusterLocalDomain)) {
-				hostnames = append(hostnames, svc.Host)
-			}
-		}
-	case fg.Spec.ExternalServiceProvider != nil:
-		for _, host := range fg.Spec.ExternalServiceProvider.Hosts {
-			if host != "" && (a.clusterLocalDomain == "" || !strings.HasSuffix(host, a.clusterLocalDomain)) {
-				hostnames = append(hostnames, host)
-			}
-		}
-	}
-
-	return a.newIngress(TypeFabricGateway, fg.Metadata, host, hostnames)
-}
-
 func (a *Adapter) newIngress(typ IngressType, metadata kubeItemMetadata, host string, hostnames []string) (*Ingress, error) {
 	annotations := metadata.Annotations
 
@@ -290,10 +259,10 @@ func (a *Adapter) IngressFiltersString() string {
 	return strings.TrimSpace(strings.Join(a.ingressFilters, ","))
 }
 
-// ListResources can be used to obtain the list of ingress, routegroup and
-// fabric resources for all namespaces filtered by class. It
+// ListResources can be used to obtain the list of ingress and routegroup
+// resources for all namespaces filtered by class. It
 // returns the Ingress business object, that for the controller does
-// not matter to be fabricgateway, routegroup or ingress..
+// not matter to be routegroup or ingress..
 func (a *Adapter) ListResources() ([]*Ingress, error) {
 	ings, err := a.ListIngress()
 	if err != nil {
@@ -314,28 +283,14 @@ func (a *Adapter) ListResources() ([]*Ingress, error) {
 		}
 	}
 
-	var fgs []*Ingress
-	if a.fabricSupport {
-		fgs, err = a.ListFabricgateways()
-		if err != nil {
-			if errors.Is(err, ErrResourceNotFound) || errors.Is(err, ErrNoPermissionToAccessResource) {
-				a.fabricSupport = false
-				log.Warnf("Disabling FabricGateway support because listing FabricGateways failed: %v, to get more information https://opensource.zalando.com/skipper/kubernetes/fabric/#fabricgateways", err)
-			} else {
-				// Generic error, FabricGateway CRD exists and we have permission to access
-				return nil, err
-			}
-		}
-	}
-
 	ings = append(ings, rgs...)
-	return append(ings, fgs...), nil
+	return ings, nil
 }
 
 // ListIngress can be used to obtain the list of ingress resources for
 // all namespaces filtered by class. It returns the Ingress business
 // object, that for the controller does not matter to be
-// fabricgateway, routegroup or ingress..
+// routegroup or ingress..
 func (a *Adapter) ListIngress() ([]*Ingress, error) {
 	il, err := a.ingressClient.listIngress(a.kubeClient)
 	if err != nil {
@@ -363,7 +318,7 @@ func (a *Adapter) ListIngress() ([]*Ingress, error) {
 // ListRoutegroups can be used to obtain the list of Ingress resources
 // for all namespaces filtered by class. It returns the Ingress
 // business object, that for the controller does not matter to be
-// fabricgateway, routegroup or ingress.
+// routegroup or ingress.
 func (a *Adapter) ListRoutegroups() ([]*Ingress, error) {
 	rgs, err := listRoutegroups(a.kubeClient)
 	if err != nil {
@@ -389,35 +344,6 @@ func (a *Adapter) ListRoutegroups() ([]*Ingress, error) {
 	return ret, nil
 }
 
-// ListFabricgateways can be used to obtain the list of Ingress resources
-// for all namespaces filtered by class. It returns the Ingress
-// business object, that for the controller does not matter to be
-// fabricgateway, routegroup or ingress.
-func (a *Adapter) ListFabricgateways() ([]*Ingress, error) {
-	fgs, err := listFabricgateways(a.kubeClient)
-	if err != nil {
-		return nil, err
-	}
-
-	var ret []*Ingress
-	for _, fg := range fgs.Items {
-		if !a.supportedCRD(fg.Metadata) {
-			continue
-		}
-		ing, err := a.newIngressFromFabric(fg)
-		if err == nil {
-			ret = append(ret, ing)
-		} else {
-			log.WithFields(log.Fields{
-				"type": TypeFabricGateway,
-				"ns":   fg.Metadata.Namespace,
-				"name": fg.Metadata.Name,
-			}).Errorf("%v", err)
-		}
-	}
-	return ret, nil
-}
-
 func (a *Adapter) supportedCRD(metadata kubeItemMetadata) bool {
 	if len(a.ingressFilters) == 0 {
 		return true
@@ -464,8 +390,6 @@ func (a *Adapter) UpdateIngressLoadBalancer(ingress *Ingress, loadBalancerDNSNam
 	}
 
 	switch ingress.ResourceType {
-	case TypeFabricGateway:
-		return updateFabricgatewayLoadBalancer(a.kubeClient, ingress.Namespace, ingress.Name, loadBalancerDNSName)
 	case TypeRouteGroup:
 		return updateRoutegroupLoadBalancer(a.kubeClient, ingress.Namespace, ingress.Name, loadBalancerDNSName)
 	case TypeIngress:

diff --git a/kubernetes/adapter_test.go b/kubernetes/adapter_test.go
@@ -488,8 +488,6 @@ func (c *mockClient) get(res string) (io.ReadCloser, error) {
 
 	var fixture string
 	switch res {
-	case fabricListResource:
-		fixture = "testdata/fixture01_fg.json"
 	case routegroupListResource:
 		fixture = "testdata/fixture01_rg.json"
 	case fmt.Sprintf(ingressListResource, IngressAPIVersionNetworking):
@@ -514,8 +512,6 @@ func (c *mockClient) patch(res string, payload []byte) (io.ReadCloser, error) {
 			return io.NopCloser(strings.NewReader(":)")), nil
 		case "/apis/zalando.org/v1/namespaces/default/routegroups/foo/status":
 			return io.NopCloser(strings.NewReader(":)")), nil
-		case "/apis/zalando.org/v1/namespaces/default/fabricgateways/foo/status":
-			return io.NopCloser(strings.NewReader(":)")), nil
 		}
 	}
 	return nil, errors.New("mocked error")
@@ -597,35 +593,6 @@ func TestUpdateRouteGroupLoadBalancer(t *testing.T) {
 	}
 }
 
-func TestUpdateFabricGatewayLoadBalancer(t *testing.T) {
-	a, _ := NewAdapter(testConfig, IngressAPIVersionNetworking, testIngressFilter, testSecurityGroup, testSSLPolicy, aws.LoadBalancerTypeApplication, DefaultClusterLocalDomain, false)
-	client := &mockClient{}
-	a.kubeClient = client
-	ing := &Ingress{
-		Namespace:      "default",
-		Name:           "foo",
-		Hostname:       "bar",
-		CertificateARN: "zbr",
-		ResourceType:   TypeFabricGateway,
-	}
-	if err := a.UpdateIngressLoadBalancer(ing, "bar"); err != ErrUpdateNotNeeded {
-		t.Error("expected ErrUpdateNotNeeded")
-	}
-	if err := a.UpdateIngressLoadBalancer(ing, "xpto"); err != nil {
-		t.Error(err)
-	}
-	client.broken = true
-	if err := a.UpdateIngressLoadBalancer(ing, "xpto"); err == nil {
-		t.Error("expected an error")
-	}
-	if err := a.UpdateIngressLoadBalancer(ing, ""); err == nil {
-		t.Error("expected an error")
-	}
-	if err := a.UpdateIngressLoadBalancer(nil, "xpto"); err == nil {
-		t.Error("expected an error")
-	}
-}
-
 func TestBrokenConfig(t *testing.T) {
 	for _, test := range []struct {
 		name string
@@ -684,9 +651,6 @@ func TestListIngressFilterClass(t *testing.T) {
 				"fixture-rg01",
 				"fixture-rg02",
 				"fixture-rg03",
-				"fixture-fg01",
-				"fixture-fg02",
-				"fixture-fg03",
 			},
 		},
 		"emptyIngressClassFilters2": {
@@ -700,25 +664,20 @@ func TestListIngressFilterClass(t *testing.T) {
 				"fixture-rg01",
 				"fixture-rg02",
 				"fixture-rg03",
-				"fixture-fg01",
-				"fixture-fg02",
-				"fixture-fg03",
 			},
 		},
 		"singleIngressClass1": {
 			ingressClassFilters: []string{"skipper"},
 			expectedIngressNames: []string{
 				"fixture02",
 				"fixture-rg02",
-				"fixture-fg02",
 			},
 		},
 		"singleIngressClass2": {
 			ingressClassFilters: []string{"other"},
 			expectedIngressNames: []string{
 				"fixture03",
 				"fixture-rg03",
-				"fixture-fg03",
 			},
 		},
 		"multipleIngressClass": {
@@ -728,8 +687,6 @@ func TestListIngressFilterClass(t *testing.T) {
 				"fixture03",
 				"fixture-rg02",
 				"fixture-rg03",
-				"fixture-fg02",
-				"fixture-fg03",
 			},
 		},
 		"multipleIngressClassWithDefault": {
@@ -739,8 +696,6 @@ func TestListIngressFilterClass(t *testing.T) {
 				"fixture02",
 				"fixture-rg01",
 				"fixture-rg02",
-				"fixture-fg01",
-				"fixture-fg02",
 			},
 		},
 		"multipleIngressClassWithDefault2": {
@@ -750,16 +705,13 @@ func TestListIngressFilterClass(t *testing.T) {
 				"fixture03",
 				"fixture-rg01",
 				"fixture-rg03",
-				"fixture-fg01",
-				"fixture-fg03",
 			},
 		},
 		"multipleIngressClassMixedAnnotationAndSpec": {
 			ingressClassFilters: []string{"other", "another"},
 			expectedIngressNames: []string{
 				"fixture03",
 				"fixture-rg03",
-				"fixture-fg03",
 				"fixture04",
 			},
 		},