Skip to content

Commit

Permalink
add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
czeslavo committed Jun 12, 2023
1 parent d2799ee commit d653950
Show file tree
Hide file tree
Showing 9 changed files with 381 additions and 39 deletions.
14 changes: 0 additions & 14 deletions internal/adminapi/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,14 +171,6 @@ func (c *Client) PodReference() (k8stypes.NamespacedName, bool) {
return k8stypes.NamespacedName{}, false
}

type ClientNotReadyError struct {
Err error
}

func (e ClientNotReadyError) Error() string {
return fmt.Errorf("client not ready: %w", e.Err).Error()
}

type ClientFactory struct {
workspace string
httpClientOpts HTTPClientOpts
Expand All @@ -202,12 +194,6 @@ func (cf ClientFactory) CreateAdminAPIClient(ctx context.Context, discoveredAdmi
if err != nil {
return nil, err
}

_, err = cl.AdminAPIClient().Status(ctx)
if err != nil {
return nil, ClientNotReadyError{Err: err}
}

cl.AttachPodReference(discoveredAdminAPI.PodRef)
return cl, nil
}
152 changes: 152 additions & 0 deletions internal/adminapi/client_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
package adminapi_test

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

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

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

type mockAdminAPIServer struct {
mux *http.ServeMux
t *testing.T

workspaceWasCreated bool
m sync.RWMutex
}

func newMockAdminAPIServer(t *testing.T, ready, workspaceExists bool) *mockAdminAPIServer {
srv := &mockAdminAPIServer{
t: t,
}

mux := http.NewServeMux()
mux.HandleFunc("/status", func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodGet {
if !ready {
w.WriteHeader(http.StatusServiceUnavailable)
}
_, _ = w.Write([]byte(`{}`))
return
}

t.Errorf("unexpected request: %s %s", r.Method, r.URL)
})
mux.HandleFunc("/workspace/workspaces/", func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodGet {
if workspaceExists {
w.WriteHeader(http.StatusOK)
} else {
w.WriteHeader(http.StatusNotFound)
}
return
}

t.Errorf("unexpected request: %s %s", r.Method, r.URL)
})
mux.HandleFunc("/workspaces", func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodPost {
if !workspaceExists {
srv.m.Lock()
defer srv.m.Unlock()
srv.workspaceWasCreated = true
_, _ = w.Write([]byte(`{"id": "workspace"}`))
w.WriteHeader(http.StatusCreated)
} else {
t.Errorf("unexpected workspace creation")
w.WriteHeader(http.StatusInternalServerError)
}
return
}

t.Errorf("unexpected request: %s %s", r.Method, r.URL)
})
srv.mux = mux

return srv
}

func (m *mockAdminAPIServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
m.t.Logf("mockAdminAPIServer received request: %s %s", r.Method, r.URL)
m.mux.ServeHTTP(w, r)
}

func (m *mockAdminAPIServer) WasWorkspaceCreated() bool {
m.m.RLock()
defer m.m.RUnlock()
return m.workspaceWasCreated
}

func TestClientFactory_CreateAdminAPIClient(t *testing.T) {
const (
workspace = "workspace"
adminToken = "token"
)

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{},
},
}

factory := adminapi.NewClientFactoryForWorkspace(workspace, adminapi.HTTPClientOpts{}, adminToken)

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
adminAPIServer := newMockAdminAPIServer(t, tc.adminAPIReady, tc.workspaceExists)
adminAPI := httptest.NewServer(adminAPIServer)
t.Cleanup(func() {
adminAPI.Close()
})

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

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, adminAPIServer.WasWorkspaceCreated(), "expected workspace to be created")
}

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)
})
}
}
4 changes: 0 additions & 4 deletions internal/adminapi/endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ import (
cfgtypes "github.com/kong/kubernetes-ingress-controller/v2/internal/manager/config/types"
)

type ReadinessChecker interface {
AdminAPIReady(ctx context.Context, address string) error
}

