Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 1 addition & 58 deletions pkg/apis/externaldns/binders.go → internal/flags/binders.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package externaldns
package flags

import (
"fmt"
Expand All @@ -23,7 +23,6 @@ import (
"time"

"github.com/alecthomas/kingpin/v2"
"github.com/spf13/cobra"
)

// FlagBinder abstracts flag registration for different CLI backends.
Expand Down Expand Up @@ -143,59 +142,3 @@ func setRegexpDefault(rs regexpSetter, def *regexp.Regexp, name string) {
}
}
}

// CobraBinder implements FlagBinder using github.com/spf13/cobra.
type CobraBinder struct {
Cmd *cobra.Command
}

// NewCobraBinder creates a FlagBinder backed by a Cobra command.
func NewCobraBinder(cmd *cobra.Command) *CobraBinder {
return &CobraBinder{Cmd: cmd}
}

func (b *CobraBinder) StringVar(name, help, def string, target *string) {
b.Cmd.Flags().StringVar(target, name, def, help)
}

func (b *CobraBinder) BoolVar(name, help string, def bool, target *bool) {
b.Cmd.Flags().BoolVar(target, name, def, help)
}

func (b *CobraBinder) DurationVar(name, help string, def time.Duration, target *time.Duration) {
b.Cmd.Flags().DurationVar(target, name, def, help)
}

func (b *CobraBinder) IntVar(name, help string, def int, target *int) {
b.Cmd.Flags().IntVar(target, name, def, help)
}

func (b *CobraBinder) Int64Var(name, help string, def int64, target *int64) {
b.Cmd.Flags().Int64Var(target, name, def, help)
}

func (b *CobraBinder) StringsVar(name, help string, def []string, target *[]string) {
// Preserve repeatable flag semantics.
b.Cmd.Flags().StringArrayVar(target, name, def, help)
}

func (b *CobraBinder) EnumVar(name, help, def string, target *string, allowed ...string) {
b.Cmd.Flags().StringVar(target, name, def, help)
}

func (b *CobraBinder) StringsEnumVar(name, help string, def []string, target *[]string, allowed ...string) {
// pflag does not enforce enums.
b.Cmd.Flags().StringArrayVar(target, name, def, help)
}

func (b *CobraBinder) StringMapVar(name, help string, target *map[string]string) {
// Use StringToStringVar for key=value pairs.
b.Cmd.Flags().StringToStringVar(target, name, map[string]string{}, help)
}

