From 5e01e9fd173c387dd4a58ebb57c546957fa0fe72 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Wed, 27 May 2026 15:28:08 +0800 Subject: [PATCH] fix(config-validate): emit structured output for -o json/yaml on success `a7 config validate -f -o json` previously printed the human-readable string `Config is valid` on stdout and exited 0, ignoring the requested format. Pipelines like `... -o json | jq '.valid'` failed with a parse error. Read the inherited --output flag in RunE; on the success path, emit a `validateResult{Valid: true, Message: "Config is valid"}` via the shared cmdutil exporter for json/yaml. Table / unset output is unchanged, so interactive users keep seeing the friendly message. Adds three unit tests covering json, yaml, and the default path. --- pkg/cmd/config/validate/validate.go | 19 ++++++- pkg/cmd/config/validate/validate_test.go | 69 ++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/config/validate/validate.go b/pkg/cmd/config/validate/validate.go index 421107c..b91dda2 100644 --- a/pkg/cmd/config/validate/validate.go +++ b/pkg/cmd/config/validate/validate.go @@ -26,7 +26,8 @@ type Options struct { Client func() (*http.Client, error) Config func() (config.Config, error) - File string + File string + Output string } func NewCmdValidate(f *cmd.Factory) *cobra.Command { @@ -44,6 +45,7 @@ func NewCmdValidate(f *cmd.Factory) *cobra.Command { if opts.File == "" { return &cmdutil.FlagError{Err: fmt.Errorf("required flag \"file\" not set")} } + opts.Output, _ = c.Flags().GetString("output") return validateRun(opts) }, } @@ -53,6 +55,14 @@ func NewCmdValidate(f *cmd.Factory) *cobra.Command { return c } +// validateResult is emitted on the success path when -o json or -o yaml is +// requested. Keeping it small and stable means downstream automation can pipe +// `a7 config validate -o json | jq '.valid'` without parsing English. +type validateResult struct { + Valid bool `json:"valid" yaml:"valid"` + Message string `json:"message,omitempty" yaml:"message,omitempty"` +} + func validateRun(opts *Options) error { data, err := readFile(opts.File) if err != nil { @@ -76,6 +86,13 @@ func validateRun(opts *Options) error { return fmt.Errorf("config validation failed:\n- %s", strings.Join(errs, "\n- ")) } + switch opts.Output { + case "json", "yaml": + return cmdutil.NewExporter(opts.Output, opts.IO.Out).Write(validateResult{ + Valid: true, + Message: "Config is valid", + }) + } fmt.Fprintln(opts.IO.Out, "Config is valid") return nil } diff --git a/pkg/cmd/config/validate/validate_test.go b/pkg/cmd/config/validate/validate_test.go index 5ae55e7..b50f188 100644 --- a/pkg/cmd/config/validate/validate_test.go +++ b/pkg/cmd/config/validate/validate_test.go @@ -1,11 +1,13 @@ package validate import ( + "encoding/json" "net/http" "os" "path/filepath" "testing" + "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -14,6 +16,18 @@ import ( "github.com/api7/a7/pkg/iostreams" ) +// newCmdWithOutputFlag returns NewCmdValidate wrapped under a parent cobra.Command +// that registers the global --output persistent flag, matching how the root +// command in pkg/cmd/root/root.go wires things up. Unit tests need this so +// `c.SetArgs("-o", "json")` resolves the inherited flag. +func newCmdWithOutputFlag(t *testing.T, ios *iostreams.IOStreams) *cobra.Command { + t.Helper() + root := &cobra.Command{Use: "a7"} + root.PersistentFlags().StringP("output", "o", "", "Output format: json, yaml (default: table)") + root.AddCommand(NewCmdValidate(factoryWithIO(ios))) + return root +} + type mockConfig struct { baseURL string } @@ -287,6 +301,61 @@ func TestConfigValidate_MissingFileFlag(t *testing.T) { assert.Contains(t, err.Error(), "required flag \"file\" not set") } +// TestConfigValidate_SuccessOutputJSON asserts that `-o json` on the success +// path emits a parseable JSON object rather than the human-readable string. +// Regression for the case where the CLI ignored -o on success and broke +// scripted callers piping into `jq`. +func TestConfigValidate_SuccessOutputJSON(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + + filePath := filepath.Join(t.TempDir(), "config.yaml") + require.NoError(t, os.WriteFile(filePath, []byte("version: \"1\"\nservices: []\n"), 0o644)) + + root := newCmdWithOutputFlag(t, ios) + root.SetArgs([]string{"validate", "-f", filePath, "-o", "json"}) + require.NoError(t, root.Execute()) + + var result struct { + Valid bool `json:"valid"` + Message string `json:"message"` + } + require.NoError(t, json.Unmarshal(stdout.Bytes(), &result), "stdout must be valid JSON, got: %s", stdout.String()) + assert.True(t, result.Valid) + assert.Equal(t, "Config is valid", result.Message) +} + +// TestConfigValidate_SuccessOutputYAML asserts the parallel YAML path. +func TestConfigValidate_SuccessOutputYAML(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + + filePath := filepath.Join(t.TempDir(), "config.yaml") + require.NoError(t, os.WriteFile(filePath, []byte("version: \"1\"\nservices: []\n"), 0o644)) + + root := newCmdWithOutputFlag(t, ios) + root.SetArgs([]string{"validate", "-f", filePath, "-o", "yaml"}) + require.NoError(t, root.Execute()) + + out := stdout.String() + assert.Contains(t, out, "valid: true") + assert.Contains(t, out, "message: Config is valid") +} + +// TestConfigValidate_SuccessDefaultOutput keeps the existing +// "Config is valid" behavior for table / unset output, so users running the +// command interactively still see the friendly message. +func TestConfigValidate_SuccessDefaultOutput(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + + filePath := filepath.Join(t.TempDir(), "config.yaml") + require.NoError(t, os.WriteFile(filePath, []byte("version: \"1\"\nservices: []\n"), 0o644)) + + root := newCmdWithOutputFlag(t, ios) + root.SetArgs([]string{"validate", "-f", filePath}) + require.NoError(t, root.Execute()) + + assert.Contains(t, stdout.String(), "Config is valid") +} + // TestConfigValidate_EmptyUnsupportedSections asserts that declaring an // unsupported top-level section (upstreams, consumer_groups, service_templates) // is rejected even when the section is explicitly empty. Presence alone is