feat: add --json output mode to all func subcommands - #3822
feat: add --json output mode to all func subcommands#3822Ankitsinghsisodya wants to merge 4 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Ankitsinghsisodya The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @Ankitsinghsisodya. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a global --json mode with a standard response envelope for machine-readable CLI output, including structured error reporting, and updates command docs/tests accordingly.
Changes:
- Introduces JSON envelope helpers (
WriteJSONSuccess,WriteJSONError) and applies them across multiple commands. - Adds a global
--jsonpersistent flag and updates the top-level error sink to emit JSON errors. - Updates generated reference docs and adjusts tests to validate the new JSON envelope shape.
Reviewed changes
Copilot reviewed 59 out of 59 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/app/app.go | Emits structured JSON errors from the top-level error sink when JSON mode is enabled |
| cmd/json.go | Adds JSON envelope types/helpers and error classification mapping |
| cmd/json_test.go | Adds tests for JSON envelope shape and error classification mapping |
| cmd/root.go | Adds global persistent --json flag and binds it to viper |
| cmd/version.go | Emits version output as a JSON envelope when JSON mode is enabled |
| cmd/tkn_tasks.go | Rejects --json for raw YAML output command |
| cmd/templates.go | Switches templates output to JSON envelope when JSON mode is enabled |
| cmd/templates_test.go | Updates templates tests to expect JSON envelope |
| cmd/subscribe.go | Emits subscribe success payload in JSON envelope |
| cmd/run.go | Emits run success payload in JSON envelope |
| cmd/repository.go | Emits repository subcommand success payloads in JSON envelope |
| cmd/mcp.go | Rejects --json for long-running stdio-protocol server |
| cmd/logs.go | Rejects --json for streaming logs |
| cmd/list.go | Wraps list JSON output inside the standard envelope |
| cmd/languages.go | Switches languages output to JSON envelope when JSON mode is enabled |
| cmd/languages_test.go | Updates languages tests to expect JSON envelope |
| cmd/invoke.go | Emits invoke response in JSON envelope and moves verbose text to stderr |
| cmd/environment.go | Emits environment output in JSON envelope when JSON mode is enabled |
| cmd/describe.go | Wraps describe JSON output inside the standard envelope |
| cmd/deploy.go | Moves deploy status text to stderr and emits deploy result JSON envelope |
| cmd/deploy_test.go | Adjusts deploy tests for messages now written to stderr |
| cmd/delete.go | Emits delete success payload in JSON envelope |
| cmd/create.go | Emits create success payload in JSON envelope |
| cmd/config_git.go | Adds JSON envelope behavior for unimplemented config git command |
| cmd/completion.go | Rejects --json for raw shell completion script output |
| cmd/build.go | Emits build success payload in JSON envelope |
| docs/reference/func.md | Documents global --json flag |
| docs/reference/func_version.md | Documents inherited --json flag |
| docs/reference/func_subscribe.md | Documents inherited --json flag |
| docs/reference/func_repository_rename.md | Documents inherited --json flag |
| docs/reference/func_repository_remove.md | Documents inherited --json flag |
| docs/reference/func_repository_list.md | Documents inherited --json flag |
| docs/reference/func_repository_add.md | Documents inherited --json flag |
| docs/reference/func_repository.md | Documents inherited --json flag |
| docs/reference/func_mcp_start.md | Documents inherited --json flag |
| docs/reference/func_mcp.md | Documents inherited --json flag |
| docs/reference/func_logs.md | Documents inherited --json flag |
| docs/reference/func_list.md | Documents inherited --json flag |
| docs/reference/func_invoke.md | Documents inherited --json flag |
| docs/reference/func_environment.md | Documents inherited --json flag |
| docs/reference/func_describe.md | Documents inherited --json flag |
| docs/reference/func_deploy.md | Documents inherited --json flag |
| docs/reference/func_delete.md | Documents inherited --json flag |
| docs/reference/func_create.md | Documents inherited --json flag |
| docs/reference/func_config_volumes_remove.md | Documents inherited --json flag |
| docs/reference/func_config_volumes_add.md | Documents inherited --json flag |
| docs/reference/func_config_volumes.md | Documents inherited --json flag |
| docs/reference/func_config_labels_remove.md | Documents inherited --json flag |
| docs/reference/func_config_labels_add.md | Documents inherited --json flag |
| docs/reference/func_config_labels.md | Documents inherited --json flag |
| docs/reference/func_config_git_set.md | Documents inherited --json flag |
| docs/reference/func_config_git_remove.md | Documents inherited --json flag |
| docs/reference/func_config_git.md | Documents inherited --json flag |
| docs/reference/func_config_envs_remove.md | Documents inherited --json flag |
| docs/reference/func_config_envs_add.md | Documents inherited --json flag |
| docs/reference/func_config_envs.md | Documents inherited --json flag |
| docs/reference/func_config.md | Documents inherited --json flag |
| docs/reference/func_completion.md | Documents inherited --json flag |
| docs/reference/func_build.md | Documents inherited --json flag |
Comments suppressed due to low confidence (1)
pkg/app/app.go:1
- The error from
cmd.WriteJSONErroris ignored. Since this is the top-level error sink, failing to write JSON should fall back to a plain-text error on stderr (or at least print an additional message) so failures don't become silent/no-output in JSON mode.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // isJSONEnabled reports whether --json was explicitly set for this execution. | ||
| // Using cmd.Flag("json").Changed (rather than viper.GetBool("json")) avoids | ||
| // stale viper state polluting test runs. | ||
| func isJSONEnabled(cmd *cobra.Command) bool { | ||
| f := cmd.Flag("json") | ||
| return f != nil && f.Changed | ||
| } |
| var resp JSONResponse | ||
| if err := json.Unmarshal([]byte(buf()), &resp); err != nil { | ||
| t.Fatalf("output is not valid JSON: %v", err) | ||
| } | ||
| if resp.APIVersion != "v1" { | ||
| t.Errorf("expected apiVersion 'v1', got %q", resp.APIVersion) | ||
| } | ||
| if resp.Status != "ok" { | ||
| t.Errorf("expected status 'ok', got %q", resp.Status) | ||
| } | ||
| if resp.Data == nil { | ||
| t.Error("expected non-nil data in templates JSON response") | ||
| } | ||
| _ = cmp.Diff // keep import used |
| var resp JSONResponse | ||
| if err := json.Unmarshal([]byte(buf()), &resp); err != nil { | ||
| t.Fatalf("output is not valid JSON: %v", err) | ||
| } | ||
| if resp.APIVersion != "v1" { | ||
| t.Errorf("expected apiVersion 'v1', got %q", resp.APIVersion) | ||
| } | ||
| if resp.Status != "ok" { | ||
| t.Errorf("expected status 'ok', got %q", resp.Status) | ||
| } | ||
| if resp.Data == nil { | ||
| t.Error("expected non-nil data") | ||
| } |
lkingland
left a comment
There was a problem hiding this comment.
This one probably needs to be revisited in light of our recent MCP updates.
describe/list/version already have --output json (raw Instance / ListItem / Version). This adds a second JSON dialect:
{"apiVersion":"v1","status":"ok","data":{...}}
MCP (describe, list, version) already parses the first dialect. We need to pick one and go with it across the board.
Copilot is right on FUNC_JSON.
Tests only check the envelope, not the payload (_ = cmp.Diff), which is a regression.
func logs --json errors as unsupported (stream) needs to be updated to use our "snapshot" (default).
- Added JSON output functionality to multiple commands including build, create, delete, deploy, describe, environment, invoke, languages, list, logs, repository, run, subscribe, templates, and version. - Introduced a standardized JSON response structure for success and error messages, enhancing the user experience and consistency across the CLI. - Updated existing commands to check for the --json flag and return structured JSON responses when enabled. - Added tests to ensure the correctness of JSON output and error handling in various scenarios. This enhancement improves the usability of the CLI by allowing users to easily parse command outputs programmatically.
- Added JSON output support for various commands, ensuring structured responses when the --json flag is enabled. - Updated error handling in commands like 'completion', 'mcp start', and 'tkn-tasks' to return appropriate error messages when JSON output is requested. - Redirected non-error messages to stderr in commands such as 'deploy' and 'invoke' to prevent contamination of JSON output on stdout. - Improved test cases to validate the new output behavior and error handling across commands. These changes enhance the user experience by providing clearer error messages and maintaining output consistency across the CLI.
- Updated the `invoke` command to return the response body directly as JSON when the `--json` flag is enabled, improving output consistency. - Removed the internal alias for `writeJSONError` in the `json.go` file to streamline the codebase. - Added documentation to multiple command reference files to include the `--json` option, ensuring users are aware of the new output format. These changes enhance the user experience by providing structured JSON responses across various commands, facilitating easier parsing and integration.
func had grown two JSON dialects. describe, list and version emitted raw payloads via --output json, which pkg/mcp parses directly, while languages and templates emitted raw payloads via --json. The new --json envelope added a third shape, leaving consumers to guess which one a given command speaks. Fold all of them into the one versioned envelope. write() wraps the JSON case in a single place, so --json is now simply the shorthand spelling of --output json and the two produce byte-identical output. pkg/mcp unwraps the envelope's data payload, and reports a status:error envelope as the CLI's own category/code/hint rather than as a parse failure. Alongside that: - isJSONEnabled consults viper, so $FUNC_JSON is honored by the success paths and not only by the top-level error sink. - logs --json reports a finite snapshot of the logs available now instead of refusing to run; streaming stays the default for the human path until knative#3999 flips it. - The templates and languages tests assert their payloads again, rather than only that the envelope carries non-nil data. - The error sink falls back to plain text on stderr when the envelope itself cannot be written, so a failure is never silent. - config git --json reports a failure: an empty success envelope claimed a result for a command that does nothing. Issue knative#3769
eadfb64 to
29adc24
Compare
Summary
Implements structured JSON output for every
funcsubcommand, as described in #3769.All JSON is written to stdout using a single versioned envelope:
{"apiVersion":"v1","status":"ok","data":{...}} {"apiVersion":"v1","status":"error","error":{"category":"...","code":"...","retryable":false,"message":"...","hint":"..."}}One dialect, two spellings
Per review feedback:
funcpreviously had two raw JSON shapes —--output jsonon
describe/list/version(parsed directly bypkg/mcp) and--jsononlanguages/templates. Rather than add a third, both spellings now emit thesame envelope:
--jsonis the shorthand spelling of--output json; the two produce byte-identical output (TestJSON_DialectParity).JSONcase ofcmd/format.go'swrite()— so the spellings cannot drift apart again.pkg/mcp(describe,version) unwraps.data, and surfaces astatus:"error"envelope as the CLI's owncategory/code/hintinstead of a parse failure.Breaking:
func describe|list|version --output jsonandfunc languages|templates --jsonnow return the envelope rather than a bare payload. Consumers read
.data.Human, plain, yaml and url formats are unchanged.
Changes
New files
cmd/json.go—JSONResponse/JSONError,WriteJSONSuccess,WriteJSONError,errorToJSONError(maps 14+ typed errors to category/code/retryable),isJSONEnabled/outputFormat/JSONOutputRequestedcmd/json_test.go— unit tests for every error-type mapping, envelope shape,apiVersionpresence, round-trip validitycmd/json_envelope_test.go— envelope decode helper; dialect-parity and$FUNC_JSONtestspkg/mcp/envelope.go+ test — envelope unwrapping for the MCP tools that parse CLI JSONGlobal plumbing
cmd/root.go—--jsonas a persistent flag on the root command, bound to viper with env lookup configured so$FUNC_JSONworks for every subcommandcmd/json.go— JSON mode is the effective setting (flag,$FUNC_JSON, or--output json), not merely "was the flag typed"pkg/app/app.go— top-level error sink writes the error envelope to stdout in JSON mode, falling back to plain text on stderr if the envelope itself cannot be writtenPer-command success payloads
fn.Instanceobjectfunc logs --jsonA structured response has to be finite, so
--jsonreports a snapshot of thelogs available at the time of the call (
Follow: false, no pod watch) ratherthan refusing to run. New
k8s.GetPodLogsSnapshotBySelector/knative.GetKServiceLogsSnapshotback this. Streaming remains the default forthe human path until #3999 flips it, at which point
--jsonsimply rides thesame snapshot code.
Test plan
make test— greenmake check— 0 issuesTestJSON_DialectParity—--jsonand--output jsonemit identical bytesTestJSON_EnvVar—$FUNC_JSONenables JSON mode on the success pathTestTemplates_JSON,TestLanguages_JSON,TestListEnvs— assert the decoded payload, not just thatdatais non-nilpkg/mcpenvelope tests — unwrapping,status:"error"classification, malformed and payload-less output