Skip to content

Commit

Permalink
Remove FabricGateway support
Browse files Browse the repository at this point in the history
Initial support #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]>
  • Loading branch information
AlexanderYastrebov committed Jul 25, 2022
1 parent 27d524a commit 268c4b2
Show file tree
Hide file tree
Showing 11 changed files with 9 additions and 509 deletions.
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 0 additions & 2 deletions deploy/ingress-serviceaccount.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,13 @@ rules:
- zalando.org
resources:
- routegroups
- fabricgateways
verbs:
- get
- list
- apiGroups:
- zalando.org
resources:
- routegroups/status
- fabricgateways/status
verbs:
- patch
- update
Expand Down
1 change: 0 additions & 1 deletion deploy/skipper-serviceaccount.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ rules:
- zalando.org
resources:
- routegroups
- fabricgateways
verbs:
- get
- list
Expand Down
92 changes: 8 additions & 84 deletions kubernetes/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -126,7 +124,6 @@ func NewAdapter(config *Config, ingressAPIVersion string, ingressClassFilters []
ingressDefaultLoadBalancerType: loadBalancerTypesAWSToIngress[ingressDefaultLoadBalancerType],
clusterLocalDomain: clusterLocalDomain,
routeGroupSupport: true,
fabricSupport: true,
}, nil
}

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand Down
48 changes: 0 additions & 48 deletions kubernetes/adapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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")
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -684,9 +651,6 @@ func TestListIngressFilterClass(t *testing.T) {
"fixture-rg01",
"fixture-rg02",
"fixture-rg03",
"fixture-fg01",
"fixture-fg02",
"fixture-fg03",
},
},
"emptyIngressClassFilters2": {
Expand All @@ -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": {
Expand All @@ -728,8 +687,6 @@ func TestListIngressFilterClass(t *testing.T) {
"fixture03",
"fixture-rg02",
"fixture-rg03",
"fixture-fg02",
"fixture-fg03",
},
},
"multipleIngressClassWithDefault": {
Expand All @@ -739,8 +696,6 @@ func TestListIngressFilterClass(t *testing.T) {
"fixture02",
"fixture-rg01",
"fixture-rg02",
"fixture-fg01",
"fixture-fg02",
},
},
"multipleIngressClassWithDefault2": {
Expand All @@ -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",
},
},
Expand Down
Loading

0 comments on commit 268c4b2

Please sign in to comment.