// DiscoveredAdminAPI represents an Admin API discovered from a Kubernetes Service.
type DiscoveredAdminAPI struct {
Address string
Expand Down
14 changes: 14 additions & 0 deletions internal/adminapi/kong.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,16 @@ import (
tlsutil "github.com/kong/kubernetes-ingress-controller/v2/internal/util/tls"
)

type KongClientNotReadyError struct {
Err error
}

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

// 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,
Expand All @@ -30,6 +39,11 @@ func NewKongClientForWorkspace(ctx context.Context, adminURL string, wsName stri
return NewClient(client), nil
}

_, err = client.Status(ctx)
if 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 {
Expand Down
7 changes: 6 additions & 1 deletion internal/clients/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,12 @@ func (c *AdminAPIClientsManager) adjustGatewayClients(discoveredAdminAPIs []admi
for _, adminAPI := range toAdd {
client, err := c.adminAPIClientFactory.CreateAdminAPIClient(c.ctx, adminAPI)
if err != nil {
c.logger.WithError(err).Errorf("failed to create a client for %s", adminAPI)
if errors.As(err, &adminapi.KongClientNotReadyError{}) {
c.logger.WithError(err).Infof("client for %q is not ready yet", adminAPI.Address)
continue
}

c.logger.WithError(err).Errorf("failed to create a client for %q", adminAPI.Address)
continue
}

Expand Down
20 changes: 18 additions & 2 deletions internal/clients/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package clients

import (
"context"
"errors"
"fmt"
"net/http"
"net/http/httptest"
Expand All @@ -23,8 +24,9 @@ import (
// - client for an unexpected address gets created
// - client which already got created was tried to be created second time.
type clientFactoryWithExpected struct {
expected map[string]bool
t *testing.T
returnErrors map[string]error
expected map[string]bool
t *testing.T
}

func (cf clientFactoryWithExpected) CreateAdminAPIClient(_ context.Context, discoveredAdminAPI adminapi.DiscoveredAdminAPI) (*adminapi.Client, error) {
Expand All @@ -38,6 +40,9 @@ func (cf clientFactoryWithExpected) CreateAdminAPIClient(_ context.Context, disc
return nil, fmt.Errorf("got %s more than once", discoveredAdminAPI)
}
cf.expected[discoveredAdminAPI.Address] = false
if errToReturn, ok := cf.returnErrors[discoveredAdminAPI.Address]; ok {
return nil, errToReturn
}

c, err := adminapi.NewTestClient(discoveredAdminAPI.Address)
if err != nil {
Expand Down Expand Up @@ -99,7 +104,14 @@ func TestClientAddressesNotifications(t *testing.T) {
defer srv2.Close()
expected[srv2.URL] = true

notReadySrv := createTestServer()
defer notReadySrv.Close()
expected[notReadySrv.URL] = true

testClientFactoryWithExpected := clientFactoryWithExpected{
returnErrors: map[string]error{
notReadySrv.URL: adminapi.KongClientNotReadyError{Err: errors.New("admin api not ready")},
},
expected: expected,
t: t,
}
Expand Down Expand Up @@ -154,6 +166,10 @@ func TestClientAddressesNotifications(t *testing.T) {
requireClientsCountEventually(t, manager, []string{srv.URL},
"notifying again with just one URL should decrease the set of URLs to just this one")

manager.Notify([]adminapi.DiscoveredAdminAPI{testDiscoveredAdminAPI(srv.URL), testDiscoveredAdminAPI(notReadySrv.URL)})
requireClientsCountEventually(t, manager, []string{srv.URL},
"notifying again with twos URL where one of them is not ready should not change the set of URLs")

manager.Notify([]adminapi.DiscoveredAdminAPI{})
requireClientsCountEventually(t, manager, []string{})

Expand Down
4 changes: 2 additions & 2 deletions internal/dataplane/sendconfig/config_change_detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/kong/deck/file"
"github.com/kong/go-kong/kong"
"github.com/samber/lo"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -79,7 +78,8 @@ func (d *DefaultConfigurationChangeDetector) HasConfigurationChanged(
if cmp.Equal(targetConfig, &file.Content{},
cmp.FilterPath(
func(p cmp.Path) bool {
return lo.Contains([]string{"FormatVersion", "Info", "Transform"}, p.String())
path := p.String()
return path == "FormatVersion" || path == "Info"
},
cmp.Ignore(),
),
Expand Down
Loading

0 comments on commit d653950

Please sign in to comment.