From 91d667a4e1b593e6088bcfdb99ee11e13b76c84e Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Sun, 17 May 2026 12:25:15 +0530 Subject: [PATCH 1/4] feat: implement JSON output support across various commands - 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. --- cmd/build.go | 21 ++- cmd/create.go | 18 ++- cmd/delete.go | 27 +++- cmd/deploy.go | 27 +++- cmd/describe.go | 9 ++ cmd/environment.go | 4 + cmd/invoke.go | 12 ++ cmd/json.go | 326 ++++++++++++++++++++++++++++++++++++++++++ cmd/json_test.go | 250 ++++++++++++++++++++++++++++++++ cmd/languages.go | 15 +- cmd/languages_test.go | 27 ++-- cmd/list.go | 10 ++ cmd/logs.go | 3 + cmd/repository.go | 37 ++++- cmd/root.go | 9 ++ cmd/run.go | 23 ++- cmd/subscribe.go | 19 ++- cmd/templates.go | 22 +-- cmd/templates_test.go | 65 +++------ cmd/version.go | 3 + pkg/app/app.go | 21 +-- 21 files changed, 832 insertions(+), 116 deletions(-) create mode 100644 cmd/json.go create mode 100644 cmd/json_test.go diff --git a/cmd/build.go b/cmd/build.go index f7f4e25822..c679974c36 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -202,7 +202,26 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro } // Stamp is a performance optimization: treat the function as being built // (cached) unless the fs changes. - return f.Stamp() + if err = f.Stamp(); err != nil { + return + } + if isJSONEnabled(cmd) { + image := f.Build.Image + if image == "" { + image = f.Image + } + err = writeJSONSuccess(cmd.OutOrStdout(), buildJSONResult{ + Name: f.Name, + Image: image, + }) + } + return +} + +// buildJSONResult is the data payload emitted on success when --json is set. +type buildJSONResult struct { + Name string `json:"name"` + Image string `json:"image,omitempty"` } // warnRegistryInsecureChange checks if the registry has changed but diff --git a/cmd/create.go b/cmd/create.go index b526c0338a..eef90fd76b 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -136,7 +136,7 @@ func runCreate(cmd *cobra.Command, args []string, newClient ClientFactory) (err } // Create - _, err = client.Init(fn.Function{ + f, err := client.Init(fn.Function{ Name: cfg.Name, Root: cfg.Path, Runtime: cfg.Runtime, @@ -145,11 +145,27 @@ func runCreate(cmd *cobra.Command, args []string, newClient ClientFactory) (err if err != nil { return err } + if isJSONEnabled(cmd) { + return writeJSONSuccess(cmd.OutOrStdout(), createJSONResult{ + Name: f.Name, + Path: f.Root, + Runtime: f.Runtime, + Template: cfg.Template, + }) + } // Confirm fmt.Fprintf(cmd.OutOrStderr(), "Created %v function in %v\n", cfg.Runtime, cfg.Path) return nil } +// createJSONResult is the data payload emitted on success when --json is set. +type createJSONResult struct { + Name string `json:"name"` + Path string `json:"path"` + Runtime string `json:"runtime"` + Template string `json:"template,omitempty"` +} + type createConfig struct { Path string // Absolute path to function source Runtime string // Language Runtime diff --git a/cmd/delete.go b/cmd/delete.go index 7cf2a24627..b24c877142 100644 --- a/cmd/delete.go +++ b/cmd/delete.go @@ -82,21 +82,42 @@ func runDelete(cmd *cobra.Command, args []string, newClient ClientFactory) (err client, done := newClient(ClientConfig{Verbose: cfg.Verbose}) defer done() + var deletedName, deletedNamespace string if cfg.Name != "" { // Delete by name if provided - _, err = client.Remove(cmd.Context(), cfg.Name, cfg.Namespace, fn.Function{}, cfg.All) - return err + deletedName = cfg.Name + deletedNamespace = cfg.Namespace + if _, err = client.Remove(cmd.Context(), cfg.Name, cfg.Namespace, fn.Function{}, cfg.All); err != nil { + return + } } else { // Otherwise; delete the function at path (cwd by default) f, err := fn.NewFunction(cfg.Path) if err != nil { return err } + deletedName = f.Name + deletedNamespace = f.Deploy.Namespace // updates f.Deploy. (clears them on success) f, err = client.Remove(cmd.Context(), "", "", f, cfg.All) if err != nil { return err } - return f.Write() + if err = f.Write(); err != nil { + return err + } + } + if isJSONEnabled(cmd) { + err = writeJSONSuccess(cmd.OutOrStdout(), deleteJSONResult{ + Name: deletedName, + Namespace: deletedNamespace, + }) } + return +} + +// deleteJSONResult is the data payload emitted on success when --json is set. +type deleteJSONResult struct { + Name string `json:"name"` + Namespace string `json:"namespace,omitempty"` } type deleteConfig struct { diff --git a/cmd/deploy.go b/cmd/deploy.go index 50ec7647c6..153c4bcf39 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -329,6 +329,7 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { defer done() // Deploy + var deployedURL string if cfg.Remote { // Write func.yaml before the pipeline uploads sources to the PVC, // so that the on-cluster deploy step sees the latest config @@ -343,7 +344,10 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { if url, f, err = client.RunPipeline(cmd.Context(), f); err != nil { return wrapDeploymentError(err) } - fmt.Fprintf(cmd.OutOrStdout(), "Function Deployed at %v\n", url) + deployedURL = url + if !isJSONEnabled(cmd) { + fmt.Fprintf(cmd.OutOrStdout(), "Function Deployed at %v\n", url) + } } else { var buildOptions []fn.BuildOption if buildOptions, err = cfg.buildOptions(); err != nil { @@ -410,7 +414,26 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { // Updates the build stamp because building must have been accomplished // during this process, and a future call to deploy without any appreciable // changes to the filesystem should not rebuild again unless `--build` - return f.Stamp() + if err = f.Stamp(); err != nil { + return + } + if isJSONEnabled(cmd) { + err = writeJSONSuccess(cmd.OutOrStdout(), deployJSONResult{ + Name: f.Name, + Namespace: f.Deploy.Namespace, + URL: deployedURL, + Image: f.Deploy.Image, + }) + } + return +} + +// deployJSONResult is the data payload emitted on success when --json is set. +type deployJSONResult struct { + Name string `json:"name"` + Namespace string `json:"namespace,omitempty"` + URL string `json:"url,omitempty"` + Image string `json:"image,omitempty"` } // build determines if the function should be built based on given flag diff --git a/cmd/describe.go b/cmd/describe.go index f8beb99af2..07f0af2210 100644 --- a/cmd/describe.go +++ b/cmd/describe.go @@ -1,6 +1,7 @@ package cmd import ( + "bytes" "encoding/json" "fmt" "io" @@ -89,6 +90,14 @@ func runDescribe(cmd *cobra.Command, args []string, newClient ClientFactory) (er } } + if isJSONEnabled(cmd) { + var buf bytes.Buffer + if err = info(details).JSON(&buf); err != nil { + return + } + var raw json.RawMessage = buf.Bytes() + return writeJSONSuccess(cmd.OutOrStdout(), raw) + } return write(os.Stdout, info(details), cfg.Output) } diff --git a/cmd/environment.go b/cmd/environment.go index 31aee04f31..bf7f393172 100644 --- a/cmd/environment.go +++ b/cmd/environment.go @@ -154,6 +154,10 @@ func runEnvironment(cmd *cobra.Command, newClient ClientFactory, v *Version) (er environment.Instance = instance } + if isJSONEnabled(cmd) { + return writeJSONSuccess(cmd.OutOrStdout(), environment) + } + var s []byte switch cfg.Format { case "json": diff --git a/cmd/invoke.go b/cmd/invoke.go index 657edd9b55..d9ddbb959e 100644 --- a/cmd/invoke.go +++ b/cmd/invoke.go @@ -236,12 +236,24 @@ func runInvoke(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err } } + // When --json: wrap response in envelope + if isJSONEnabled(cmd) { + return writeJSONSuccess(cmd.OutOrStdout(), invokeJSONResult{ + Response: string(body), + }) + } + // Always print the response's default stringification // Note body already includes a linebreak. fmt.Fprint(cmd.OutOrStdout(), body) return } +// invokeJSONResult is the data payload emitted on success when --json is set. +type invokeJSONResult struct { + Response string `json:"response"` +} + type invokeConfig struct { Path string Target string diff --git a/cmd/json.go b/cmd/json.go new file mode 100644 index 0000000000..e104d401e8 --- /dev/null +++ b/cmd/json.go @@ -0,0 +1,326 @@ +package cmd + +import ( + "encoding/json" + "errors" + "io" + + "github.com/spf13/cobra" + "knative.dev/func/pkg/docker" + fn "knative.dev/func/pkg/functions" + "knative.dev/func/pkg/oci" +) + +const jsonAPIVersion = "v1" + +// 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 +} + +// JSONResponse is the top-level envelope for all --json output. +type JSONResponse struct { + APIVersion string `json:"apiVersion"` + Status string `json:"status"` // "ok" or "error" + Data any `json:"data,omitempty"` + Error *JSONError `json:"error,omitempty"` +} + +// JSONError carries structured failure information for machine consumers. +type JSONError struct { + Category string `json:"category"` + Code string `json:"code"` + Retryable bool `json:"retryable"` + Message string `json:"message"` + Hint string `json:"hint,omitempty"` + Context map[string]string `json:"context,omitempty"` +} + +// WriteJSONSuccess writes a success envelope containing data to w. +// Exported for use in tests and pkg/app. +func WriteJSONSuccess(w io.Writer, data any) error { + return json.NewEncoder(w).Encode(JSONResponse{ + APIVersion: jsonAPIVersion, + Status: "ok", + Data: data, + }) +} + +// writeJSONSuccess is the package-internal alias. +func writeJSONSuccess(w io.Writer, data any) error { + return WriteJSONSuccess(w, data) +} + +// WriteJSONError classifies err and writes an error envelope to w. +// Exported so that pkg/app can call it from the top-level error sink. +func WriteJSONError(w io.Writer, err error) error { + return json.NewEncoder(w).Encode(JSONResponse{ + APIVersion: jsonAPIVersion, + Status: "error", + Error: errorToJSONError(err), + }) +} + +// writeJSONError is the package-internal alias. +func writeJSONError(w io.Writer, err error) error { + return WriteJSONError(w, err) +} + +// errorToJSONError maps a Go error to a structured JSONError by inspecting +// the known typed errors in the cmd and pkg/functions layers. +func errorToJSONError(err error) *JSONError { + if err == nil { + return nil + } + + // --- CLUSTER errors --- + + var clusterNotAccessible *ErrClusterNotAccessible + if errors.As(err, &clusterNotAccessible) { + return &JSONError{ + Category: "CLUSTER_ERROR", + Code: "CLUSTER_NOT_ACCESSIBLE", + Retryable: true, + Message: clusterNotAccessible.Err.Error(), + Hint: "Verify your cluster is running: kubectl cluster-info", + } + } + + var listClusterConn *ErrListClusterConnection + if errors.As(err, &listClusterConn) { + return &JSONError{ + Category: "CLUSTER_ERROR", + Code: "CLUSTER_NOT_ACCESSIBLE", + Retryable: true, + Message: listClusterConn.Err.Error(), + Hint: "Verify your cluster is running: kubectl cluster-info", + } + } + + var invalidKubeconfig *ErrInvalidKubeconfig + if errors.As(err, &invalidKubeconfig) { + return &JSONError{ + Category: "CLUSTER_ERROR", + Code: "INVALID_KUBECONFIG", + Retryable: false, + Message: invalidKubeconfig.Err.Error(), + Hint: "Check your KUBECONFIG environment variable or ~/.kube/config", + } + } + + // --- AUTH / REGISTRY errors --- + + var registryRequiredCLI *ErrRegistryRequired + if errors.As(err, ®istryRequiredCLI) { + return &JSONError{ + Category: "VALIDATION_ERROR", + Code: "REGISTRY_REQUIRED", + Retryable: false, + Message: registryRequiredCLI.Err.Error(), + Hint: "Provide --registry or set FUNC_REGISTRY", + Context: map[string]string{"command": registryRequiredCLI.Cmd}, + } + } + + if errors.Is(err, fn.ErrRegistryRequired) { + return &JSONError{ + Category: "VALIDATION_ERROR", + Code: "REGISTRY_REQUIRED", + Retryable: false, + Message: err.Error(), + Hint: "Provide --registry or set FUNC_REGISTRY", + } + } + + // --- VALIDATION errors --- + + var conflictImageRegistry *ErrConflictImageRegistry + if errors.As(err, &conflictImageRegistry) { + return &JSONError{ + Category: "VALIDATION_ERROR", + Code: "CONFLICTING_IMAGE_REGISTRY", + Retryable: false, + Message: conflictImageRegistry.Err.Error(), + Hint: "Use either --image or --registry, not both", + Context: map[string]string{"command": conflictImageRegistry.Cmd}, + } + } + + var invalidNamespace *ErrInvalidNamespace + if errors.As(err, &invalidNamespace) { + return &JSONError{ + Category: "VALIDATION_ERROR", + Code: "INVALID_NAMESPACE", + Retryable: false, + Message: invalidNamespace.Err.Error(), + Hint: "Namespace must be lowercase alphanumeric and hyphens only, max 63 chars", + Context: map[string]string{"command": invalidNamespace.Cmd}, + } + } + + var invalidDomain *ErrInvalidDomain + if errors.As(err, &invalidDomain) { + return &JSONError{ + Category: "VALIDATION_ERROR", + Code: "INVALID_DOMAIN", + Retryable: false, + Message: invalidDomain.Err.Error(), + Hint: "Domain must be a valid DNS subdomain", + Context: map[string]string{"command": invalidDomain.Cmd}, + } + } + + var platformNotSupported *ErrPlatformNotSupported + if errors.As(err, &platformNotSupported) { + return &JSONError{ + Category: "VALIDATION_ERROR", + Code: "PLATFORM_NOT_SUPPORTED", + Retryable: false, + Message: platformNotSupported.Err.Error(), + Hint: "--platform is only supported with s2i and pack builders", + Context: map[string]string{"command": platformNotSupported.Cmd}, + } + } + + var notInitializedCLI *ErrNotInitialized + if errors.As(err, ¬InitializedCLI) { + ctx := map[string]string{} + if notInitializedCLI.Cmd != "" { + ctx["command"] = notInitializedCLI.Cmd + } + return &JSONError{ + Category: "VALIDATION_ERROR", + Code: "NOT_INITIALIZED", + Retryable: false, + Message: notInitializedCLI.Err.Error(), + Hint: "Run 'func create' to initialize a function first", + Context: ctx, + } + } + + var notInitializedCore *fn.ErrNotInitialized + if errors.As(err, ¬InitializedCore) { + ctx := map[string]string{} + if notInitializedCore.Path != "" { + ctx["path"] = notInitializedCore.Path + } + return &JSONError{ + Category: "VALIDATION_ERROR", + Code: "NOT_INITIALIZED", + Retryable: false, + Message: notInitializedCore.Error(), + Hint: "Run 'func create' to initialize a function first", + Context: ctx, + } + } + + var deleteNameRequired *ErrDeleteNameRequired + if errors.As(err, &deleteNameRequired) { + return &JSONError{ + Category: "VALIDATION_ERROR", + Code: "NAME_REQUIRED", + Retryable: false, + Message: deleteNameRequired.Err.Error(), + Hint: "Provide a function name or use --path", + } + } + + var deleteNamespaceRequired *ErrDeleteNamespaceRequired + if errors.As(err, &deleteNamespaceRequired) { + return &JSONError{ + Category: "VALIDATION_ERROR", + Code: "NAMESPACE_REQUIRED", + Retryable: false, + Message: deleteNamespaceRequired.Err.Error(), + Hint: "Provide --namespace or use --path to a function with a recorded namespace", + } + } + + if errors.Is(err, fn.ErrNameRequired) { + return &JSONError{ + Category: "VALIDATION_ERROR", + Code: "NAME_REQUIRED", + Retryable: false, + Message: err.Error(), + } + } + + if errors.Is(err, fn.ErrNamespaceRequired) { + return &JSONError{ + Category: "VALIDATION_ERROR", + Code: "NAMESPACE_REQUIRED", + Retryable: false, + Message: err.Error(), + } + } + + // --- PORT / RUN errors --- + + var portPermissionDenied *ErrPortPermissionDenied + if errors.As(err, &portPermissionDenied) { + return &JSONError{ + Category: "VALIDATION_ERROR", + Code: "PORT_PERMISSION_DENIED", + Retryable: false, + Message: portPermissionDenied.Error(), + Hint: "Use a non-privileged port (>1024) or run with elevated permissions", + Context: map[string]string{"port": portPermissionDenied.Port}, + } + } + + var portUnavailable *ErrPortUnavailable + if errors.As(err, &portUnavailable) { + return &JSONError{ + Category: "VALIDATION_ERROR", + Code: "PORT_UNAVAILABLE", + Retryable: true, + Message: portUnavailable.Err.Error(), + Hint: "Try a different port with --address", + Context: map[string]string{"port": portUnavailable.Port}, + } + } + + // --- BUILD errors --- + + if errors.Is(err, docker.ErrNoDocker) { + return &JSONError{ + Category: "BUILD_ERROR", + Code: "DOCKER_NOT_AVAILABLE", + Retryable: true, + Message: err.Error(), + Hint: "Ensure Docker or Podman daemon is running", + } + } + + var buildErr oci.BuildErr + if errors.As(err, &buildErr) { + return &JSONError{ + Category: "BUILD_ERROR", + Code: "BUILD_FAILED", + Retryable: true, + Message: buildErr.Err.Error(), + } + } + + if errors.Is(err, fn.ErrNotBuilt) { + return &JSONError{ + Category: "BUILD_ERROR", + Code: "NOT_BUILT", + Retryable: false, + Message: err.Error(), + Hint: "Run 'func build' before deploying", + } + } + + // --- fallback --- + + return &JSONError{ + Category: "UNKNOWN_ERROR", + Code: "UNKNOWN", + Retryable: false, + Message: err.Error(), + } +} diff --git a/cmd/json_test.go b/cmd/json_test.go new file mode 100644 index 0000000000..b27f9a3ac8 --- /dev/null +++ b/cmd/json_test.go @@ -0,0 +1,250 @@ +package cmd_test + +import ( + "bytes" + "encoding/json" + "errors" + "testing" + + "knative.dev/func/cmd" + fn "knative.dev/func/pkg/functions" + "knative.dev/func/pkg/oci" +) + +// -- envelope shape --------------------------------------------------------- + +func TestWriteJSONSuccess_EnvelopeShape(t *testing.T) { + var buf bytes.Buffer + type payload struct { + Name string `json:"name"` + } + if err := cmd.WriteJSONSuccess(&buf, payload{Name: "myfunc"}); err != nil { + t.Fatalf("WriteJSONSuccess returned error: %v", err) + } + + var resp cmd.JSONResponse + if err := json.Unmarshal(buf.Bytes(), &resp); err != nil { + t.Fatalf("success envelope 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.Error != nil { + t.Errorf("expected nil error in success envelope, got %+v", resp.Error) + } + if resp.Data == nil { + t.Error("expected non-nil data in success envelope") + } +} + +func TestWriteJSONError_EnvelopeShape(t *testing.T) { + var buf bytes.Buffer + if err := cmd.WriteJSONError(&buf, errors.New("something broke")); err != nil { + t.Fatalf("WriteJSONError returned error: %v", err) + } + + var resp cmd.JSONResponse + if err := json.Unmarshal(buf.Bytes(), &resp); err != nil { + t.Fatalf("error envelope is not valid JSON: %v", err) + } + if resp.APIVersion != "v1" { + t.Errorf("expected apiVersion 'v1', got %q", resp.APIVersion) + } + if resp.Status != "error" { + t.Errorf("expected status 'error', got %q", resp.Status) + } + if resp.Error == nil { + t.Fatal("expected non-nil error field in error envelope") + } + if resp.Data != nil { + t.Errorf("expected nil data in error envelope, got %v", resp.Data) + } +} + +func TestAPIVersionAlwaysPresent(t *testing.T) { + tests := []struct { + name string + fn func(*bytes.Buffer) error + }{ + {"success", func(b *bytes.Buffer) error { return cmd.WriteJSONSuccess(b, "data") }}, + {"error", func(b *bytes.Buffer) error { return cmd.WriteJSONError(b, errors.New("err")) }}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var buf bytes.Buffer + if err := tc.fn(&buf); err != nil { + t.Fatal(err) + } + var resp cmd.JSONResponse + if err := json.Unmarshal(buf.Bytes(), &resp); err != nil { + t.Fatalf("not valid JSON: %v", err) + } + if resp.APIVersion != "v1" { + t.Errorf("apiVersion: want 'v1' got %q", resp.APIVersion) + } + }) + } +} + +// -- errorToJSONError category/code mapping --------------------------------- + +func TestErrorToJSONError_ClusterNotAccessible(t *testing.T) { + inner := errors.New("dial tcp: connection refused") + err := cmd.NewErrClusterNotAccessible(inner) + assertJSONError(t, err, "CLUSTER_ERROR", "CLUSTER_NOT_ACCESSIBLE", true) +} + +func TestErrorToJSONError_InvalidKubeconfig(t *testing.T) { + inner := errors.New("kubeconfig not found") + err := cmd.NewErrInvalidKubeconfig(inner) + assertJSONError(t, err, "CLUSTER_ERROR", "INVALID_KUBECONFIG", false) +} + +func TestErrorToJSONError_RegistryRequiredCLI(t *testing.T) { + inner := fn.ErrRegistryRequired + err := cmd.NewErrRegistryRequired(inner, "build") + assertJSONError(t, err, "VALIDATION_ERROR", "REGISTRY_REQUIRED", false) +} + +func TestErrorToJSONError_RegistryRequiredCore(t *testing.T) { + assertJSONError(t, fn.ErrRegistryRequired, "VALIDATION_ERROR", "REGISTRY_REQUIRED", false) +} + +func TestErrorToJSONError_ConflictImageRegistry(t *testing.T) { + inner := fn.ErrConflictingImageAndRegistry + err := cmd.NewErrConflictImageRegistry(inner, "build") + assertJSONError(t, err, "VALIDATION_ERROR", "CONFLICTING_IMAGE_REGISTRY", false) +} + +func TestErrorToJSONError_InvalidNamespace(t *testing.T) { + inner := fn.ErrInvalidNamespace + err := cmd.NewErrInvalidNamespace(inner, "deploy") + assertJSONError(t, err, "VALIDATION_ERROR", "INVALID_NAMESPACE", false) +} + +func TestErrorToJSONError_InvalidDomain(t *testing.T) { + inner := fn.ErrInvalidDomain + err := cmd.NewErrInvalidDomain(inner, "deploy") + assertJSONError(t, err, "VALIDATION_ERROR", "INVALID_DOMAIN", false) +} + +func TestErrorToJSONError_PlatformNotSupported(t *testing.T) { + inner := fn.ErrPlatformNotSupported + err := cmd.NewErrPlatformNotSupported(inner, "build") + assertJSONError(t, err, "VALIDATION_ERROR", "PLATFORM_NOT_SUPPORTED", false) +} + +func TestErrorToJSONError_NotInitializedCLI(t *testing.T) { + inner := fn.NewErrNotInitialized("/path") + err := cmd.NewErrNotInitialized(inner, "deploy") + assertJSONError(t, err, "VALIDATION_ERROR", "NOT_INITIALIZED", false) +} + +func TestErrorToJSONError_NotInitializedCore(t *testing.T) { + err := fn.NewErrNotInitialized("/some/path") + assertJSONError(t, err, "VALIDATION_ERROR", "NOT_INITIALIZED", false) +} + +func TestErrorToJSONError_DeleteNameRequired(t *testing.T) { + err := cmd.NewErrDeleteNameRequired(fn.ErrNameRequired) + assertJSONError(t, err, "VALIDATION_ERROR", "NAME_REQUIRED", false) +} + +func TestErrorToJSONError_DeleteNamespaceRequired(t *testing.T) { + err := cmd.NewErrDeleteNamespaceRequired(fn.ErrNamespaceRequired) + assertJSONError(t, err, "VALIDATION_ERROR", "NAMESPACE_REQUIRED", false) +} + +func TestErrorToJSONError_NotBuilt(t *testing.T) { + assertJSONError(t, fn.ErrNotBuilt, "BUILD_ERROR", "NOT_BUILT", false) +} + +func TestErrorToJSONError_BuildFailed(t *testing.T) { + err := oci.BuildErr{Err: errors.New("build failed")} + assertJSONError(t, err, "BUILD_ERROR", "BUILD_FAILED", true) +} + +func TestErrorToJSONError_Unknown(t *testing.T) { + assertJSONError(t, errors.New("totally unknown error"), "UNKNOWN_ERROR", "UNKNOWN", false) +} + +func TestErrorToJSONError_MessagePreserved(t *testing.T) { + msg := "my specific error message" + var buf bytes.Buffer + if err := cmd.WriteJSONError(&buf, errors.New(msg)); err != nil { + t.Fatal(err) + } + var resp cmd.JSONResponse + if err := json.Unmarshal(buf.Bytes(), &resp); err != nil { + t.Fatal(err) + } + if resp.Error.Message != msg { + t.Errorf("expected message %q, got %q", msg, resp.Error.Message) + } +} + +// -- round-trip validity ---------------------------------------------------- + +func TestWriteJSONSuccess_RoundTrip(t *testing.T) { + type payload struct { + Name string `json:"name"` + Namespace string `json:"namespace"` + } + p := payload{Name: "myfunc", Namespace: "prod"} + + var buf bytes.Buffer + if err := cmd.WriteJSONSuccess(&buf, p); err != nil { + t.Fatal(err) + } + + var resp struct { + APIVersion string `json:"apiVersion"` + Status string `json:"status"` + Data payload `json:"data"` + } + if err := json.Unmarshal(buf.Bytes(), &resp); err != nil { + t.Fatalf("round-trip decode failed: %v", err) + } + if resp.Data.Name != p.Name { + t.Errorf("round-trip name: want %q got %q", p.Name, resp.Data.Name) + } + if resp.Data.Namespace != p.Namespace { + t.Errorf("round-trip namespace: want %q got %q", p.Namespace, resp.Data.Namespace) + } + if resp.APIVersion != "v1" { + t.Errorf("round-trip apiVersion: want 'v1' got %q", resp.APIVersion) + } + if resp.Status != "ok" { + t.Errorf("round-trip status: want 'ok' got %q", resp.Status) + } +} + +// -- helpers ---------------------------------------------------------------- + +// assertJSONError writes the error as a JSON envelope and checks category/code/retryable. +func assertJSONError(t *testing.T, err error, wantCategory, wantCode string, wantRetryable bool) { + t.Helper() + var buf bytes.Buffer + if werr := cmd.WriteJSONError(&buf, err); werr != nil { + t.Fatalf("WriteJSONError returned error: %v", werr) + } + var resp cmd.JSONResponse + if derr := json.Unmarshal(buf.Bytes(), &resp); derr != nil { + t.Fatalf("result is not valid JSON: %v", derr) + } + if resp.Error == nil { + t.Fatal("expected non-nil error field") + } + if resp.Error.Category != wantCategory { + t.Errorf("category: want %q got %q", wantCategory, resp.Error.Category) + } + if resp.Error.Code != wantCode { + t.Errorf("code: want %q got %q", wantCode, resp.Error.Code) + } + if resp.Error.Retryable != wantRetryable { + t.Errorf("retryable: want %v got %v", wantRetryable, resp.Error.Retryable) + } +} diff --git a/cmd/languages.go b/cmd/languages.go index d0ad7eb187..004956a941 100644 --- a/cmd/languages.go +++ b/cmd/languages.go @@ -1,7 +1,6 @@ package cmd import ( - "encoding/json" "fmt" "github.com/ory/viper" @@ -86,16 +85,10 @@ func runLanguages(cmd *cobra.Command, newClient ClientFactory) (err error) { } if cfg.JSON { - var s []byte - s, err = json.MarshalIndent(runtimes, "", " ") - if err != nil { - return - } - fmt.Fprintln(cmd.OutOrStdout(), string(s)) - } else { - for _, runtime := range runtimes { - fmt.Fprintln(cmd.OutOrStdout(), runtime) - } + return writeJSONSuccess(cmd.OutOrStdout(), runtimes) + } + for _, runtime := range runtimes { + fmt.Fprintln(cmd.OutOrStdout(), runtime) } return } diff --git a/cmd/languages_test.go b/cmd/languages_test.go index 4874b6b2ad..fd8da45590 100644 --- a/cmd/languages_test.go +++ b/cmd/languages_test.go @@ -1,6 +1,7 @@ package cmd import ( + "encoding/json" "testing" . "knative.dev/func/pkg/testing" @@ -33,7 +34,7 @@ typescript` } // TestLanguages_JSON ensures that listing languages in --json format returns -// builtin languages as a JSON array. +// builtin languages wrapped in a structured JSON envelope. func TestLanguages_JSON(t *testing.T) { _ = FromTempDirectory(t) @@ -44,17 +45,17 @@ func TestLanguages_JSON(t *testing.T) { t.Fatal(err) } - expected := `[ - "go", - "node", - "python", - "quarkus", - "rust", - "springboot", - "typescript" -]` - output := buf() - if output != expected { - t.Fatalf("expected:\n%v\ngot:\n%v\n", expected, output) + 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") } } diff --git a/cmd/list.go b/cmd/list.go index 41845890b7..5eebc68dd5 100644 --- a/cmd/list.go +++ b/cmd/list.go @@ -1,6 +1,7 @@ package cmd import ( + "bytes" "encoding/json" "fmt" "io" @@ -91,6 +92,15 @@ func runList(cmd *cobra.Command, _ []string, newClient ClientFactory) (err error return NewErrListClusterConnection(err) } + if isJSONEnabled(cmd) { + var buf bytes.Buffer + if err = listItems(items).JSON(&buf); err != nil { + return + } + var raw json.RawMessage = buf.Bytes() + return writeJSONSuccess(cmd.OutOrStdout(), raw) + } + if len(items) == 0 { printNoFunctionsFound(cmd, cfg.Namespace) return diff --git a/cmd/logs.go b/cmd/logs.go index 8a0bcb6135..18f46a4982 100644 --- a/cmd/logs.go +++ b/cmd/logs.go @@ -63,6 +63,9 @@ specified with --path. Abstracts away the underlying service name and pod detail } func runLogs(cmd *cobra.Command, newClient ClientFactory) error { + if isJSONEnabled(cmd) { + return fmt.Errorf("--json is not supported for streaming commands such as 'logs'") + } cfg, err := newLogsConfig(cmd) if err != nil { return err diff --git a/cmd/repository.go b/cmd/repository.go index fb29337016..efa9df1f62 100644 --- a/cmd/repository.go +++ b/cmd/repository.go @@ -297,7 +297,7 @@ func runRepository(cmd *cobra.Command, args []string, newClient ClientFactory) ( } // List -func runRepositoryList(_ *cobra.Command, newClient ClientFactory) (err error) { +func runRepositoryList(cmd *cobra.Command, newClient ClientFactory) (err error) { cfg, err := newRepositoryConfig() if err != nil { return @@ -312,6 +312,18 @@ func runRepositoryList(_ *cobra.Command, newClient ClientFactory) (err error) { return } + if isJSONEnabled(cmd) { + type repoItem struct { + Name string `json:"name"` + URL string `json:"url,omitempty"` + } + items := make([]repoItem, len(rr)) + for i, r := range rr { + items[i] = repoItem{Name: r.Name, URL: r.URL()} + } + return writeJSONSuccess(cmd.OutOrStdout(), items) + } + // Print repository names, or name plus url if verbose // This follows the format of `git remote`, as it is likely familiar. for _, r := range rr { @@ -325,7 +337,7 @@ func runRepositoryList(_ *cobra.Command, newClient ClientFactory) (err error) { } // Add -func runRepositoryAdd(_ *cobra.Command, args []string, newClient ClientFactory) (err error) { +func runRepositoryAdd(cmd *cobra.Command, args []string, newClient ClientFactory) (err error) { // Supports both composable, discrete CLI commands or prompt-based "config" // by setting the argument values (name and ulr) to value of positional args, // but only requires them if not prompting. If prompting, those values @@ -415,6 +427,12 @@ func runRepositoryAdd(_ *cobra.Command, args []string, newClient ClientFactory) if n, err = client.Repositories().Add(params.Name, params.URL); err != nil { return } + if isJSONEnabled(cmd) { + return writeJSONSuccess(cmd.OutOrStdout(), struct { + Name string `json:"name"` + URL string `json:"url"` + }{Name: n, URL: params.URL}) + } if cfg.Verbose { fmt.Fprintf(os.Stdout, "Repository added: %s\n", n) } @@ -422,7 +440,7 @@ func runRepositoryAdd(_ *cobra.Command, args []string, newClient ClientFactory) } // Rename -func runRepositoryRename(_ *cobra.Command, args []string, newClient ClientFactory) (err error) { +func runRepositoryRename(cmd *cobra.Command, args []string, newClient ClientFactory) (err error) { cfg, err := newRepositoryConfig() if err != nil { return @@ -485,6 +503,12 @@ func runRepositoryRename(_ *cobra.Command, args []string, newClient ClientFactor if err = client.Repositories().Rename(params.Old, params.New); err != nil { return } + if isJSONEnabled(cmd) { + return writeJSONSuccess(cmd.OutOrStdout(), struct { + Old string `json:"old"` + New string `json:"new"` + }{Old: params.Old, New: params.New}) + } if cfg.Verbose { fmt.Fprintln(os.Stdout, "Repository renamed") } @@ -492,7 +516,7 @@ func runRepositoryRename(_ *cobra.Command, args []string, newClient ClientFactor } // Remove -func runRepositoryRemove(_ *cobra.Command, args []string, newClient ClientFactory) (err error) { +func runRepositoryRemove(cmd *cobra.Command, args []string, newClient ClientFactory) (err error) { cfg, err := newRepositoryConfig() if err != nil { return @@ -578,6 +602,11 @@ func runRepositoryRemove(_ *cobra.Command, args []string, newClient ClientFactor if err = client.Repositories().Remove(params.Name); err != nil { return } + if isJSONEnabled(cmd) { + return writeJSONSuccess(cmd.OutOrStdout(), struct { + Name string `json:"name"` + }{Name: params.Name}) + } if cfg.Verbose { fmt.Fprintln(os.Stdout, "Repository removed") } diff --git a/cmd/root.go b/cmd/root.go index ec1e2e680f..dbbb68c06b 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -132,9 +132,18 @@ Learn more about Knative at: https://knative.dev`, cfg.Name), groups.AddTo(cmd) groups.SetRootUsage(cmd, nil) + addJSONFlag(cmd) + return cmd } +// addJSONFlag ensures common text/wording when the --json flag is used +func addJSONFlag(cmd *cobra.Command) { + cmd.PersistentFlags().Bool("json", false, "Output results as JSON ($FUNC_JSON)") + // Bind to viper so app.go can check viper.GetBool("json") in the error sink. + _ = viper.BindPFlag("json", cmd.PersistentFlags().Lookup("json")) +} + // Helpers // ------------------------------------------ diff --git a/cmd/run.go b/cmd/run.go index f60ef3e9aa..2ef0699abe 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -2,7 +2,6 @@ package cmd import ( "context" - "encoding/json" "errors" "fmt" "net" @@ -273,22 +272,13 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { // Output based on format if cfg.JSON { - // Create JSON output structure - output := struct { - Address string `json:"address"` - Host string `json:"host"` - Port string `json:"port"` - }{ + if err = writeJSONSuccess(cmd.OutOrStdout(), runJSONResult{ Address: fmt.Sprintf("http://%s:%s", job.Host, job.Port), Host: job.Host, Port: job.Port, + }); err != nil { + return fmt.Errorf("failed to write JSON output: %w", err) } - - jsonData, err := json.Marshal(output) - if err != nil { - return fmt.Errorf("failed to marshal JSON output: %w", err) - } - fmt.Fprintln(cmd.OutOrStdout(), string(jsonData)) } else { fmt.Fprintf(cmd.OutOrStderr(), "Function running on %s\n", net.JoinHostPort(job.Host, job.Port)) } @@ -434,3 +424,10 @@ func (c runConfig) Validate(cmd *cobra.Command, f fn.Function) (err error) { return } + +// runJSONResult is the data payload emitted on success when --json is set. +type runJSONResult struct { + Address string `json:"address"` + Host string `json:"host"` + Port string `json:"port"` +} diff --git a/cmd/subscribe.go b/cmd/subscribe.go index 91ace2c032..e2e582fde4 100644 --- a/cmd/subscribe.go +++ b/cmd/subscribe.go @@ -62,7 +62,24 @@ func runSubscribe(cmd *cobra.Command) (err error) { f.Deploy.Subscriptions = updateOrAddSubscription(f.Deploy.Subscriptions, cfg) // pump it - return f.Write() + if err = f.Write(); err != nil { + return + } + if isJSONEnabled(cmd) { + err = writeJSONSuccess(cmd.OutOrStdout(), subscribeJSONResult{ + Name: f.Name, + Source: cfg.Source, + Filters: extractFilterMap(cfg.Filter), + }) + } + return +} + +// subscribeJSONResult is the data payload emitted on success when --json is set. +type subscribeJSONResult struct { + Name string `json:"name"` + Source string `json:"source"` + Filters map[string]string `json:"filters,omitempty"` } func extractFilterMap(filters []string) map[string]string { diff --git a/cmd/templates.go b/cmd/templates.go index e0c343dfb4..f8786e7e31 100644 --- a/cmd/templates.go +++ b/cmd/templates.go @@ -1,7 +1,6 @@ package cmd import ( - "encoding/json" "errors" "fmt" "net/http" @@ -107,15 +106,10 @@ func runTemplates(cmd *cobra.Command, args []string, newClient ClientFactory) (e return err } if cfg.JSON { - s, err := json.MarshalIndent(templates, "", " ") - if err != nil { - return err - } - fmt.Fprintln(cmd.OutOrStdout(), string(s)) - } else { - for _, template := range templates { - fmt.Fprintln(cmd.OutOrStdout(), template) - } + return writeJSONSuccess(cmd.OutOrStdout(), templates) + } + for _, template := range templates { + fmt.Fprintln(cmd.OutOrStdout(), template) } return nil } else if len(args) > 1 { @@ -129,7 +123,7 @@ func runTemplates(cmd *cobra.Command, args []string, newClient ClientFactory) (e return } if cfg.JSON { - // Gather into a single data structure for printing as json + // Gather into a single data structure for the envelope templateMap := make(map[string][]string) for _, runtime := range runtimes { templates, err := client.Templates().List(runtime) @@ -138,11 +132,7 @@ func runTemplates(cmd *cobra.Command, args []string, newClient ClientFactory) (e } templateMap[runtime] = templates } - s, err := json.MarshalIndent(templateMap, "", " ") - if err != nil { - return err - } - fmt.Fprintln(cmd.OutOrStdout(), string(s)) + return writeJSONSuccess(cmd.OutOrStdout(), templateMap) } else { // print using a formatted writer (sorted) builder := strings.Builder{} diff --git a/cmd/templates_test.go b/cmd/templates_test.go index 92076d75cd..5aa85b9cca 100644 --- a/cmd/templates_test.go +++ b/cmd/templates_test.go @@ -1,6 +1,7 @@ package cmd import ( + "encoding/json" "errors" "testing" @@ -43,7 +44,7 @@ typescript http` } // TestTemplates_JSON ensures that listing templates respects the --json -// output format. +// output format, returning an envelope with the template map as data. func TestTemplates_JSON(t *testing.T) { _ = FromTempDirectory(t) @@ -54,40 +55,20 @@ func TestTemplates_JSON(t *testing.T) { t.Fatal(err) } - expected := `{ - "go": [ - "cloudevents", - "http" - ], - "node": [ - "cloudevents", - "http" - ], - "python": [ - "cloudevents", - "http" - ], - "quarkus": [ - "cloudevents", - "http" - ], - "rust": [ - "cloudevents", - "http" - ], - "springboot": [ - "cloudevents", - "http" - ], - "typescript": [ - "cloudevents", - "http" - ] -}` - - if d := cmp.Diff(expected, buf()); d != "" { - t.Error("output mismatch (-want, +got):", d) + 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 } // TestTemplates_ByLanguage ensures that the output is correctly filtered @@ -112,21 +93,19 @@ http` t.Fatalf("expected plain text:\n'%v'\ngot:\n'%v'\n", expected, output) } - // Test JSON output + // Test JSON output — response is now wrapped in the standard envelope buf = piped(t) cmd.SetArgs([]string{"go", "--json"}) if err := cmd.Execute(); err != nil { t.Fatal(err) } - expected = `[ - "cloudevents", - "http" -]` - - output = buf() - if output != expected { - t.Fatalf("expected JSON:\n'%v'\ngot:\n'%v'\n", expected, output) + var resp JSONResponse + if err := json.Unmarshal([]byte(buf()), &resp); err != nil { + t.Fatalf("expected JSON:\n'%v'\n", err) + } + if resp.Status != "ok" || resp.Data == nil { + t.Fatalf("unexpected envelope: status=%q data=%v", resp.Status, resp.Data) } } diff --git a/cmd/version.go b/cmd/version.go index da82b99b58..de7a36ea1b 100644 --- a/cmd/version.go +++ b/cmd/version.go @@ -95,6 +95,9 @@ func runVersion(cmd *cobra.Command, v Version) error { } v.MiddlewareVersions = latestMW + if isJSONEnabled(cmd) { + return writeJSONSuccess(cmd.OutOrStdout(), v) + } return write(cmd.OutOrStdout(), v, output) } diff --git a/pkg/app/app.go b/pkg/app/app.go index eae55e63a8..1e7ef65afb 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -10,6 +10,7 @@ import ( "syscall" "github.com/AlecAivazis/survey/v2/terminal" + "github.com/ory/viper" "knative.dev/func/cmd" "knative.dev/func/pkg/docker" @@ -38,23 +39,27 @@ func Main() { }} if err := cmd.NewRootCmd(cfg).ExecuteContext(ctx); err != nil { - if !errors.Is(err, terminal.InterruptErr) { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - } if ctx.Err() != nil || errors.Is(err, terminal.InterruptErr) { os.Exit(130) } - if errors.Is(err, docker.ErrNoDocker) { - if !dockerOrPodmanInstalled() { - fmt.Fprintln(os.Stderr, `Docker/Podman not installed. + if viper.GetBool("json") { + _ = cmd.WriteJSONError(os.Stdout, err) + } else { + if !errors.Is(err, terminal.InterruptErr) { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + } + if errors.Is(err, docker.ErrNoDocker) { + if !dockerOrPodmanInstalled() { + fmt.Fprintln(os.Stderr, `Docker/Podman not installed. Please consider installing one of these: https://podman-desktop.io/ https://www.docker.com/products/docker-desktop/`) - } else { - fmt.Fprintln(os.Stderr, `Possible causes: + } else { + fmt.Fprintln(os.Stderr, `Possible causes: The docker/podman daemon is not running. The DOCKER_HOST environment variable is not set.`) + } } } From baedca996cf2dcbda75e63aaf2bcba258865fd8b Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Sun, 17 May 2026 14:13:52 +0530 Subject: [PATCH 2/4] feat: enhance command error handling and output formatting - 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. --- cmd/completion.go | 4 ++ cmd/config_git.go | 10 +-- cmd/deploy.go | 14 ++++- cmd/deploy_test.go | 11 ++-- cmd/invoke.go | 22 +++---- cmd/json.go | 154 +++++++++++++++++++++++++++++++++++++++++++++ cmd/mcp.go | 3 + cmd/tkn_tasks.go | 3 + 8 files changed, 198 insertions(+), 23 deletions(-) diff --git a/cmd/completion.go b/cmd/completion.go index 34311c3cbb..aa2e16fc77 100644 --- a/cmd/completion.go +++ b/cmd/completion.go @@ -2,6 +2,7 @@ package cmd import ( "errors" + "fmt" "os" "github.com/spf13/cobra" @@ -27,6 +28,9 @@ source <(func completion bash) ValidArgs: []string{"bash", "zsh", "fish"}, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) (err error) { + if isJSONEnabled(cmd) { + return fmt.Errorf("--json is not supported for 'completion': it outputs raw shell completion scripts") + } if len(args) < 1 { return errors.New("missing argument") } diff --git a/cmd/config_git.go b/cmd/config_git.go index c000188edb..af9eb90abb 100644 --- a/cmd/config_git.go +++ b/cmd/config_git.go @@ -48,9 +48,11 @@ the current directory or from the directory specified with --path. return cmd } -func runConfigGitCmd(_ *cobra.Command, _ ClientFactory) (err error) { - fmt.Printf("--------------------------- Function Git config ---------------------------\n") - fmt.Printf("Not implemented yet.\n") - +func runConfigGitCmd(cmd *cobra.Command, _ ClientFactory) (err error) { + if isJSONEnabled(cmd) { + return writeJSONSuccess(cmd.OutOrStdout(), nil) + } + fmt.Fprintln(cmd.ErrOrStderr(), "--------------------------- Function Git config ---------------------------") + fmt.Fprintln(cmd.ErrOrStderr(), "Not implemented yet.") return nil } diff --git a/cmd/deploy.go b/cmd/deploy.go index 153c4bcf39..9aecc70205 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -312,12 +312,13 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { if changingNamespace(f) && k8s.IsOpenShift() && k8s.IsOpenShiftInternalRegistry(f.Registry) { f.Registry = "image-registry.openshift-image-registry.svc:5000/" + f.Namespace if cfg.Verbose { - fmt.Fprintf(cmd.OutOrStdout(), "Info: Overriding openshift registry to %s\n", f.Registry) + fmt.Fprintf(cmd.ErrOrStderr(), "Info: Overriding openshift registry to %s\n", f.Registry) } } - // Informative non-error messages regarding the final deployment request - printDeployMessages(cmd.OutOrStdout(), f) + // Informative non-error messages: always go to stderr so that --json + // output on stdout is not contaminated with human-readable status text. + printDeployMessages(cmd.ErrOrStderr(), f) // Get options based on the value of the config such as concrete impls // of builders and pushers based on the value of the --builder flag @@ -403,6 +404,13 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { if f, err = client.Deploy(cmd.Context(), f, fn.WithDeploySkipBuildCheck(cfg.Build == "false")); err != nil { return wrapDeploymentError(err) } + // Capture the deployed URL for --json output. The URL is not + // returned directly by Deploy; a lightweight Describe call fetches it. + if isJSONEnabled(cmd) { + if inst, descErr := client.Describe(cmd.Context(), "", "", f); descErr == nil && len(inst.Routes) > 0 { + deployedURL = inst.Routes[0] + } + } } // Write diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index f7122e8a6c..17a214a2eb 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -1173,17 +1173,18 @@ func TestDeploy_NamespaceRedeployWarning(t *testing.T) { fn.WithRegistry(TestRegistry), )) cmd.SetArgs([]string{}) - stdout := strings.Builder{} - cmd.SetOut(&stdout) + stderr := strings.Builder{} + cmd.SetErr(&stderr) if err := cmd.Execute(); err != nil { t.Fatal(err) } expected := "Warning: namespace chosen is 'funcns', but currently active namespace is 'mynamespace'. Continuing with deployment to 'funcns'." - // Ensure output contained warning if changing namespace - if !strings.Contains(stdout.String(), expected) { - t.Log("STDOUT:\n" + stdout.String()) + // Ensure warning appears on stderr (deploy messages always go to stderr + // so they don't contaminate stdout when --json is active). + if !strings.Contains(stderr.String(), expected) { + t.Log("STDERR:\n" + stderr.String()) t.Fatalf("Expected warning not found:\n%v", expected) } diff --git a/cmd/invoke.go b/cmd/invoke.go index d9ddbb959e..2831d9d330 100644 --- a/cmd/invoke.go +++ b/cmd/invoke.go @@ -213,6 +213,13 @@ func runInvoke(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err return err } + // When --json: emit structured envelope only – no human text on stdout. + if isJSONEnabled(cmd) { + return writeJSONSuccess(cmd.OutOrStdout(), invokeJSONResult{ + Response: string(body), + }) + } + // When Verbose // - Print an explicit "Received response" indicator // - Print metadata (headers for HTTP requests, CloudEvents already include @@ -222,27 +229,20 @@ func runInvoke(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err // stdout could be confusing on a first-time run, viewing a proper echo. // user feedback suggests this actually be placed behind the --verbose // setting: - fmt.Println("Function invoked. Response:") + fmt.Fprintln(cmd.ErrOrStderr(), "Function invoked. Response:") if len(metadata) > 0 { - fmt.Println(" Metadata:") + fmt.Fprintln(cmd.ErrOrStderr(), " Metadata:") } for k, vv := range metadata { values := strings.Join(vv, ";") - fmt.Fprintf(cmd.OutOrStdout(), " %v: %v\n", k, values) + fmt.Fprintf(cmd.ErrOrStderr(), " %v: %v\n", k, values) } if len(metadata) > 0 { - fmt.Println(" Content:") + fmt.Fprintln(cmd.ErrOrStderr(), " Content:") } } - // When --json: wrap response in envelope - if isJSONEnabled(cmd) { - return writeJSONSuccess(cmd.OutOrStdout(), invokeJSONResult{ - Response: string(body), - }) - } - // Always print the response's default stringification // Note body already includes a linebreak. fmt.Fprint(cmd.OutOrStdout(), body) diff --git a/cmd/json.go b/cmd/json.go index e104d401e8..1723de9893 100644 --- a/cmd/json.go +++ b/cmd/json.go @@ -283,6 +283,160 @@ func errorToJSONError(err error) *JSONError { } } + // --- TEMPLATE / REPOSITORY errors --- + + if errors.Is(err, fn.ErrTemplateNotFound) { + return &JSONError{ + Category: "TEMPLATE_ERROR", + Code: "TEMPLATE_NOT_FOUND", + Retryable: false, + Message: err.Error(), + Hint: "Run 'func repository list' to see available repositories and templates", + } + } + + if errors.Is(err, fn.ErrTemplatesNotFound) { + return &JSONError{ + Category: "TEMPLATE_ERROR", + Code: "TEMPLATES_NOT_FOUND", + Retryable: false, + Message: err.Error(), + Hint: "The repository may be missing a 'templates' directory", + } + } + + if errors.Is(err, fn.ErrTemplateMissingRepository) { + return &JSONError{ + Category: "TEMPLATE_ERROR", + Code: "TEMPLATE_MISSING_REPOSITORY", + Retryable: false, + Message: err.Error(), + Hint: "Specify the repository prefix, e.g. 'myrepo/http'", + } + } + + if errors.Is(err, fn.ErrRepositoryNotFound) { + return &JSONError{ + Category: "TEMPLATE_ERROR", + Code: "REPOSITORY_NOT_FOUND", + Retryable: false, + Message: err.Error(), + Hint: "Add the repository with 'func repository add'", + } + } + + if errors.Is(err, fn.ErrRepositoriesNotDefined) { + return &JSONError{ + Category: "TEMPLATE_ERROR", + Code: "REPOSITORIES_NOT_DEFINED", + Retryable: false, + Message: err.Error(), + Hint: "Set FUNC_REPOSITORIES_PATH or add a repository with 'func repository add'", + } + } + + // --- RUNTIME errors --- + + if errors.Is(err, fn.ErrRuntimeNotFound) { + return &JSONError{ + Category: "RUNTIME_ERROR", + Code: "RUNTIME_NOT_FOUND", + Retryable: false, + Message: err.Error(), + Hint: "Run 'func languages' to see supported runtimes", + } + } + + if errors.Is(err, fn.ErrRuntimeRequired) { + return &JSONError{ + Category: "RUNTIME_ERROR", + Code: "RUNTIME_REQUIRED", + Retryable: false, + Message: err.Error(), + Hint: "Provide --language or set the runtime in func.yaml", + } + } + + var runtimeNotRecognized fn.ErrRuntimeNotRecognized + if errors.As(err, &runtimeNotRecognized) { + return &JSONError{ + Category: "RUNTIME_ERROR", + Code: "RUNTIME_NOT_RECOGNIZED", + Retryable: false, + Message: runtimeNotRecognized.Error(), + Hint: "Run 'func languages' to see supported runtimes", + Context: map[string]string{"runtime": runtimeNotRecognized.Runtime}, + } + } + + var runnerNotImplemented fn.ErrRunnerNotImplemented + if errors.As(err, &runnerNotImplemented) { + return &JSONError{ + Category: "RUNTIME_ERROR", + Code: "RUNNER_NOT_IMPLEMENTED", + Retryable: false, + Message: runnerNotImplemented.Error(), + Hint: "Use 'func deploy' to run containerized functions", + Context: map[string]string{"runtime": runnerNotImplemented.Runtime}, + } + } + + var runTimeout fn.ErrRunTimeout + if errors.As(err, &runTimeout) { + return &JSONError{ + Category: "RUNTIME_ERROR", + Code: "RUN_TIMEOUT", + Retryable: true, + Message: runTimeout.Error(), + Hint: "The function did not become ready in time; check container logs", + } + } + + // --- FUNCTION state errors --- + + if errors.Is(err, fn.ErrFunctionNotFound) { + return &JSONError{ + Category: "NOT_FOUND", + Code: "FUNCTION_NOT_FOUND", + Retryable: false, + Message: err.Error(), + } + } + + if errors.Is(err, fn.ErrNotRunning) { + return &JSONError{ + Category: "NOT_FOUND", + Code: "FUNCTION_NOT_RUNNING", + Retryable: false, + Message: err.Error(), + Hint: "Start the function with 'func run' or 'func deploy'", + } + } + + // --- PORT / RUN errors (pkg/functions layer) --- + + var portUnavailableCore *fn.ErrPortUnavailableError + if errors.As(err, &portUnavailableCore) { + if portUnavailableCore.IsPermissionDenied() { + return &JSONError{ + Category: "VALIDATION_ERROR", + Code: "PORT_PERMISSION_DENIED", + Retryable: false, + Message: portUnavailableCore.Error(), + Hint: "Use a non-privileged port (>1024) or run with elevated permissions", + Context: map[string]string{"port": portUnavailableCore.Port}, + } + } + return &JSONError{ + Category: "VALIDATION_ERROR", + Code: "PORT_UNAVAILABLE", + Retryable: true, + Message: portUnavailableCore.Error(), + Hint: "Try a different port with --address", + Context: map[string]string{"port": portUnavailableCore.Port}, + } + } + // --- BUILD errors --- if errors.Is(err, docker.ErrNoDocker) { diff --git a/cmd/mcp.go b/cmd/mcp.go index 391817001c..1ef950772c 100644 --- a/cmd/mcp.go +++ b/cmd/mcp.go @@ -96,6 +96,9 @@ DESCRIPTION } func runMCPStart(cmd *cobra.Command, args []string, newClient ClientFactory) error { + if isJSONEnabled(cmd) { + return fmt.Errorf("--json is not supported for 'mcp start': it is a long-running server process with its own stdio protocol") + } // Configure write mode writeEnabled := false if val := os.Getenv("FUNC_ENABLE_MCP_WRITE"); val != "" { diff --git a/cmd/tkn_tasks.go b/cmd/tkn_tasks.go index 57b7dd793f..84b3de134c 100644 --- a/cmd/tkn_tasks.go +++ b/cmd/tkn_tasks.go @@ -18,6 +18,9 @@ Installation: func tkn-tasks | kubectl apply -f - `, Hidden: true, RunE: func(cmd *cobra.Command, args []string) error { + if isJSONEnabled(cmd) { + return fmt.Errorf("--json is not supported for 'tkn-tasks': it outputs raw multi-document YAML") + } _, err := fmt.Fprintln(cmd.OutOrStdout(), tekton.GetClusterTasks()) return err }, From adfc5b07a1d2d980f7011c308fe076c19fa5f0e1 Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Sun, 17 May 2026 14:21:23 +0530 Subject: [PATCH 3/4] feat: enhance JSON output support across commands - 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. --- cmd/invoke.go | 2 +- cmd/json.go | 5 ----- docs/reference/func.md | 1 + docs/reference/func_build.md | 6 ++++++ docs/reference/func_completion.md | 6 ++++++ docs/reference/func_config.md | 6 ++++++ docs/reference/func_config_envs.md | 6 ++++++ docs/reference/func_config_envs_add.md | 6 ++++++ docs/reference/func_config_envs_remove.md | 6 ++++++ docs/reference/func_config_git.md | 6 ++++++ docs/reference/func_config_git_remove.md | 6 ++++++ docs/reference/func_config_git_set.md | 6 ++++++ docs/reference/func_config_labels.md | 6 ++++++ docs/reference/func_config_labels_add.md | 6 ++++++ docs/reference/func_config_labels_remove.md | 6 ++++++ docs/reference/func_config_volumes.md | 6 ++++++ docs/reference/func_config_volumes_add.md | 6 ++++++ docs/reference/func_config_volumes_remove.md | 6 ++++++ docs/reference/func_create.md | 6 ++++++ docs/reference/func_delete.md | 6 ++++++ docs/reference/func_deploy.md | 6 ++++++ docs/reference/func_describe.md | 6 ++++++ docs/reference/func_environment.md | 6 ++++++ docs/reference/func_invoke.md | 6 ++++++ docs/reference/func_list.md | 6 ++++++ docs/reference/func_logs.md | 6 ++++++ docs/reference/func_mcp.md | 6 ++++++ docs/reference/func_mcp_start.md | 6 ++++++ docs/reference/func_repository.md | 6 ++++++ docs/reference/func_repository_add.md | 6 ++++++ docs/reference/func_repository_list.md | 6 ++++++ docs/reference/func_repository_remove.md | 6 ++++++ docs/reference/func_repository_rename.md | 6 ++++++ docs/reference/func_subscribe.md | 6 ++++++ docs/reference/func_version.md | 6 ++++++ 35 files changed, 194 insertions(+), 6 deletions(-) diff --git a/cmd/invoke.go b/cmd/invoke.go index 2831d9d330..5acca3a918 100644 --- a/cmd/invoke.go +++ b/cmd/invoke.go @@ -216,7 +216,7 @@ func runInvoke(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err // When --json: emit structured envelope only – no human text on stdout. if isJSONEnabled(cmd) { return writeJSONSuccess(cmd.OutOrStdout(), invokeJSONResult{ - Response: string(body), + Response: body, }) } diff --git a/cmd/json.go b/cmd/json.go index 1723de9893..9d11c2597a 100644 --- a/cmd/json.go +++ b/cmd/json.go @@ -64,11 +64,6 @@ func WriteJSONError(w io.Writer, err error) error { }) } -// writeJSONError is the package-internal alias. -func writeJSONError(w io.Writer, err error) error { - return WriteJSONError(w, err) -} - // errorToJSONError maps a Go error to a structured JSONError by inspecting // the known typed errors in the cmd and pkg/functions layers. func errorToJSONError(err error) *JSONError { diff --git a/docs/reference/func.md b/docs/reference/func.md index 0c01edaf36..8adef2f0e3 100644 --- a/docs/reference/func.md +++ b/docs/reference/func.md @@ -19,6 +19,7 @@ Learn more about Knative at: https://knative.dev ``` -h, --help help for func + --json Output results as JSON ($FUNC_JSON) ``` ### SEE ALSO diff --git a/docs/reference/func_build.md b/docs/reference/func_build.md index 344574a334..81f4365a9e 100644 --- a/docs/reference/func_build.md +++ b/docs/reference/func_build.md @@ -76,6 +76,12 @@ func build -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func](func.md) - func manages Knative Functions diff --git a/docs/reference/func_completion.md b/docs/reference/func_completion.md index d562c2f6fc..ad2efa62f4 100644 --- a/docs/reference/func_completion.md +++ b/docs/reference/func_completion.md @@ -28,6 +28,12 @@ func completion -h, --help help for completion ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func](func.md) - func manages Knative Functions diff --git a/docs/reference/func_config.md b/docs/reference/func_config.md index ef2aea0ff3..1e10f44566 100644 --- a/docs/reference/func_config.md +++ b/docs/reference/func_config.md @@ -23,6 +23,12 @@ func config -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func](func.md) - func manages Knative Functions diff --git a/docs/reference/func_config_envs.md b/docs/reference/func_config_envs.md index d353f6b415..7d13f6e3a5 100644 --- a/docs/reference/func_config_envs.md +++ b/docs/reference/func_config_envs.md @@ -23,6 +23,12 @@ func config envs -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func config](func_config.md) - Configure a function diff --git a/docs/reference/func_config_envs_add.md b/docs/reference/func_config_envs_add.md index 57a638a4ed..5731110a68 100644 --- a/docs/reference/func_config_envs_add.md +++ b/docs/reference/func_config_envs_add.md @@ -48,6 +48,12 @@ func config envs add --value='{{ configMap:confMapName }}' -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func config envs](func_config_envs.md) - List and manage configured environment variable for a function diff --git a/docs/reference/func_config_envs_remove.md b/docs/reference/func_config_envs_remove.md index bb0caacee4..9cd2d5eef6 100644 --- a/docs/reference/func_config_envs_remove.md +++ b/docs/reference/func_config_envs_remove.md @@ -23,6 +23,12 @@ func config envs remove -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func config envs](func_config_envs.md) - List and manage configured environment variable for a function diff --git a/docs/reference/func_config_git.md b/docs/reference/func_config_git.md index 18653d363a..fffffd9d3d 100644 --- a/docs/reference/func_config_git.md +++ b/docs/reference/func_config_git.md @@ -22,6 +22,12 @@ func config git -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func config](func_config.md) - Configure a function diff --git a/docs/reference/func_config_git_remove.md b/docs/reference/func_config_git_remove.md index 29d06c562e..586289819b 100644 --- a/docs/reference/func_config_git_remove.md +++ b/docs/reference/func_config_git_remove.md @@ -27,6 +27,12 @@ func config git remove -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func config git](func_config_git.md) - Manage Git configuration of a function diff --git a/docs/reference/func_config_git_set.md b/docs/reference/func_config_git_set.md index 6e480265b8..ff4024a31e 100644 --- a/docs/reference/func_config_git_set.md +++ b/docs/reference/func_config_git_set.md @@ -36,6 +36,12 @@ func config git set -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func config git](func_config_git.md) - Manage Git configuration of a function diff --git a/docs/reference/func_config_labels.md b/docs/reference/func_config_labels.md index d56e57a8c8..8c15524c0e 100644 --- a/docs/reference/func_config_labels.md +++ b/docs/reference/func_config_labels.md @@ -23,6 +23,12 @@ func config labels -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func config](func_config.md) - Configure a function diff --git a/docs/reference/func_config_labels_add.md b/docs/reference/func_config_labels_add.md index af8c54e4c0..31879bf3f0 100644 --- a/docs/reference/func_config_labels_add.md +++ b/docs/reference/func_config_labels_add.md @@ -36,6 +36,12 @@ func config labels add --name=Foo --value='{{ env:FOO }}' -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func config labels](func_config_labels.md) - List and manage configured labels for a function diff --git a/docs/reference/func_config_labels_remove.md b/docs/reference/func_config_labels_remove.md index 24356300bb..2c45b633e3 100644 --- a/docs/reference/func_config_labels_remove.md +++ b/docs/reference/func_config_labels_remove.md @@ -23,6 +23,12 @@ func config labels remove -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func config labels](func_config_labels.md) - List and manage configured labels for a function diff --git a/docs/reference/func_config_volumes.md b/docs/reference/func_config_volumes.md index 1d115b1ccf..da45a47f4c 100644 --- a/docs/reference/func_config_volumes.md +++ b/docs/reference/func_config_volumes.md @@ -22,6 +22,12 @@ func config volumes -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func config](func_config.md) - Configure a function diff --git a/docs/reference/func_config_volumes_add.md b/docs/reference/func_config_volumes_add.md index e2a732b373..6d400c963b 100644 --- a/docs/reference/func_config_volumes_add.md +++ b/docs/reference/func_config_volumes_add.md @@ -48,6 +48,12 @@ func config volumes add --type=emptydir --path=/tmp/cache --size=1Gi --medium=Me -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func config volumes](func_config_volumes.md) - List and manage configured volumes for a function diff --git a/docs/reference/func_config_volumes_remove.md b/docs/reference/func_config_volumes_remove.md index c717a1fd13..3afbbe57fb 100644 --- a/docs/reference/func_config_volumes_remove.md +++ b/docs/reference/func_config_volumes_remove.md @@ -32,6 +32,12 @@ func config volumes remove --mount-path=/etc/config -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func config volumes](func_config_volumes.md) - List and manage configured volumes for a function diff --git a/docs/reference/func_create.md b/docs/reference/func_create.md index 34480a6bd8..b12bad777b 100644 --- a/docs/reference/func_create.md +++ b/docs/reference/func_create.md @@ -76,6 +76,12 @@ func create -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func](func.md) - func manages Knative Functions diff --git a/docs/reference/func_delete.md b/docs/reference/func_delete.md index de3587951c..92b7db32aa 100644 --- a/docs/reference/func_delete.md +++ b/docs/reference/func_delete.md @@ -41,6 +41,12 @@ func delete myfunc --namespace apps -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func](func.md) - func manages Knative Functions diff --git a/docs/reference/func_deploy.md b/docs/reference/func_deploy.md index b8257073ad..2b49d35335 100644 --- a/docs/reference/func_deploy.md +++ b/docs/reference/func_deploy.md @@ -146,6 +146,12 @@ func deploy -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func](func.md) - func manages Knative Functions diff --git a/docs/reference/func_describe.md b/docs/reference/func_describe.md index 7e5254392b..7cb2426600 100644 --- a/docs/reference/func_describe.md +++ b/docs/reference/func_describe.md @@ -36,6 +36,12 @@ func describe --output yaml --path myotherfunc -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func](func.md) - func manages Knative Functions diff --git a/docs/reference/func_environment.md b/docs/reference/func_environment.md index 87ee7de689..68652921f2 100644 --- a/docs/reference/func_environment.md +++ b/docs/reference/func_environment.md @@ -31,6 +31,12 @@ func environment -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func](func.md) - func manages Knative Functions diff --git a/docs/reference/func_invoke.md b/docs/reference/func_invoke.md index 21cf277cdb..edd0c21305 100644 --- a/docs/reference/func_invoke.md +++ b/docs/reference/func_invoke.md @@ -113,6 +113,12 @@ func invoke -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func](func.md) - func manages Knative Functions diff --git a/docs/reference/func_list.md b/docs/reference/func_list.md index 522620a030..4e3547cdc8 100644 --- a/docs/reference/func_list.md +++ b/docs/reference/func_list.md @@ -38,6 +38,12 @@ func list --all-namespaces --output json -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func](func.md) - func manages Knative Functions diff --git a/docs/reference/func_logs.md b/docs/reference/func_logs.md index 8423776f5f..98f36cc91c 100644 --- a/docs/reference/func_logs.md +++ b/docs/reference/func_logs.md @@ -43,6 +43,12 @@ func logs --since 5m -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func](func.md) - func manages Knative Functions diff --git a/docs/reference/func_mcp.md b/docs/reference/func_mcp.md index 17f13b7157..530ef18832 100644 --- a/docs/reference/func_mcp.md +++ b/docs/reference/func_mcp.md @@ -54,6 +54,12 @@ EXAMPLES -h, --help help for mcp ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func](func.md) - func manages Knative Functions diff --git a/docs/reference/func_mcp_start.md b/docs/reference/func_mcp_start.md index 23d5674d01..a0c316fa84 100644 --- a/docs/reference/func_mcp_start.md +++ b/docs/reference/func_mcp_start.md @@ -31,6 +31,12 @@ func mcp start -h, --help help for start ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func mcp](func_mcp.md) - Model Context Protocol (MCP) server diff --git a/docs/reference/func_repository.md b/docs/reference/func_repository.md index bc99b53026..b11255b1b1 100644 --- a/docs/reference/func_repository.md +++ b/docs/reference/func_repository.md @@ -141,6 +141,12 @@ func repository -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func](func.md) - func manages Knative Functions diff --git a/docs/reference/func_repository_add.md b/docs/reference/func_repository_add.md index 2fe7afb52e..12a841d64f 100644 --- a/docs/reference/func_repository_add.md +++ b/docs/reference/func_repository_add.md @@ -14,6 +14,12 @@ func repository add -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func repository](func_repository.md) - Manage installed template repositories diff --git a/docs/reference/func_repository_list.md b/docs/reference/func_repository_list.md index 14198ef33f..387131d370 100644 --- a/docs/reference/func_repository_list.md +++ b/docs/reference/func_repository_list.md @@ -14,6 +14,12 @@ func repository list -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func repository](func_repository.md) - Manage installed template repositories diff --git a/docs/reference/func_repository_remove.md b/docs/reference/func_repository_remove.md index 081d81e7c2..4fb92687dc 100644 --- a/docs/reference/func_repository_remove.md +++ b/docs/reference/func_repository_remove.md @@ -14,6 +14,12 @@ func repository remove -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func repository](func_repository.md) - Manage installed template repositories diff --git a/docs/reference/func_repository_rename.md b/docs/reference/func_repository_rename.md index 920bd80980..b17efdde58 100644 --- a/docs/reference/func_repository_rename.md +++ b/docs/reference/func_repository_rename.md @@ -14,6 +14,12 @@ func repository rename -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func repository](func_repository.md) - Manage installed template repositories diff --git a/docs/reference/func_subscribe.md b/docs/reference/func_subscribe.md index 5d3a5ef613..ddba722f24 100644 --- a/docs/reference/func_subscribe.md +++ b/docs/reference/func_subscribe.md @@ -38,6 +38,12 @@ func subscribe --filter type=com.example --filter extension=my-extension-value - -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func](func.md) - func manages Knative Functions diff --git a/docs/reference/func_version.md b/docs/reference/func_version.md index bbb61b7865..adc503b5fe 100644 --- a/docs/reference/func_version.md +++ b/docs/reference/func_version.md @@ -43,6 +43,12 @@ func version -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` +### Options inherited from parent commands + +``` + --json Output results as JSON ($FUNC_JSON) +``` + ### SEE ALSO * [func](func.md) - func manages Knative Functions From 29adc24c99d82aedb56d8338a3aaa505ca9a8e76 Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Thu, 20 Aug 2026 12:55:27 +0530 Subject: [PATCH 4/4] feat: unify func's JSON output on one envelope 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 #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 #3769 --- cmd/build.go | 2 +- cmd/config_envs.go | 7 +-- cmd/config_git.go | 6 +- cmd/config_labels.go | 7 +-- cmd/config_test.go | 16 +++++- cmd/create.go | 2 +- cmd/delete.go | 2 +- cmd/deploy.go | 13 +++-- cmd/describe.go | 11 +--- cmd/environment.go | 2 +- cmd/format.go | 12 +++- cmd/invoke.go | 2 +- cmd/json.go | 53 +++++++++++++---- cmd/json_envelope_test.go | 98 ++++++++++++++++++++++++++++++++ cmd/languages.go | 4 +- cmd/languages_test.go | 21 +++---- cmd/list.go | 17 ++---- cmd/logs.go | 39 ++++++++++++- cmd/repository.go | 8 +-- cmd/root.go | 17 +++++- cmd/run.go | 4 +- cmd/subscribe.go | 2 +- cmd/templates.go | 6 +- cmd/templates_test.go | 37 ++++++------ cmd/version.go | 5 +- docs/reference/func_languages.md | 2 +- docs/reference/func_logs.md | 6 ++ docs/reference/func_run.md | 2 +- docs/reference/func_templates.md | 2 +- pkg/app/app.go | 10 +++- pkg/k8s/logs.go | 90 ++++++++++++++++++++++------- pkg/knative/logs.go | 10 ++++ pkg/mcp/envelope.go | 62 ++++++++++++++++++++ pkg/mcp/envelope_test.go | 82 ++++++++++++++++++++++++++ pkg/mcp/tools_describe.go | 5 +- pkg/mcp/tools_describe_test.go | 6 +- pkg/mcp/tools_version.go | 5 +- pkg/mcp/tools_version_test.go | 4 +- 38 files changed, 535 insertions(+), 144 deletions(-) create mode 100644 cmd/json_envelope_test.go create mode 100644 pkg/mcp/envelope.go create mode 100644 pkg/mcp/envelope_test.go diff --git a/cmd/build.go b/cmd/build.go index c679974c36..5178ebae9f 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -210,7 +210,7 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro if image == "" { image = f.Image } - err = writeJSONSuccess(cmd.OutOrStdout(), buildJSONResult{ + err = WriteJSONSuccess(cmd.OutOrStdout(), buildJSONResult{ Name: f.Name, Image: image, }) diff --git a/cmd/config_envs.go b/cmd/config_envs.go index 4d03a0f347..dd7cb14d7c 100644 --- a/cmd/config_envs.go +++ b/cmd/config_envs.go @@ -2,7 +2,6 @@ package cmd import ( "context" - "encoding/json" "errors" "fmt" "io" @@ -10,7 +9,6 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/AlecAivazis/survey/v2/terminal" - "github.com/ory/viper" "github.com/spf13/cobra" "knative.dev/func/cmd/common" @@ -38,7 +36,7 @@ the current directory or from the directory specified with --path. return } - return listEnvs(function, cmd.OutOrStdout(), Format(viper.GetString("output"))) + return listEnvs(function, cmd.OutOrStdout(), Format(outputFormat())) }, } cfg, err := config.NewDefault() @@ -204,8 +202,7 @@ func listEnvs(f fn.Function, w io.Writer, outputFormat Format) error { } return nil case JSON: - enc := json.NewEncoder(w) - return enc.Encode(f.Run.Envs) + return WriteJSONSuccess(w, f.Run.Envs) default: return fmt.Errorf("bad format: %v", outputFormat) } diff --git a/cmd/config_git.go b/cmd/config_git.go index af9eb90abb..2fc8ddb50b 100644 --- a/cmd/config_git.go +++ b/cmd/config_git.go @@ -1,6 +1,7 @@ package cmd import ( + "errors" "fmt" "github.com/spf13/cobra" @@ -49,8 +50,11 @@ the current directory or from the directory specified with --path. } func runConfigGitCmd(cmd *cobra.Command, _ ClientFactory) (err error) { + // Reporting an empty success envelope here would tell a machine consumer + // the command produced a result when it did nothing at all, so say so as + // a failure instead. if isJSONEnabled(cmd) { - return writeJSONSuccess(cmd.OutOrStdout(), nil) + return errors.New("'config git' is not implemented yet") } fmt.Fprintln(cmd.ErrOrStderr(), "--------------------------- Function Git config ---------------------------") fmt.Fprintln(cmd.ErrOrStderr(), "Not implemented yet.") diff --git a/cmd/config_labels.go b/cmd/config_labels.go index 389a8a0735..b43d21b0b1 100644 --- a/cmd/config_labels.go +++ b/cmd/config_labels.go @@ -2,13 +2,11 @@ package cmd import ( "context" - "encoding/json" "fmt" "io" "os" "github.com/AlecAivazis/survey/v2" - "github.com/ory/viper" "github.com/spf13/cobra" "knative.dev/func/cmd/common" @@ -35,7 +33,7 @@ the current directory or from the directory specified with --path. return } - return listLabels(function, cmd.OutOrStdout(), Format(viper.GetString("output"))) + return listLabels(function, cmd.OutOrStdout(), Format(outputFormat())) }, } @@ -178,8 +176,7 @@ func listLabels(f fn.Function, w io.Writer, outputFormat Format) error { } return nil case JSON: - enc := json.NewEncoder(w) - return enc.Encode(f.Deploy.Labels) + return WriteJSONSuccess(w, f.Deploy.Labels) default: return fmt.Errorf("invalid format: %v", outputFormat) } diff --git a/cmd/config_test.go b/cmd/config_test.go index 0ce09cdb68..dfed2c0299 100644 --- a/cmd/config_test.go +++ b/cmd/config_test.go @@ -39,8 +39,22 @@ func TestListEnvs(t *testing.T) { t.Fatal(err) } + // All JSON output, whether requested with --json or --output json, is + // wrapped in the versioned envelope; the payload is under "data". + var envelope struct { + APIVersion string `json:"apiVersion"` + Status string `json:"status"` + Data json.RawMessage `json:"data"` + } + if err = json.Unmarshal(buff.Bytes(), &envelope); err != nil { + t.Fatal(err) + } + if envelope.APIVersion != "v1" || envelope.Status != "ok" { + t.Fatalf("unexpected envelope: %s", buff.Bytes()) + } + var data []fn.Env - err = json.Unmarshal(buff.Bytes(), &data) + err = json.Unmarshal(envelope.Data, &data) if err != nil { t.Fatal(err) } diff --git a/cmd/create.go b/cmd/create.go index eef90fd76b..6c76790e7d 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -146,7 +146,7 @@ func runCreate(cmd *cobra.Command, args []string, newClient ClientFactory) (err return err } if isJSONEnabled(cmd) { - return writeJSONSuccess(cmd.OutOrStdout(), createJSONResult{ + return WriteJSONSuccess(cmd.OutOrStdout(), createJSONResult{ Name: f.Name, Path: f.Root, Runtime: f.Runtime, diff --git a/cmd/delete.go b/cmd/delete.go index b24c877142..96af08b131 100644 --- a/cmd/delete.go +++ b/cmd/delete.go @@ -106,7 +106,7 @@ func runDelete(cmd *cobra.Command, args []string, newClient ClientFactory) (err } } if isJSONEnabled(cmd) { - err = writeJSONSuccess(cmd.OutOrStdout(), deleteJSONResult{ + err = WriteJSONSuccess(cmd.OutOrStdout(), deleteJSONResult{ Name: deletedName, Namespace: deletedNamespace, }) diff --git a/cmd/deploy.go b/cmd/deploy.go index 9aecc70205..7c40ddd825 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -404,10 +404,15 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { if f, err = client.Deploy(cmd.Context(), f, fn.WithDeploySkipBuildCheck(cfg.Build == "false")); err != nil { return wrapDeploymentError(err) } - // Capture the deployed URL for --json output. The URL is not - // returned directly by Deploy; a lightweight Describe call fetches it. + // Deploy does not return the resulting URL, so --json fetches it with + // a follow-up Describe. A failure there is not fatal — the deploy + // itself succeeded — but it must not be silent either, or the caller + // cannot tell an absent url from a function which has none. if isJSONEnabled(cmd) { - if inst, descErr := client.Describe(cmd.Context(), "", "", f); descErr == nil && len(inst.Routes) > 0 { + inst, descErr := client.Describe(cmd.Context(), "", "", f) + if descErr != nil { + fmt.Fprintf(cmd.ErrOrStderr(), "Warning: deployed, but could not determine the function URL: %v\n", descErr) + } else if len(inst.Routes) > 0 { deployedURL = inst.Routes[0] } } @@ -426,7 +431,7 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { return } if isJSONEnabled(cmd) { - err = writeJSONSuccess(cmd.OutOrStdout(), deployJSONResult{ + err = WriteJSONSuccess(cmd.OutOrStdout(), deployJSONResult{ Name: f.Name, Namespace: f.Deploy.Namespace, URL: deployedURL, diff --git a/cmd/describe.go b/cmd/describe.go index 07f0af2210..261d3701c1 100644 --- a/cmd/describe.go +++ b/cmd/describe.go @@ -1,7 +1,6 @@ package cmd import ( - "bytes" "encoding/json" "fmt" "io" @@ -90,14 +89,6 @@ func runDescribe(cmd *cobra.Command, args []string, newClient ClientFactory) (er } } - if isJSONEnabled(cmd) { - var buf bytes.Buffer - if err = info(details).JSON(&buf); err != nil { - return - } - var raw json.RawMessage = buf.Bytes() - return writeJSONSuccess(cmd.OutOrStdout(), raw) - } return write(os.Stdout, info(details), cfg.Output) } @@ -120,7 +111,7 @@ func newDescribeConfig(cmd *cobra.Command, args []string) (cfg describeConfig, e cfg = describeConfig{ Name: name, Namespace: viper.GetString("namespace"), - Output: viper.GetString("output"), + Output: outputFormat(), Path: viper.GetString("path"), Verbose: viper.GetBool("verbose"), } diff --git a/cmd/environment.go b/cmd/environment.go index bf7f393172..96db115561 100644 --- a/cmd/environment.go +++ b/cmd/environment.go @@ -155,7 +155,7 @@ func runEnvironment(cmd *cobra.Command, newClient ClientFactory, v *Version) (er } if isJSONEnabled(cmd) { - return writeJSONSuccess(cmd.OutOrStdout(), environment) + return WriteJSONSuccess(cmd.OutOrStdout(), environment) } var s []byte diff --git a/cmd/format.go b/cmd/format.go index bcad1bc925..57b52bc2fb 100644 --- a/cmd/format.go +++ b/cmd/format.go @@ -1,6 +1,8 @@ package cmd import ( + "bytes" + "encoding/json" "fmt" "io" ) @@ -33,7 +35,15 @@ func write(out io.Writer, s Formatter, formatName string) error { case Plain: return s.Plain(out) case JSON: - return s.JSON(out) + // Every JSON payload func emits is wrapped in the versioned envelope + // (see cmd/json.go), whether it was requested with --json or with + // --output json. Wrapping here rather than at each call site keeps + // the two spellings from drifting into separate dialects. + var buf bytes.Buffer + if err := s.JSON(&buf); err != nil { + return err + } + return WriteJSONSuccess(out, json.RawMessage(buf.Bytes())) case YAML: return s.YAML(out) case URL: diff --git a/cmd/invoke.go b/cmd/invoke.go index 5acca3a918..ba8b6761d3 100644 --- a/cmd/invoke.go +++ b/cmd/invoke.go @@ -215,7 +215,7 @@ func runInvoke(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err // When --json: emit structured envelope only – no human text on stdout. if isJSONEnabled(cmd) { - return writeJSONSuccess(cmd.OutOrStdout(), invokeJSONResult{ + return WriteJSONSuccess(cmd.OutOrStdout(), invokeJSONResult{ Response: body, }) } diff --git a/cmd/json.go b/cmd/json.go index 9d11c2597a..2a6aecb3e7 100644 --- a/cmd/json.go +++ b/cmd/json.go @@ -5,6 +5,7 @@ import ( "errors" "io" + "github.com/ory/viper" "github.com/spf13/cobra" "knative.dev/func/pkg/docker" fn "knative.dev/func/pkg/functions" @@ -13,12 +14,40 @@ import ( const jsonAPIVersion = "v1" -// 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. +// isJSONEnabled reports whether the caller asked for structured JSON output, +// spelled either as the global --json flag (or $FUNC_JSON), or as +// "--output json" on the commands which accept an output format. Both +// spellings emit the same envelope, so machine consumers have exactly one +// shape to parse. func isJSONEnabled(cmd *cobra.Command) bool { - f := cmd.Flag("json") - return f != nil && f.Changed + if viper.GetBool("json") { + return true + } + // --output is command-local, so only consult it for commands which + // actually define it; otherwise a value bound by an earlier command in + // the same process could switch an unrelated command into JSON. + if cmd.Flags().Lookup("output") != nil { + return Format(viper.GetString("output")) == JSON + } + return false +} + +// outputFormat returns the effective format for commands which accept an +// --output flag. The global --json flag is simply the shorthand spelling of +// "--output json", so it wins when set. +func outputFormat() string { + if viper.GetBool("json") { + return JSON + } + return viper.GetString("output") +} + +// JSONOutputRequested reports whether this invocation asked for JSON output. +// It is the *cobra.Command-less form of isJSONEnabled, exported for the +// top-level error sink in pkg/app which runs after Execute has returned and +// therefore has no command to consult. +func JSONOutputRequested() bool { + return viper.GetBool("json") || Format(viper.GetString("output")) == JSON } // JSONResponse is the top-level envelope for all --json output. @@ -42,22 +71,26 @@ type JSONError struct { // WriteJSONSuccess writes a success envelope containing data to w. // Exported for use in tests and pkg/app. func WriteJSONSuccess(w io.Writer, data any) error { - return json.NewEncoder(w).Encode(JSONResponse{ + return encodeJSON(w, JSONResponse{ APIVersion: jsonAPIVersion, Status: "ok", Data: data, }) } -// writeJSONSuccess is the package-internal alias. -func writeJSONSuccess(w io.Writer, data any) error { - return WriteJSONSuccess(w, data) +// encodeJSON writes one envelope to w. Output is indented, matching what +// --output json produced before it was folded into the envelope, so that +// output piped through a terminal stays readable. +func encodeJSON(w io.Writer, response JSONResponse) error { + enc := json.NewEncoder(w) + enc.SetIndent("", " ") + return enc.Encode(response) } // WriteJSONError classifies err and writes an error envelope to w. // Exported so that pkg/app can call it from the top-level error sink. func WriteJSONError(w io.Writer, err error) error { - return json.NewEncoder(w).Encode(JSONResponse{ + return encodeJSON(w, JSONResponse{ APIVersion: jsonAPIVersion, Status: "error", Error: errorToJSONError(err), diff --git a/cmd/json_envelope_test.go b/cmd/json_envelope_test.go new file mode 100644 index 0000000000..e8fb8d5f55 --- /dev/null +++ b/cmd/json_envelope_test.go @@ -0,0 +1,98 @@ +package cmd + +import ( + "bytes" + "encoding/json" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/ory/viper" +) + +// jsonEnvelope mirrors JSONResponse but defers decoding of the data payload, +// so tests can assert on the payload in its real type rather than on the +// interface{} soup a direct JSONResponse unmarshal produces. +type jsonEnvelope struct { + APIVersion string `json:"apiVersion"` + Status string `json:"status"` + Data json.RawMessage `json:"data,omitempty"` + Error *JSONError `json:"error,omitempty"` +} + +// decodeJSONEnvelope asserts that b is a well-formed success envelope and +// unmarshals its data payload into out. Pass a nil out to check only the +// envelope. Tests use this rather than checking `data != nil` so that a +// regression in what a command actually reports cannot slip through. +func decodeJSONEnvelope(t *testing.T, b []byte, out any) { + t.Helper() + var env jsonEnvelope + if err := json.Unmarshal(b, &env); err != nil { + t.Fatalf("output is not valid JSON: %v\ngot: %s", err, b) + } + if env.APIVersion != jsonAPIVersion { + t.Errorf("expected apiVersion %q, got %q", jsonAPIVersion, env.APIVersion) + } + if env.Status != "ok" { + t.Fatalf("expected status 'ok', got %q (error: %+v)", env.Status, env.Error) + } + if out == nil { + return + } + if len(env.Data) == 0 { + t.Fatalf("envelope carries no data payload: %s", b) + } + if err := json.Unmarshal(env.Data, out); err != nil { + t.Fatalf("data payload does not decode into %T: %v\ngot: %s", out, err, env.Data) + } +} + +// runVersionCapture runs the root command's version subcommand with args and +// returns what it wrote to stdout. +func runVersionCapture(t *testing.T, args ...string) string { + t.Helper() + viper.Reset() + var out bytes.Buffer + cmd := NewRootCmd(RootCommandConfig{ + Name: "func", + Version: Version{Vers: "v0.42.0"}, + }) + cmd.SetArgs(append([]string{"version"}, args...)) + cmd.SetOut(&out) + cmd.SetErr(&out) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + return out.String() +} + +// TestJSON_DialectParity ensures --json and --output json emit byte-identical +// envelopes. func has exactly one JSON dialect, so a consumer written against +// either spelling parses the other; a second dialect is the regression this +// guards against. +func TestJSON_DialectParity(t *testing.T) { + viaJSONFlag := runVersionCapture(t, "--json") + viaOutputFlag := runVersionCapture(t, "--output", "json") + + if d := cmp.Diff(viaOutputFlag, viaJSONFlag); d != "" { + t.Error("--json and --output json disagree (-output, +json):", d) + } + + var v Version + decodeJSONEnvelope(t, []byte(viaJSONFlag), &v) + if v.Vers != "v0.42.0" { + t.Errorf("expected version 'v0.42.0' in the data payload, got %q", v.Vers) + } +} + +// TestJSON_EnvVar ensures $FUNC_JSON enables JSON mode, which the --json flag +// help advertises. Detecting the flag alone would leave the env var honored by +// the top-level error sink but ignored by the success paths. +func TestJSON_EnvVar(t *testing.T) { + t.Setenv("FUNC_JSON", "true") + + var v Version + decodeJSONEnvelope(t, []byte(runVersionCapture(t)), &v) + if v.Vers != "v0.42.0" { + t.Errorf("expected version 'v0.42.0' in the data payload, got %q", v.Vers) + } +} diff --git a/cmd/languages.go b/cmd/languages.go index 004956a941..16f48d2a30 100644 --- a/cmd/languages.go +++ b/cmd/languages.go @@ -61,7 +61,7 @@ EXAMPLES fmt.Fprintf(cmd.OutOrStdout(), "error loading config at '%v'. %v\n", config.File(), err) } - cmd.Flags().BoolP("json", "", false, "Set output to JSON format. ($FUNC_JSON)") + cmd.Flags().Bool("json", false, jsonFlagUsage) cmd.Flags().StringP("repository", "r", "", "URI to a specific repository to consider ($FUNC_REPOSITORY)") addVerboseFlag(cmd, cfg.Verbose) @@ -85,7 +85,7 @@ func runLanguages(cmd *cobra.Command, newClient ClientFactory) (err error) { } if cfg.JSON { - return writeJSONSuccess(cmd.OutOrStdout(), runtimes) + return WriteJSONSuccess(cmd.OutOrStdout(), runtimes) } for _, runtime := range runtimes { fmt.Fprintln(cmd.OutOrStdout(), runtime) diff --git a/cmd/languages_test.go b/cmd/languages_test.go index fd8da45590..7c60a49973 100644 --- a/cmd/languages_test.go +++ b/cmd/languages_test.go @@ -1,9 +1,10 @@ package cmd import ( - "encoding/json" "testing" + "github.com/google/go-cmp/cmp" + . "knative.dev/func/pkg/testing" ) @@ -45,17 +46,11 @@ func TestLanguages_JSON(t *testing.T) { t.Fatal(err) } - 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") + var runtimes []string + decodeJSONEnvelope(t, []byte(buf()), &runtimes) + + expected := []string{"go", "node", "python", "quarkus", "rust", "springboot", "typescript"} + if d := cmp.Diff(expected, runtimes); d != "" { + t.Error("runtime list mismatch (-want, +got):", d) } } diff --git a/cmd/list.go b/cmd/list.go index 5eebc68dd5..1b6ae921c6 100644 --- a/cmd/list.go +++ b/cmd/list.go @@ -1,7 +1,6 @@ package cmd import ( - "bytes" "encoding/json" "fmt" "io" @@ -92,16 +91,10 @@ func runList(cmd *cobra.Command, _ []string, newClient ClientFactory) (err error return NewErrListClusterConnection(err) } - if isJSONEnabled(cmd) { - var buf bytes.Buffer - if err = listItems(items).JSON(&buf); err != nil { - return - } - var raw json.RawMessage = buf.Bytes() - return writeJSONSuccess(cmd.OutOrStdout(), raw) - } - - if len(items) == 0 { + // An empty result is reported as an empty JSON array rather than the + // human "no functions found" notice: a machine consumer asking for JSON + // must always receive parseable JSON on stdout. + if len(items) == 0 && !isJSONEnabled(cmd) { printNoFunctionsFound(cmd, cfg.Namespace) return } @@ -121,7 +114,7 @@ type listConfig struct { func newListConfig(cmd *cobra.Command) (cfg listConfig, err error) { cfg = listConfig{ Namespace: viper.GetString("namespace"), - Output: viper.GetString("output"), + Output: outputFormat(), Verbose: viper.GetBool("verbose"), } // If --all-namespaces, zero out any value for namespace (such as) diff --git a/cmd/logs.go b/cmd/logs.go index 18f46a4982..ba29dc53b8 100644 --- a/cmd/logs.go +++ b/cmd/logs.go @@ -1,10 +1,12 @@ package cmd import ( + "bytes" "context" "fmt" "os" "os/signal" + "strings" "syscall" "time" @@ -24,6 +26,9 @@ func NewLogsCmd(newClient ClientFactory) *cobra.Command { Streams logs for the function in the current directory or from the directory specified with --path. Abstracts away the underlying service name and pod details. + +With --json, the logs available at the time of the call are reported as a +finite snapshot instead of being streamed, so the command terminates. `, Example: ` # Stream logs for the function in the current directory @@ -37,6 +42,9 @@ specified with --path. Abstracts away the underlying service name and pod detail # Stream logs with a specific time window {{rootCmdUse}} logs --since 5m + +# Report a finite snapshot of the last 5 minutes of logs as JSON +{{rootCmdUse}} logs --since 5m --json `, SuggestFor: []string{"log", "tail"}, ValidArgsFunction: CompleteFunctionList, @@ -63,9 +71,6 @@ specified with --path. Abstracts away the underlying service name and pod detail } func runLogs(cmd *cobra.Command, newClient ClientFactory) error { - if isJSONEnabled(cmd) { - return fmt.Errorf("--json is not supported for streaming commands such as 'logs'") - } cfg, err := newLogsConfig(cmd) if err != nil { return err @@ -129,6 +134,25 @@ func runLogs(cmd *cobra.Command, newClient ClientFactory) error { sinceTime = &t } + // A structured response has to be finite, so --json reports a snapshot of + // the logs available now rather than following the stream. Streaming stays + // the default for the human path until #3999 flips it. + if isJSONEnabled(cmd) { + var buf bytes.Buffer + if err = knative.GetKServiceLogsSnapshot(cmd.Context(), f.Namespace, f.Name, f.Image, sinceTime, &buf); err != nil { + return fmt.Errorf("failed to read logs: %w", err) + } + lines := []string{} + if trimmed := strings.TrimRight(buf.String(), "\n"); trimmed != "" { + lines = strings.Split(trimmed, "\n") + } + return WriteJSONSuccess(cmd.OutOrStdout(), logsJSONResult{ + Name: f.Name, + Namespace: f.Namespace, + Lines: lines, + }) + } + // Create context that can be cancelled with Ctrl+C ctx, cancel := context.WithCancel(cmd.Context()) defer cancel() @@ -153,6 +177,15 @@ func runLogs(cmd *cobra.Command, newClient ClientFactory) error { return nil } +// logsJSONResult is the data payload emitted on success when --json is set. +type logsJSONResult struct { + Name string `json:"name"` + Namespace string `json:"namespace,omitempty"` + // Lines is the snapshot of log lines available at the time of the call, + // oldest first, with trailing newlines stripped. + Lines []string `json:"lines"` +} + // CLI Configuration (parameters) // ------------------------------ diff --git a/cmd/repository.go b/cmd/repository.go index efa9df1f62..e8fbdea7d3 100644 --- a/cmd/repository.go +++ b/cmd/repository.go @@ -321,7 +321,7 @@ func runRepositoryList(cmd *cobra.Command, newClient ClientFactory) (err error) for i, r := range rr { items[i] = repoItem{Name: r.Name, URL: r.URL()} } - return writeJSONSuccess(cmd.OutOrStdout(), items) + return WriteJSONSuccess(cmd.OutOrStdout(), items) } // Print repository names, or name plus url if verbose @@ -428,7 +428,7 @@ func runRepositoryAdd(cmd *cobra.Command, args []string, newClient ClientFactory return } if isJSONEnabled(cmd) { - return writeJSONSuccess(cmd.OutOrStdout(), struct { + return WriteJSONSuccess(cmd.OutOrStdout(), struct { Name string `json:"name"` URL string `json:"url"` }{Name: n, URL: params.URL}) @@ -504,7 +504,7 @@ func runRepositoryRename(cmd *cobra.Command, args []string, newClient ClientFact return } if isJSONEnabled(cmd) { - return writeJSONSuccess(cmd.OutOrStdout(), struct { + return WriteJSONSuccess(cmd.OutOrStdout(), struct { Old string `json:"old"` New string `json:"new"` }{Old: params.Old, New: params.New}) @@ -603,7 +603,7 @@ func runRepositoryRemove(cmd *cobra.Command, args []string, newClient ClientFact return } if isJSONEnabled(cmd) { - return writeJSONSuccess(cmd.OutOrStdout(), struct { + return WriteJSONSuccess(cmd.OutOrStdout(), struct { Name string `json:"name"` }{Name: params.Name}) } diff --git a/cmd/root.go b/cmd/root.go index dbbb68c06b..f96e138249 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -137,11 +137,22 @@ Learn more about Knative at: https://knative.dev`, cfg.Name), return cmd } -// addJSONFlag ensures common text/wording when the --json flag is used +// jsonFlagUsage is the single wording for --json, shared by the root's +// persistent flag and by the commands which declared their own --json before +// it became global (run, templates, languages). +const jsonFlagUsage = "Output results as JSON ($FUNC_JSON)" + +// addJSONFlag registers --json as a persistent flag so every subcommand +// inherits it, and binds it to viper so that $FUNC_JSON is honored and the +// top-level error sink in pkg/app can consult it after Execute returns. func addJSONFlag(cmd *cobra.Command) { - cmd.PersistentFlags().Bool("json", false, "Output results as JSON ($FUNC_JSON)") - // Bind to viper so app.go can check viper.GetBool("json") in the error sink. + cmd.PersistentFlags().Bool("json", false, jsonFlagUsage) _ = viper.BindPFlag("json", cmd.PersistentFlags().Lookup("json")) + // Commands whose PreRunE does not bindEnv would otherwise never enable + // viper's environment lookup, leaving $FUNC_JSON silently ignored. + viper.AutomaticEnv() + viper.SetEnvPrefix("func") + viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_")) } // Helpers diff --git a/cmd/run.go b/cmd/run.go index 2ef0699abe..93c1e6f579 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -129,7 +129,7 @@ EXAMPLES cmd.Flags().Lookup("build").NoOptDefVal = "true" // register `--build` as equivalient to `--build=true` cmd.Flags().String("address", "", "Interface and port on which to bind and listen. Default is 127.0.0.1:8080, or an available port if 8080 is not available. ($FUNC_ADDRESS)") - cmd.Flags().Bool("json", false, "Output as JSON. ($FUNC_JSON)") + cmd.Flags().Bool("json", false, jsonFlagUsage) // Oft-shared flags: addConfirmFlag(cmd, cfg.Confirm) @@ -272,7 +272,7 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { // Output based on format if cfg.JSON { - if err = writeJSONSuccess(cmd.OutOrStdout(), runJSONResult{ + if err = WriteJSONSuccess(cmd.OutOrStdout(), runJSONResult{ Address: fmt.Sprintf("http://%s:%s", job.Host, job.Port), Host: job.Host, Port: job.Port, diff --git a/cmd/subscribe.go b/cmd/subscribe.go index e2e582fde4..b73b8e1ed9 100644 --- a/cmd/subscribe.go +++ b/cmd/subscribe.go @@ -66,7 +66,7 @@ func runSubscribe(cmd *cobra.Command) (err error) { return } if isJSONEnabled(cmd) { - err = writeJSONSuccess(cmd.OutOrStdout(), subscribeJSONResult{ + err = WriteJSONSuccess(cmd.OutOrStdout(), subscribeJSONResult{ Name: f.Name, Source: cfg.Source, Filters: extractFilterMap(cfg.Filter), diff --git a/cmd/templates.go b/cmd/templates.go index f8786e7e31..3ab2bdb482 100644 --- a/cmd/templates.go +++ b/cmd/templates.go @@ -66,7 +66,7 @@ EXAMPLES fmt.Fprintf(cmd.OutOrStdout(), "error loading config at '%v'. %v\n", config.File(), err) } - cmd.Flags().Bool("json", false, "Set output to JSON format. (Env: $FUNC_JSON)") + cmd.Flags().Bool("json", false, jsonFlagUsage) cmd.Flags().StringP("repository", "r", "", "URI to a specific repository to consider ($FUNC_REPOSITORY)") addVerboseFlag(cmd, cfg.Verbose) @@ -106,7 +106,7 @@ func runTemplates(cmd *cobra.Command, args []string, newClient ClientFactory) (e return err } if cfg.JSON { - return writeJSONSuccess(cmd.OutOrStdout(), templates) + return WriteJSONSuccess(cmd.OutOrStdout(), templates) } for _, template := range templates { fmt.Fprintln(cmd.OutOrStdout(), template) @@ -132,7 +132,7 @@ func runTemplates(cmd *cobra.Command, args []string, newClient ClientFactory) (e } templateMap[runtime] = templates } - return writeJSONSuccess(cmd.OutOrStdout(), templateMap) + return WriteJSONSuccess(cmd.OutOrStdout(), templateMap) } else { // print using a formatted writer (sorted) builder := strings.Builder{} diff --git a/cmd/templates_test.go b/cmd/templates_test.go index 5aa85b9cca..3b64ff7636 100644 --- a/cmd/templates_test.go +++ b/cmd/templates_test.go @@ -1,7 +1,6 @@ package cmd import ( - "encoding/json" "errors" "testing" @@ -55,20 +54,21 @@ func TestTemplates_JSON(t *testing.T) { t.Fatal(err) } - var resp JSONResponse - if err := json.Unmarshal([]byte(buf()), &resp); err != nil { - t.Fatalf("output is not valid JSON: %v", err) + var templates map[string][]string + decodeJSONEnvelope(t, []byte(buf()), &templates) + + expected := map[string][]string{ + "go": {"cloudevents", "http"}, + "node": {"cloudevents", "http"}, + "python": {"cloudevents", "http"}, + "quarkus": {"cloudevents", "http"}, + "rust": {"cloudevents", "http"}, + "springboot": {"cloudevents", "http"}, + "typescript": {"cloudevents", "http"}, } - if resp.APIVersion != "v1" { - t.Errorf("expected apiVersion 'v1', got %q", resp.APIVersion) + if d := cmp.Diff(expected, templates); d != "" { + t.Error("template map mismatch (-want, +got):", d) } - 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 } // TestTemplates_ByLanguage ensures that the output is correctly filtered @@ -100,12 +100,11 @@ http` t.Fatal(err) } - var resp JSONResponse - if err := json.Unmarshal([]byte(buf()), &resp); err != nil { - t.Fatalf("expected JSON:\n'%v'\n", err) - } - if resp.Status != "ok" || resp.Data == nil { - t.Fatalf("unexpected envelope: status=%q data=%v", resp.Status, resp.Data) + var templates []string + decodeJSONEnvelope(t, []byte(buf()), &templates) + + if d := cmp.Diff([]string{"cloudevents", "http"}, templates); d != "" { + t.Error("template list mismatch (-want, +got):", d) } } diff --git a/cmd/version.go b/cmd/version.go index de7a36ea1b..404c533377 100644 --- a/cmd/version.go +++ b/cmd/version.go @@ -71,7 +71,7 @@ DESCRIPTION // Run func runVersion(cmd *cobra.Command, v Version) error { verbose := viper.GetBool("verbose") - output := viper.GetString("output") + output := outputFormat() // Set verbose flag v.Verbose = verbose @@ -95,9 +95,6 @@ func runVersion(cmd *cobra.Command, v Version) error { } v.MiddlewareVersions = latestMW - if isJSONEnabled(cmd) { - return writeJSONSuccess(cmd.OutOrStdout(), v) - } return write(cmd.OutOrStdout(), v, output) } diff --git a/docs/reference/func_languages.md b/docs/reference/func_languages.md index a49c02b716..7a4f35b65f 100644 --- a/docs/reference/func_languages.md +++ b/docs/reference/func_languages.md @@ -46,7 +46,7 @@ func languages ``` -h, --help help for languages - --json Set output to JSON format. ($FUNC_JSON) + --json Output results as JSON ($FUNC_JSON) -r, --repository string URI to a specific repository to consider ($FUNC_REPOSITORY) -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` diff --git a/docs/reference/func_logs.md b/docs/reference/func_logs.md index 98f36cc91c..4033140640 100644 --- a/docs/reference/func_logs.md +++ b/docs/reference/func_logs.md @@ -9,6 +9,9 @@ Stream logs from a deployed function Streams logs for the function in the current directory or from the directory specified with --path. Abstracts away the underlying service name and pod details. +With --json, the logs available at the time of the call are reported as a +finite snapshot instead of being streamed, so the command terminates. + ``` func logs @@ -30,6 +33,9 @@ func logs --namespace my-namespace # Stream logs with a specific time window func logs --since 5m +# Report a finite snapshot of the last 5 minutes of logs as JSON +func logs --since 5m --json + ``` ### Options diff --git a/docs/reference/func_run.md b/docs/reference/func_run.md index 20092f91dc..86b7a17a8f 100644 --- a/docs/reference/func_run.md +++ b/docs/reference/func_run.md @@ -69,7 +69,7 @@ func run -e, --env stringArray Environment variable to set in the form NAME=VALUE. You may provide this flag multiple times for setting multiple environment variables. To unset, specify the environment variable name followed by a "-" (e.g., NAME-). -h, --help help for run -i, --image string Full image name in the form [registry]/[namespace]/[name]:[tag]. This option takes precedence over --registry. Specifying tag is optional. ($FUNC_IMAGE) - --json Output as JSON. ($FUNC_JSON) + --json Output results as JSON ($FUNC_JSON) -p, --path string Path to the function. Default is current directory ($FUNC_PATH) -r, --registry string Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. ($FUNC_REGISTRY) -v, --verbose Print verbose logs ($FUNC_VERBOSE) diff --git a/docs/reference/func_templates.md b/docs/reference/func_templates.md index e9bf6b36de..fd985b06f7 100644 --- a/docs/reference/func_templates.md +++ b/docs/reference/func_templates.md @@ -47,7 +47,7 @@ func templates ``` -h, --help help for templates - --json Set output to JSON format. (Env: $FUNC_JSON) + --json Output results as JSON ($FUNC_JSON) -r, --repository string URI to a specific repository to consider ($FUNC_REPOSITORY) -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` diff --git a/pkg/app/app.go b/pkg/app/app.go index 1e7ef65afb..1c4f3b8fc9 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -10,7 +10,6 @@ import ( "syscall" "github.com/AlecAivazis/survey/v2/terminal" - "github.com/ory/viper" "knative.dev/func/cmd" "knative.dev/func/pkg/docker" @@ -43,8 +42,13 @@ func Main() { os.Exit(130) } - if viper.GetBool("json") { - _ = cmd.WriteJSONError(os.Stdout, err) + if cmd.JSONOutputRequested() { + if jsonErr := cmd.WriteJSONError(os.Stdout, err); jsonErr != nil { + // The envelope could not be written; fall back to plain text + // rather than exiting non-zero with no diagnostic at all. + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + fmt.Fprintf(os.Stderr, "Error: could not write JSON error output: %v\n", jsonErr) + } } else { if !errors.Is(err, terminal.InterruptErr) { fmt.Fprintf(os.Stderr, "Error: %v\n", err) diff --git a/pkg/k8s/logs.go b/pkg/k8s/logs.go index ed06caa1b1..ef7b08562a 100644 --- a/pkg/k8s/logs.go +++ b/pkg/k8s/logs.go @@ -96,24 +96,6 @@ func GetPodLogsBySelector(ctx context.Context, namespace, labelSelector, contain return nil } - mayReadLogs := func(pod corev1.Pod) bool { - for _, status := range pod.Status.ContainerStatuses { - if status.Name == containerName { - return status.State.Running != nil || status.State.Terminated != nil - } - } - return false - } - - getImage := func(pod corev1.Pod) string { - for _, ctr := range pod.Spec.Containers { - if ctr.Name == containerName { - return ctr.Image - } - } - return "" - } - var eg errgroup.Group for event := range w.ResultChan() { @@ -124,7 +106,7 @@ func GetPodLogsBySelector(ctx context.Context, namespace, labelSelector, contain _, loggingAlready := beingProcessed[pod.Name] beingProcessedMu.Unlock() - if !loggingAlready && (image == "" || image == getImage(pod)) && mayReadLogs(pod) { + if !loggingAlready && (image == "" || image == containerImage(pod, containerName)) && mayReadLogs(pod, containerName) { beingProcessedMu.Lock() beingProcessed[pod.Name] = true @@ -144,6 +126,76 @@ func GetPodLogsBySelector(ctx context.Context, namespace, labelSelector, contain return nil } +// GetPodLogsSnapshotBySelector writes the logs currently available from the +// given container of every pod matching labelSelector to out, and returns. +// +// Unlike GetPodLogsBySelector it neither follows the log streams nor watches +// for new pods, so it always terminates. Callers which need a finite result +// (scripts, --json output, agents) use this. +func GetPodLogsSnapshotBySelector(ctx context.Context, namespace, labelSelector, containerName, image string, since *time.Time, out io.Writer) error { + client, namespace, err := NewClientAndResolvedNamespace(namespace) + if err != nil { + return fmt.Errorf("cannot create k8s client: %w", err) + } + + pods := client.CoreV1().Pods(namespace) + + list, err := pods.List(ctx, metav1.ListOptions{LabelSelector: labelSelector}) + if err != nil { + return fmt.Errorf("cannot list pods: %w", err) + } + + podLogOpts := corev1.PodLogOptions{Container: containerName} + if since != nil { + sinceTime := metav1.NewTime(*since) + podLogOpts.SinceTime = &sinceTime + } + + // Pods are read sequentially rather than concurrently: a snapshot has a + // definite end, so concurrency would only buy interleaved, nondeterministic + // output. + for _, pod := range list.Items { + if image != "" && image != containerImage(pod, containerName) { + continue + } + if !mayReadLogs(pod, containerName) { + continue + } + r, e := pods.GetLogs(pod.Name, &podLogOpts).Stream(ctx) + if e != nil { + return fmt.Errorf("cannot get logs of pod %q: %w", pod.Name, e) + } + _, e = io.Copy(out, r) + r.Close() + if e != nil { + return fmt.Errorf("error copying logs of pod %q: %w", pod.Name, e) + } + } + return nil +} + +// mayReadLogs reports whether the named container of pod has reached a state +// in which the API server will serve its logs. +func mayReadLogs(pod corev1.Pod, containerName string) bool { + for _, status := range pod.Status.ContainerStatuses { + if status.Name == containerName { + return status.State.Running != nil || status.State.Terminated != nil + } + } + return false +} + +// containerImage returns the image of the named container of pod, or "" when +// the pod has no such container. +func containerImage(pod corev1.Pod, containerName string) string { + for _, ctr := range pod.Spec.Containers { + if ctr.Name == containerName { + return ctr.Image + } + } + return "" +} + type SynchronizedBuffer struct { b bytes.Buffer mu sync.Mutex diff --git a/pkg/knative/logs.go b/pkg/knative/logs.go index 19790f7d09..20a62d3653 100644 --- a/pkg/knative/logs.go +++ b/pkg/knative/logs.go @@ -20,3 +20,13 @@ func GetKServiceLogs(ctx context.Context, namespace, kServiceName, image string, selector := fmt.Sprintf("serving.knative.dev/service=%s", kServiceName) return k8s.GetPodLogsBySelector(ctx, namespace, selector, "user-container", image, since, out) } + +// GetKServiceLogsSnapshot writes the logs currently available from the +// user-container of the Knative service's pods, and returns. +// +// Unlike GetKServiceLogs it does not follow the log streams, so it terminates +// on its own; callers which need a finite result use this. +func GetKServiceLogsSnapshot(ctx context.Context, namespace, kServiceName, image string, since *time.Time, out io.Writer) error { + selector := fmt.Sprintf("serving.knative.dev/service=%s", kServiceName) + return k8s.GetPodLogsSnapshotBySelector(ctx, namespace, selector, "user-container", image, since, out) +} diff --git a/pkg/mcp/envelope.go b/pkg/mcp/envelope.go new file mode 100644 index 0000000000..8f5922894f --- /dev/null +++ b/pkg/mcp/envelope.go @@ -0,0 +1,62 @@ +package mcp + +import ( + "encoding/json" + "fmt" +) + +// jsonEnvelope mirrors the versioned envelope which every `func` command wraps +// its JSON output in (see cmd/json.go). It is duplicated here rather than +// imported because cmd/mcp.go imports this package, so importing cmd back +// would be an import cycle. +type jsonEnvelope struct { + APIVersion string `json:"apiVersion"` + Status string `json:"status"` + Data json.RawMessage `json:"data,omitempty"` + Error *jsonEnvelopeErr `json:"error,omitempty"` +} + +// jsonEnvelopeErr is the structured failure the CLI reports in JSON mode. +type jsonEnvelopeErr struct { + Category string `json:"category"` + Code string `json:"code"` + Retryable bool `json:"retryable"` + Message string `json:"message"` + Hint string `json:"hint,omitempty"` + Context map[string]string `json:"context,omitempty"` +} + +func (e jsonEnvelopeErr) Error() string { + msg := fmt.Sprintf("%s/%s: %s", e.Category, e.Code, e.Message) + if e.Hint != "" { + msg += " (" + e.Hint + ")" + } + return msg +} + +// unwrapJSON parses a func command's JSON stdout and unmarshals the envelope's +// data payload into out. A "status":"error" envelope is returned as an error +// carrying the CLI's own classification, so tools surface the category, code +// and remediation hint rather than a bare parse failure. +func unwrapJSON(stdout []byte, out any) error { + var env jsonEnvelope + if err := json.Unmarshal(stdout, &env); err != nil { + return fmt.Errorf("failed to parse JSON output: %w\n%s", err, string(stdout)) + } + if env.Status == "error" { + if env.Error != nil { + return *env.Error + } + return fmt.Errorf("command reported an error without detail\n%s", string(stdout)) + } + if env.Status != "ok" { + return fmt.Errorf("unexpected envelope status %q\n%s", env.Status, string(stdout)) + } + if len(env.Data) == 0 { + return fmt.Errorf("JSON output carried no data payload\n%s", string(stdout)) + } + if err := json.Unmarshal(env.Data, out); err != nil { + return fmt.Errorf("failed to parse JSON data payload: %w\n%s", err, string(env.Data)) + } + return nil +} diff --git a/pkg/mcp/envelope_test.go b/pkg/mcp/envelope_test.go new file mode 100644 index 0000000000..32befa5279 --- /dev/null +++ b/pkg/mcp/envelope_test.go @@ -0,0 +1,82 @@ +package mcp + +import ( + "strings" + "testing" +) + +// enveloped wraps a raw payload in the versioned envelope the func CLI emits +// in JSON mode, so executor fixtures state the payload they care about rather +// than repeating envelope boilerplate. +func enveloped(data string) []byte { + return []byte(`{"apiVersion":"v1","status":"ok","data":` + data + `}`) +} + +func TestUnwrapJSON_Success(t *testing.T) { + var out struct { + Name string `json:"name"` + } + if err := unwrapJSON(enveloped(`{"name":"my-function"}`), &out); err != nil { + t.Fatal(err) + } + if out.Name != "my-function" { + t.Errorf("expected name 'my-function', got %q", out.Name) + } +} + +// TestUnwrapJSON_ErrorEnvelope ensures a "status":"error" envelope surfaces the +// CLI's own classification, which is the whole point of the structured +// contract: consumers must not have to substring-match the message. +func TestUnwrapJSON_ErrorEnvelope(t *testing.T) { + stdout := []byte(`{"apiVersion":"v1","status":"error","error":{` + + `"category":"CLUSTER_ERROR","code":"CLUSTER_NOT_ACCESSIBLE",` + + `"retryable":true,"message":"connection refused",` + + `"hint":"Verify your cluster is running"}}`) + + var out map[string]any + err := unwrapJSON(stdout, &out) + if err == nil { + t.Fatal("expected an error for a status:error envelope") + } + + var envErr jsonEnvelopeErr + if !asEnvelopeErr(err, &envErr) { + t.Fatalf("expected a jsonEnvelopeErr, got %T: %v", err, err) + } + if envErr.Category != "CLUSTER_ERROR" { + t.Errorf("expected category 'CLUSTER_ERROR', got %q", envErr.Category) + } + if envErr.Code != "CLUSTER_NOT_ACCESSIBLE" { + t.Errorf("expected code 'CLUSTER_NOT_ACCESSIBLE', got %q", envErr.Code) + } + if !envErr.Retryable { + t.Error("expected retryable true") + } + if !strings.Contains(err.Error(), "Verify your cluster is running") { + t.Errorf("expected the hint in the error text, got %q", err.Error()) + } +} + +func TestUnwrapJSON_NotJSON(t *testing.T) { + var out map[string]any + if err := unwrapJSON([]byte("not json"), &out); err == nil { + t.Fatal("expected an error for non-JSON output") + } +} + +// TestUnwrapJSON_MissingData guards against a bare envelope silently +// unmarshaling into a zero-valued payload. +func TestUnwrapJSON_MissingData(t *testing.T) { + var out map[string]any + if err := unwrapJSON([]byte(`{"apiVersion":"v1","status":"ok"}`), &out); err == nil { + t.Fatal("expected an error when the envelope carries no data") + } +} + +func asEnvelopeErr(err error, target *jsonEnvelopeErr) bool { + e, ok := err.(jsonEnvelopeErr) + if ok { + *target = e + } + return ok +} diff --git a/pkg/mcp/tools_describe.go b/pkg/mcp/tools_describe.go index 270c5b995b..e0e510410d 100644 --- a/pkg/mcp/tools_describe.go +++ b/pkg/mcp/tools_describe.go @@ -2,7 +2,6 @@ package mcp import ( "context" - "encoding/json" "fmt" "strings" @@ -50,8 +49,8 @@ func (s *Server) describeHandler(ctx context.Context, r *mcp.CallToolRequest, in } var instance fn.Instance - if err = json.Unmarshal(stdout, &instance); err != nil { - err = fmt.Errorf("failed to parse describe output: %w\n%s", err, string(stdout)) + if err = unwrapJSON(stdout, &instance); err != nil { + err = fmt.Errorf("failed to read describe output: %w", err) return } diff --git a/pkg/mcp/tools_describe_test.go b/pkg/mcp/tools_describe_test.go index 83776d076d..28027d2b48 100644 --- a/pkg/mcp/tools_describe_test.go +++ b/pkg/mcp/tools_describe_test.go @@ -57,7 +57,7 @@ func TestTool_Describe_Args(t *testing.T) { // fields), so real `func describe --output json` output emits // "Route" capitalized. Using that exact casing here (rather than // "route") keeps this test honest about the real CLI wire format. - return []byte(`{ + return enveloped(`{ "name": "my-function", "namespace": "prod", "Route": "https://my-function.prod.example.com", @@ -144,7 +144,7 @@ func TestTool_Describe_Args(t *testing.T) { func TestTool_Describe_NoMiddleware(t *testing.T) { executor := mock.NewExecutor() executor.ExecuteSplitFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, []byte, error) { - return []byte(`{"name": "my-function"}`), nil, nil + return enveloped(`{"name": "my-function"}`), nil, nil } client, _, err := newTestPair(t, WithExecutor(executor)) @@ -282,7 +282,7 @@ func TestTool_Describe_MalformedJSON(t *testing.T) { func TestTool_Describe_StderrWarningDoesNotBreakParsing(t *testing.T) { executor := mock.NewExecutor() executor.ExecuteSplitFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, []byte, error) { - stdout := []byte(`{ + stdout := enveloped(`{ "name": "my-function", "namespace": "prod", "ready": "true" diff --git a/pkg/mcp/tools_version.go b/pkg/mcp/tools_version.go index 3370869278..e0fec4d897 100644 --- a/pkg/mcp/tools_version.go +++ b/pkg/mcp/tools_version.go @@ -2,7 +2,6 @@ package mcp import ( "context" - "encoding/json" "fmt" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -33,8 +32,8 @@ func (s *Server) versionHandler(ctx context.Context, r *mcp.CallToolRequest, inp Vers string `json:"version,omitempty"` Hash string `json:"commit,omitempty"` } - if err = json.Unmarshal(out, &raw); err != nil { - err = fmt.Errorf("error parsing version output: %w\n%s", err, string(out)) + if err = unwrapJSON(out, &raw); err != nil { + err = fmt.Errorf("error reading version output: %w", err) return } diff --git a/pkg/mcp/tools_version_test.go b/pkg/mcp/tools_version_test.go index 571d1ca260..e838c4fe80 100644 --- a/pkg/mcp/tools_version_test.go +++ b/pkg/mcp/tools_version_test.go @@ -26,7 +26,7 @@ func TestTool_Version(t *testing.T) { }{ "output": {"output", "--output", "json"}, }) - return []byte(`{"version":"v1.16.0","commit":"abc123"}`), nil + return enveloped(`{"version":"v1.16.0","commit":"abc123"}`), nil } client, _, err := newTestPair(t, WithExecutor(executor)) @@ -65,7 +65,7 @@ func TestTool_Version(t *testing.T) { func TestTool_Version_NoCommit(t *testing.T) { executor := mock.NewExecutor() executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { - return []byte(`{"version":"v0.0.0+source"}`), nil + return enveloped(`{"version":"v0.0.0+source"}`), nil } client, _, err := newTestPair(t, WithExecutor(executor))