Skip to content

Commit

Permalink
feat: make NewKongClientForWorkspace first verify status + tests (#4357)
Browse files Browse the repository at this point in the history 
Makes NewKongClientForWorkspace verify the status before calling other 
endpoints. Returns a new KongClientNotReadyError when the client is not ready
yet. It gives callers a context of whether it makes sense to retry while
awaiting for readiness (will be used in the next PRs for #3499).
  • Loading branch information
@czeslavo
czeslavo authored Jul 14, 2023
1 parent 0fda6b5 commit 3cea973
Show file tree
Hide file tree
Showing 5 changed files with 296 additions and 69 deletions.
38 changes: 38 additions & 0 deletions internal/adminapi/client_test.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package adminapi_test

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

"github.com/stretchr/testify/require"
k8stypes "k8s.io/apimachinery/pkg/types"

"github.com/kong/kubernetes-ingress-controller/v2/internal/adminapi"
"github.com/kong/kubernetes-ingress-controller/v2/test/mocks"
)

func TestClientFactory_CreateAdminAPIClientAttachesPodReference(t *testing.T) {
factory := adminapi.NewClientFactoryForWorkspace("workspace", adminapi.HTTPClientOpts{}, "")

adminAPIHandler := mocks.NewAdminAPIHandler(t)
adminAPIServer := httptest.NewServer(adminAPIHandler)
t.Cleanup(func() { adminAPIServer.Close() })

client, err := factory.CreateAdminAPIClient(context.Background(), adminapi.DiscoveredAdminAPI{
Address: adminAPIServer.URL,
PodRef: k8stypes.NamespacedName{
Namespace: "namespace",
Name: "name",
},
})
require.NoError(t, err)
require.NotNil(t, client)

ref, ok := client.PodReference()
require.True(t, ok, "expected pod reference to be attached to the client")
require.Equal(t, k8stypes.NamespacedName{
Namespace: "namespace",
Name: "name",
}, ref)
}
34 changes: 27 additions & 7 deletions internal/adminapi/kong.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,27 @@ import (
tlsutil "github.com/kong/kubernetes-ingress-controller/v2/internal/util/tls"
)

// KongClientNotReadyError is returned when the Kong client is not ready to be used yet.
// This can happen if the Kong Admin API is not reachable, or if it's reachable but `GET /status` does not return 200.
type KongClientNotReadyError struct {
Err error
}

func (e KongClientNotReadyError) Error() string {
return fmt.Sprintf("client not ready: %s", e.Err)
}

func (e KongClientNotReadyError) Unwrap() error {
return e.Err
}

// 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.
// If the workspace does not already exist, NewKongClientForWorkspace will create it.
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.
// Create the base client, and if no workspace was provided then return that.
client, err := kong.NewClient(kong.String(adminURL), httpclient)
if err != nil {
return nil, fmt.Errorf("creating Kong client: %w", err)
Expand All @@ -30,13 +45,18 @@ func NewKongClientForWorkspace(ctx context.Context, adminURL string, wsName stri
return NewClient(client), nil
}

// if a workspace was provided, verify whether or not it exists.
// Ensure that the client is ready to be used by performing a status check.
if _, err = client.Status(ctx); err != nil {
return nil, KongClientNotReadyError{Err: err}
}

// If a workspace was provided, verify whether or not it exists.
exists, err := client.Workspaces.ExistsByName(ctx, kong.String(wsName))
if err != nil {
return nil, fmt.Errorf("looking up workspace: %w", err)
}

// if the provided workspace does not exist, for convenience we create it.
// If the provided workspace does not exist, for convenience we create it.
if !exists {
workspace := kong.Workspace{
Name: kong.String(wsName),
Expand All @@ -47,7 +67,7 @@ func NewKongClientForWorkspace(ctx context.Context, adminURL string, wsName stri
}
}

// ensure that we set the workspace appropriately
// Ensure that we set the workspace appropriately.
client.SetWorkspace(wsName)

return NewClient(client), nil
Expand All @@ -70,7 +90,7 @@ type HTTPClientOpts struct {
}

const (
headerNameAdminToken = "Kong-Admin-Token"
HeaderNameAdminToken = "Kong-Admin-Token"
)

// MakeHTTPClient returns an HTTP client with the specified mTLS/headers configuration.
Expand Down Expand Up @@ -133,11 +153,11 @@ func MakeHTTPClient(opts *HTTPClientOpts, kongAdminToken string) (*http.Client,
func prepareHeaders(headers []string, kongAdminToken string) []string {
if kongAdminToken != "" {
contains := lo.ContainsBy(headers, func(header string) bool {
return strings.HasPrefix(header, headerNameAdminToken+":")
return strings.HasPrefix(header, HeaderNameAdminToken+":")
})

if !contains {
headers = append(headers, headerNameAdminToken+":"+kongAdminToken)
headers = append(headers, HeaderNameAdminToken+":"+kongAdminToken)
}
}
return headers
Expand Down
91 changes: 78 additions & 13 deletions internal/adminapi/kong_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
package adminapi
package adminapi_test

import (
"bytes"
"context"
"crypto/rand"
"crypto/rsa"
"crypto/tls"
Expand All @@ -20,6 +21,9 @@ import (
"time"

"github.com/stretchr/testify/require"

"github.com/kong/kubernetes-ingress-controller/v2/internal/adminapi"
"github.com/kong/kubernetes-ingress-controller/v2/test/mocks"
)

func TestMakeHTTPClientWithTLSOpts(t *testing.T) {
Expand All @@ -31,27 +35,27 @@ func TestMakeHTTPClientWithTLSOpts(t *testing.T) {
caPEM, certPEM, certPrivateKeyPEM, err = buildTLS(t)
require.NoError(t, err, "Fail to build TLS certificates")

opts := HTTPClientOpts{
opts := adminapi.HTTPClientOpts{
TLSSkipVerify: true,
TLSServerName: "",
CACertPath: "",
CACert: caPEM.String(),
Headers: nil,
TLSClient: TLSClientConfig{
TLSClient: adminapi.TLSClientConfig{
Cert: certPEM.String(),
Key: certPrivateKeyPEM.String(),
},
}

t.Run("without kong admin token", func(t *testing.T) {
httpclient, err := MakeHTTPClient(&opts, "")
httpclient, err := adminapi.MakeHTTPClient(&opts, "")
require.NoError(t, err)
require.NotNil(t, httpclient)
require.NoError(t, validate(t, httpclient, caPEM, certPEM, certPrivateKeyPEM, ""))
})

t.Run("with kong admin token", func(t *testing.T) {
httpclient, err := MakeHTTPClient(&opts, "my-token")
httpclient, err := adminapi.MakeHTTPClient(&opts, "my-token")
require.NoError(t, err)
require.NotNil(t, httpclient)
require.NoError(t, validate(t, httpclient, caPEM, certPEM, certPrivateKeyPEM, "my-token"))
Expand Down Expand Up @@ -88,33 +92,94 @@ func TestMakeHTTPClientWithTLSOptsAndFilePaths(t *testing.T) {
require.Equal(t, certPrivateKeyPEM.Len(), writtenBytes)
defer os.Remove(caFile.Name())

opts := HTTPClientOpts{
opts := adminapi.HTTPClientOpts{
TLSSkipVerify: true,
TLSServerName: "",
CACertPath: caFile.Name(),
CACert: "",
Headers: nil,
TLSClient: TLSClientConfig{
TLSClient: adminapi.TLSClientConfig{
CertFile: certFile.Name(),
KeyFile: certPrivateKeyFile.Name(),
},
}

t.Run("without kong admin token", func(t *testing.T) {
httpclient, err := MakeHTTPClient(&opts, "")
httpclient, err := adminapi.MakeHTTPClient(&opts, "")
require.NoError(t, err)
require.NotNil(t, httpclient)
require.NoError(t, validate(t, httpclient, caPEM, certPEM, certPrivateKeyPEM, ""))
})

t.Run("with kong admin token", func(t *testing.T) {
httpclient, err := MakeHTTPClient(&opts, "my-token")
httpclient, err := adminapi.MakeHTTPClient(&opts, "my-token")
require.NoError(t, err)
require.NotNil(t, httpclient)
require.NoError(t, validate(t, httpclient, caPEM, certPEM, certPrivateKeyPEM, "my-token"))
})
}

func TestNewKongClientForWorkspace(t *testing.T) {
const testWorkspace = "workspace"

testCases := []struct {
name string
adminAPIReady bool
workspaceExists bool
expectError error
}{
{
name: "admin api is ready and workspace exists",
adminAPIReady: true,
workspaceExists: true,
},
{
name: "admin api is ready and workspace doesn't exist",
adminAPIReady: true,
workspaceExists: false,
},
{
name: "admin api is not ready",
adminAPIReady: false,
expectError: adminapi.KongClientNotReadyError{},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
adminAPIHandler := mocks.NewAdminAPIHandler(
t,
mocks.WithWorkspaceExists(tc.workspaceExists),
mocks.WithReady(tc.adminAPIReady),
)
adminAPIServer := httptest.NewServer(adminAPIHandler)
t.Cleanup(func() { adminAPIServer.Close() })

client, err := adminapi.NewKongClientForWorkspace(
context.Background(),
adminAPIServer.URL,
testWorkspace,
adminAPIServer.Client(),
)

if tc.expectError != nil {
require.IsType(t, err, tc.expectError)
return
}
require.NoError(t, err)
require.NotNil(t, client)

if !tc.workspaceExists {
require.True(t, adminAPIHandler.WasWorkspaceCreated(), "expected workspace to be created")
}

require.Equal(t, client.AdminAPIClient().Workspace(), testWorkspace)
_, ok := client.PodReference()
require.False(t, ok, "expected no pod reference to be attached to the client")
})
}
}

func buildTLS(t *testing.T) (caPEM *bytes.Buffer, certPEM *bytes.Buffer, certPrivateKeyPEM *bytes.Buffer, err error) {
const rsaKeySize = 2048

Expand Down Expand Up @@ -251,19 +316,19 @@ func validate(t *testing.T,
successMessage := "connection successful"
server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if kongAdminToken != "" {
v, ok := r.Header[http.CanonicalHeaderKey(headerNameAdminToken)]
v, ok := r.Header[http.CanonicalHeaderKey(adminapi.HeaderNameAdminToken)]
if !ok {
fmt.Fprintf(w, "%s header not found", headerNameAdminToken)
fmt.Fprintf(w, "%s header not found", adminapi.HeaderNameAdminToken)
return
}
if len(v) != 1 {
fmt.Fprintf(w, "%s header expected to contain %s but found %v",
headerNameAdminToken, kongAdminToken, v)
adminapi.HeaderNameAdminToken, kongAdminToken, v)
return
}
if v[0] != kongAdminToken {
fmt.Fprintf(w, "%s header expected to contain %s but found %s",
headerNameAdminToken, kongAdminToken, v[0])
adminapi.HeaderNameAdminToken, kongAdminToken, v[0])
return
}
}
Expand Down
53 changes: 4 additions & 49 deletions test/envtest/adminapimock.go
View file
Original file line number Diff line number Diff line change
@@ -1,64 +1,19 @@
package envtest

import (
"io"
"net/http"
"net/http/httptest"
"testing"
)

const dblessConfig = `{
"version": "3.3.0",
"configuration": {
"database": "off",
"router_flavor": "traditional",
"role": "traditional",
"proxy_listeners": [
{
"backlog=%d+": false,
"ipv6only=on": false,
"ipv6only=off": false,
"ssl": false,
"so_keepalive=off": false,
"so_keepalive=%w*:%w*:%d*": false,
"listener": "0.0.0.0:8000",
"bind": false,
"port": 8000,
"deferred": false,
"so_keepalive=on": false,
"http2": false,
"proxy_protocol": false,
"ip": "0.0.0.0",
"reuseport": false
}
]
}
}`
"github.com/kong/kubernetes-ingress-controller/v2/test/mocks"
)

// StartAdminAPIServerMock starts a mock Kong Admin API server.
// Server's .Close() method will be called during test's cleanup.
func StartAdminAPIServerMock(t *testing.T) *httptest.Server {
t.Helper()

mux := http.NewServeMux()
mux.HandleFunc("/config", func(w http.ResponseWriter, r *http.Request) {
body, _ := io.ReadAll(r.Body)
t.Logf("Admin API config: %s %s %s", r.Method, r.URL, string(body))
})
mux.HandleFunc("/status", func(w http.ResponseWriter, r *http.Request) {
if _, err := w.Write([]byte(dblessConfig)); err != nil {
w.WriteHeader(500)
return
}
})
mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
if _, err := w.Write([]byte(dblessConfig)); err != nil {
w.WriteHeader(500)
return
}
})

s := httptest.NewServer(mux)
handler := mocks.NewAdminAPIHandler(t)
s := httptest.NewServer(handler)
t.Cleanup(func() {
s.Close()
})
Expand Down
Loading

0 comments on commit 3cea973

Please sign in to comment.