fix: Explain refused org-wide listing and reject org/project names

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 := "<project>"
+	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),
+	)
+}

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 <project>"},
+		{name: "flag with too many segments", project: "a/b/c", wantHint: "Use --project <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)
+			}
+		})
+	}
+}

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 := "<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 <project> -A
+To find project names, run:
+  datumctl get projects --organization %s`, kind, scope.Organization)
+}

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 <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 <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 <project-id> --all-namespaces") {
+		t.Error("example should show listing across all namespaces in a project")
+	}
+}

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 {

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 <project-id> --namespace default
 
-  # List all resources of a type across all namespaces
-  datumctl get dnszones --organization <org-id> --all-namespaces
+  # List all resources of a type across every namespace in a project
+  datumctl get dnszones --project <project-id> --all-namespaces
 
   # Output as YAML
   datumctl get project my-project-id --organization <org-id> -o yaml

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)
 	})