Skip to content

Commit

Permalink
fix: set proper User-Agent for request made to Kong and Konnect (#5753)
Browse files Browse the repository at this point in the history
  • Loading branch information
programmer04 authored Mar 28, 2024
1 parent 2825d8e commit c393bb5
Show file tree
Hide file tree
Showing 25 changed files with 232 additions and 28 deletions.
2 changes: 2 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ linters-settings:
- 'gatewayv1alpha2|gatewayv1beta1|gatewayv1(# use internal/gatewayapi aliases instead)?'
- 'CoreV1\(\)\.Endpoints(# use DiscoveryV1 EndpointSlices API instead)?'
- 'corev1\.Endpoint(# use DiscoveryV1 EndpointSlices API instead)?'
- '(gokong|kong)\.NewClient(# use adminapi.NewKongAPIClient instead )?'

gomodguard:
blocked:
modules:
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ Adding a new version? You'll need three changes:
[#5658](https://github.com/Kong/kubernetes-ingress-controller/issues/5658)
- Do not require `rsa_public_key` field in credential `Secret`s when working with jwt HMAC credentials.
[#5737](https://github.com/Kong/kubernetes-ingress-controller/issues/5737)
- Set proper User-Agent for request made to Kong and Konnect.
[#5753](https://github.com/Kong/kubernetes-ingress-controller/pull/5753)

## [3.1.2]

Expand Down
14 changes: 13 additions & 1 deletion internal/adminapi/kong.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/kong/go-kong/kong"
"github.com/samber/lo"

"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/metadata"
tlsutil "github.com/kong/kubernetes-ingress-controller/v3/internal/util/tls"
"github.com/kong/kubernetes-ingress-controller/v3/internal/versions"
)
Expand All @@ -40,6 +41,17 @@ func (e KongGatewayUnsupportedVersionError) Error() string {
return fmt.Sprintf("Kong Gateway version is not supported: %s", e.msg)
}

// NewKongAPIClient returns a Kong API client for a given root API URL.
// It ensures that proper User-Agent is set. Do not use kong.NewClient directly.
func NewKongAPIClient(adminURL string, httpClient *http.Client) (*kong.Client, error) {
client, err := kong.NewClient(kong.String(adminURL), httpClient) //nolint:forbidigo
if err != nil {
return nil, fmt.Errorf("creating Kong client: %w", err)
}
client.UserAgent = metadata.UserAgent()
return client, nil
}

// NewKongClientForWorkspace returns a Kong API client for a given root API URL and workspace.
// It ensures that the client is ready to be used by performing a status check, returns KongClientNotReadyError if not
// or KongGatewayUnsupportedVersionError if it can't check Kong Gateway's version or it is not >= 3.4.1.
Expand All @@ -48,7 +60,7 @@ func NewKongClientForWorkspace(
ctx context.Context, adminURL string, wsName string, httpClient *http.Client,
) (*Client, error) {
// Create the base client, and if no workspace was provided then return that.
client, err := kong.NewClient(kong.String(adminURL), httpClient)
client, err := NewKongAPIClient(adminURL, httpClient)
if err != nil {
return nil, fmt.Errorf("creating Kong client: %w", err)
}
Expand Down
7 changes: 3 additions & 4 deletions internal/adminapi/konnect.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/avast/retry-go/v4"
"github.com/go-logr/logr"
"github.com/kong/go-kong/kong"
"github.com/samber/lo"

tlsutil "github.com/kong/kubernetes-ingress-controller/v3/internal/util/tls"
)
Expand Down Expand Up @@ -42,7 +41,7 @@ func NewKongClientForKonnectControlPlane(c KonnectConfig) (*KonnectClient, error
return nil, fmt.Errorf("failed to extract client certificates: %w", err)
}
if clientCertificate == nil {
return nil, fmt.Errorf("client ceritficate is missing")
return nil, fmt.Errorf("client certificate is missing")
}

tlsConfig := tls.Config{
Expand All @@ -51,8 +50,8 @@ func NewKongClientForKonnectControlPlane(c KonnectConfig) (*KonnectClient, error
}
transport := http.DefaultTransport.(*http.Transport).Clone()
transport.TLSClientConfig = &tlsConfig
client, err := kong.NewClient(
lo.ToPtr(fmt.Sprintf("%s/%s/%s", c.Address, "kic/api/control-planes", c.ControlPlaneID)),
client, err := NewKongAPIClient(
fmt.Sprintf("%s/%s/%s", c.Address, "kic/api/control-planes", c.ControlPlaneID),
&http.Client{
Transport: transport,
},
Expand Down
2 changes: 1 addition & 1 deletion internal/dataplane/kong_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,7 @@ func mustSampleGatewayClient(t *testing.T) *adminapi.Client {
func mustSampleKonnectClient(t *testing.T) *adminapi.KonnectClient {
t.Helper()

c, err := kong.NewClient(lo.ToPtr(fmt.Sprintf("https://%s.konghq.tech", uuid.NewString())), &http.Client{})
c, err := adminapi.NewKongAPIClient(fmt.Sprintf("https://%s.konghq.tech", uuid.NewString()), &http.Client{})
require.NoError(t, err)

rgID := uuid.NewString()
Expand Down
6 changes: 6 additions & 0 deletions internal/konnect/controlplanes/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"strings"
"time"

"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/metadata"
"github.com/oapi-codegen/runtime"
openapi_types "github.com/oapi-codegen/runtime/types"
)
Expand Down Expand Up @@ -379,6 +380,11 @@ func NewClient(server string, opts ...ClientOption) (*Client, error) {
if client.Client == nil {
client.Client = &http.Client{}
}
client.RequestEditors = append([]RequestEditorFn{func(ctx context.Context, req *http.Request) error {
req.Header.Set("User-Agent", metadata.UserAgent())
return nil
}}, client.RequestEditors...)

return &client, nil
}

Expand Down
44 changes: 44 additions & 0 deletions internal/konnect/controlplanes/client_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package controlplanes_test

import (
"context"
"net/http"
"net/http/httptest"
"testing"

"github.com/google/uuid"
"github.com/stretchr/testify/require"

"github.com/kong/kubernetes-ingress-controller/v3/internal/konnect/controlplanes"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/metadata"
)

type mockControlPlanesServer struct {
t *testing.T
}

func newMockControlPlanesServer(t *testing.T) *mockControlPlanesServer {
return &mockControlPlanesServer{
t: t,
}
}

func (m *mockControlPlanesServer) ServeHTTP(_ http.ResponseWriter, r *http.Request) {
require.Equal(m.t, metadata.UserAgent(), r.Header.Get("User-Agent"))
}

func TestControlPlanesClientUserAgent(t *testing.T) {
ts := httptest.NewServer(newMockControlPlanesServer(t))
t.Cleanup(ts.Close)

c, err := controlplanes.NewClient(ts.URL)
require.NoError(t, err)

r, err := c.GetControlPlane(context.Background(), uuid.New())
require.NoError(t, err)
r.Body.Close()

r, err = c.DeleteControlPlane(context.Background(), uuid.New())
require.NoError(t, err)
r.Body.Close()
}
5 changes: 5 additions & 0 deletions internal/konnect/controlplanesconfig/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"net/url"
"strings"

"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/metadata"
"github.com/oapi-codegen/runtime"
)

Expand Down Expand Up @@ -195,6 +196,10 @@ func NewClient(server string, opts ...ClientOption) (*Client, error) {
if client.Client == nil {
client.Client = &http.Client{}
}
client.RequestEditors = append([]RequestEditorFn{func(ctx context.Context, req *http.Request) error {
req.Header.Set("User-Agent", metadata.UserAgent())
return nil
}}, client.RequestEditors...)
return &client, nil
}

Expand Down
43 changes: 43 additions & 0 deletions internal/konnect/controlplanesconfig/client_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package controlplanesconfig_test

import (
"context"
"net/http"
"net/http/httptest"
"testing"

"github.com/stretchr/testify/require"

"github.com/kong/kubernetes-ingress-controller/v3/internal/konnect/controlplanesconfig"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/metadata"
)

type mockControlPlanesConfigServer struct {
t *testing.T
}

func newMockControlPlanesConfigServer(t *testing.T) *mockControlPlanesConfigServer {
return &mockControlPlanesConfigServer{
t: t,
}
}

func (m *mockControlPlanesConfigServer) ServeHTTP(_ http.ResponseWriter, r *http.Request) {
require.Equal(m.t, metadata.UserAgent(), r.Header.Get("User-Agent"))
}

func TestControlPlanesConfigClientUserAgent(t *testing.T) {
ts := httptest.NewServer(newMockControlPlanesConfigServer(t))
t.Cleanup(ts.Close)

c, err := controlplanesconfig.NewClient(ts.URL)
require.NoError(t, err)

r, err := c.GetNodes(context.Background(), &controlplanesconfig.GetNodesParams{})
require.NoError(t, err)
r.Body.Close()

r, err = c.DeleteCoreEntities(context.Background(), "test")
require.NoError(t, err)
r.Body.Close()
}
3 changes: 2 additions & 1 deletion internal/konnect/license/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/samber/mo"

"github.com/kong/kubernetes-ingress-controller/v3/internal/adminapi"
"github.com/kong/kubernetes-ingress-controller/v3/internal/konnect/useragent"
"github.com/kong/kubernetes-ingress-controller/v3/internal/license"
tlsutil "github.com/kong/kubernetes-ingress-controller/v3/internal/util/tls"
)
Expand Down Expand Up @@ -44,7 +45,7 @@ func NewClient(cfg adminapi.KonnectConfig) (*Client, error) {
c := &http.Client{}
transport := http.DefaultTransport.(*http.Transport).Clone()
transport.TLSClientConfig = &tlsConfig
c.Transport = transport
c.Transport = useragent.NewTransport(transport)

return &Client{
address: cfg.Address,
Expand Down
10 changes: 7 additions & 3 deletions internal/konnect/license/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,25 @@ import (

"github.com/kong/kubernetes-ingress-controller/v3/internal/adminapi"
"github.com/kong/kubernetes-ingress-controller/v3/internal/konnect/license"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/metadata"
)

type mockKonnectLicenseServer struct {
response []byte
statusCode int
t *testing.T
}

func newMockKonnectLicenseServer(response []byte, statusCode int) *mockKonnectLicenseServer {
func newMockKonnectLicenseServer(t *testing.T, response []byte, statusCode int) *mockKonnectLicenseServer {
return &mockKonnectLicenseServer{
t: t,
response: response,
statusCode: statusCode,
}
}

func (m *mockKonnectLicenseServer) ServeHTTP(w http.ResponseWriter, _ *http.Request) {
func (m *mockKonnectLicenseServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
require.Equal(m.t, metadata.UserAgent(), r.Header.Get("User-Agent"))
w.WriteHeader(m.statusCode)
_, _ = w.Write(m.response)
}
Expand Down Expand Up @@ -150,7 +154,7 @@ func TestLicenseClient(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
server := newMockKonnectLicenseServer(tc.response, tc.status)
server := newMockKonnectLicenseServer(t, tc.response, tc.status)
ts := httptest.NewServer(server)
defer ts.Close()

Expand Down
3 changes: 2 additions & 1 deletion internal/konnect/nodes/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/samber/lo"

"github.com/kong/kubernetes-ingress-controller/v3/internal/adminapi"
"github.com/kong/kubernetes-ingress-controller/v3/internal/konnect/useragent"
tlsutil "github.com/kong/kubernetes-ingress-controller/v3/internal/util/tls"
)

Expand Down Expand Up @@ -43,7 +44,7 @@ func NewClient(cfg adminapi.KonnectConfig) (*Client, error) {
c := &http.Client{}
transport := http.DefaultTransport.(*http.Transport).Clone()
transport.TLSClientConfig = &tlsConfig
c.Transport = transport
c.Transport = useragent.NewTransport(transport)

return &Client{
address: cfg.Address,
Expand Down
42 changes: 42 additions & 0 deletions internal/konnect/nodes/client_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package nodes_test

import (
"context"
"net/http"
"net/http/httptest"
"testing"

"github.com/stretchr/testify/require"

"github.com/kong/kubernetes-ingress-controller/v3/internal/adminapi"
"github.com/kong/kubernetes-ingress-controller/v3/internal/konnect/nodes"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/metadata"
)

type mockNodesServer struct {
t *testing.T
}

func newMockNodesServer(t *testing.T) *mockNodesServer {
return &mockNodesServer{
t: t,
}
}

func (m *mockNodesServer) ServeHTTP(_ http.ResponseWriter, r *http.Request) {
require.Equal(m.t, metadata.UserAgent(), r.Header.Get("User-Agent"))
}

func TestNodesClientUserAgent(t *testing.T) {
ts := httptest.NewServer(newMockNodesServer(t))
t.Cleanup(ts.Close)

c, err := nodes.NewClient(adminapi.KonnectConfig{Address: ts.URL})
require.NoError(t, err)

_, err = c.GetNode(context.Background(), "test-node-id")
require.Error(t, err)

err = c.DeleteNode(context.Background(), "test-node-id")
require.NoError(t, err)
}
3 changes: 3 additions & 0 deletions internal/konnect/roles/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"fmt"
"io"
"net/http"

"github.com/kong/kubernetes-ingress-controller/v3/internal/konnect/useragent"
)

const (
Expand Down Expand Up @@ -43,6 +45,7 @@ func NewRole(id, entityID string) (Role, error) {
}

func NewClient(httpClient *http.Client, baseURL string, personalAccessToken string) *Client {
httpClient.Transport = useragent.NewTransport(httpClient.Transport)
return &Client{
baseURL: baseURL,
httpClient: httpClient,
Expand Down
8 changes: 8 additions & 0 deletions internal/konnect/roles/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/stretchr/testify/require"

"github.com/kong/kubernetes-ingress-controller/v3/internal/konnect/roles"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/metadata"
)

const (
Expand All @@ -23,8 +24,13 @@ func newMockRolesServer(t *testing.T) *httptest.Server {
token := r.Header.Get("Authorization")
require.Equal(t, "Bearer "+testToken, token)
}
requireUserAgent := func(r *http.Request) {
userAgent := r.Header.Get("User-Agent")
require.Equal(t, metadata.UserAgent(), userAgent)
}

mux.HandleFunc("/users/me", func(w http.ResponseWriter, r *http.Request) {
requireUserAgent(r)
requireToken(r)

w.WriteHeader(http.StatusOK)
Expand All @@ -41,6 +47,7 @@ func newMockRolesServer(t *testing.T) *httptest.Server {
})

mux.HandleFunc("/users/"+currentUserID+"/assigned-roles/", func(w http.ResponseWriter, r *http.Request) {
requireUserAgent(r)
requireToken(r)

switch r.Method {
Expand Down Expand Up @@ -73,6 +80,7 @@ func newMockRolesServer(t *testing.T) *httptest.Server {
]
}`))
case http.MethodDelete:
requireUserAgent(r)
w.WriteHeader(http.StatusNoContent)
default:
t.Errorf("unexpected method %s", r.Method)
Expand Down
Loading

0 comments on commit c393bb5

Please sign in to comment.