diff --git a/.changelog/14635.txt b/.changelog/14635.txt new file mode 100644 index 000000000..8830ed3e3 --- /dev/null +++ b/.changelog/14635.txt @@ -0,0 +1,3 @@ +```release-note:bug +cli: fixed a bug in the `operator api` command where the HTTPS scheme was not always correctly calculated +``` diff --git a/command/operator_api.go b/command/operator_api.go index 81f818362..9ff7b9c9a 100644 --- a/command/operator_api.go +++ b/command/operator_api.go @@ -386,22 +386,41 @@ func tlsToCurl(parts []string, tlsConfig *api.TLSConfig) []string { return parts } -// pathToURL converts a curl path argumet to URL. Paths without a host are +// pathToURL converts a curl path argument to URL. Paths without a host are // prefixed with $NOMAD_ADDR or http://127.0.0.1:4646. +// +// Callers should pass a config generated by Meta.clientConfig which ensures +// all default values are set correctly. Failure to do so will likely result in +// a nil-pointer. func pathToURL(config *api.Config, path string) (*url.URL, error) { - // If the scheme is missing, add it + + // If the scheme is missing from the path, it likely means the path is just + // the HTTP handler path. Attempt to infer this. if !strings.HasPrefix(path, "http://") && !strings.HasPrefix(path, "https://") { scheme := "http" - if config.TLSConfig != nil { - if config.TLSConfig.CACert != "" || - config.TLSConfig.CAPath != "" || - config.TLSConfig.ClientCert != "" || - config.TLSConfig.TLSServerName != "" || - config.TLSConfig.Insecure { - // TLS configured, but scheme not set. Assume - // https. - scheme = "https" + // If the user has set any TLS configuration value, this is a good sign + // Nomad is running with TLS enabled. Otherwise, use the address within + // the config to identify a scheme. + if config.TLSConfig.CACert != "" || + config.TLSConfig.CAPath != "" || + config.TLSConfig.ClientCert != "" || + config.TLSConfig.TLSServerName != "" || + config.TLSConfig.Insecure { + + // TLS configured, but scheme not set. Assume https. + scheme = "https" + } else if config.Address != "" { + + confURL, err := url.Parse(config.Address) + if err != nil { + return nil, fmt.Errorf("unable to parse configured address: %v", err) + } + + // Ensure we only overwrite the set scheme value if the parsing + // identified a valid scheme. + if confURL.Scheme == "http" || confURL.Scheme == "https" { + scheme = confURL.Scheme } } @@ -413,7 +432,7 @@ func pathToURL(config *api.Config, path string) (*url.URL, error) { return nil, err } - // If URL.Scheme is empty, use defaults from client config + // If URL.Host is empty, use defaults from client config. if u.Host == "" { confURL, err := url.Parse(config.Address) if err != nil { diff --git a/command/operator_api_test.go b/command/operator_api_test.go index 813534a8d..4292cd586 100644 --- a/command/operator_api_test.go +++ b/command/operator_api_test.go @@ -8,8 +8,10 @@ import ( "testing" "time" + "github.com/hashicorp/nomad/api" "github.com/hashicorp/nomad/ci" "github.com/mitchellh/cli" + "github.com/shoenig/test/must" "github.com/stretchr/testify/require" ) @@ -106,3 +108,65 @@ func TestOperatorAPICommand_Curl(t *testing.T) { ` require.Equal(t, expected, buf.String()) } + +func Test_pathToURL(t *testing.T) { + ci.Parallel(t) + + testCases := []struct { + name string + inputConfig *api.Config + inputPath string + expectedOutputURL string + }{ + { + name: "https address via config", + inputConfig: &api.Config{ + Address: "https://nomad.systems:4646", + TLSConfig: &api.TLSConfig{}, + }, + inputPath: "/v1/jobs", + expectedOutputURL: "https://nomad.systems:4646/v1/jobs", + }, + { + name: "http address via config", + inputConfig: &api.Config{ + Address: "http://nomad.systems:4646", + TLSConfig: &api.TLSConfig{}, + }, + inputPath: "/v1/jobs", + expectedOutputURL: "http://nomad.systems:4646/v1/jobs", + }, + { + name: "https address via path", + inputConfig: api.DefaultConfig(), + inputPath: "https://nomad.systems:4646/v1/jobs", + expectedOutputURL: "https://nomad.systems:4646/v1/jobs", + }, + { + name: "http address via path", + inputConfig: api.DefaultConfig(), + inputPath: "http://nomad.systems:4646/v1/jobs", + expectedOutputURL: "http://nomad.systems:4646/v1/jobs", + }, + { + name: "https inferred by tls config", + inputConfig: &api.Config{ + Address: "http://127.0.0.1:4646", + TLSConfig: &api.TLSConfig{ + CAPath: "/path/to/nowhere", + }, + }, + inputPath: "/v1/jobs", + expectedOutputURL: "https://127.0.0.1:4646/v1/jobs", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actualOutput, err := pathToURL(tc.inputConfig, tc.inputPath) + must.NoError(t, err) + must.NotNil(t, actualOutput) + must.Eq(t, actualOutput.String(), tc.expectedOutputURL) + }) + } +}