Skip to content

Commit

Permalink
fix: ensure deprecated flags are correctly hidden (#1974)
Browse files Browse the repository at this point in the history
* ensure flags are correctly hidden

* rename file

* fix lint

* oops

* more generic

* fix linting errors

* print out error

* Update go.mod

* have dev flags shown

* fix typo
  • Loading branch information
raulb authored Nov 14, 2024
1 parent e5c5ea3 commit 3100bc4
Show file tree
Hide file tree
Showing 6 changed files with 187 additions and 19 deletions.
11 changes: 11 additions & 0 deletions cmd/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (

"github.com/conduitio/conduit/pkg/conduit"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
)

var (
Expand Down Expand Up @@ -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
}

Expand Down
83 changes: 83 additions & 0 deletions cmd/cli/cli_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
9 changes: 1 addition & 8 deletions cmd/cli/conduit_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"fmt"
"os"
"path/filepath"
"strings"

"github.com/conduitio/conduit/cmd/cli/internal"
"github.com/conduitio/conduit/pkg/conduit"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
25 changes: 15 additions & 10 deletions pkg/conduit/entrypoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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",
)
Expand Down Expand Up @@ -163,18 +166,20 @@ 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")

// 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)
Expand Down
76 changes: 76 additions & 0 deletions pkg/conduit/entrypoint_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
}

0 comments on commit 3100bc4

Please sign in to comment.