Merge pull request #1680 from entireio/move-json-flag-to-specific-commands · Entire

Merge pull request #1680 from entireio/move-json-flag-to-specific-commands

d1683dd→main·

gtrrz-victor·1w ago·11 files·+158 added/-17 removed

Move control-plane --json off the shared persistent flag

Changes

11

20 unmodified lines

// addControlPlaneFlags registers the persistent flags shared by every
// control-plane command group. Persistent so they're inherited by nested
// subcommands (e.g. `entire repo mirror list`):
//   - --json: emit the raw wire JSON instead of the default human table.
//   - --insecure-http-auth: permit the token exchange over plain http://
//     (local/dev deployments where the core isn't behind TLS). Hidden, as
//     elsewhere in the CLI.
//     elsewhere in the CLI. Applies to every subcommand because they all build
//     a control-plane client.
//
// --json is deliberately NOT persistent here: it only makes sense on the read
// and mutation verbs that render a wire payload, so it's registered per-command
// with addJSONFlag. A persistent --json was inherited by side-effect verbs
// (delete, clone, mirror create/remove, grant remove) that silently ignored it;
// cobra can't hide a persistent flag from a subset of children, so the flag
// lives on exactly the commands that honor it.
func addControlPlaneFlags(cmd *cobra.Command) {
    cmd.PersistentFlags().Bool("json", false, "Output raw JSON instead of a table")
    cmd.PersistentFlags().Bool("insecure-http-auth", false, "Allow authentication over plain HTTP (insecure, for local development only)")
    if err := cmd.PersistentFlags().MarkHidden("insecure-http-auth"); err != nil {
        panic(fmt.Sprintf("hide insecure-http-auth flag: %v", err))
    }
}

// addJSONFlag registers the local --json flag on a command that renders a wire
// payload (list/get/create/mutation verbs routed through the runCore* helpers).
// Local, not persistent, so only these commands advertise and accept it — see
// addControlPlaneFlags for why. Read it with jsonRequested.
func addJSONFlag(cmd *cobra.Command) {
    cmd.Flags().Bool("json", false, "Output raw JSON instead of a table")
}

// jsonRequested reports whether --json was set on cmd or an ancestor. A
// lookup error means the flag isn't defined on this command tree, which is
// treated as "not requested".

Mcmd/entire/cli/corecmd.go+17/-3

package cli

import (
    "sort"
    "testing"

"github.com/spf13/cobra"
    "github.com/stretchr/testify/require"
)

// TestControlPlaneJSONFlag_OnlyOnHonoringCommands pins the structural fix that
// moved --json off the shared control-plane persistent flag and onto a local
// flag registered only where the command actually renders JSON.
//
// The old design registered --json persistently on each group root, so it was
// inherited by every subcommand — including side-effect verbs (delete, clone,
// mirror create/remove, grant remove) that ignored it, silently accepting a
// no-op flag. Now the flag exists exactly on the commands that honor it, so the
// non-honoring commands reject --json with "unknown flag" and their help never
// advertises it.
func TestControlPlaneJSONFlag_OnlyOnHonoringCommands(t *testing.T) {
    t.Parallel()

// path (relative to the group root) -> honors --json.
    want := map[string]bool{
        // org
        "org create": true,
        "org list":   true,
        "org get":    true,
        "org delete": false,
        // project
        "project create": true,
        "project list":   true,
        "project get":    true,
        "project delete": false,
        // repo
        "repo create":                    true,
        "repo list":                      true,
        "repo get":                       true,
        "repo delete":                    false,
        "repo clone":                     false,
        "repo mirror create":             false,
        "repo mirror list":               true,
        "repo mirror get":                true,
        "repo mirror remove":             false,
        "repo mirror collaborators list": true,
        "repo visibility get":            true,
        "repo visibility set":            true,
        // grant
        "grant org add":        true,
        "grant org list":       true,
        "grant org remove":     false,
        "grant project add":    true,
        "grant project list":   true,
        "grant project remove": false,
        "grant repo add":       true,
        "grant repo list":      true,
        "grant repo remove":    false,
    }

got := map[string]bool{}
    for _, root := range []*cobra.Command{newOrgCmd(), newProjectCmd(), newRepoCmd(), newGrantCmd()} {
        collectJSONFlag(t, root, root.Name(), got)
    }

// Every command we expect an answer for must exist in the tree, and vice
    // versa — a drift in either direction (renamed/removed command, or a new
    // leaf we forgot to classify) should fail loudly.
    require.Equal(t, sortedKeys(want), sortedKeys(got), "command tree drifted from the expected --json map")
    for path, expected := range want {
        require.Equal(t, expected, got[path], "command %q: --json presence mismatch", path)
    }
}

