From 1a6f7ccdf51f8ff49b495c4c74f8fd6a748d48c6 Mon Sep 17 00:00:00 2001 From: Jakub Warczarek Date: Wed, 29 Jan 2025 16:12:10 +0100 Subject: [PATCH] remove option validation --- internal/adminapi/endpoints.go | 12 +++++++----- internal/adminapi/kong.go | 3 --- internal/adminapi/kong_test.go | 14 -------------- 3 files changed, 7 insertions(+), 22 deletions(-) diff --git a/internal/adminapi/endpoints.go b/internal/adminapi/endpoints.go index 958776d3bc..89c1dc0435 100644 --- a/internal/adminapi/endpoints.go +++ b/internal/adminapi/endpoints.go @@ -14,8 +14,7 @@ import ( // DiscoveredAdminAPI represents an Admin API discovered from a Kubernetes Service. // For field Address use format https://:, and for field TLSServerName -// use format ...svc, where podIPDashes -// is the IP address separated by dashes instead of dots. +// use format pod...svc. type DiscoveredAdminAPI struct { // Address format is https://10.68.0.5:8444. Address string @@ -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 - // *...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: + // *...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. diff --git a/internal/adminapi/kong.go b/internal/adminapi/kong.go index e840f1e371..17eccc1c49 100644 --- a/internal/adminapi/kong.go +++ b/internal/adminapi/kong.go @@ -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 diff --git a/internal/adminapi/kong_test.go b/internal/adminapi/kong_test.go index fbaf83fde2..88007e1054 100644 --- a/internal/adminapi/kong_test.go +++ b/internal/adminapi/kong_test.go @@ -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, "") }) @@ -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, "") })