diff --git a/internal/auth/clouddomains_test.go b/internal/auth/clouddomains_test.go new file mode 100644 index 0000000..f03117f --- /dev/null +++ b/internal/auth/clouddomains_test.go @@ -0,0 +1,130 @@ +package auth + +import ( + "testing" + + "github.com/gophercloud/gophercloud/v2" +) + +// gophercloud v2.15.0 split the user domain from the project domain in +// clouds.Parse and stopped coalescing them. These cases are written against the +// AuthOptions shape Parse returns, not against Parse itself, so they hold +// whichever version is vendored. +func TestReconcileCloudDomains(t *testing.T) { + tests := []struct { + name string + in gophercloud.AuthOptions + // want* are the user domain and the scope's project domain after + // reconciliation. + wantUserDomain string + wantProjectDomain string + }{ + { + name: "only project_domain_name: the user borrows it", + in: gophercloud.AuthOptions{ + Username: "u", TenantName: "proj", + Scope: &gophercloud.AuthScope{ProjectName: "proj", DomainName: "ProjDom"}, + }, + wantUserDomain: "ProjDom", wantProjectDomain: "ProjDom", + }, + { + name: "only user_domain_name: the project scope borrows it", + in: gophercloud.AuthOptions{ + Username: "u", DomainName: "UserDom", TenantName: "proj", + Scope: &gophercloud.AuthScope{ProjectName: "proj"}, + }, + wantUserDomain: "UserDom", wantProjectDomain: "UserDom", + }, + { + name: "both named: neither is touched", + in: gophercloud.AuthOptions{ + Username: "u", DomainName: "UserDom", TenantName: "proj", + Scope: &gophercloud.AuthScope{ProjectName: "proj", DomainName: "ProjDom"}, + }, + wantUserDomain: "UserDom", wantProjectDomain: "ProjDom", + }, + { + name: "project by ID needs no project domain, user still borrows nothing", + in: gophercloud.AuthOptions{ + Username: "u", DomainName: "UserDom", + Scope: &gophercloud.AuthScope{ProjectID: "p-1"}, + }, + wantUserDomain: "UserDom", wantProjectDomain: "", + }, + { + name: "gophercloud < 2.15.0 returns no scope: nothing to do", + in: gophercloud.AuthOptions{ + Username: "u", DomainName: "CloudsDom", TenantName: "proj", + }, + wantUserDomain: "CloudsDom", wantProjectDomain: "", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ao := tc.in + reconcileCloudDomains(&ao) + if ao.DomainName != tc.wantUserDomain { + t.Errorf("user domain = %q, want %q", ao.DomainName, tc.wantUserDomain) + } + var gotProjectDomain string + if ao.Scope != nil { + gotProjectDomain = ao.Scope.DomainName + } + if gotProjectDomain != tc.wantProjectDomain { + t.Errorf("scope domain = %q, want %q", gotProjectDomain, tc.wantProjectDomain) + } + }) + } +} + +// The ID spellings coalesce the same way the name spellings do. +func TestReconcileCloudDomains_IDs(t *testing.T) { + ao := gophercloud.AuthOptions{ + Username: "u", TenantName: "proj", + Scope: &gophercloud.AuthScope{ProjectName: "proj", DomainID: "dom-1"}, + } + reconcileCloudDomains(&ao) + if ao.DomainID != "dom-1" { + t.Errorf("user DomainID = %q, want dom-1", ao.DomainID) + } +} + +// A system- or trust-scoped token is not domain-qualified, so reconciliation +// must leave it exactly as gophercloud built it. +func TestReconcileCloudDomains_SystemAndTrustUntouched(t *testing.T) { + for _, tc := range []struct { + name string + scope *gophercloud.AuthScope + }{ + {"system", &gophercloud.AuthScope{System: true}}, + {"trust", &gophercloud.AuthScope{TrustID: "t-1"}}, + } { + t.Run(tc.name, func(t *testing.T) { + ao := gophercloud.AuthOptions{Username: "u", Scope: tc.scope} + reconcileCloudDomains(&ao) + if ao.DomainName != "" || ao.DomainID != "" { + t.Errorf("%s scope must not gain a user domain, got %q/%q", tc.name, ao.DomainID, ao.DomainName) + } + }) + } +} + +// An explicit koc flag still outranks the reconciled clouds.yaml value: the +// flag path runs after this one and overwrites both halves. +func TestReconcileCloudDomains_FlagsStillWin(t *testing.T) { + ao := gophercloud.AuthOptions{ + Username: "u", TenantName: "proj", + Scope: &gophercloud.AuthScope{ProjectName: "proj", DomainName: "ProjDom"}, + } + reconcileCloudDomains(&ao) + + o := &Options{UserDomainName: "FlagDom", ProjectName: "proj"} + o.applyAuthOverrides(&ao) + + if ao.DomainName != "FlagDom" { + t.Errorf("user domain = %q, want the flag to win with FlagDom", ao.DomainName) + } + if ao.Scope == nil || ao.Scope.DomainName != "FlagDom" { + t.Errorf("scope = %#v, want the flag's domain", ao.Scope) + } +} diff --git a/internal/auth/provider.go b/internal/auth/provider.go index c7dbc22..f5154ac 100644 --- a/internal/auth/provider.go +++ b/internal/auth/provider.go @@ -118,6 +118,7 @@ func (o *Options) resolveAuth() (gophercloud.AuthOptions, gophercloud.EndpointOp if err != nil { return ao, eo, nil, fmt.Errorf("loading cloud %q from clouds.yaml: %w", o.Cloud, err) } + reconcileCloudDomains(&ao) } else { // Build the auth options from OS_* / flags directly rather than via // gophercloud's AuthOptionsFromEnv, which only understands OS_DOMAIN_NAME @@ -139,6 +140,45 @@ func (o *Options) resolveAuth() (gophercloud.AuthOptions, gophercloud.EndpointOp return ao, eo, baseTLS, nil } +// reconcileCloudDomains restores the domain coalescing clouds.Parse used to do +// for itself, for a clouds.yaml that names only one of the two domains. +// +// Keystone qualifies two different things by domain: the *user* being +// authenticated, and the *project* being scoped to. gophercloud v2.14.0 and +// earlier folded them together — AuthOptions.DomainName fell back through +// user_domain_name, project_domain_name, domain_name, and Scope was left nil +// for gophercloud to derive from TenantName and that one domain. v2.15.0 split +// them: DomainName no longer falls back to project_domain_name, and Parse +// returns an explicit Scope carrying the project's domain. A clouds.yaml that +// sets only one of the pair therefore loses the other half: +// +// - only project_domain_name → the user is unqualified, and Keystone rejects +// the password grant as ambiguous; +// - only user_domain_name → the project-by-name scope is unqualified, and +// Keystone cannot tell which project of that name is meant. +// +// Single-domain clouds — the common case, and what the fleet runs — write only +// one of the two and mean it for both. So each half falls back to the other, +// which is what v2.14.0 did and what upstream OSC does. An explicit --os-*-domain +// flag or OS_*_DOMAIN_* variable still wins: applyDomainScope runs after this and +// overwrites both. +// +// Inert against gophercloud v2.14.0, which returns a nil Scope here. +func reconcileCloudDomains(ao *gophercloud.AuthOptions) { + // A system- or trust-scoped token is not domain-qualified at all, and a + // project-by-ID scope needs no domain, so there is nothing to reconcile. + if ao.Scope == nil || ao.Scope.System || ao.Scope.TrustID != "" { + return + } + if ao.DomainID == "" && ao.DomainName == "" { + ao.DomainID, ao.DomainName = ao.Scope.DomainID, ao.Scope.DomainName + } + if ao.Scope.ProjectName != "" && ao.Scope.ProjectID == "" && + ao.Scope.DomainID == "" && ao.Scope.DomainName == "" { + ao.Scope.DomainID, ao.Scope.DomainName = ao.DomainID, ao.DomainName + } +} + // applyAuthOverrides layers explicitly-set auth flags over whatever the // clouds.yaml / env path produced. Every value goes through Options.override so // an env-derived default cannot outrank an explicitly named cloud. diff --git a/internal/cli/server/server_show.go b/internal/cli/server/server_show.go index b3c336b..6251249 100644 --- a/internal/cli/server/server_show.go +++ b/internal/cli/server/server_show.go @@ -95,6 +95,14 @@ func powerStateLabel(v any) any { n = int(t) case int: n = t + case json.Number: + // gophercloud v2.15.0 decodes numbers into any with UseNumber(), so + // nova's power_state arrives as a json.Number rather than a float64. + i, err := t.Int64() + if err != nil { + return v + } + n = int(i) default: return v } @@ -237,6 +245,10 @@ func scalarString(v any) string { return strconv.FormatBool(t) case float64: return strconv.FormatFloat(t, 'f', -1, 64) + case json.Number: + // Already the literal the server sent (gophercloud v2.15.0 decodes with + // UseNumber()), so it needs no reformatting to avoid a trailing ".0". + return t.String() default: b, _ := json.Marshal(t) return string(b) diff --git a/internal/cli/server/server_show_jsonnumber_test.go b/internal/cli/server/server_show_jsonnumber_test.go new file mode 100644 index 0000000..d81b256 --- /dev/null +++ b/internal/cli/server/server_show_jsonnumber_test.go @@ -0,0 +1,79 @@ +package server + +import ( + "bytes" + "encoding/json" + "net/http" + "strings" + "testing" + + th "github.com/gophercloud/gophercloud/v2/testhelper" + + "github.com/ftarasenko/go-openstackclient/internal/output" +) + +// gophercloud v2.15.0 decodes JSON numbers into `any` with UseNumber(), so every +// number that reaches koc through a map[string]any is a json.Number rather than +// a float64. These tests pin the handling directly instead of through the SDK, +// so they hold whichever decoder the vendored version happens to use. + +func TestPowerStateLabel_NumberKinds(t *testing.T) { + for _, tc := range []struct { + name string + in any + want any + }{ + {"json.Number (gophercloud >= 2.15.0)", json.Number("1"), "Running"}, + {"float64 (gophercloud < 2.15.0)", float64(1), "Running"}, + {"int", 4, "Shutdown"}, + {"json.Number, shutdown", json.Number("4"), "Shutdown"}, + {"json.Number, unmapped code passes through", json.Number("9"), json.Number("9")}, + {"json.Number, not an integer passes through", json.Number("1.5"), json.Number("1.5")}, + {"unrelated type passes through", "Running", "Running"}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := powerStateLabel(tc.in); got != tc.want { + t.Errorf("powerStateLabel(%#v) = %#v, want %#v", tc.in, got, tc.want) + } + }) + } +} + +func TestScalarString_JSONNumber(t *testing.T) { + // A json.Number is already the literal the server sent, so it renders + // without the trailing ".0" a float64 round-trip would risk. + for _, tc := range []struct{ in, want string }{ + {"1", "1"}, {"0", "0"}, {"2048", "2048"}, {"1.5", "1.5"}, + } { + if got := scalarString(json.Number(tc.in)); got != tc.want { + t.Errorf("scalarString(json.Number(%q)) = %q, want %q", tc.in, got, tc.want) + } + } + // ... and matches what the float64 path produces for the same value. + if got, want := scalarString(json.Number("1")), scalarString(float64(1)); got != want { + t.Errorf("json.Number and float64 disagree: %q vs %q", got, want) + } +} + +// TestRunServerShow_PowerStateFromDecodedBody is the end-to-end guard: the body +// is decoded the way the SDK decodes it, so this fails if a future vendor bump +// changes the number kind again and the humanizer is not taught about it. +func TestRunServerShow_PowerStateFromDecodedBody(t *testing.T) { + fakeServer := th.SetupHTTP() + defer fakeServer.Teardown() + + fakeServer.Mux.HandleFunc("/servers/"+serverUUID, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"server":{"id":"` + serverUUID + + `","name":"web-1","status":"ACTIVE","OS-EXT-STS:power_state":1}}`)) + }) + + var buf bytes.Buffer + o := &output.Options{Format: output.FormatTable} + if err := runServerShow(t.Context(), computeClient(fakeServer, "2.93"), o, serverUUID, false, &buf); err != nil { + t.Fatalf("runServerShow: %v", err) + } + if !strings.Contains(buf.String(), "Running") { + t.Errorf("power_state was not humanized; table:\n%s", buf.String()) + } +} diff --git a/internal/output/output.go b/internal/output/output.go index 18b544c..96beef7 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -346,6 +346,13 @@ func numericValue(v any) (float64, bool) { return float64(n), true case float64: return n, true + case json.Number: + // gophercloud decodes JSON numbers into any with UseNumber() (v2.15.0), + // so a cell that used to arrive as float64 is now a json.Number. Without + // this case it would fall through to the string branch's named-type + // mismatch and sort as text. + f, err := n.Float64() + return f, err == nil case string: f, err := strconv.ParseFloat(strings.TrimSpace(n), 64) return f, err == nil && strings.TrimSpace(n) != "" diff --git a/internal/output/output_test.go b/internal/output/output_test.go index e4dd3c8..85dd2ae 100644 --- a/internal/output/output_test.go +++ b/internal/output/output_test.go @@ -519,6 +519,15 @@ func TestCompareCells_NumericAndString(t *testing.T) { {"number vs word", 10, "ACTIVE", -1}, {"nil sorts first", nil, "a", -1}, {"empty string is not a number", "", "1", -1}, + // gophercloud v2.15.0 decodes JSON numbers into any with UseNumber(), so + // a cell that used to be a float64 is now a json.Number. It is a named + // string type, so without its own case it would sort as text and put + // "10" before "9". + {"json.Number", json.Number("9"), json.Number("10"), -1}, + {"json.Number descending", json.Number("10"), json.Number("9"), 1}, + {"json.Number against float64", json.Number("9"), 10.0, -1}, + {"json.Number against int", json.Number("10"), 9, 1}, + {"equal json.Numbers", json.Number("5"), json.Number("5"), 0}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) {