// collectJSONFlag walks the command tree rooted at cmd, recording for each leaf
// command whether --json is visible on it (local flags merged with inherited).
func collectJSONFlag(t *testing.T, cmd *cobra.Command, path string, out map[string]bool) {
    t.Helper()
    children := cmd.Commands()
    if len(children) == 0 {
        // Merge parent persistent flags so an accidentally-inherited --json is
        // still caught here, not just a locally-registered one.
        out[path] = cmd.Flags().Lookup("json") != nil || cmd.InheritedFlags().Lookup("json") != nil
        return
    }
    for _, child := range children {
        collectJSONFlag(t, child, path+" "+child.Name(), out)
    }
}

func sortedKeys(m map[string]bool) []string {
    keys := make([]string, 0, len(m))
    for k := range m {
        keys = append(keys, k)
    }
    sort.Strings(keys)
    return keys
}

Mcmd/entire/cli/corecmd_json_flag_test.go+98

// Not parallel: runCoreCmd swaps the package-level activeCoreClient seam.
func TestRunCoreList_EmptyJSONIsArray(t *testing.T) {
    srv := serveOrgList(t, nil)
    // org list's --json is persistent on the group root, so drive the full
    // group command with "list" as a subcommand arg.
    // Drive the full group command so the test covers Cobra's command-tree
    // wiring as well as the leaf's local --json flag.
    out, _, err := runCoreCmd(t, newOrgCmd, srv.URL, "list", "--json")
    require.NoError(t, err)
    require.JSONEq(t, "[]", out, "empty --json list must be [], not null")
}

Mcmd/entire/cli/corecmd_list_test.go+2/-2

// Not parallel: runCoreCmd swaps the package-level activeCoreClient seam.
func TestOrgCreate_JSONOnRequest(t *testing.T) {
    srv := newCreateOrgServer(t)
    // org create's --json is persistent on the group root, so drive the
    // full group command with "create" as a subcommand arg.
    // Drive the full group command so the test covers Cobra's command-tree
    // wiring as well as the leaf's local --json flag.
    out, _, err := runCoreCmd(t, newOrgCmd, srv.URL, "create", "acme", "--json")
    require.NoError(t, err)
    require.Contains(t, out, `"name": "acme"`)
}

Mcmd/entire/cli/corecmd_mutation_test.go+2/-2


// Not parallel: runCoreCmd swaps the package-level activeCoreClient seam.
func TestRunCoreList_EmptyJSONIsArray(t *testing.T) {
    srv := serveOrgList(t, nil)
    // org list's --json is persistent on the group root, so drive the full
    // group command with "list" as a subcommand arg.
    // Drive the full group command so the test covers Cobra's command-tree
    // wiring as well as the leaf's local --json flag.
    out, _, err := runCoreCmd(t, newOrgCmd, srv.URL, "list", "--json")
    require.NoError(t, err)
    require.JSONEq(t, "[]", out, "empty --json list must be [], not null")
}

Mcmd/entire/cli/grant.go+10/-2

Mcmd/entire/cli/org.go+7/-2


Mcmd/entire/cli/project.go+5/-1

```go

Mcmd/entire/cli/repo.go+7/-1

```go

Mcmd/entire/cli/repo_mirror.go+4/-1

```go

Mcmd/entire/cli/repo_mirror_collaborators.go+3/-1

```go

Mcmd/entire/cli/repo_mirror_test.go+3/-2

```go