cli: fix a bug in operator API when setting HTTPS via address. (#14635)

Operators may have a setup whereby the TLS config comes from a
source other than setting Nomad specific env vars. In this case,
we should attempt to identify the scheme using the config setting
as a fallback.
This commit is contained in:
James Rasell 2022-09-22 15:43:58 +02:00 committed by GitHub
parent 2088ca3345
commit a25028c412
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 98 additions and 12 deletions

3
.changelog/14635.txt Normal file
View File

@ -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
```

View File

@ -386,22 +386,41 @@ func tlsToCurl(parts []string, tlsConfig *api.TLSConfig) []string {
return parts 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. // 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) { 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://") { if !strings.HasPrefix(path, "http://") && !strings.HasPrefix(path, "https://") {
scheme := "http" 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 // If the user has set any TLS configuration value, this is a good sign
// https. // Nomad is running with TLS enabled. Otherwise, use the address within
scheme = "https" // 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 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 == "" { if u.Host == "" {
confURL, err := url.Parse(config.Address) confURL, err := url.Parse(config.Address)
if err != nil { if err != nil {

View File

@ -8,8 +8,10 @@ import (
"testing" "testing"
"time" "time"
"github.com/hashicorp/nomad/api"
"github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/ci"
"github.com/mitchellh/cli" "github.com/mitchellh/cli"
"github.com/shoenig/test/must"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
@ -106,3 +108,65 @@ func TestOperatorAPICommand_Curl(t *testing.T) {
` `
require.Equal(t, expected, buf.String()) 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)
})
}
}