refactor!: rename --address flag to --url; require scheme

cmd/otf-agent/main.go

@@ -57,7 +57,7 @@ func run(ctx context.Context, args []string) error {
 		},
 	}
 
-	cmd.Flags().StringVar(&clientConfig.Address, "address", otfapi.DefaultAddress, "Address of OTF server")
+	cmd.Flags().StringVar(&clientConfig.URL, "url", otfapi.DefaultURL, "URL of OTF server")
 	cmd.Flags().StringVar(&clientConfig.Token, "token", "", "Agent token for authentication")
 	cmd.MarkFlagRequired("token")
 	cmd.SetArgs(args)

docs/agents.md

@@ -61,7 +61,7 @@ Give the token a description and click **Create token**.
 Copy the token to your system clipboard. Now you can run the agent:
 
 ```bash
-otf-agent --token <token> --address <otfd-hostname>
+otf-agent --token <token> --url https://<otfd-hostname>
 ```
 
 The agent should confirm it has registered successfully:

docs/cli.md

@@ -11,19 +11,21 @@ Usage:
   otf [command]
 
 Available Commands:
-  agents        Agent management
-  help          Help about any command
-  organizations Organization management
-  runs          Runs management
-  state         State version management
-  teams         Team management
-  users         User account management
-  workspaces    Workspace management
+  agents          Agent management
+  completion      Generate the autocompletion script for the specified shell
+  help            Help about any command
+  organizations   Organization management
+  runs            Runs management
+  state           State version management
+  team-membership Team membership management
+  teams           Team management
+  users           User account management
+  workspaces      Workspace management
 
 Flags:
-      --address string   Address of OTF server (default "localhost:8080")
-  -h, --help             help for otf
-      --token string     API authentication token
+  -h, --help           help for otf
+      --token string   API authentication token
+      --url string     URL of OTF server (default "https://localhost:8080")
 
 Use "otf [command] --help" for more information about a command.
 ```

docs/config/flags.md

@@ -245,6 +245,13 @@ otfd --site-token=643f57a1016cdde7e7e39914785d36d61fd
 
 The default, an empty string, disables the site admin account.
 
+## `--url`
+
+* System: `otf-agent`, `otf`
+* Default: `https://localhost:8080`
+
+Specifies the URL of `otfd` to connect to. You must include the scheme, which is either `https://` or `http://`.
+
 ## `--v`, `-v`
 
 * System: `otfd`, `otf-agent`

internal/agent/daemon_client.go

@@ -28,8 +28,8 @@ type (
 		logs       logsClient
 		server     hostnameClient
 
-		// address of OTF server peer - only populated when daemonClient is using RPC
-		address string
+		// URL of OTF server peer - only populated when daemonClient is using RPC
+		url string
 	}
 
 	// InProcClient is a client for in-process communication with the server.
