Skip to content

Commit

Permalink
feat(adminapi): support specification of http/https via appProtocol o…
Browse files Browse the repository at this point in the history
…n the admin port.
  • Loading branch information
Jonah Back committed Nov 29, 2023
1 parent b956d41 commit 1119f78
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 7 deletions.
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"
if port.AppProtocol != nil && strings.Compare(*port.AppProtocol, "http") == 0 {
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

1 comment on commit 1119f78

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Go Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 1119f78 Previous: b0f92b6 Ratio
BenchmarkDeckgenGenerateSHA - ns/op 76039 ns/op 29319 ns/op 2.59

This comment was automatically generated by workflow using github-action-benchmark.

CC: @Kong/k8s-maintainers

Please sign in to comment.