Skip to content

Commit

Permalink
wip: add --gateway-discovery-dns-strategy flag
Browse files Browse the repository at this point in the history
  • Loading branch information
pmalek committed May 25, 2023
1 parent e237dc0 commit dd20d7b
Show file tree
Hide file tree
Showing 14 changed files with 388 additions and 282 deletions.
18 changes: 15 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ Adding a new version? You'll need three changes:
[#3963](https://github.com/Kong/kubernetes-ingress-controller/pull/3963)
- Added translator to translate `HTTPRoute` and `GRPCRoute` in gateway APIs to
expression based kong routes. Similar to ingresses, this translator is only
enabled when feature gate `ExpressionRoutes` is turned on and the managed
enabled when feature gate `ExpressionRoutes` is turned on and the managed
Kong gateway runs in router flavor `expressions`.
[#3956](https://github.com/Kong/kubernetes-ingress-controller/pull/3956)
[#3988](https://github.com/Kong/kubernetes-ingress-controller/pull/3988)
Expand All @@ -125,13 +125,25 @@ Adding a new version? You'll need three changes:
Runtime Group Admin API.
[#4029](https://github.com/Kong/kubernetes-ingress-controller/pull/4029)
- Record an event attached to KIC pod after applying configuration to Kong. If
the applying succeeded, a `Normal` event with `KongConfigurationSucceeded`
reason is recorded. If the applying failed, a `Warning` event with
the applying succeeded, a `Normal` event with `KongConfigurationSucceeded`
reason is recorded. If the applying failed, a `Warning` event with
`KongConfigurationApplyFailed` reason is recorded.
[#4054](https://github.com/Kong/kubernetes-ingress-controller/pull/4054)
- Disable translation to expression routes when feature gate `ExpressionRoutes`
is enabled but feature gate `CombinedRoutes` is not enabled.
[#4057](https://github.com/Kong/kubernetes-ingress-controller/pull/4057)
- Added `--gateway-discovery-dns-strategy` flag which allows specifying which
DNS strategy to use when generating Gateway's Admin API addresses.
[#4071](https://github.com/Kong/kubernetes-ingress-controller/pull/4071)

There are 3 options available
- `ip` (default): which will make KIC create Admin API addresses built out of
IP addresses.
- `pod`: will make KIC build addresses using the following template:
`pod-ip-address.my-namespace.pod`.
- `service`: will make KIC build addresses using the following template:
`pod-ip-address.service-name.my-namespace.svc`.
This is known to not work on GKE becuase it uses `kube-dns` instead of coredns.

### Changed

Expand Down
110 changes: 79 additions & 31 deletions internal/adminapi/endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
k8stypes "k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
"sigs.k8s.io/controller-runtime/pkg/client"

cfgtypes "github.com/kong/kubernetes-ingress-controller/v2/internal/manager/config/types"
)

// DiscoveredAdminAPI represents an Admin API discovered from a Kubernetes Service.
Expand All @@ -27,6 +29,7 @@ func GetAdminAPIsForService(
kubeClient client.Client,
service k8stypes.NamespacedName,
portNames sets.Set[string],
dnsStrategy cfgtypes.DNSStrategy,
) (sets.Set[DiscoveredAdminAPI], error) {
const (
defaultEndpointSliceListPagingLimit = 100
Expand Down Expand Up @@ -55,7 +58,11 @@ func GetAdminAPIsForService(
}

for _, es := range endpointsList.Items {
addresses = addresses.Union(AdminAPIsFromEndpointSlice(es, portNames))
adminAPI, err := AdminAPIsFromEndpointSlice(es, portNames, dnsStrategy)
if err != nil {
return nil, err
}
addresses = addresses.Union(adminAPI)
}

if endpointsList.Continue == "" {
Expand All @@ -68,7 +75,11 @@ func GetAdminAPIsForService(

// AdminAPIsFromEndpointSlice returns a list of Admin APIs when given
// an EndpointSlice.
func AdminAPIsFromEndpointSlice(endpoints discoveryv1.EndpointSlice, portNames sets.Set[string]) sets.Set[DiscoveredAdminAPI] {
func AdminAPIsFromEndpointSlice(
endpoints discoveryv1.EndpointSlice,
portNames sets.Set[string],
dnsStrategy cfgtypes.DNSStrategy,
) (sets.Set[DiscoveredAdminAPI], error) {
discoveredAdminAPIs := sets.New[DiscoveredAdminAPI]()
for _, p := range endpoints.Ports {
if p.Name == nil {
Expand Down Expand Up @@ -96,42 +107,79 @@ func AdminAPIsFromEndpointSlice(endpoints discoveryv1.EndpointSlice, portNames s
if e.TargetRef == nil || e.TargetRef.Kind != "Pod" {
continue
}
podNN := k8stypes.NamespacedName{
Name: e.TargetRef.Name,
Namespace: e.TargetRef.Namespace,
}

if len(e.Addresses) < 1 {
continue
}

// Endpoint's addresses are assumed to be fungible, therefore we pick only the first one.
// For the context please see the `Endpoint.Addresses` godoc.
addr := strings.ReplaceAll(e.Addresses[0], ".", "-")

var adminAPI DiscoveredAdminAPI
// NOTE: We assume https here because the referenced Admin API
// server will live in another Pod/elsewhere so allowing http would
// not be considered best practice.
if serviceName == "" {
// If we couldn't find a service that's the owner of provided EndpointSlice
// then fallback to providing a DNS name for the Pod only.
adminAPI = DiscoveredAdminAPI{
Address: fmt.Sprintf("https://%s.%s.pod:%d",
addr, endpoints.Namespace, *p.Port,
),
PodRef: podNN,
}
} else {
adminAPI = DiscoveredAdminAPI{
Address: fmt.Sprintf("https://%s.%s.%s.svc:%d",
addr, serviceName, endpoints.Namespace, *p.Port,
),
PodRef: podNN,
}
svc := k8stypes.NamespacedName{
Name: serviceName,
Namespace: endpoints.Namespace,
}

adminAPI, err := adminAPIFromEndpoint(e, p, svc, dnsStrategy)
if err != nil {
return nil, err
}
discoveredAdminAPIs = discoveredAdminAPIs.Insert(adminAPI)
}
}
return discoveredAdminAPIs
return discoveredAdminAPIs, nil
}

func adminAPIFromEndpoint(
endpoint discoveryv1.Endpoint,
port discoveryv1.EndpointPort,
service k8stypes.NamespacedName,
dnsStrategy cfgtypes.DNSStrategy,
) (DiscoveredAdminAPI, error) {
podNN := k8stypes.NamespacedName{
Name: endpoint.TargetRef.Name,
Namespace: endpoint.TargetRef.Namespace,
}

// NOTE: Endpoint's addresses are assumed to be fungible, therefore we pick
// only the first one.
// For the context please see the `Endpoint.Addresses` godoc.
eAddress := endpoint.Addresses[0]

// NOTE: We assume https below because the referenced Admin API
// server will live in another Pod/elsewhere so allowing http would
// not be considered best practice.

switch dnsStrategy {
case cfgtypes.ServiceScopedPodDNSStrategy:
if service.Name == "" {
return DiscoveredAdminAPI{}, fmt.Errorf(
"service name is empty for an endpoint with TargetRef %s/%s",
endpoint.TargetRef.Namespace, endpoint.TargetRef.Name,
)
}

ipAddr := strings.ReplaceAll(eAddress, ".", "-")
address := fmt.Sprintf("%s.%s.%s.svc", ipAddr, service.Name, service.Namespace)

return DiscoveredAdminAPI{
Address: fmt.Sprintf("https://%s:%d", address, *port.Port),
PodRef: podNN,
}, nil

case cfgtypes.NamespaceScopedPodDNSStrategy:
ipAddr := strings.ReplaceAll(eAddress, ".", "-")
address := fmt.Sprintf("%s.%s.pod", ipAddr, service.Namespace)

return DiscoveredAdminAPI{
Address: fmt.Sprintf("https://%s:%d", address, *port.Port),
PodRef: podNN,
}, nil

case cfgtypes.IPDNSStrategy:
return DiscoveredAdminAPI{
Address: fmt.Sprintf("https://%s:%d", eAddress, *port.Port),
PodRef: podNN,
}, nil

default:
return DiscoveredAdminAPI{}, fmt.Errorf("unknown dns strategy: %s", dnsStrategy)
}
}
11 changes: 4 additions & 7 deletions internal/adminapi/endpoints_envtest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"

"github.com/kong/kubernetes-ingress-controller/v2/internal/adminapi"
cfgtypes "github.com/kong/kubernetes-ingress-controller/v2/internal/manager/config/types"
"github.com/kong/kubernetes-ingress-controller/v2/internal/util/builder"
"github.com/kong/kubernetes-ingress-controller/v2/test/envtest"
)

Expand Down Expand Up @@ -88,18 +90,13 @@ func TestGetAdminAPIsForServiceReturnsAllAddressesCorrectlyPagingThroughResults(
TargetRef: testPodReference("pod-1", ns.Name),
},
},
Ports: []discoveryv1.EndpointPort{
{
Name: lo.ToPtr("admin"),
Port: lo.ToPtr(int32(8444)),
},
},
Ports: builder.NewEndpointPort(8444).WithName("admin").IntoSlice(),
}
require.NoError(t, client.Create(ctx, &es))
}
}

got, err := adminapi.GetAdminAPIsForService(ctx, client, service, sets.New("admin"))
got, err := adminapi.GetAdminAPIsForService(ctx, client, service, sets.New("admin"), cfgtypes.IPDNSStrategy)
require.NoError(t, err)
require.Len(t, got, tc.subnetD*tc.subnetC, "GetAdminAPIsForService should return all valid addresses")
})
Expand Down
Loading

0 comments on commit dd20d7b

Please sign in to comment.