@@ -93,15 +93,15 @@ func newRPCDaemonClient(cfg otfapi.Config, agentID *string) (*daemonClient, erro
 		configs:    &configversion.Client{Client: apiClient},
 		logs:       &logs.Client{Client: apiClient},
 		server:     apiClient,
-		address:    cfg.Address,
+		url:        cfg.URL,
 	}, nil
 }
 
 // newJobClient constructs a client for communicating with services via RPC on
 // behalf of a job, authenticating as a job using the job token arg.
 func (c *daemonClient) newJobClient(agentID string, token []byte, logger logr.Logger) (*daemonClient, error) {
 	return newRPCDaemonClient(otfapi.Config{
-		Address:       c.address,
+		URL:           c.url,
 		Token:         string(token),
 		RetryRequests: true,
 		RetryLogHook: func(_ retryablehttp.Logger, r *http.Request, n int) {

internal/api/api.go

@@ -11,7 +11,7 @@ import (
 const (
 	DefaultBasePath = "/otfapi"
 	PingEndpoint    = "ping"
-	DefaultAddress  = "localhost:8080"
+	DefaultURL      = "https://localhost:8080"
 )
 
 type Handlers struct{}

internal/api/client.go

@@ -8,6 +8,7 @@ import (
 	"io"
 	"net/http"
 	"net/url"
+	"path"
 	"reflect"
 	"strings"
 	"time"
@@ -28,8 +29,8 @@ type (
 
 	// Config provides configuration details to the API client.
 	Config struct {
-		// The address of the otf API.
-		Address string
+		// The URL of the otf API.
+		URL string
 		// The base path on which the API is served.
 		BasePath string
 		// API token used to access the otf API.
@@ -47,8 +48,8 @@ type (
 
 func NewClient(config Config) (*Client, error) {
 	// set defaults
-	if config.Address == "" {
-		config.Address = DefaultAddress
+	if config.URL == "" {
+		config.URL = DefaultURL
 	}
 	if config.BasePath == "" {
 		config.BasePath = DefaultBasePath
@@ -61,15 +62,11 @@ func NewClient(config Config) (*Client, error) {
 	}
 	config.Headers.Set("User-Agent", "otf-agent")
 
-	addr, err := otfhttp.SanitizeAddress(config.Address)
+	baseURL, err := otfhttp.ParseURL(config.URL)
 	if err != nil {
-		return nil, err
-	}
-	baseURL, err := url.Parse(addr)
-	if err != nil {
-		return nil, fmt.Errorf("invalid address: %v", err)
+		return nil, fmt.Errorf("invalid server url: %v", err)
 	}
-	baseURL.Path = config.BasePath
+	baseURL.Path = path.Join(baseURL.Path, config.BasePath)
 	if !strings.HasSuffix(baseURL.Path, "/") {
 		baseURL.Path += "/"
 	}

internal/cli/cli.go

@@ -48,7 +48,7 @@ func (a *CLI) Run(ctx context.Context, args []string, out io.Writer) error {
 		PersistentPreRunE: a.newClient(&cfg),
 	}
 
-	cmd.PersistentFlags().StringVar(&cfg.Address, "address", api.DefaultAddress, "Address of OTF server")
+	cmd.PersistentFlags().StringVar(&cfg.URL, "url", api.DefaultURL, "URL of OTF server")
 	cmd.PersistentFlags().StringVar(&cfg.Token, "token", "", "API authentication token")
 
 	cmd.SetArgs(args)
@@ -80,7 +80,7 @@ func (a *CLI) newClient(cfg *api.Config) func(*cobra.Command, []string) error {
 
 		if cfg.Token == "" {
 			// not set via flag, so try lower precedence options
-			token, err := a.getToken(cfg.Address)
+			token, err := a.getToken(cfg.URL)
 			if err != nil {
 				return err
 			}

internal/http/http.go

@@ -4,6 +4,7 @@ Package http provides an HTTP interface allowing HTTP clients to interact with o
 package http
 
 import (
+	"errors"
 	"fmt"
 	"net"
 	"net/http"
@@ -59,18 +60,22 @@ func SanitizeHostname(hostname string) (string, error) {
 	return u.Host, nil
 }
 
-// SanitizeAddress ensures address is in format https://<host>:<port>
-func SanitizeAddress(address string) (string, error) {
-	u, err := url.ParseRequestURI(address)
-	if err != nil || u.Host == "" {
-		u, er := url.ParseRequestURI("https://" + address)
-		if er != nil {
-			return "", fmt.Errorf("could not parse hostname: %w", err)
-		}
-		return u.String(), nil
+var ErrParseURL = errors.New("url must begin with http:// or https:// and contain a hostname or ip address")
+
+// ParseURL parses address into a URL. The URL must be absolute with an http(s)
+// scheme.
+func ParseURL(address string) (*url.URL, error) {
+	u, err := url.Parse(address)
+	if err != nil {
+		return nil, fmt.Errorf("%w: %w", ErrParseURL, err)
+	}
+	if u.Scheme != "https" && u.Scheme != "http" {
+		return nil, ErrParseURL
+	}
+	if u.Host == "" {
+		return nil, ErrParseURL
 	}
-	u.Scheme = "https"
-	return u.String(), nil
+	return u, nil
 }
 
 // GetClientIP gets the client's IP address

internal/http/http_test.go

@@ -34,34 +34,48 @@ func TestSanitizeHostname(t *testing.T) {
 	}
 }
 
-func TestSanitizeAddress(t *testing.T) {
+func TestParseURL(t *testing.T) {
 	tests := []struct {
 		name    string
 		address string
-		want    string
+		want    error
 	}{
 		{
-			name:    "add scheme",
-			address: "localhost:8080",
-			want:    "https://localhost:8080",
+			name:    "valid http url",
+			address: "http://localhost:8080",
 		},
 		{
-			name:    "no change",
+			name:    "valid https url",
 			address: "https://localhost:8080",
-			want:    "https://localhost:8080",
 		},
 		{
-			name:    "fix scheme",
-			address: "http://localhost:8080",
-			want:    "https://localhost:8080",
+			name:    "valid https url with path",
+			address: "https://localhost:8080/otf",
+		},
+		{
+			name:    "valid https url using ip address",
+			address: "https://127.0.0.1:8080",
+		},
+		{
+			name:    "invalid address missing scheme",
+			address: "localhost:8080",
+			want:    ErrParseURL,
+		},
+		{
+			name:    "invalid address with non-http(s) scheme",
+			address: "ftp://localhost:8080",
+			want:    ErrParseURL,
+		},
+		{
+			name:    "invalid address without host",
+			address: "http:///otf",
+			want:    ErrParseURL,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := SanitizeAddress(tt.address)
-			if assert.NoError(t, err) {
-				assert.Equal(t, tt.want, got)
-			}
+			_, err := ParseURL(tt.address)
+			assert.ErrorIs(t, err, tt.want)
 		})
 	}
 }

internal/integration/daemon_helpers_test.go

@@ -432,8 +432,8 @@ func (s *testDaemon) startAgent(t *testing.T, ctx context.Context, org, poolID,
 	}
 
 	agentDaemon, err := agent.NewPoolDaemon(logger, cfg, api.Config{
-		Token:   token,
-		Address: s.System.Hostname(),
+		Token: token,
+		URL:   s.System.URL("/"),
 	})
 	require.NoError(t, err)
 
@@ -491,7 +491,7 @@ func (s *testDaemon) otfcli(t *testing.T, ctx context.Context, args ...string) s
 	user := userFromContext(t, ctx)
 	_, token := s.createToken(t, ctx, user)
 
-	cmdargs := []string{"--address", s.System.Hostname(), "--token", string(token)}
+	cmdargs := []string{"--url", s.System.URL("/"), "--token", string(token)}
 	cmdargs = append(cmdargs, args...)
 
 	var buf bytes.Buffer

internal/integration/organization_token_test.go

@@ -25,8 +25,8 @@ func TestIntegration_OrganizationTokens(t *testing.T) {
 	assert.Equal(t, org.Name, ot.Organization)
 
 	apiClient, err := api.NewClient(api.Config{
-		Address: daemon.System.Hostname(),
-		Token:   string(token),
+		URL:   daemon.System.URL("/"),
+		Token: string(token),
 	})
 	require.NoError(t, err)
 

internal/integration/run_api_test.go

@@ -30,7 +30,7 @@ func TestIntegration_RunAPI(t *testing.T) {
 	_, token := daemon.createToken(t, ctx, nil)
 
 	tfeClient, err := tfe.NewClient(&tfe.Config{
-		Address:           "https://" + daemon.System.Hostname(),
+		Address:           daemon.System.URL("/"),
 		Token:             string(token),
 		RetryServerErrors: true,
 	})

internal/integration/workspace_api_test.go

@@ -74,7 +74,7 @@ func TestIntegration_WorkspaceAPI_CreateConnected(t *testing.T) {
 	_, token := daemon.createToken(t, ctx, nil)
 
 	client, err := tfe.NewClient(&tfe.Config{
-		Address: "https://" + daemon.System.Hostname(),
+		Address: daemon.System.URL("/"),
 		Token:   string(token),
 	})
 	require.NoError(t, err)