Skip to content

Commit

Permalink
remove option validation
Browse files Browse the repository at this point in the history
  • Loading branch information
programmer04 committed Jan 29, 2025
1 parent 3fde843 commit 1a6f7cc
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 22 deletions.
12 changes: 7 additions & 5 deletions internal/adminapi/endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ import (

// DiscoveredAdminAPI represents an Admin API discovered from a Kubernetes Service.
// For field Address use format https://<podIP>:<port>, and for field TLSServerName
// use format <podIPDashes>.<serviceName>.<namespace>.svc, where podIPDashes
// is the IP address separated by dashes instead of dots.
// use format pod.<serviceName>.<namespace>.svc.
type DiscoveredAdminAPI struct {
// Address format is https://10.68.0.5:8444.
Address string
Expand Down Expand Up @@ -181,9 +180,12 @@ func adminAPIFromEndpoint(
Address: fmt.Sprintf("https://%s:%d", podIPAddr, *port.Port),
// TLSServerName format doesn't need to include the IP address part, it's the same for
// ipv4 and ipv6: pod.dataplane-admin-kong-rqwr9-sc49t.default.svc.
// Currently everywhere (KGO, Chart) certificates are generated like that
// *.<service-name>.<namespace>.svc e.g.: *.dataplane-admin-kong-rqwr9-sc49t.default.svc
// so we have to follow the same 4-parts pattern here, to satisfy wildcard. The first part
// Currently for KGO certificates are generated like that:
// *.<service-name>.<namespace>.svc e.g.: *.dataplane-admin-kong-rqwr9-sc49t.default.svc, see
// https://github.com/Kong/gateway-operator/blob/8f009b49b622197ac083abae1c3606bc2c35114f/controller/dataplane/owned_resources.go#L30-L53
// In case of Ingress Chart TLS never worked out of the box (by default it's disabled), due to hardcoded service name in the
// https://github.com/Kong/charts/blob/1903dd5370e2d028ec31f6b3cec1414a55462ebd/charts/kong/templates/service-kong-admin.yaml#L35-L37
// Let's follow the same 4-parts pattern here, to satisfy wildcard. The first part
// can be arbitral so let's use "pod". Ditching the first part (wildcard certificate) is
// problematic, because this requires changes in the certificate generation logic and may
// break existing users' setups, but it can be done one day.
Expand Down
3 changes: 0 additions & 3 deletions internal/adminapi/kong.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,6 @@ func makeHTTPClient(opts ClientOpts, kongAdminToken string) (*http.Client, error

if opts.TLSSkipVerify {
tlsConfig.InsecureSkipVerify = true //nolint:gosec
if opts.TLSServerName != "" || opts.CACertPath != "" || opts.CACert != "" || !opts.TLSClient.IsZero() {
return nil, errors.New("when TLSSkipVerify is set, no other TLS options can be set")
}
}

tlsConfig.ServerName = opts.TLSServerName
Expand Down
14 changes: 0 additions & 14 deletions internal/adminapi/kong_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,6 @@ func TestAdminAPIClientWithTLSOpts(t *testing.T) {
},
}

t.Run("with mutually exclusive options set, it should fail", func(t *testing.T) {
optsConflict := opts
optsConflict.TLSSkipVerify = true
_, err := adminapi.NewKongAPIClient("https://localhost", optsConflict, "")
require.ErrorContains(t, err, "when TLSSkipVerify is set, no other TLS options can be set")
})

t.Run("without kong admin token", func(t *testing.T) {
validate(t, opts, caCert, cert, key, "")
})
Expand Down Expand Up @@ -89,13 +82,6 @@ func TestAdminAPIClientWithTLSOptsAndFilePaths(t *testing.T) {
},
}

t.Run("with mutually exclusive options set, it should fail", func(t *testing.T) {
optsConflict := opts
optsConflict.TLSSkipVerify = true
_, err := adminapi.NewKongAPIClient("https://localhost", optsConflict, "")
require.ErrorContains(t, err, "when TLSSkipVerify is set, no other TLS options can be set")
})

t.Run("without kong admin token", func(t *testing.T) {
validate(t, opts, caCert, cert, key, "")
})
Expand Down

0 comments on commit 1a6f7cc

Please sign in to comment.