From 9eb36b6f98efc3103af6a737092028380d81aad7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 15:17:50 +0000 Subject: [PATCH 1/2] fix(output): render and sort json.Number the way float64 was rendered gophercloud v2.15.0 decodes JSON numbers into `any` with UseNumber(), so every number koc reads out of a map[string]any is a json.Number rather than a float64. Three type switches only knew the float64 spelling: - powerStateLabel fell through to its default and printed nova's raw OS-EXT-STS:power_state, so `server show` showed "1" where it used to show "Running"; - numericValue returned not-a-number, so a column carrying one sorted as text and put "10" before "9" (json.Number is a named string type, so the existing string case does not match it either); - scalarString fell through to json.Marshal, which happens to be right for a well-formed number but relies on the default branch by accident. Teach all three the json.Number spelling. The change is inert against the currently vendored v2.14.0 -- nothing produces a json.Number there -- and is what keeps `server show` correct once the bump lands, so it goes in ahead of it rather than with it. Tests pin the handling directly rather than through the SDK, so they hold whichever decoder the vendored version uses, plus one end-to-end guard that decodes a real response body and fails if a future bump changes the number kind again. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EkWzWSEyrjJ1gd1v5zJNtE --- internal/cli/server/server_show.go | 12 +++ .../cli/server/server_show_jsonnumber_test.go | 79 +++++++++++++++++++ internal/output/output.go | 7 ++ internal/output/output_test.go | 9 +++ 4 files changed, 107 insertions(+) create mode 100644 internal/cli/server/server_show_jsonnumber_test.go 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) { From b27bdce9658d0569c2d90da1a0e15743033d1218 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 15:21:57 +0000 Subject: [PATCH 2/2] fix(auth): keep clouds.yaml usable when it names only one domain 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 in clouds.Parse -- 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 splits 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 names only one of the pair loses the other half. Driven against the real v2.15.0 clouds.Parse, a file with project_domain_name and no user_domain_name yields an empty user domain, which Keystone rejects as an ambiguous password grant; the mirror case leaves the project-by-name scope unqualified. Single-domain clouds write one of the two and mean it for both, so each half now falls back to the other -- what v2.14.0 did and what upstream OSC does. koc's own flags are unaffected either way: applyDomainScope runs after this and overwrites both halves, and it is only reached when a domain flag or OS_*_DOMAIN_* variable is actually set, which is why the clouds.yaml path was exposed at all. A system- or trust-scoped token is not domain-qualified and is left exactly as gophercloud built it. Inert against the vendored v2.14.0, which returns a nil Scope here, so it lands ahead of the bump rather than with it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EkWzWSEyrjJ1gd1v5zJNtE --- internal/auth/clouddomains_test.go | 130 +++++++++++++++++++++++++++++ internal/auth/provider.go | 40 +++++++++ 2 files changed, 170 insertions(+) create mode 100644 internal/auth/clouddomains_test.go 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.