refactor: test node agent against node client interface (#3976)

internal/clients/config_status.go

@@ -42,6 +42,13 @@ type ChannelConfigNotifier struct {
 
 var _ ConfigStatusNotifier = &ChannelConfigNotifier{}
 
+func NewChannelConfigNotifier(logger logr.Logger) *ChannelConfigNotifier {
+ return &ChannelConfigNotifier{
+ ch: make(chan ConfigStatus),
+ logger: logger,
+ }
+}
+
 // NotifyConfigStatus sends the status in a separate goroutine. If the notification is not received in 1s, it's dropped.
 func (n *ChannelConfigNotifier) NotifyConfigStatus(ctx context.Context, status ConfigStatus) {
  const notifyTimeout = time.Second
@@ -64,10 +71,3 @@ func (n *ChannelConfigNotifier) SubscribeConfigStatus() chan ConfigStatus {
  // TODO: in case of multiple subscribers, we should use a fan-out pattern.
  return n.ch
 }
-
-func NewChannelConfigNotifier(logger logr.Logger) *ChannelConfigNotifier {
- return &ChannelConfigNotifier{
- ch: make(chan ConfigStatus, 1),
- logger: logger,
- }
-}

internal/konnect/license_client.go → internal/konnect/license/client.go

@@ -1,4 +1,4 @@
-package konnect
+package license
 
 import (
  "context"
@@ -14,18 +14,18 @@ import (
  tlsutil "github.com/kong/kubernetes-ingress-controller/v2/internal/util/tls"
 )
 
-// LicenseAPIClient interacts with the Konnect license API.
-type LicenseAPIClient struct {
- Address string
- RuntimeGroupID string
- Client  *http.Client
+// Client interacts with the Konnect license API.
+type Client struct {
+ address string
+ runtimeGroupID string
+ httpClient *http.Client
 }
 
 // KICLicenseAPIPathPattern is the path pattern for KIC license operations.
 var KICLicenseAPIPathPattern = "%s/kic/api/runtime_groups/%s/v1/licenses"
 
-// NewLicenseAPIClient creates a Konnect client.
-func NewLicenseAPIClient(cfg adminapi.KonnectConfig) (*LicenseAPIClient, error) {
+// NewClient creates a License API Konnect client.
+func NewClient(cfg adminapi.KonnectConfig) (*Client, error) {
  tlsConfig := tls.Config{
  MinVersion: tls.VersionTLS12,
  }
@@ -42,18 +42,18 @@ func NewLicenseAPIClient(cfg adminapi.KonnectConfig) (*LicenseAPIClient, error)
  transport.TLSClientConfig = &tlsConfig
  c.Transport = transport
 
- return &LicenseAPIClient{
- Address: cfg.Address,
- RuntimeGroupID: cfg.RuntimeGroupID,
- Client:  c,
+ return &Client{
+ address: cfg.Address,
+ runtimeGroupID: cfg.RuntimeGroupID,
+ httpClient: c,
  }, nil
 }
 
-func (c *LicenseAPIClient) kicLicenseAPIEndpoint() string {
- return fmt.Sprintf(KICLicenseAPIPathPattern, c.Address, c.RuntimeGroupID)
+func (c *Client) kicLicenseAPIEndpoint() string {
+ return fmt.Sprintf(KICLicenseAPIPathPattern, c.address, c.runtimeGroupID)
 }
 
-func (c *LicenseAPIClient) List(ctx context.Context, pageNumber int) (*ListLicenseResponse, error) {
+func (c *Client) List(ctx context.Context, pageNumber int) (*ListLicenseResponse, error) {
  // TODO this is another case where we have a pseudo-unary object. The page is always 0 in practice, but if we have
  // separate functions per entity, we end up with effectively dead code for some
  url, _ := neturl.Parse(c.kicLicenseAPIEndpoint())
@@ -67,7 +67,7 @@ func (c *LicenseAPIClient) List(ctx context.Context, pageNumber int) (*ListLicen
  return nil, fmt.Errorf("failed to create request: %w", err)
  }
 
- httpResp, err := c.Client.Do(req)
+ httpResp, err := c.httpClient.Do(req)
  if err != nil {
  return nil, fmt.Errorf("failed to get response: %w", err)
  }
@@ -90,3 +90,8 @@ func (c *LicenseAPIClient) List(ctx context.Context, pageNumber int) (*ListLicen
  }
  return resp, nil
 }
+
+// isOKStatusCode returns true if the input HTTP status code is 2xx, in [200,300).
+func isOKStatusCode(code int) bool {
+ return code >= 200 && code < 300
+}

internal/konnect/license_types.go → internal/konnect/license/types.go

@@ -1,16 +1,16 @@
-package konnect
+package license
 
 type ListLicenseResponse struct {
- Items []*LicenseItem `json:"items"`
+ Items []*Item `json:"items"`
  // TODO our APIs generally assume that there are no unary objects. Any object type can have multiple instances,
  // and lists of instances can be paginated. However, the license API doesn't return pagination info, as it is
  // effectively a unary object. We should sort that out, to at least have a guarantee as to whether or not we'll
  // represent unary objects as a collection that coincidentally always only has one page with one entry.
  // Page *PaginationInfo `json:"page"`
 }
 
-// LicenseItem is a single license from the upstream license API.
-type LicenseItem struct {
+// Item is a single license from the upstream license API.
+type Item struct {
  License string `json:"payload,omitempty"`
  UpdatedAt uint64 `json:"updated_at,omitempty"`
  CreatedAt uint64 `json:"created_at,omitempty"`

internal/konnect/mock_node_api_test.go

@@ -0,0 +1,98 @@
+package konnect_test
+
+import (
+ "context"
+ "sync"
+
+ "github.com/google/uuid"
+ "github.com/samber/lo"
+
+ "github.com/kong/kubernetes-ingress-controller/v2/internal/konnect/nodes"
+)
+
+// mockNodeClient is a mock implementation of the NodeClient interface.
+type mockNodeClient struct {
+ nodes map[string]*nodes.NodeItem
+ returnErrorFromListNodes bool
+ wasListAllNodesCalled bool
+ lock sync.RWMutex
+}
+
+func newMockNodeClient(initialNodes []*nodes.NodeItem) *mockNodeClient {
+ nodesMap := lo.SliceToMap(initialNodes, func(i *nodes.NodeItem) (string, *nodes.NodeItem) {
+ return i.ID, i
+ })
+ return &mockNodeClient{nodes: nodesMap}
+}
+
+func (m *mockNodeClient) CreateNode(_ context.Context, req *nodes.CreateNodeRequest) (*nodes.CreateNodeResponse, error) {
+ node := m.upsertNode(&nodes.NodeItem{
+ ID: req.ID,
+ Version: req.Version,
+ Hostname: req.Hostname,
+ LastPing: req.LastPing,
+ Type: req.Type,
+ Status: req.Status,
+ })
+ return &nodes.CreateNodeResponse{Item: node}, nil
+}
+
+func (m *mockNodeClient) UpdateNode(_ context.Context, nodeID string, req *nodes.UpdateNodeRequest) (*nodes.UpdateNodeResponse, error) {
+ node := m.upsertNode(&nodes.NodeItem{
+ ID: nodeID,
+ Version: req.Version,
+ Hostname: req.Hostname,
+ LastPing: req.LastPing,
+ Type: req.Type,
+ Status: req.Status,
+ })
+ return &nodes.UpdateNodeResponse{Item: node}, nil
+}
+
+func (m *mockNodeClient) DeleteNode(_ context.Context, nodeID string) error {
+ m.lock.Lock()
+ defer m.lock.Unlock()
+ delete(m.nodes, nodeID)
+ return nil
+}
+
+func (m *mockNodeClient) ListAllNodes(_ context.Context) ([]*nodes.NodeItem, error) {
+ m.lock.Lock()
+ defer m.lock.Unlock()
+
+ m.wasListAllNodesCalled = true
+ return lo.MapToSlice(m.nodes, func(_ string, i *nodes.NodeItem) *nodes.NodeItem {
+ return i
+ }), nil
+}
+
+func (m *mockNodeClient) upsertNode(node *nodes.NodeItem) *nodes.NodeItem {
+ m.lock.Lock()
+ defer m.lock.Unlock()
+
+ if node.ID == "" {
+ node.ID = uuid.New().String()
+ }
+ m.nodes[node.ID] = node
+ return node
+}
+
+func (m *mockNodeClient) MustAllNodes() []*nodes.NodeItem {
+ ns, err := m.ListAllNodes(context.Background())
+ if err != nil {
+ panic(err)
+ }
+ return ns
+}
+
+func (m *mockNodeClient) WasListAllNodesCalled() bool {
+ m.lock.RLock()
+ defer m.lock.RUnlock()
+ return m.wasListAllNodesCalled
+}
+
+func (m *mockNodeClient) ReturnErrorFromListAllNodes(v bool) {
+ m.lock.Lock()
+ defer m.lock.Unlock()
+ m.returnErrorFromListNodes = v
+}