Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(adminapi): support specification of http/https via appProtocol on the admin port #5251

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ Adding a new version? You'll need three changes:
[#5220](https://github.com/Kong/kubernetes-ingress-controller/pull/5220)
- Added support for GRPC over HTTP (without TLS) in Gateway API.
[#5128](https://github.com/Kong/kubernetes-ingress-controller/pull/5128)
- Added support in Gateway Discovery for specifying that the Admin API should use HTTP via the appProtocol field on the admin port.
- [#5251](https://github.com/Kong/kubernetes-ingress-controller/pull/5251)

### Fixed

Expand Down
18 changes: 11 additions & 7 deletions internal/adminapi/endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,15 +162,19 @@ func adminAPIFromEndpoint(
Namespace: endpoint.TargetRef.Namespace,
}

// Format for calling the Admin API. If the port explicitly indicates http as the AppProtocol, use http.
// Otherwise, default to HTTPS as a best practice. Consumers may want to use HTTP if they have a service mesh in place which
// is already handling TLS authentication for them.
format := "https://%s:%d"
backjo marked this conversation as resolved.
Show resolved Hide resolved
if port.AppProtocol != nil && strings.Compare(*port.AppProtocol, "http") == 0 {
backjo marked this conversation as resolved.
Show resolved Hide resolved
format = "http://%s:%d"
}

// 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 == "" {
Expand All @@ -184,7 +188,7 @@ func adminAPIFromEndpoint(
address := fmt.Sprintf("%s.%s.%s.svc", ipAddr, service.Name, service.Namespace)

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

Expand All @@ -193,7 +197,7 @@ func adminAPIFromEndpoint(
address := fmt.Sprintf("%s.%s.pod", ipAddr, service.Namespace)

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

Expand All @@ -203,7 +207,7 @@ func adminAPIFromEndpoint(
bounded = fmt.Sprintf("[%s]", bounded)
}
return DiscoveredAdminAPI{
Address: fmt.Sprintf("https://%s:%d", bounded, *port.Port),
Address: fmt.Sprintf(format, bounded, *port.Port),
PodRef: podNN,
}, nil

Expand Down
28 changes: 28 additions & 0 deletions internal/adminapi/endpoints_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,34 @@ func TestDiscoverer_AddressesFromEndpointSlice(t *testing.T) {
),
dnsStrategy: cfgtypes.NamespaceScopedPodDNSStrategy,
},
{
name: "basic with appProtocol http",
endpoints: discoveryv1.EndpointSlice{
ObjectMeta: endpointsSliceObjectMeta,
AddressType: discoveryv1.AddressTypeIPv4,
Endpoints: []discoveryv1.Endpoint{
{
Addresses: []string{"10.0.0.1", "10.0.0.2"},
Conditions: discoveryv1.EndpointConditions{
Ready: lo.ToPtr(true),
Terminating: lo.ToPtr(false),
},
TargetRef: testPodReference(namespaceName, "pod-1"),
},
},
Ports: builder.NewEndpointPort(8444).WithName("admin").WithAppProtocol("http").IntoSlice(),
},
portNames: sets.New("admin"),
want: sets.New(
DiscoveredAdminAPI{
Address: "http://10-0-0-1.ns.pod:8444",
PodRef: k8stypes.NamespacedName{
Name: "pod-1", Namespace: namespaceName,
},
},
),
dnsStrategy: cfgtypes.NamespaceScopedPodDNSStrategy,
},
{
name: "basic",
endpoints: discoveryv1.EndpointSlice{
Expand Down
6 changes: 6 additions & 0 deletions internal/util/builder/endpointport.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ func (b *EndpointPortBuilder) WithName(name string) *EndpointPortBuilder {
return b
}

// WithAppProtocol sets the appProtocol on the endpoint port.
func (b *EndpointPortBuilder) WithAppProtocol(appProtocol string) *EndpointPortBuilder {
b.ep.AppProtocol = lo.ToPtr(appProtocol)
return b
}

// Build returns the configured EndpointPort.
func (b *EndpointPortBuilder) Build() discoveryv1.EndpointPort {
return b.ep
Expand Down