refactor: move things around to make building a client easier from other projects (#123)

* refactor: split logic for building API config into public func
* refactor: move config code out of internal/

api/configuration_default.go

@@ -0,0 +1,44 @@
+package api
+
+import (
+	"crypto/tls"
+	"fmt"
+	"net/http"
+	"net/url"
+)
+
+type ConfigParams struct {
+	Host             *url.URL
+	UserAgent        string
+	Token            *string
+	TraceId          *string
+	AllowInsecureTLS bool
+	Debug            bool
+}
+
+// NewAPIConfig builds a configuration tailored to the InfluxDB v2 API.
+func NewAPIConfig(params ConfigParams) *Configuration {
+	clientTransport := http.DefaultTransport.(*http.Transport).Clone()
+	clientTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: params.AllowInsecureTLS}
+
+	apiConfig := NewConfiguration()
+	apiConfig.Host = params.Host.Host
+	apiConfig.Scheme = params.Host.Scheme
+	apiConfig.UserAgent = params.UserAgent
+	apiConfig.HTTPClient = &http.Client{Transport: clientTransport}
+	if params.Token != nil {
+		apiConfig.DefaultHeader["Authorization"] = fmt.Sprintf("Token %s", *params.Token)
+	}
+	if params.TraceId != nil {
+		// NOTE: This is circumventing our codegen. If the header we use for tracing ever changes,
+		// we'll need to manually update the string here to match.
+		//
+		// The alternative is to pass the trace ID to the business logic for every CLI command, and
+		// use codegen'd logic to set the header on every HTTP request. Early versions of the CLI
+		// used that technique, and we found it to be error-prone and easy to forget during testing.
+		apiConfig.DefaultHeader["Zap-Trace-Span"] = *params.TraceId
+	}
+	apiConfig.Debug = params.Debug
+
+	return apiConfig
+}

clients/bucket/create_test.go

