Add linter; Expose LB API Client (#20)

* export lb api client

Signed-off-by: Tyler Auerbeck <[email protected]>

* add .golangci.yml

Signed-off-by: Tyler Auerbeck <[email protected]>

* New lint, New Problems

Signed-off-by: Tyler Auerbeck <[email protected]>

---------

Signed-off-by: Tyler Auerbeck <[email protected]>
Co-authored-by: Tyler Auerbeck <[email protected]>

.golangci.yml

@@ -0,0 +1,57 @@
+linters-settings:
+  goimports:
+    local-prefixes: go.infratographer.com/loadbalanceroperator
+
+linters:
+  enable:
+    # default linters
+    - deadcode
+    - errcheck
+    - gosimple
+    - govet
+    - ineffassign
+    - staticcheck
+    - structcheck
+    - typecheck
+    - unused
+    - varcheck
+
+    # additional linters
+    - bodyclose
+    - gocritic
+    - gocyclo
+    - goerr113
+    - gofmt
+    - goimports
+    - gomnd
+    - govet
+    - misspell
+    - noctx
+    - stylecheck
+    - whitespace
+    - wsl
+
+issues:
+  exclude:
+    # Default excludes from `golangci-lint run --help` with EXC0002 removed
+    # EXC0001 errcheck: Almost all programs ignore errors on these functions and in most cases it's ok
+    - Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked
+    # EXC0002 golint: Annoying issue about not having a comment. The rare codebase has such comments
+    # - (comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form)
+    # EXC0003 golint: False positive when tests are defined in package 'test'
+    - func name will be used as test\.Test.* by other packages, and that stutters; consider calling this
+    # EXC0004 govet: Common false positives
+    - (possible misuse of unsafe.Pointer|should have signature)
+    # EXC0005 staticcheck: Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore
+    - ineffective break statement. Did you mean to break out of the outer loop
+    # EXC0006 gosec: Too many false-positives on 'unsafe' usage
+    - Use of unsafe calls should be audited
+    # EXC0007 gosec: Too many false-positives for parametrized shell calls
+    - Subprocess launch(ed with variable|ing should be audited)
+    # EXC0008 gosec: Duplicated errcheck checks
+    - (G104|G307)
+    # EXC0009 gosec: Too many issues in popular repos
+    - (Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less)
+    # EXC0010 gosec: False positive is triggered by 'src, err := ioutil.ReadFile(filename)'
+    - Potential file inclusion via variable
+  exclude-use-default: false

cmd/run.go

@@ -8,8 +8,8 @@ import (
 	"strings"
 
 	"go.infratographer.com/loadbalancer-manager-haproxy/internal/dataplaneapi"
-	"go.infratographer.com/loadbalancer-manager-haproxy/internal/lbapi"
 	"go.infratographer.com/loadbalancer-manager-haproxy/internal/manager"
+	"go.infratographer.com/loadbalancer-manager-haproxy/pkg/lbapi"
 
 	"github.com/nats-io/nats.go"
 	"github.com/spf13/cobra"

internal/dataplaneapi/client.go

@@ -27,8 +27,8 @@ func NewClient(url string) *Client {
 	}
 }
 
-// ApiIsReady returns true when a 200 is returned for a GET request to the Data Plane API
-func (c *Client) ApiIsReady(ctx context.Context) bool {
+// APIIsReady returns true when a 200 is returned for a GET request to the Data Plane API
+func (c *Client) APIIsReady(ctx context.Context) bool {
 	req, _ := http.NewRequestWithContext(ctx, http.MethodGet, c.baseURL, nil)
 	req.SetBasicAuth(viper.GetString("dataplane.user.name"), viper.GetString("dataplane.user.pwd"))
 

internal/dataplaneapi/client_test.go

@@ -63,7 +63,7 @@ func TestAPIIsReady(t *testing.T) {
 		baseURL: "http://localhost:5555/v2",
 	}
 
-	ready := dc.ApiIsReady(context.TODO())
+	ready := dc.APIIsReady(context.TODO())
 	if !ready {
 		t.Error("expected dataplane api readiness to be true, got:", ready)
 	}
@@ -80,7 +80,7 @@ func TestAPIIsReady(t *testing.T) {
 		baseURL: "http://localhost:5555/v2",
 	}
 
-	ready = dc.ApiIsReady(context.TODO())
+	ready = dc.APIIsReady(context.TODO())
 	if ready {
 		t.Error("expected dataplane api readiness to be false, got:", ready)
 	}

internal/dataplaneapi/doc.go

@@ -0,0 +1,2 @@
+// Package dataplaneapi provides a client for interacting with the haproxy dataplane api
+package dataplaneapi

internal/manager/doc.go

@@ -0,0 +1,3 @@
+// Package manager provides core functionality of the load balancer manager for
+// processing messages and managing load balancers
+package manager

internal/manager/errors.go

@@ -0,0 +1,34 @@
+package manager
+
+import (
+	"errors"
+	"fmt"
+)
+
+var (
+	// errInvalidLBID is returned when an invalid load balancer ID is provided
+	errInvalidLBID = errors.New("optional lbID param must be not set or set to a singular loadbalancer ID")
+
+	// errFrontendSectionLabelFailure is returned when a frontend section cannot be created
+	errFrontendSectionLabelFailure = errors.New("failed to create frontend section with label")
+
+	// errUseBackendFailure is returned when the use_backend attr cannot be applied to a frontend
+	errUseBackendFailure = errors.New("failed to create frontend attr use_backend")
+
+	// errFrontendBindFailure is returned when the bind attribute cannot be applied to a frontend
+	errFrontendBindFailure = errors.New("failed to create frontend attr bind")
+
+	// errBackendSectionLabelFailure is returned when a backend section cannot be created
+	errBackendSectionLabelFailure = errors.New("failed to create section backend with label")
+
+	// errBackendServerFailure is returned when a server cannot be applied to a backend
+	errBackendServerFailure = errors.New("failed to add backend attr server: ")
+)
+
+func newLabelError(label string, err error, labelErr error) error {
+	return fmt.Errorf("%w %q: %v", err, label, labelErr)
+}
+
+func newAttrError(err error, attrErr error) error {
+	return fmt.Errorf("%w: %v", err, attrErr)
+}

internal/manager/manager.go

@@ -13,7 +13,7 @@ import (
 	"github.com/nats-io/nats.go"
 	"github.com/spf13/viper"
 	"go.infratographer.com/loadbalancer-manager-haproxy/internal/dataplaneapi"
-	"go.infratographer.com/loadbalancer-manager-haproxy/internal/lbapi"
+	"go.infratographer.com/loadbalancer-manager-haproxy/pkg/lbapi"
 
 	"go.infratographer.com/x/pubsubx"
 	"go.infratographer.com/x/urnx"
@@ -34,7 +34,7 @@ type lbAPI interface {
 
 type dataPlaneAPI interface {
 	PostConfig(ctx context.Context, config string) error
-	ApiIsReady(ctx context.Context) bool
+	APIIsReady(ctx context.Context) bool
 }
 
 // Manager contains configuration and client connections
@@ -124,7 +124,7 @@ func (m Manager) processMsg(msg *pubsub.Message) error {
 // updateConfigToLatest update the haproxy cfg to either baseline or one requested from lbapi with optional lbID param
 func (m *Manager) updateConfigToLatest(lbID ...string) error {
 	if len(lbID) > 1 {
-		return fmt.Errorf("optional lbID param must be not set or set to a singular loadbalancer ID")
+		return errInvalidLBID
 	}
 
 	m.Logger.Info("updating the config")
@@ -201,7 +201,7 @@ func (m *Manager) updateConfigToLatest(lbID ...string) error {
 
 func (m Manager) waitForDataPlaneReady(retries int, sleep time.Duration) error {
 	for i := 0; i < retries; i++ {
-		if m.DataPlaneClient.ApiIsReady(m.Context) {
+		if m.DataPlaneClient.APIIsReady(m.Context) {
 			m.Logger.Info("dataplaneapi is ready")
 			return nil
 		}
@@ -218,22 +218,22 @@ func mergeConfig(cfg parser.Parser, lb *loadBalancer) (parser.Parser, error) {
 	for _, p := range lb.Ports {
 		// create port
 		if err := cfg.SectionsCreate(parser.Frontends, p.Name); err != nil {
-			return nil, fmt.Errorf("failed to create frontend section with label %q: %v", p.Name, err)
+			return nil, newLabelError(p.Name, errFrontendSectionLabelFailure, err)
 		}
 
 		if err := cfg.Insert(parser.Frontends, p.Name, "bind", types.Bind{
 			Path: fmt.Sprintf("%s@:%d", p.AddressFamily, p.Port)}); err != nil {
-			return nil, fmt.Errorf("failed to create frontend attr bind: %v", err)
+			return nil, newAttrError(errFrontendBindFailure, err)
 		}
 
 		// map frontend to backend
 		if err := cfg.Set(parser.Frontends, p.Name, "use_backend", types.UseBackend{Name: p.Name}); err != nil {
-			return nil, fmt.Errorf("failed to create frontend attr use_backend: %v", err)
+			return nil, newAttrError(errUseBackendFailure, err)
 		}
 
 		// create backend
 		if err := cfg.SectionsCreate(parser.Backends, p.Name); err != nil {
-			return nil, fmt.Errorf("failed to create section backend with label %q': %v", p.Name, err)
+			return nil, newLabelError(p.Name, errBackendSectionLabelFailure, err)
 		}
 
 		for _, pool := range p.Pools {
@@ -250,7 +250,7 @@ func mergeConfig(cfg parser.Parser, lb *loadBalancer) (parser.Parser, error) {
 				}
 
 				if err := cfg.Set(parser.Backends, p.Name, "server", srvr); err != nil {
-					return nil, fmt.Errorf("failed to add backend %q attr server: %v", p.Name, err)
+					return nil, newLabelError(p.Name, errBackendServerFailure, err)
 				}
 			}
 		}

internal/manager/manager_test.go

@@ -11,8 +11,8 @@ import (
 	"github.com/haproxytech/config-parser/v4/options"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"go.infratographer.com/loadbalancer-manager-haproxy/internal/lbapi"
 	"go.infratographer.com/loadbalancer-manager-haproxy/internal/manager/mock"
+	"go.infratographer.com/loadbalancer-manager-haproxy/pkg/lbapi"
 	"go.uber.org/zap"
 )
 
@@ -58,14 +58,15 @@ func TestMergeConfig(t *testing.T) {
 func TestUpdateConfigToLatest(t *testing.T) {
 	l, err := zap.NewDevelopmentConfig().Build()
 	logger := l.Sugar()
+
 	require.Nil(t, err)
 
 	t.Run("errors on failure to query for loadbalancers/:id", func(t *testing.T) {
 		t.Parallel()
 
 		mockLBAPI := &mock.LBAPIClient{
 			DoGetLoadBalancer: func(ctx context.Context, id string) (*lbapi.LoadBalancerResponse, error) {
-				return nil, fmt.Errorf("failure")
+				return nil, fmt.Errorf("failure") // nolint:goerr113
 			},
 		}
 
@@ -101,7 +102,7 @@ func TestUpdateConfigToLatest(t *testing.T) {
 				}, nil
 			},
 			DoGetPool: func(ctx context.Context, id string) (*lbapi.PoolResponse, error) {
-				return nil, fmt.Errorf("failure")
+				return nil, fmt.Errorf("failure") // nolint:goerr113
 			},
 		}
 
@@ -138,7 +139,7 @@ func TestUpdateConfigToLatest(t *testing.T) {
 
 		// remove that 'unnamed_defaults_1' thing the haproxy parser library puts in the default section,
 		// even though the library is configured to not include default section labels
-		mgr.currentConfig = strings.Replace(mgr.currentConfig, " unnamed_defaults_1", "", -1)
+		mgr.currentConfig = strings.ReplaceAll(mgr.currentConfig, " unnamed_defaults_1", "")
 
 		assert.Equal(t, strings.TrimSpace(string(contents)), strings.TrimSpace(mgr.currentConfig))
 	})

internal/manager/mock/doc.go

@@ -0,0 +1,2 @@
+// Package mock provides mock implementations of the lb haproxy manager
+package mock

internal/manager/mock/mock.go

@@ -3,7 +3,7 @@ package mock
 import (
 	"context"
 
-	"go.infratographer.com/loadbalancer-manager-haproxy/internal/lbapi"
+	"go.infratographer.com/loadbalancer-manager-haproxy/pkg/lbapi"
 )
 
 // LBAPIClient mock client
@@ -23,13 +23,13 @@ func (c LBAPIClient) GetPool(ctx context.Context, id string) (*lbapi.PoolRespons
 // DataplaneAPIClient mock client
 type DataplaneAPIClient struct {
 	DoPostConfig func(ctx context.Context, config string) error
-	DoApiIsReady func(ctx context.Context) bool
+	DoAPIIsReady func(ctx context.Context) bool
 }
 
 func (c *DataplaneAPIClient) PostConfig(ctx context.Context, config string) error {
 	return c.DoPostConfig(ctx, config)
 }
 
-func (c DataplaneAPIClient) ApiIsReady(ctx context.Context) bool {
-	return c.DoApiIsReady(ctx)
+func (c DataplaneAPIClient) APIIsReady(ctx context.Context) bool {
+	return c.DoAPIIsReady(ctx)
 }

internal/lbapi/client.go → pkg/lbapi/client.go

@@ -1,4 +1,3 @@
-// lbapi TODO: will move to https://github.com/infratographer/load-balancer-api
 package lbapi
 
 import (
@@ -13,24 +12,27 @@ import (
 )
 
 const (
-	apiVersion = "v1"
+	apiVersion           = "v1"
+	clientTimeoutSeconds = 5
 )
 
 // HTTPClient interface
 type HTTPClient interface {
 	Do(req *http.Request) (*http.Response, error)
 }
 
+// Client creates a new lb api client against a specific endpoint
 type Client struct {
 	client  HTTPClient
 	baseURL string
 }
 
+// NewClient creates a new lb api client
 func NewClient(url string, opts ...func(*Client)) *Client {
 	// default retryable http client
 	retryCli := retryablehttp.NewClient()
 	retryCli.RetryMax = 3
-	retryCli.HTTPClient.Timeout = time.Second * 5
+	retryCli.HTTPClient.Timeout = time.Second * time.Duration(clientTimeoutSeconds)
 	retryCli.Logger = nil
 
 	c := &Client{
@@ -71,7 +73,7 @@ func (c Client) GetLoadBalancer(ctx context.Context, id string) (*LoadBalancerRe
 	switch resp.StatusCode {
 	case http.StatusOK:
 		if err := json.NewDecoder(resp.Body).Decode(lb); err != nil {
-			return nil, fmt.Errorf("failed to decode load balancer: %v", err)
+			return nil, newError(errDecodeLB, err)
 		}
 	case http.StatusNotFound:
 		return nil, ErrLBHTTPNotfound
@@ -82,8 +84,9 @@ func (c Client) GetLoadBalancer(ctx context.Context, id string) (*LoadBalancerRe
 	default:
 		b, err := io.ReadAll(resp.Body)
 		if err != nil {
-			return nil, fmt.Errorf("failed to read resp body")
+			return nil, errReadResponse
 		}
+
 		return nil, fmt.Errorf("%s: %w", fmt.Sprintf("StatusCode (%d) - %s ", resp.StatusCode, string(b)), ErrLBHTTPError)
 	}
 
@@ -109,7 +112,7 @@ func (c Client) GetPool(ctx context.Context, id string) (*PoolResponse, error) {
 	switch resp.StatusCode {
 	case http.StatusOK:
 		if err := json.NewDecoder(resp.Body).Decode(pool); err != nil {
-			return nil, fmt.Errorf("failed to decode load balancer: %v", err)
+			return nil, newError(errDecodeLB, err)
 		}
 	case http.StatusNotFound:
 		return nil, ErrLBHTTPNotfound
@@ -120,8 +123,9 @@ func (c Client) GetPool(ctx context.Context, id string) (*PoolResponse, error) {
 	default:
 		b, err := io.ReadAll(resp.Body)
 		if err != nil {
-			return nil, fmt.Errorf("failed to read resp body")
+			return nil, errReadResponse
 		}
+
 		return nil, fmt.Errorf("%s: %w", fmt.Sprintf("StatusCode (%d) - %s ", resp.StatusCode, string(b)), ErrLBHTTPError)
 	}
 

internal/lbapi/client_test.go → pkg/lbapi/client_test.go

@@ -9,7 +9,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"go.infratographer.com/loadbalancer-manager-haproxy/internal/lbapi/mock"
+	"go.infratographer.com/loadbalancer-manager-haproxy/pkg/lbapi/internal/mock"
 )
 
 func newLBAPIMock(respJSON string, respCode int) *mock.HTTPClient {
@@ -18,6 +18,7 @@ func newLBAPIMock(respJSON string, respCode int) *mock.HTTPClient {
 		json := respJSON
 
 		r := io.NopCloser(strings.NewReader(json))
+
 		return &http.Response{
 			StatusCode: respCode,
 			Body:       r,

pkg/lbapi/doc.go

@@ -0,0 +1,2 @@
+// Package lbapi provides a client for interacting with the load balancer api
+package lbapi