func (b *CobraBinder) RegexpVar(name, help string, def *regexp.Regexp, target **regexp.Regexp) {
rv := &regexpValue{target: target}
// set default value to mirror kingpin's Default(def.String()) behavior
setRegexpDefault(rv, def, name)
b.Cmd.Flags().Var(rv, name, help)
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package externaldns
package flags

import (
"errors"
Expand All @@ -23,7 +23,6 @@ import (
"time"

"github.com/alecthomas/kingpin/v2"
"github.com/spf13/cobra"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -96,67 +95,6 @@ func TestKingpinBinderStringsVarNoDefaultAndBoolDefaultFalse(t *testing.T) {
assert.False(t, b2)
}

func TestCobraBinderParsesAllTypes(t *testing.T) {
cmd := &cobra.Command{Use: "test"}
b := NewCobraBinder(cmd)

var (
s string
bval bool
d time.Duration
i int
i64 int64
ss []string
e string
)

b.StringVar("s", "string flag", "def", &s)
b.BoolVar("b", "bool flag", true, &bval)
b.DurationVar("d", "duration flag", 5*time.Second, &d)
b.IntVar("i", "int flag", 7, &i)
b.Int64Var("i64", "int64 flag", 9, &i64)
b.StringsVar("ss", "strings flag", []string{"x"}, &ss)
b.EnumVar("e", "enum flag", "a", &e, "a", "b")

cmd.SetArgs([]string{"--s=abc", "--b=false", "--d=2s", "--i=42", "--i64=64", "--ss=one", "--ss=two", "--e=b"})
err := cmd.Execute()
require.NoError(t, err)

assert.Equal(t, "abc", s)
assert.False(t, bval)
assert.Equal(t, 2*time.Second, d)
assert.Equal(t, 42, i)
assert.Equal(t, int64(64), i64)
assert.ElementsMatch(t, []string{"one", "two"}, ss)
assert.Equal(t, "b", e)
}

func TestCobraBinderEnumNotValidatedHere(t *testing.T) {
cmd := &cobra.Command{Use: "test"}
b := NewCobraBinder(cmd)

var e string
b.EnumVar("e", "enum flag", "a", &e, "a", "b")

cmd.SetArgs([]string{"--e=c"})
err := cmd.Execute()
require.NoError(t, err)
assert.Equal(t, "c", e)
}

// Cobra requires --<flag>=false
func TestCobraBinderNoBooleanNegationFormUnsupported(t *testing.T) {
cmd := &cobra.Command{Use: "test"}
b := NewCobraBinder(cmd)

var v bool
b.BoolVar("v", "bool flag", true, &v)

cmd.SetArgs([]string{"--no-v"})
err := cmd.Execute()
require.Error(t, err)
}

func TestCobraRegexValueSetStringType(t *testing.T) {
var r *regexp.Regexp
rv := &regexpValue{target: &r}
Expand All @@ -177,40 +115,6 @@ func TestCobraRegexValueSetStringType(t *testing.T) {
assert.Equal(t, "^foo$", rv.String())
}

func TestCobraRegexpVarDefaultAndInvalidValue(t *testing.T) {
cmd := &cobra.Command{Use: "test"}
b := NewCobraBinder(cmd)

var r *regexp.Regexp
// Provide a valid default: should set target immediately
b.RegexpVar("re", "help", regexp.MustCompile("^x+$"), &r)
require.NotNil(t, r)
assert.Equal(t, "^x+$", r.String())

// Executing with an invalid value should produce an error
cmd2 := &cobra.Command{Use: "test2"}
b2 := NewCobraBinder(cmd2)
var r2 *regexp.Regexp
b2.RegexpVar("re", "help", nil, &r2)
cmd2.SetArgs([]string{"--re=invalid("})
err := cmd2.Execute()
require.Error(t, err)
}

func TestCobraStringMapVarDefaultEmpty(t *testing.T) {
cmd := &cobra.Command{Use: "test"}
b := NewCobraBinder(cmd)

var m map[string]string
b.StringMapVar("m", "help", &m)

cmd.SetArgs([]string{})
err := cmd.Execute()
require.NoError(t, err)
require.NotNil(t, m)
assert.Empty(t, m)
}

func TestKingpinRegexpVarDefaultAndParse(t *testing.T) {
app := kingpin.New("test", "")
b := NewKingpinBinder(app)
Expand Down Expand Up @@ -254,26 +158,6 @@ func TestKingpinStringsEnumVarWithAndWithoutDefault(t *testing.T) {
assert.ElementsMatch(t, []string{"a", "c"}, vals2)
}

func TestCobraStringsEnumVarWithAndWithoutDefault(t *testing.T) {
cmd := &cobra.Command{Use: "test"}
b := NewCobraBinder(cmd)

var vals []string
b.StringsEnumVar("se", "help", []string{"x", "y"}, &vals, "x", "y")
cmd.SetArgs([]string{})
require.NoError(t, cmd.Execute())
assert.ElementsMatch(t, []string{"x", "y"}, vals)

// without default
cmd2 := &cobra.Command{Use: "test2"}
b2 := NewCobraBinder(cmd2)
var vals2 []string
b2.StringsEnumVar("se", "help", nil, &vals2, "x", "y")
cmd2.SetArgs([]string{"--se=a", "--se=b"})
require.NoError(t, cmd2.Execute())
assert.ElementsMatch(t, []string{"a", "b"}, vals2)
}

func TestSetRegexDefaultPanicsOnInvalidDefault(t *testing.T) {
bs := &badSetter{}
def := regexp.MustCompile("^")
Expand Down
120 changes: 9 additions & 111 deletions pkg/apis/externaldns/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,20 @@ package externaldns

import (
"fmt"
"os"
"reflect"
"regexp"
"strings"
"time"

"k8s.io/apimachinery/pkg/labels"

"sigs.k8s.io/external-dns/internal/flags"

"sigs.k8s.io/external-dns/endpoint"
"sigs.k8s.io/external-dns/source/annotations"

"github.com/alecthomas/kingpin/v2"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
)

const (
Expand Down Expand Up @@ -490,109 +490,13 @@ func allLogLevelsAsStrings() []string {

// ParseFlags adds and parses flags from command line
func (cfg *Config) ParseFlags(args []string) error {
backend := ""
pruned := make([]string, 0, len(args))
skipNext := false
for i := 0; i < len(args); i++ {
if skipNext {
skipNext = false
continue
}
a := args[i]
if strings.HasPrefix(a, "--cli-backend") {
val := ""
if a == "--cli-backend" {
if i+1 < len(args) {
val = args[i+1]
skipNext = true
}
} else if strings.HasPrefix(a, "--cli-backend=") {
val = strings.TrimPrefix(a, "--cli-backend=")
}
if val != "" {
backend = val
}
continue
}
pruned = append(pruned, a)
}
if backend == "" {
backend = os.Getenv("EXTERNAL_DNS_CLI")
}
if strings.EqualFold(backend, "cobra") {
cmd := newCobraCommand(cfg)
cmd.SetArgs(pruned)
if err := cmd.Execute(); err != nil {
return err
}
return nil
}

app := App(cfg)
_, err := app.Parse(pruned)
if err != nil {
if _, err := App(cfg).Parse(args); err != nil {
return err
}

return nil
}

func newCobraCommand(cfg *Config) *cobra.Command {
cmd := &cobra.Command{
Use: "external-dns",
Short: "ExternalDNS synchronizes exposed Kubernetes Services and Ingresses with DNS providers.",
SilenceUsage: true,
SilenceErrors: true,
RunE: func(cmd *cobra.Command, args []string) error {
return nil
},
}

// Recreate a minimal post-parse validation for Cobra so it behaves like
// Kingpin's Required/Enum validations.
cmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
// Enforce required provider (must be present) like Kingpin.
if cfg.Provider == "" {
return fmt.Errorf("--provider is required when using cobra backend")
}
validProvider := false
for _, p := range providerNames {
if p == cfg.Provider {
validProvider = true
break
}
}
if !validProvider {
return fmt.Errorf("invalid provider %q; valid values: %s", cfg.Provider, strings.Join(providerNames, ", "))
}

// Enforce at least one source is present and validate allowed values.
if len(cfg.Sources) == 0 {
return fmt.Errorf("--source is required when using cobra backend")
}
for _, src := range cfg.Sources {
valid := false
for _, as := range allowedSources {
if src == as {
valid = true
break
}
}
if !valid {
return fmt.Errorf("invalid source %q; valid values: %s", src, strings.Join(allowedSources, ", "))
}
}

return nil
}

b := NewCobraBinder(cmd)
bindFlags(b, cfg)

return cmd
}

func bindFlags(b FlagBinder, cfg *Config) {
func bindFlags(b flags.FlagBinder, cfg *Config) {
// Flags related to Kubernetes
b.StringVar("server", "The Kubernetes API server to connect to (default: auto-detect)", defaultConfig.APIServerURL, &cfg.APIServerURL)
b.StringVar("kubeconfig", "Retrieve target cluster configuration from a Kubernetes configuration file (default: auto-detect)", defaultConfig.KubeConfig, &cfg.KubeConfig)
Expand Down Expand Up @@ -651,14 +555,6 @@ func bindFlags(b FlagBinder, cfg *Config) {

b.StringsVar("events-emit", "Events that should be emitted. Specify multiple times for multiple events support (optional, default: none, expected: RecordReady, RecordDeleted, RecordError)", defaultConfig.EmitEvents, &cfg.EmitEvents)

// Flags related to providers
if _, ok := b.(*CobraBinder); ok {
providerHelp := "The DNS provider where the DNS records will be created (required, options: " + strings.Join(providerNames, ", ") + ")"
b.StringVar("provider", providerHelp, cfg.Provider, &cfg.Provider)

sourceHelp := "The resource types that are queried for endpoints; specify multiple times for multiple sources (required, options: " + strings.Join(allowedSources, ", ") + ")"
b.StringsVar("source", sourceHelp, append([]string(nil), cfg.Sources...), &cfg.Sources)
}
b.DurationVar("provider-cache-time", "The time to cache the DNS provider record list requests.", defaultConfig.ProviderCacheTime, &cfg.ProviderCacheTime)
b.StringsVar("domain-filter", "Limit possible target zones by a domain suffix; specify multiple times for multiple domains (optional)", []string{""}, &cfg.DomainFilter)
b.StringsVar("exclude-domains", "Exclude subdomains (optional)", []string{""}, &cfg.ExcludeDomains)
Expand Down Expand Up @@ -821,14 +717,16 @@ func App(cfg *Config) *kingpin.Application {
app.Version(Version)
app.DefaultEnvars()

bindFlags(NewKingpinBinder(app), cfg)
bindFlags(flags.NewKingpinBinder(app), cfg)

// Kingpin-only semantics: preserve Required/PlaceHolder and enum validation
// that Kingpin provided before the flags were migrated into the binder.
app.Flag("provider", "The DNS provider where the DNS records will be created (required, options: akamai, alibabacloud, aws, aws-sd, azure, azure-dns, azure-private-dns, civo, cloudflare, coredns, digitalocean, dnsimple, exoscale, gandi, godaddy, google, inmemory, linode, ns1, oci, ovh, pdns, pihole, plural, rfc2136, scaleway, skydns, transip, webhook)").Required().PlaceHolder("provider").EnumVar(&cfg.Provider, providerNames...)
providerHelp := "The DNS provider where the DNS records will be created (required, options: " + strings.Join(providerNames, ", ") + ")"
app.Flag("provider", providerHelp).Required().PlaceHolder("provider").EnumVar(&cfg.Provider, providerNames...)

// Reintroduce source enum/required validation in Kingpin to match previous behavior.
app.Flag("source", "The resource types that are queried for endpoints; specify multiple times for multiple sources (required, options: service, ingress, node, pod, fake, connector, gateway-httproute, gateway-grpcroute, gateway-tlsroute, gateway-tcproute, gateway-udproute, istio-gateway, istio-virtualservice, cloudfoundry, contour-httpproxy, gloo-proxy, crd, empty, skipper-routegroup, openshift-route, ambassador-host, kong-tcpingress, f5-virtualserver, f5-transportserver, traefik-proxy)").Required().PlaceHolder("source").EnumsVar(&cfg.Sources, "service", "ingress", "node", "pod", "gateway-httproute", "gateway-grpcroute", "gateway-tlsroute", "gateway-tcproute", "gateway-udproute", "istio-gateway", "istio-virtualservice", "cloudfoundry", "contour-httpproxy", "gloo-proxy", "fake", "connector", "crd", "empty", "skipper-routegroup", "openshift-route", "ambassador-host", "kong-tcpingress", "f5-virtualserver", "f5-transportserver", "traefik-proxy")
sourceHelp := "The resource types that are queried for endpoints; specify multiple times for multiple sources (required, options: " + strings.Join(allowedSources, ", ") + ")"
app.Flag("source", sourceHelp).Required().PlaceHolder("source").EnumsVar(&cfg.Sources, allowedSources...)

return app
}
Loading
Loading