@@ -10,7 +10,7 @@ import (
 	"github.com/influxdata/influx-cli/v2/api"
 	"github.com/influxdata/influx-cli/v2/clients"
 	"github.com/influxdata/influx-cli/v2/clients/bucket"
-	"github.com/influxdata/influx-cli/v2/internal/config"
+	"github.com/influxdata/influx-cli/v2/config"
 	"github.com/influxdata/influx-cli/v2/internal/mock"
 	"github.com/influxdata/influx-cli/v2/internal/testutils"
 	"github.com/stretchr/testify/assert"

clients/bucket/delete_test.go

@@ -10,7 +10,7 @@ import (
 	"github.com/influxdata/influx-cli/v2/api"
 	"github.com/influxdata/influx-cli/v2/clients"
 	"github.com/influxdata/influx-cli/v2/clients/bucket"
-	"github.com/influxdata/influx-cli/v2/internal/config"
+	"github.com/influxdata/influx-cli/v2/config"
 	"github.com/influxdata/influx-cli/v2/internal/mock"
 	"github.com/influxdata/influx-cli/v2/internal/testutils"
 	"github.com/stretchr/testify/assert"

clients/bucket/list_test.go

@@ -10,7 +10,7 @@ import (
 	"github.com/influxdata/influx-cli/v2/api"
 	"github.com/influxdata/influx-cli/v2/clients"
 	"github.com/influxdata/influx-cli/v2/clients/bucket"
-	"github.com/influxdata/influx-cli/v2/internal/config"
+	"github.com/influxdata/influx-cli/v2/config"
 	"github.com/influxdata/influx-cli/v2/internal/mock"
 	"github.com/influxdata/influx-cli/v2/internal/testutils"
 	"github.com/stretchr/testify/assert"

clients/cli.go

@@ -3,7 +3,7 @@ package clients
 import (
 	"encoding/json"
 
-	"github.com/influxdata/influx-cli/v2/internal/config"
+	"github.com/influxdata/influx-cli/v2/config"
 	"github.com/influxdata/influx-cli/v2/internal/tabwriter"
 	"github.com/influxdata/influx-cli/v2/pkg/stdio"
 )

clients/config/config.go

@@ -7,7 +7,7 @@ import (
 
 	"github.com/influxdata/influx-cli/v2/api"
 	"github.com/influxdata/influx-cli/v2/clients"
-	"github.com/influxdata/influx-cli/v2/internal/config"
+	"github.com/influxdata/influx-cli/v2/config"
 )
 
 var ErrInvalidHostUrlScheme = errors.New("a scheme of http or https must be provided for host url")

clients/config/config_test.go

@@ -9,8 +9,8 @@ import (
 	"github.com/golang/mock/gomock"
 	"github.com/influxdata/influx-cli/v2/api"
 	"github.com/influxdata/influx-cli/v2/clients"
-	"github.com/influxdata/influx-cli/v2/clients/config"
+	cmd "github.com/influxdata/influx-cli/v2/clients/config"
-	iconfig "github.com/influxdata/influx-cli/v2/internal/config"
+	"github.com/influxdata/influx-cli/v2/config"
 	"github.com/influxdata/influx-cli/v2/internal/mock"
 	"github.com/influxdata/influx-cli/v2/internal/testutils"
 	"github.com/stretchr/testify/require"
@@ -25,7 +25,7 @@ func TestClient_SwitchActive(t *testing.T) {
 	stdio.EXPECT().Write(gomock.Any()).DoAndReturn(writtenBytes.Write).AnyTimes()
 
 	name := "foo"
-	cfg := iconfig.Config{
+	cfg := config.Config{
 		Name:   name,
 		Active: true,
 		Host:   "http://localhost:8086",
@@ -35,7 +35,7 @@ func TestClient_SwitchActive(t *testing.T) {
 	svc := mock.NewMockConfigService(ctrl)
 	svc.EXPECT().SwitchActive(gomock.Eq(name)).Return(cfg, nil)
 
-	cli := config.Client{CLI: clients.CLI{ConfigService: svc, StdIO: stdio}}
+	cli := cmd.Client{CLI: clients.CLI{ConfigService: svc, StdIO: stdio}}
 	require.NoError(t, cli.SwitchActive(name))
 	testutils.MatchLines(t, []string{
 		`Active\s+Name\s+URL\s+Org`,
@@ -51,7 +51,7 @@ func TestClient_PrintActive(t *testing.T) {
 	writtenBytes := bytes.Buffer{}
 	stdio.EXPECT().Write(gomock.Any()).DoAndReturn(writtenBytes.Write).AnyTimes()
 
-	cfg := iconfig.Config{
+	cfg := config.Config{
 		Name:   "foo",
 		Active: true,
 		Host:   "http://localhost:8086",
@@ -61,7 +61,7 @@ func TestClient_PrintActive(t *testing.T) {
 	svc := mock.NewMockConfigService(ctrl)
 	svc.EXPECT().Active().Return(cfg, nil)
 
-	cli := config.Client{CLI: clients.CLI{ConfigService: svc, StdIO: stdio}}
+	cli := cmd.Client{CLI: clients.CLI{ConfigService: svc, StdIO: stdio}}
 	require.NoError(t, cli.PrintActive())
 	testutils.MatchLines(t, []string{
 		`Active\s+Name\s+URL\s+Org`,
@@ -77,7 +77,7 @@ func TestClient_Create(t *testing.T) {
 	writtenBytes := bytes.Buffer{}
 	stdio.EXPECT().Write(gomock.Any()).DoAndReturn(writtenBytes.Write).AnyTimes()
 
-	cfg := iconfig.Config{
+	cfg := config.Config{
 		Name:   "foo",
 		Active: true,
 		Host:   "http://localhost:8086",
@@ -87,7 +87,7 @@ func TestClient_Create(t *testing.T) {
 	svc := mock.NewMockConfigService(ctrl)
 	svc.EXPECT().CreateConfig(cfg).Return(cfg, nil)
 
-	cli := config.Client{CLI: clients.CLI{ConfigService: svc, StdIO: stdio}}
+	cli := cmd.Client{CLI: clients.CLI{ConfigService: svc, StdIO: stdio}}
 	require.NoError(t, cli.Create(cfg))
 	testutils.MatchLines(t, []string{
 		`Active\s+Name\s+URL\s+Org`,
@@ -112,7 +112,7 @@ func TestClient_Delete(t *testing.T) {
 			in:   []string{"foo"},
 			registerExpectations: func(svc *mock.MockConfigService) {
 				svc.EXPECT().DeleteConfig(gomock.Eq("foo")).
-					Return(iconfig.Config{Name: "foo", Host: "bar", Org: "baz"}, nil)
+					Return(config.Config{Name: "foo", Host: "bar", Org: "baz"}, nil)
 			},
 			out: []string{`\s+foo\s+bar\s+baz\s+true`},
 		},
@@ -121,11 +121,11 @@ func TestClient_Delete(t *testing.T) {
 			in:   []string{"foo", "qux", "wibble"},
 			registerExpectations: func(svc *mock.MockConfigService) {
 				svc.EXPECT().DeleteConfig(gomock.Eq("foo")).
-					Return(iconfig.Config{Name: "foo", Host: "bar", Org: "baz"}, nil)
+					Return(config.Config{Name: "foo", Host: "bar", Org: "baz"}, nil)
 				svc.EXPECT().DeleteConfig(gomock.Eq("qux")).
-					Return(iconfig.Config{}, &api.Error{Code: api.ERRORCODE_NOT_FOUND})
+					Return(config.Config{}, &api.Error{Code: api.ERRORCODE_NOT_FOUND})
 				svc.EXPECT().DeleteConfig(gomock.Eq("wibble")).
-					Return(iconfig.Config{Name: "wibble", Host: "bar", Active: true}, nil)
+					Return(config.Config{Name: "wibble", Host: "bar", Active: true}, nil)
 			},
 			out: []string{
 				`\s+foo\s+bar\s+baz\s+true`,
@@ -149,7 +149,7 @@ func TestClient_Delete(t *testing.T) {
 				tc.registerExpectations(svc)
 			}
 
-			cli := config.Client{CLI: clients.CLI{ConfigService: svc, StdIO: stdio}}
+			cli := cmd.Client{CLI: clients.CLI{ConfigService: svc, StdIO: stdio}}
 			require.NoError(t, cli.Delete(tc.in))
 
 			// Can't use our usual 'MatchLines' because list output depends on map iteration,
@@ -170,12 +170,12 @@ func TestClient_Update(t *testing.T) {
 	writtenBytes := bytes.Buffer{}
 	stdio.EXPECT().Write(gomock.Any()).DoAndReturn(writtenBytes.Write).AnyTimes()
 
-	updates := iconfig.Config{
+	updates := config.Config{
 		Name:   "foo",
 		Active: true,
 		Token:  "doublesecret",
 	}
-	cfg := iconfig.Config{
+	cfg := config.Config{
 		Name:   updates.Name,
 		Active: updates.Active,
 		Host:   "http://localhost:8086",
@@ -185,7 +185,7 @@ func TestClient_Update(t *testing.T) {
 	svc := mock.NewMockConfigService(ctrl)
 	svc.EXPECT().UpdateConfig(updates).Return(cfg, nil)
 
-	cli := config.Client{CLI: clients.CLI{ConfigService: svc, StdIO: stdio}}
+	cli := cmd.Client{CLI: clients.CLI{ConfigService: svc, StdIO: stdio}}
 	require.NoError(t, cli.Update(updates))
 	testutils.MatchLines(t, []string{
 		`Active\s+Name\s+URL\s+Org`,
@@ -198,7 +198,7 @@ func TestClient_List(t *testing.T) {
 
 	testCases := []struct {
 		name     string
-		cfgs     iconfig.Configs
+		cfgs     config.Configs
 		expected []string
 	}{
 		{
@@ -206,16 +206,16 @@ func TestClient_List(t *testing.T) {
 		},
 		{
 			name: "one",
-			cfgs: iconfig.Configs{
+			cfgs: config.Configs{
-				"foo": iconfig.Config{Name: "foo", Host: "bar", Org: "baz"},
+				"foo": config.Config{Name: "foo", Host: "bar", Org: "baz"},
 			},
 			expected: []string{`\s+foo\s+bar\s+baz`},
 		},
 		{
 			name: "many",
-			cfgs: iconfig.Configs{
+			cfgs: config.Configs{
-				"foo":    iconfig.Config{Name: "foo", Host: "bar", Org: "baz"},
+				"foo":    config.Config{Name: "foo", Host: "bar", Org: "baz"},
-				"wibble": iconfig.Config{Name: "wibble", Host: "bar", Active: true},
+				"wibble": config.Config{Name: "wibble", Host: "bar", Active: true},
 			},
 			expected: []string{
 				`\s+foo\s+bar\s+baz`,
@@ -237,7 +237,7 @@ func TestClient_List(t *testing.T) {
 			svc := mock.NewMockConfigService(ctrl)
 			svc.EXPECT().ListConfigs().Return(tc.cfgs, nil)
 
-			cli := config.Client{CLI: clients.CLI{ConfigService: svc, StdIO: stdio}}
+			cli := cmd.Client{CLI: clients.CLI{ConfigService: svc, StdIO: stdio}}
 			require.NoError(t, cli.List())
 
 			// Can't use our usual 'MatchLines' because list output depends on map iteration,

clients/delete/delete_test.go

@@ -9,7 +9,7 @@ import (
 	"github.com/influxdata/influx-cli/v2/api"
 	"github.com/influxdata/influx-cli/v2/clients"
 	"github.com/influxdata/influx-cli/v2/clients/delete"
-	"github.com/influxdata/influx-cli/v2/internal/config"
+	"github.com/influxdata/influx-cli/v2/config"
 	"github.com/influxdata/influx-cli/v2/internal/mock"
 	"github.com/influxdata/influx-cli/v2/pkg/influxid"
 	"github.com/stretchr/testify/assert"

clients/org/org_members_test.go

@@ -12,7 +12,7 @@ import (
 	"github.com/influxdata/influx-cli/v2/api"
 	"github.com/influxdata/influx-cli/v2/clients"
 	"github.com/influxdata/influx-cli/v2/clients/org"
-	"github.com/influxdata/influx-cli/v2/internal/config"
+	"github.com/influxdata/influx-cli/v2/config"
 	"github.com/influxdata/influx-cli/v2/internal/mock"
 	"github.com/influxdata/influx-cli/v2/internal/testutils"
 	"github.com/influxdata/influx-cli/v2/pkg/influxid"

clients/query/query_test.go

@@ -14,7 +14,7 @@ import (
 	"github.com/influxdata/influx-cli/v2/api"
 	"github.com/influxdata/influx-cli/v2/clients"
 	"github.com/influxdata/influx-cli/v2/clients/query"
-	"github.com/influxdata/influx-cli/v2/internal/config"
+	"github.com/influxdata/influx-cli/v2/config"
 	"github.com/influxdata/influx-cli/v2/internal/mock"
 	"github.com/influxdata/influx-cli/v2/pkg/influxid"
 	"github.com/stretchr/testify/assert"

clients/setup/setup.go

@@ -11,7 +11,7 @@ import (
 	"github.com/influxdata/influx-cli/v2/api"
 	"github.com/influxdata/influx-cli/v2/clients"
 	"github.com/influxdata/influx-cli/v2/clients/bucket"
-	"github.com/influxdata/influx-cli/v2/internal/config"
+	"github.com/influxdata/influx-cli/v2/config"
 	"github.com/influxdata/influx-cli/v2/internal/duration"
 )
 

clients/setup/setup_test.go

@@ -13,7 +13,7 @@ import (
 	"github.com/influxdata/influx-cli/v2/api"
 	"github.com/influxdata/influx-cli/v2/clients"
 	"github.com/influxdata/influx-cli/v2/clients/setup"
-	"github.com/influxdata/influx-cli/v2/internal/config"
+	"github.com/influxdata/influx-cli/v2/config"
 	"github.com/influxdata/influx-cli/v2/internal/duration"
 	"github.com/influxdata/influx-cli/v2/internal/mock"
 	"github.com/influxdata/influx-cli/v2/internal/testutils"

clients/user/user_test.go

@@ -11,7 +11,7 @@ import (
 	"github.com/influxdata/influx-cli/v2/api"
 	"github.com/influxdata/influx-cli/v2/clients"
 	"github.com/influxdata/influx-cli/v2/clients/user"
-	"github.com/influxdata/influx-cli/v2/internal/config"
+	 "github.com/influxdata/influx-cli/v2/config"
 	"github.com/influxdata/influx-cli/v2/internal/mock"
 	"github.com/influxdata/influx-cli/v2/internal/testutils"
 	"github.com/influxdata/influx-cli/v2/pkg/influxid"

clients/write/dryrun_test.go

@@ -10,7 +10,7 @@ import (
 	"github.com/golang/mock/gomock"
 	"github.com/influxdata/influx-cli/v2/clients"
 	"github.com/influxdata/influx-cli/v2/clients/write"
-	"github.com/influxdata/influx-cli/v2/internal/config"
+	"github.com/influxdata/influx-cli/v2/config"
 	"github.com/influxdata/influx-cli/v2/internal/mock"
 	"github.com/stretchr/testify/require"
 )

clients/write/write_test.go

@@ -13,7 +13,7 @@ import (
 	"github.com/influxdata/influx-cli/v2/api"
 	"github.com/influxdata/influx-cli/v2/clients"
 	"github.com/influxdata/influx-cli/v2/clients/write"
-	"github.com/influxdata/influx-cli/v2/internal/config"
+	"github.com/influxdata/influx-cli/v2/config"
 	"github.com/influxdata/influx-cli/v2/internal/mock"
 	"github.com/stretchr/testify/assert"
 	tmock "github.com/stretchr/testify/mock"

cmd/influx/config.go

@@ -5,8 +5,8 @@ import (
 	"os"
 	"path"
 
-	"github.com/influxdata/influx-cli/v2/clients/config"
+	cmd "github.com/influxdata/influx-cli/v2/clients/config"
-	iconfig "github.com/influxdata/influx-cli/v2/internal/config"
+	"github.com/influxdata/influx-cli/v2/config"
 	"github.com/urfave/cli/v2"
 )
 
@@ -48,7 +48,7 @@ https://docs.influxdata.com/influxdb/latest/reference/cli/influx/config/
 			if ctx.NArg() > 1 {
 				return fmt.Errorf("usage: %s config [config name]", prog)
 			}
-			client := config.Client{CLI: getCLI(ctx)}
+			client := cmd.Client{CLI: getCLI(ctx)}
 			if ctx.NArg() == 1 {
 				return client.SwitchActive(ctx.Args().Get(0))
 			}
@@ -64,7 +64,7 @@ https://docs.influxdata.com/influxdb/latest/reference/cli/influx/config/
 }
 
 func newConfigCreateCmd() *cli.Command {
-	var cfg iconfig.Config
+	var cfg config.Config
 	return &cli.Command{
 		Name:  "create",
 		Usage: "Create config",
@@ -122,7 +122,7 @@ https://docs.influxdata.com/influxdb/latest/reference/cli/influx/config/create/
 			},
 		),
 		Action: func(ctx *cli.Context) error {
-			client := config.Client{CLI: getCLI(ctx)}
+			client := cmd.Client{CLI: getCLI(ctx)}
 			return client.Create(cfg)
 		},
 	}
@@ -153,14 +153,14 @@ https://docs.influxdata.com/influxdb/latest/reference/cli/influx/config/rm/
 		Before: withCli(),
 		Flags:  append([]cli.Flag{configPathFlag()}, printFlags()...),
 		Action: func(ctx *cli.Context) error {
-			client := config.Client{CLI: getCLI(ctx)}
+			client := cmd.Client{CLI: getCLI(ctx)}
 			return client.Delete(ctx.Args().Slice())
 		},
 	}
 }
 
 func newConfigUpdateCmd() *cli.Command {
-	var cfg iconfig.Config
+	var cfg config.Config
 	return &cli.Command{
 		Name:    "set",
 		Aliases: []string{"update"},
@@ -217,7 +217,7 @@ https://docs.influxdata.com/influxdb/latest/reference/cli/influx/config/set/
 			},
 		),
 		Action: func(ctx *cli.Context) error {
-			client := config.Client{CLI: getCLI(ctx)}
+			client := cmd.Client{CLI: getCLI(ctx)}
 			return client.Update(cfg)
 		},
 	}
@@ -249,7 +249,7 @@ https://docs.influxdata.com/influxdb/latest/reference/cli/influx/config/list/
 		Before: withCli(),
 		Flags:  configPathAndPrintFlags,
 		Action: func(ctx *cli.Context) error {
-			client := config.Client{CLI: getCLI(ctx)}
+			client := cmd.Client{CLI: getCLI(ctx)}
 			return client.List()
 		},
 	}

cmd/influx/global.go

@@ -1,15 +1,13 @@
 package main
 
 import (
-	"crypto/tls"
 	"fmt"
-	"net/http"
 	"net/url"
 	"runtime"
 
 	"github.com/influxdata/influx-cli/v2/api"
 	"github.com/influxdata/influx-cli/v2/clients"
-	"github.com/influxdata/influx-cli/v2/internal/config"
+	"github.com/influxdata/influx-cli/v2/config"
 	"github.com/influxdata/influx-cli/v2/pkg/cli/middleware"
 	"github.com/influxdata/influx-cli/v2/pkg/stdio"
 	"github.com/urfave/cli/v2"
@@ -71,34 +69,26 @@ func newApiClient(ctx *cli.Context, configSvc config.Service, injectToken bool)
 		cfg.Host = ctx.String(hostFlagName)
 	}
 
+	configParams := api.ConfigParams{
+		UserAgent:        fmt.Sprintf("influx/%s (%s) Sha/%s Date/%s", version, runtime.GOOS, commit, date),
+		AllowInsecureTLS: ctx.Bool(skipVerifyFlagName),
+		Debug:            ctx.Bool(httpDebugFlagName),
+	}
+
 	parsedHost, err := url.Parse(cfg.Host)
 	if err != nil {
 		return nil, fmt.Errorf("host URL %q is invalid: %w", cfg.Host, err)
 	}
+	configParams.Host = parsedHost
 
-	clientTransport := http.DefaultTransport.(*http.Transport)
-	clientTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: ctx.Bool(skipVerifyFlagName)}
-
-	apiConfig := api.NewConfiguration()
-	apiConfig.Host = parsedHost.Host
-	apiConfig.Scheme = parsedHost.Scheme
-	apiConfig.UserAgent = fmt.Sprintf("influx/%s (%s) Sha/%s Date/%s", version, runtime.GOOS, commit, date)
-	apiConfig.HTTPClient = &http.Client{Transport: clientTransport}
 	if injectToken {
-		apiConfig.DefaultHeader["Authorization"] = fmt.Sprintf("Token %s", cfg.Token)
+		configParams.Token = &cfg.Token
 	}
 	if ctx.IsSet(traceIdFlagName) {
-		// NOTE: This is circumventing our codegen. If the header we use for tracing ever changes,
+		configParams.TraceId = api.PtrString(ctx.String(traceIdFlagName))
-		// we'll need to manually update the string here to match.
-		//
-		// The alternative is to pass the trace ID to the business logic for every CLI command, and
-		// use codegen'd logic to set the header on every HTTP request. Early versions of the CLI
-		// used that technique, and we found it to be error-prone and easy to forget during testing.
-		apiConfig.DefaultHeader["Zap-Trace-Span"] = ctx.String(traceIdFlagName)
 	}
-	apiConfig.Debug = ctx.Bool(httpDebugFlagName)
 
-	return api.NewAPIClient(apiConfig), nil
+	return api.NewAPIClient(api.NewAPIConfig(configParams)), nil
 }
 
 func withCli() cli.BeforeFunc {

internal/mock/config.gen.go

@@ -5,10 +5,10 @@
 package mock
 
 import (
-	reflect "reflect"
+	"reflect"
 
-	gomock "github.com/golang/mock/gomock"
+	"github.com/golang/mock/gomock"
-	config "github.com/influxdata/influx-cli/v2/internal/config"
+	"github.com/influxdata/influx-cli/v2/config"
 )
 
 // MockConfigService is a mock of Service interface.

internal/mock/gen.go

@@ -13,5 +13,5 @@ package mock
 //go:generate go run github.com/golang/mock/mockgen -package mock -destination api_backup.gen.go github.com/influxdata/influx-cli/v2/api BackupApi
 
 // Other mocks
-//go:generate go run github.com/golang/mock/mockgen -package mock -destination config.gen.go -mock_names Service=MockConfigService github.com/influxdata/influx-cli/v2/internal/config Service
+//go:generate go run github.com/golang/mock/mockgen -package mock -destination config.gen.go -mock_names Service=MockConfigService github.com/influxdata/influx-cli/v2/config Service
 //go:generate go run github.com/golang/mock/mockgen -package mock -destination stdio.gen.go github.com/influxdata/influx-cli/v2/pkg/stdio StdIO