diff --git a/cmd/cli/cli.go b/cmd/cli/cli.go index 7849903d3..90c39509b 100644 --- a/cmd/cli/cli.go +++ b/cmd/cli/cli.go @@ -20,6 +20,7 @@ import ( "github.com/conduitio/conduit/pkg/conduit" "github.com/spf13/cobra" + "github.com/spf13/pflag" ) var ( @@ -71,6 +72,16 @@ func buildRootCmd() *cobra.Command { }) cmd.AddCommand(buildPipelinesCmd()) + // mark hidden flags + cmd.Flags().VisitAll(func(f *pflag.Flag) { + if conduit.HiddenFlags[f.Name] { + err := cmd.Flags().MarkHidden(f.Name) + if err != nil { + fmt.Fprintf(os.Stderr, "failed to mark flag %q as hidden: %v", f.Name, err) + } + } + }) + return cmd } diff --git a/cmd/cli/cli_test.go b/cmd/cli/cli_test.go new file mode 100644 index 000000000..7bca0b0d1 --- /dev/null +++ b/cmd/cli/cli_test.go @@ -0,0 +1,83 @@ +// Copyright © 2024 Meroxa, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cli + +import ( + "bytes" + "strings" + "testing" + + "github.com/conduitio/conduit/pkg/conduit" +) + +func TestBuildRootCmd_HelpOutput(t *testing.T) { + cmd := buildRootCmd() + + var buf bytes.Buffer + cmd.SetOut(&buf) + cmd.SetArgs([]string{"--help"}) + + err := cmd.Execute() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + output := buf.String() + + expectedFlags := []string{ + "db.type", + "db.badger.path", + "db.postgres.connection-string", + "db.postgres.table", + "db.sqlite.path", + "db.sqlite.table", + "dev.blockprofileblockprofile", + "dev.cpuprofile", + "dev.memprofile", + "api.enabled", + "http.address", + "grpc.address", + "log.level", + "log.format", + "connectors.path", + "processors.path", + "pipelines.path", + "pipelines.exit-on-degraded", + "pipelines.error-recovery.min-delay", + "pipelines.error-recovery.max-delay", + "pipelines.error-recovery.backoff-factor", + "pipelines.error-recovery.max-retries", + "pipelines.error-recovery.max-retries-window", + "schema-registry.type", + "schema-registry.confluent.connection-string", + "preview.pipeline-arch-v2", + } + + unexpectedFlags := []string{ + conduit.FlagPipelinesExitOnError, //nolint:staticcheck // this will be completely removed before Conduit 1.0 + } + + for _, flag := range expectedFlags { + if !strings.Contains(output, flag) { + t.Errorf("expected flag %q not found in help output", flag) + } + } + + for _, flag := range unexpectedFlags { + if strings.Contains(output, flag) { + t.Errorf("unexpected flag %q found in help output", flag) + } + } +} diff --git a/cmd/cli/conduit_init.go b/cmd/cli/conduit_init.go index e0d23ac2f..40ece5351 100644 --- a/cmd/cli/conduit_init.go +++ b/cmd/cli/conduit_init.go @@ -19,7 +19,6 @@ import ( "fmt" "os" "path/filepath" - "strings" "github.com/conduitio/conduit/cmd/cli/internal" "github.com/conduitio/conduit/pkg/conduit" @@ -62,7 +61,7 @@ To see how you can customize your first pipeline, run 'conduit pipelines init -- func (i *ConduitInit) createConfigYAML() error { cfgYAML := internal.NewYAMLTree() i.conduitCfgFlags().VisitAll(func(f *flag.Flag) { - if i.isHiddenFlag(f.Name) { + if conduit.HiddenFlags[f.Name] { return // hide flag from output } cfgYAML.Insert(f.Name, f.DefValue, f.Usage) @@ -104,12 +103,6 @@ func (i *ConduitInit) createDirs() error { return nil } -func (i *ConduitInit) isHiddenFlag(name string) bool { - return name == "dev" || - strings.HasPrefix(name, "dev.") || - conduit.DeprecatedFlags[name] -} - func (i *ConduitInit) conduitCfgFlags() *flag.FlagSet { cfg := conduit.DefaultConfigWithBasePath(i.path()) return conduit.Flags(&cfg) diff --git a/go.mod b/go.mod index bef5f1379..d4ab79682 100644 --- a/go.mod +++ b/go.mod @@ -42,6 +42,7 @@ require ( github.com/rs/zerolog v1.33.0 github.com/sourcegraph/conc v0.3.0 github.com/spf13/cobra v1.8.1 + github.com/spf13/pflag v1.0.5 github.com/stealthrocket/wazergo v0.19.1 github.com/tetratelabs/wazero v1.8.1 github.com/twmb/franz-go/pkg/sr v1.2.0 @@ -319,7 +320,6 @@ require ( github.com/sourcegraph/go-diff v0.7.0 // indirect github.com/spf13/afero v1.11.0 // indirect github.com/spf13/cast v1.7.0 // indirect - github.com/spf13/pflag v1.0.5 // indirect github.com/spf13/viper v1.19.0 // indirect github.com/ssgreg/nlreturn/v2 v2.2.1 // indirect github.com/stbenjam/no-sprintf-host-port v0.1.1 // indirect diff --git a/pkg/conduit/entrypoint.go b/pkg/conduit/entrypoint.go index a3bf84b4b..259dd3fdf 100644 --- a/pkg/conduit/entrypoint.go +++ b/pkg/conduit/entrypoint.go @@ -20,22 +20,25 @@ import ( "fmt" "os" "os/signal" - "strings" "github.com/conduitio/conduit/pkg/foundation/cerrors" "github.com/peterbourgon/ff/v3" "github.com/peterbourgon/ff/v3/ffyaml" ) -var DeprecatedFlags = map[string]bool{ - "pipelines.exit-on-error": true, -} - const ( exitCodeErr = 1 exitCodeInterrupt = 2 + + // Deprecated: Use `pipelines.error-recovery.exit-on-degraded` instead. + FlagPipelinesExitOnError = "pipelines.exit-on-error" ) +// HiddenFlags is a map of flags that should not be shown in the help output. +var HiddenFlags = map[string]bool{ + FlagPipelinesExitOnError: true, +} + // Serve is a shortcut for Entrypoint.Serve. func Serve(cfg Config) { e := &Entrypoint{} @@ -115,7 +118,7 @@ func Flags(cfg *Config) *flag.FlagSet { // Note: If both `pipeline.exit-on-error` and `pipeline.exit-on-degraded` are set, `pipeline.exit-on-degraded` will take precedence flags.BoolVar( &cfg.Pipelines.ExitOnDegraded, - "pipelines.exit-on-error", + FlagPipelinesExitOnError, cfg.Pipelines.ExitOnDegraded, "Deprecated: use `exit-on-degraded` instead.\nexit Conduit if a pipeline experiences an error while running", ) @@ -163,8 +166,6 @@ func Flags(cfg *Config) *flag.FlagSet { flags.BoolVar(&cfg.Preview.PipelineArchV2, "preview.pipeline-arch-v2", cfg.Preview.PipelineArchV2, "enables experimental pipeline architecture v2 (note that the new architecture currently supports only 1 source and 1 destination per pipeline)") - // NB: flags with prefix dev.* are hidden from help output by default, they only show up using '-dev -help' - showDevHelp := flags.Bool("dev", false, "used together with the dev flag it shows dev flags") flags.StringVar(&cfg.dev.cpuprofile, "dev.cpuprofile", "", "write cpu profile to file") flags.StringVar(&cfg.dev.memprofile, "dev.memprofile", "", "write memory profile to file") flags.StringVar(&cfg.dev.blockprofile, "dev.blockprofile", "", "write block profile to file") @@ -172,9 +173,13 @@ func Flags(cfg *Config) *flag.FlagSet { // show user or dev flags flags.Usage = func() { tmpFlags := flag.NewFlagSet("conduit", flag.ExitOnError) + + // preserve original flag's output to the same writer + tmpFlags.SetOutput(flags.Output()) + flags.VisitAll(func(f *flag.Flag) { - if f.Name == "dev" || strings.HasPrefix(f.Name, "dev.") != *showDevHelp || DeprecatedFlags[f.Name] { - return // hide flag from output + if HiddenFlags[f.Name] { + return } // reset value to its default, to ensure default is shown correctly _ = f.Value.Set(f.DefValue) diff --git a/pkg/conduit/entrypoint_test.go b/pkg/conduit/entrypoint_test.go new file mode 100644 index 000000000..21c56bdad --- /dev/null +++ b/pkg/conduit/entrypoint_test.go @@ -0,0 +1,76 @@ +// Copyright © 2024 Meroxa, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package conduit + +import ( + "bytes" + "strings" + "testing" +) + +func TestFlags_HelpOutput(t *testing.T) { + var buf bytes.Buffer + + flags := Flags(&Config{}) + flags.SetOutput(&buf) + + flags.Usage() + output := buf.String() + + expectedFlags := []string{ + "db.type", + "db.badger.path", + "db.postgres.connection-string", + "db.postgres.table", + "db.sqlite.path", + "db.sqlite.table", + "dev.blockprofileblockprofile", + "dev.cpuprofile", + "dev.memprofile", + "api.enabled", + "http.address", + "grpc.address", + "log.level", + "log.format", + "connectors.path", + "processors.path", + "pipelines.path", + "pipelines.exit-on-degraded", + "pipelines.error-recovery.min-delay", + "pipelines.error-recovery.max-delay", + "pipelines.error-recovery.backoff-factor", + "pipelines.error-recovery.max-retries", + "pipelines.error-recovery.max-retries-window", + "schema-registry.type", + "schema-registry.confluent.connection-string", + "preview.pipeline-arch-v2", + } + + unexpectedFlags := []string{ + FlagPipelinesExitOnError, + } + + for _, flag := range expectedFlags { + if !strings.Contains(output, flag) { + t.Errorf("expected flag %q not found in help output", flag) + } + } + + for _, flag := range unexpectedFlags { + if strings.Contains(output, flag) { + t.Errorf("unexpected flag %q found in help output", flag) + } + } +}