From 721994ee90bb026c4c4116ab8abf0e498df853ae Mon Sep 17 00:00:00 2001 From: Michael Costello Date: Fri, 6 Nov 2026 13:53:57 -0500 Subject: [PATCH 1/2] fix: Explain refused org-wide get across namespaces The authorizer refuses to list a namespaced resource across all namespaces at organization scope, for every role: an organization's resources live in one namespace, and project resources live in each project's own control plane. The get help still recommended that form, and the refusal printed a bare Forbidden with no next step. The hint is attached only when the server actually returns Forbidden, so datumctl does not duplicate the server's rule and will not block the form if the server ever allows it. Key changes: - Show listing across every namespace in a project in the get example - After a Forbidden for get -A at organization scope, print a hint naming the project form and how to find project names - Carry the hint in the JSON and YAML error envelopes Co-Authored-By: Claude Opus 5.5 --- internal/cmd/get_org_scope.go | 49 +++++++++++ internal/cmd/get_org_scope_test.go | 136 +++++++++++++++++++++++++++++ internal/cmd/get_organizations.go | 2 + internal/cmd/root.go | 4 +- main.go | 6 +- 5 files changed, 194 insertions(+), 3 deletions(-) create mode 100644 internal/cmd/get_org_scope.go create mode 100644 internal/cmd/get_org_scope_test.go diff --git a/internal/cmd/get_org_scope.go b/internal/cmd/get_org_scope.go new file mode 100644 index 0000000..47b483b --- /dev/null +++ b/internal/cmd/get_org_scope.go @@ -0,0 +1,49 @@ +package cmd + +import ( + "fmt" + "strings" + + "github.com/spf13/cobra" + + "go.datum.net/datumctl/internal/client" +) + +// getFatalHint is the hint for a Forbidden that the running get command +// expects. kubectl get reports its errors through cmdutil.CheckErr, which only +// passes the formatted message on, so the hint is recorded before the command +// runs and attached by FatalHint when the error is printed. +var getFatalHint string + +// FatalHint returns guidance to print after a fatal kubectl error message, or +// "" when there is none. +func FatalHint(msg string) string { + if getFatalHint == "" || !strings.Contains(msg, "(Forbidden)") { + return "" + } + return getFatalHint +} + +// orgScopeAllNamespacesHint returns the hint for a get across all namespaces +// at organization scope. The authorizer refuses a namespaced list with no +// namespace in an organization's control plane, whatever the caller's role, +// because an organization's resources live in a single namespace and project +// resources live in each project's own control plane. +func orgScopeAllNamespacesHint(c *cobra.Command, factory *client.DatumCloudFactory, args []string) string { + allNamespaces, err := c.Flags().GetBool("all-namespaces") + if err != nil || !allNamespaces { + return "" + } + scope, err := factory.ConfigFlags.ResolvedScopeDetails() + if err != nil || scope.Organization == "" { + return "" + } + kind := "" + if len(args) > 0 { + kind, _, _ = strings.Cut(args[0], "/") + } + return fmt.Sprintf(`Listing across all namespaces is not allowed at organization scope. List a project's resources instead: + datumctl get %s --project -A +To find project names, run: + datumctl get projects --organization %s`, kind, scope.Organization) +} diff --git a/internal/cmd/get_org_scope_test.go b/internal/cmd/get_org_scope_test.go new file mode 100644 index 0000000..4edb0bd --- /dev/null +++ b/internal/cmd/get_org_scope_test.go @@ -0,0 +1,136 @@ +package cmd + +import ( + "bytes" + "context" + "strings" + "testing" + + "github.com/spf13/cobra" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/cli-runtime/pkg/genericclioptions" + cmdutil "k8s.io/kubectl/pkg/cmd/util" + + "go.datum.net/datumctl/internal/client" +) + +// newFakeGet stands in for kubectl get: the flags the wrapper reads, and a Run +// that fails the way kubectl does when the server refuses the list. +func newFakeGet(t *testing.T, runErr error) *cobra.Command { + t.Helper() + factory, err := client.NewDatumFactory(context.Background()) + if err != nil { + t.Fatal(err) + } + cmd := &cobra.Command{ + Use: "get", + Run: func(cmd *cobra.Command, args []string) { cmdutil.CheckErr(runErr) }, + } + cmd.Flags().BoolP("all-namespaces", "A", false, "") + cmd.Flags().StringP("output", "o", "", "") + factory.AddFlags(cmd.Flags()) + return WrapGetCommand(cmd, factory, genericclioptions.IOStreams{Out: &bytes.Buffer{}, ErrOut: &bytes.Buffer{}}) +} + +func captureFatal(t *testing.T) *string { + t.Helper() + var got string + cmdutil.BehaviorOnFatal(func(msg string, code int) { got = msg }) + t.Cleanup(cmdutil.DefaultBehaviorOnFatal) + t.Cleanup(func() { getFatalHint = "" }) + return &got +} + +func TestGetForbiddenAllNamespacesHint(t *testing.T) { + forbidden := apierrors.NewForbidden( + schema.GroupResource{Group: "dns.networking.miloapis.com", Resource: "dnszones"}, "", + errFake("cross-namespace queries not allowed for organization-scoped requests"), + ) + notFound := apierrors.NewNotFound(schema.GroupResource{Resource: "dnszones"}, "zone-a") + + tests := []struct { + name string + args []string + runErr error + wantHint []string + }{ + { + name: "organization scope from the context", + args: []string{"dnszones", "-A"}, + runErr: forbidden, + wantHint: []string{"datumctl get dnszones --project -A", "datumctl get projects --organization org-prod"}, + }, + { + name: "organization flag", + args: []string{"dnszones", "--all-namespaces", "--organization", "acme"}, + runErr: forbidden, + wantHint: []string{"datumctl get dnszones --project -A", "datumctl get projects --organization acme"}, + }, + { + name: "project scope gets no hint", + args: []string{"dnszones", "-A", "--project", "web"}, + runErr: forbidden, + }, + { + name: "organization scope without -A gets no hint", + args: []string{"dnszones", "--organization", "acme"}, + runErr: forbidden, + }, + { + name: "an error other than Forbidden gets no hint", + args: []string{"dnszones", "-A", "--organization", "acme"}, + runErr: notFound, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + setupOverrideEnv(t) + fatal := captureFatal(t) + + cmd := newFakeGet(t, tt.runErr) + cmd.SetArgs(tt.args) + if err := cmd.Execute(); err != nil { + t.Fatalf("execute: %v", err) + } + if *fatal == "" { + t.Fatal("the wrapped get command never reported its error") + } + + hint := FatalHint(*fatal) + if len(tt.wantHint) == 0 { + if hint != "" { + t.Errorf("expected no hint, got %q", hint) + } + return + } + for _, want := range tt.wantHint { + if !strings.Contains(hint, want) { + t.Errorf("hint %q does not contain %q", hint, want) + } + } + }) + } +} + +type errFake string + +func (e errFake) Error() string { return string(e) } + +func TestGetExampleListsAllNamespacesInAProject(t *testing.T) { + setupOverrideEnv(t) + get, _, err := RootCmd().Find([]string{"get"}) + if err != nil { + t.Fatal(err) + } + for _, line := range strings.Split(get.Example, "\n") { + if strings.Contains(line, "--all-namespaces") || strings.Contains(line, " -A") { + if strings.Contains(line, "--organization") { + t.Errorf("example lists across all namespaces at organization scope, which the server refuses: %q", line) + } + } + } + if !strings.Contains(get.Example, "--project --all-namespaces") { + t.Error("example should show listing across all namespaces in a project") + } +} diff --git a/internal/cmd/get_organizations.go b/internal/cmd/get_organizations.go index bd77adb..75896b1 100644 --- a/internal/cmd/get_organizations.go +++ b/internal/cmd/get_organizations.go @@ -43,6 +43,8 @@ func WrapGetCommand(cmd *cobra.Command, factory *datumclient.DatumCloudFactory, factory.ConfigFlags.ForceUserControlPlane = true defer func() { factory.ConfigFlags.ForceUserControlPlane = prev }() _ = c.Flags().Set("all-namespaces", "true") + } else { + getFatalHint = orgScopeAllNamespacesHint(c, factory, args) } if origRunE != nil { diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 50ab137..008dea9 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -381,8 +381,8 @@ The 'organizations' shorthand also works with 'datumctl delete', 'datumctl edit' # List DNS zones in a project namespace datumctl get dnszones --project --namespace default - # List all resources of a type across all namespaces - datumctl get dnszones --organization --all-namespaces + # List all resources of a type across every namespace in a project + datumctl get dnszones --project --all-namespaces # Output as YAML datumctl get project my-project-id --organization -o yaml diff --git a/main.go b/main.go index e5590e5..802f56c 100644 --- a/main.go +++ b/main.go @@ -42,13 +42,17 @@ func main() { // re-encode the message inside the JSON/YAML envelope. util.BehaviorOnFatal(func(msg string, code int) { msg = strings.TrimSuffix(msg, "\n") + hint := cmd.FatalHint(msg) format := formatFor(rootCmd) if format == customerrors.FormatHuman { fmt.Fprintln(os.Stderr, msg) + if hint != "" { + fmt.Fprintln(os.Stderr, hint) + } os.Exit(code) } clean := strings.TrimPrefix(msg, "error: ") - customerrors.Format(os.Stderr, errors.New(clean), format, verbosity()) + customerrors.Format(os.Stderr, customerrors.NewUserErrorWithHint(clean, hint), format, verbosity()) os.Exit(code) }) From 879bd5916cb3dddf16ffc378c500d1c0d71fc0d2 Mon Sep 17 00:00:00 2001 From: Michael Costello Date: Fri, 6 Nov 2026 13:53:57 -0500 Subject: [PATCH 2/2] fix: Refuse org/project in --project 'datumctl ctx list' prints project contexts as org/project, and that form passed to --project or DATUM_PROJECT went straight into the control plane URL, failing with discovery errors that did not name the cause. Project names cannot contain "/", so the value is now refused up front with a hint that names the bare project. Refusing is preferred over splitting the value: the project control plane URL carries no organization, so a split would silently ignore a wrong organization. Co-Authored-By: Claude Opus 5.5 --- internal/client/factory.go | 27 ++++++++++++++++++++++++++ internal/client/scope_test.go | 36 +++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/internal/client/factory.go b/internal/client/factory.go index 45e16d9..ad88d55 100644 --- a/internal/client/factory.go +++ b/internal/client/factory.go @@ -7,6 +7,7 @@ import ( "net/http" "os" "path/filepath" + "strings" "time" "github.com/spf13/pflag" @@ -414,6 +415,12 @@ func (c *CustomConfigFlags) resolveScope(ctxEntry *datumconfig.DiscoveredContext return Scope{}, fmt.Errorf("exactly one of --project or --organization must be provided") } + if source == ScopeSourceFlag || source == ScopeSourceEnv { + if err := validateProjectName(projectID, source); err != nil { + return Scope{}, err + } + } + return Scope{ Project: projectID, Organization: organizationID, @@ -515,3 +522,23 @@ func NewDatumFactory(ctx context.Context) (*DatumCloudFactory, error) { ConfigFlags: configFlags, }, nil } + +// validateProjectName rejects a project given in the "org/project" form that +// 'datumctl ctx list' prints. Project names never contain "/". +func validateProjectName(project string, source ScopeSource) error { + if !strings.Contains(project, "/") { + return nil + } + name := "" + if parts := strings.Split(project, "/"); len(parts) == 2 && parts[1] != "" { + name = parts[1] + } + setting, use := "--project", "--project "+name + if source == ScopeSourceEnv { + setting, use = "DATUM_PROJECT", "DATUM_PROJECT="+name + } + return customerrors.NewUserErrorWithHint( + fmt.Sprintf("%s takes a project name, not org/project: %q", setting, project), + fmt.Sprintf("Use %s", use), + ) +} diff --git a/internal/client/scope_test.go b/internal/client/scope_test.go index fd0edc7..f96c015 100644 --- a/internal/client/scope_test.go +++ b/internal/client/scope_test.go @@ -1,9 +1,11 @@ package client import ( + "strings" "testing" "go.datum.net/datumctl/internal/datumconfig" + customerrors "go.datum.net/datumctl/internal/errors" ) func TestResolvedScopeDetails(t *testing.T) { @@ -93,3 +95,37 @@ func TestResolvedScopeDetails(t *testing.T) { }) } } + +func TestResolvedScopeRejectsOrgProjectForm(t *testing.T) { + tests := []struct { + name string + project string + envProject string + wantHint string + }{ + {name: "flag in the org/project form", project: "acme/web", wantHint: "Use --project web"}, + {name: "flag with an empty project segment", project: "acme/", wantHint: "Use --project "}, + {name: "flag with too many segments", project: "a/b/c", wantHint: "Use --project "}, + {name: "environment in the org/project form", envProject: "acme/web", wantHint: "Use DATUM_PROJECT=web"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := setupFactoryOverrideEnv(t) + t.Setenv("DATUM_PROJECT", tt.envProject) + t.Setenv("DATUM_ORGANIZATION", "") + *f.ConfigFlags.Project = tt.project + + _, err := f.ConfigFlags.ResolvedScopeDetails() + userErr, ok := customerrors.IsUserError(err) + if !ok { + t.Fatalf("err = %v, want a UserError", err) + } + if userErr.Hint != tt.wantHint { + t.Errorf("hint = %q, want %q", userErr.Hint, tt.wantHint) + } + if !strings.Contains(userErr.Message, "takes a project name, not org/project") { + t.Errorf("message = %q", userErr.Message) + } + }) + } +}