diff --git a/pkg/apis/externaldns/binders.go b/pkg/apis/externaldns/binders.go new file mode 100644 index 0000000000..70dcd1a7b1 --- /dev/null +++ b/pkg/apis/externaldns/binders.go @@ -0,0 +1,121 @@ +/* +Copyright 2025 The Kubernetes Authors. + +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 externaldns + +import ( + "strconv" + "time" + + "github.com/alecthomas/kingpin/v2" + "github.com/spf13/cobra" +) + +// FlagBinder abstracts flag registration for different CLI backends. +type FlagBinder interface { + StringVar(name, help, def string, target *string) + BoolVar(name, help string, def bool, target *bool) + DurationVar(name, help string, def time.Duration, target *time.Duration) + IntVar(name, help string, def int, target *int) + Int64Var(name, help string, def int64, target *int64) + StringsVar(name, help string, def []string, target *[]string) + EnumVar(name, help, def string, target *string, allowed ...string) +} + +// KingpinBinder implements FlagBinder using github.com/alecthomas/kingpin/v2. +type KingpinBinder struct { + App *kingpin.Application +} + +// NewKingpinBinder creates a FlagBinder backed by a kingpin Application. +func NewKingpinBinder(app *kingpin.Application) *KingpinBinder { + return &KingpinBinder{App: app} +} + +func (b *KingpinBinder) StringVar(name, help, def string, target *string) { + b.App.Flag(name, help).Default(def).StringVar(target) +} + +func (b *KingpinBinder) BoolVar(name, help string, def bool, target *bool) { + if def { + b.App.Flag(name, help).Default("true").BoolVar(target) + } else { + b.App.Flag(name, help).Default("false").BoolVar(target) + } +} + +func (b *KingpinBinder) DurationVar(name, help string, def time.Duration, target *time.Duration) { + b.App.Flag(name, help).Default(def.String()).DurationVar(target) +} + +func (b *KingpinBinder) IntVar(name, help string, def int, target *int) { + b.App.Flag(name, help).Default(strconv.Itoa(def)).IntVar(target) +} + +func (b *KingpinBinder) Int64Var(name, help string, def int64, target *int64) { + b.App.Flag(name, help).Default(strconv.FormatInt(def, 10)).Int64Var(target) +} + +func (b *KingpinBinder) StringsVar(name, help string, def []string, target *[]string) { + if len(def) > 0 { + b.App.Flag(name, help).Default(def...).StringsVar(target) + return + } + b.App.Flag(name, help).StringsVar(target) +} + +func (b *KingpinBinder) EnumVar(name, help, def string, target *string, allowed ...string) { + b.App.Flag(name, help).Default(def).EnumVar(target, allowed...) +} + +// 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) +} diff --git a/pkg/apis/externaldns/binders_test.go b/pkg/apis/externaldns/binders_test.go new file mode 100644 index 0000000000..3200a06f37 --- /dev/null +++ b/pkg/apis/externaldns/binders_test.go @@ -0,0 +1,139 @@ +/* +Copyright 2025 The Kubernetes Authors. + +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 externaldns + +import ( + "testing" + "time" + + "github.com/alecthomas/kingpin/v2" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestKingpinBinderParsesAllTypes(t *testing.T) { + app := kingpin.New("test", "") + b := NewKingpinBinder(app) + + 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") + + _, err := app.Parse([]string{"--s=abc", "--no-b", "--d=2s", "--i=42", "--i64=64", "--ss=one", "--ss=two", "--e=b"}) + 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 TestKingpinBinderEnumValidation(t *testing.T) { + app := kingpin.New("test", "") + b := NewKingpinBinder(app) + + var e string + b.EnumVar("e", "enum flag", "a", &e, "a", "b") + + _, err := app.Parse([]string{"--e=c"}) + require.Error(t, err) +} + +func TestKingpinBinderStringsVarNoDefaultAndBoolDefaultFalse(t *testing.T) { + app := kingpin.New("test", "") + b := NewKingpinBinder(app) + + var ( + ss []string + b2 bool + ) + + b.StringsVar("ss", "strings flag", nil, &ss) + b.BoolVar("b2", "bool2 flag", false, &b2) + + _, err := app.Parse([]string{}) + require.NoError(t, err) + + assert.Empty(t, ss) + 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) +} diff --git a/pkg/apis/externaldns/types.go b/pkg/apis/externaldns/types.go index 9df6218d34..0d5c39e017 100644 --- a/pkg/apis/externaldns/types.go +++ b/pkg/apis/externaldns/types.go @@ -18,6 +18,7 @@ package externaldns import ( "fmt" + "os" "reflect" "regexp" "strconv" @@ -30,6 +31,7 @@ import ( "github.com/alecthomas/kingpin/v2" "github.com/sirupsen/logrus" + "github.com/spf13/cobra" ) const ( @@ -421,9 +423,46 @@ func allLogLevelsAsStrings() []string { // ParseFlags adds and parses flags from command line func (cfg *Config) ParseFlags(args []string) error { - app := App(cfg) + 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 + } - _, err := app.Parse(args) + app := App(cfg) + _, err := app.Parse(pruned) if err != nil { return err } @@ -431,6 +470,33 @@ func (cfg *Config) ParseFlags(args []string) error { 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 + }, + } + + b := NewCobraBinder(cmd) + + b.EnumVar("provider", "The DNS provider where the DNS records will be created.", defaultConfig.Provider, &cfg.Provider) + b.StringsVar("source", "The resource types that are queried for endpoints; specify multiple times for multiple sources.", cfg.Sources, &cfg.Sources) + + 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) + b.DurationVar("request-timeout", "Request timeout when calling Kubernetes APIs. 0s means no timeout", defaultConfig.RequestTimeout, &cfg.RequestTimeout) + + b.StringVar("namespace", "Limit resources queried for endpoints to a specific namespace (default: all namespaces)", defaultConfig.Namespace, &cfg.Namespace) + b.StringsVar("domain-filter", "Limit possible target zones by domain suffix (optional)", defaultConfig.DomainFilter, &cfg.DomainFilter) + b.StringVar("openshift-router-name", "if source is openshift-route then you can pass the ingress controller name.", cfg.OCPRouterName, &cfg.OCPRouterName) + + return cmd +} + func (cfg *Config) AddSourceWrapper(name string) { if cfg.sourceWrappers == nil { cfg.sourceWrappers = make(map[string]bool) diff --git a/pkg/apis/externaldns/types_test.go b/pkg/apis/externaldns/types_test.go index 2c869db8a9..0e65e6e153 100644 --- a/pkg/apis/externaldns/types_test.go +++ b/pkg/apis/externaldns/types_test.go @@ -532,6 +532,181 @@ func setEnv(t *testing.T, env map[string]string) map[string]string { return originalEnv } +// Additional tests to verify the experimental CLI switch behavior. +// Default path should use kingpin and parse flags correctly. +func TestParseFlagsDefaultKingpin(t *testing.T) { + t.Setenv("EXTERNAL_DNS_CLI", "") + + args := []string{ + "--provider=aws", + "--source=service", + "--source=ingress", + "--server=http://127.0.0.1:8080", + "--kubeconfig=/some/path", + "--request-timeout=2s", + "--namespace=ns", + "--domain-filter=example.org", + "--domain-filter=company.com", + "--openshift-router-name=default", + } + + cfg := NewConfig() + require.NoError(t, cfg.ParseFlags(args)) + + assert.Equal(t, "aws", cfg.Provider) + assert.ElementsMatch(t, []string{"service", "ingress"}, cfg.Sources) + assert.Equal(t, "http://127.0.0.1:8080", cfg.APIServerURL) + assert.Equal(t, "/some/path", cfg.KubeConfig) + assert.Equal(t, 2*time.Second, cfg.RequestTimeout) + assert.Equal(t, "ns", cfg.Namespace) + assert.ElementsMatch(t, []string{"example.org", "company.com"}, cfg.DomainFilter) + assert.Equal(t, "default", cfg.OCPRouterName) +} + +// When EXTERNAL_DNS_CLI=cobra is set, cobra path should parse the subset of +// flags it currently binds, yielding parity with kingpin for those fields. +func TestParseFlagsCobraSwitchParitySubset(t *testing.T) { + args := []string{ + "--provider=aws", + "--source=service", + "--source=ingress", + "--server=http://127.0.0.1:8080", + "--kubeconfig=/some/path", + "--request-timeout=2s", + "--namespace=ns", + "--domain-filter=example.org", + "--domain-filter=company.com", + "--openshift-router-name=default", + } + + // Kingpin baseline + cfgK := NewConfig() + require.NoError(t, cfgK.ParseFlags(args)) + + // Cobra path via env switch + t.Setenv("EXTERNAL_DNS_CLI", "cobra") + cfgC := NewConfig() + require.NoError(t, cfgC.ParseFlags(args)) + + // Compare selected fields bound in cobra + assert.Equal(t, cfgK.Provider, cfgC.Provider) + assert.ElementsMatch(t, cfgK.Sources, cfgC.Sources) + assert.Equal(t, cfgK.APIServerURL, cfgC.APIServerURL) + assert.Equal(t, cfgK.KubeConfig, cfgC.KubeConfig) + assert.Equal(t, cfgK.RequestTimeout, cfgC.RequestTimeout) + assert.Equal(t, cfgK.Namespace, cfgC.Namespace) + assert.ElementsMatch(t, cfgK.DomainFilter, cfgC.DomainFilter) + assert.Equal(t, cfgK.OCPRouterName, cfgC.OCPRouterName) +} + +func TestNewCobraCommandBindsAndParsesSubset(t *testing.T) { + cfg := NewConfig() + cmd := newCobraCommand(cfg) + + args := []string{ + "--provider=aws", + "--source=service", + "--source=ingress", + "--server=http://127.0.0.1:8080", + "--kubeconfig=/some/path", + "--request-timeout=2s", + "--namespace=ns", + "--domain-filter=example.org", + "--domain-filter=company.com", + "--openshift-router-name=default", + } + + cmd.SetArgs(args) + require.NoError(t, cmd.Execute()) + + assert.Equal(t, "aws", cfg.Provider) + assert.ElementsMatch(t, []string{"service", "ingress"}, cfg.Sources) + assert.Equal(t, "http://127.0.0.1:8080", cfg.APIServerURL) + assert.Equal(t, "/some/path", cfg.KubeConfig) + assert.Equal(t, 2*time.Second, cfg.RequestTimeout) + assert.Equal(t, "ns", cfg.Namespace) + assert.ElementsMatch(t, []string{"example.org", "company.com"}, cfg.DomainFilter) + assert.Equal(t, "default", cfg.OCPRouterName) +} + +func TestNewCobraCommandDefaultsApplied(t *testing.T) { + cfg := NewConfig() + // Pre-populate some defaults to confirm they persist without args. + cfg.Sources = []string{"service"} + + cmd := newCobraCommand(cfg) + cmd.SetArgs([]string{}) + require.NoError(t, cmd.Execute()) + + assert.ElementsMatch(t, []string{"service"}, cfg.Sources) + // DomainFilter uses the package default for this binding. + assert.Empty(t, cfg.DomainFilter) +} + +func TestParseFlagsCliFlagCobraParitySubset(t *testing.T) { + args := []string{ + "--cli-backend=cobra", + "--provider=aws", + "--source=service", + "--source=ingress", + "--server=http://127.0.0.1:8080", + "--kubeconfig=/some/path", + "--request-timeout=2s", + "--namespace=ns", + "--domain-filter=example.org", + "--domain-filter=company.com", + "--openshift-router-name=default", + } + + // Kingpin baseline without the hidden flag + baselineArgs := append([]string{}, args[1:]...) + cfgK := NewConfig() + require.NoError(t, cfgK.ParseFlags(baselineArgs)) + + cfgC := NewConfig() + require.NoError(t, cfgC.ParseFlags(args)) + + assert.Equal(t, cfgK.Provider, cfgC.Provider) + assert.ElementsMatch(t, cfgK.Sources, cfgC.Sources) + assert.Equal(t, cfgK.APIServerURL, cfgC.APIServerURL) + assert.Equal(t, cfgK.KubeConfig, cfgC.KubeConfig) + assert.Equal(t, cfgK.RequestTimeout, cfgC.RequestTimeout) + assert.Equal(t, cfgK.Namespace, cfgC.Namespace) + assert.ElementsMatch(t, cfgK.DomainFilter, cfgC.DomainFilter) + assert.Equal(t, cfgK.OCPRouterName, cfgC.OCPRouterName) +} + +func TestParseFlagsCliFlagOverridesEnv(t *testing.T) { + // Env requests cobra; CLI flag forces kingpin. + t.Setenv("EXTERNAL_DNS_CLI", "cobra") + args := []string{ + "--cli-backend=kingpin", + "--provider=aws", + "--source=service", + // Flag not bound in Cobra newCobraCommand path; will error if cobra is used. + "--log-format=json", + } + + cfg := NewConfig() + require.NoError(t, cfg.ParseFlags(args)) + assert.Equal(t, "aws", cfg.Provider) + assert.ElementsMatch(t, []string{"service"}, cfg.Sources) + assert.Equal(t, "json", cfg.LogFormat) +} + +func TestParseFlagsCliFlagSeparatedValue(t *testing.T) { + // Support "--cli-backend", "cobra" form as well. + args := []string{ + "--cli-backend", "cobra", + "--provider=aws", + "--source=service", + } + cfg := NewConfig() + require.NoError(t, cfg.ParseFlags(args)) + assert.Equal(t, "aws", cfg.Provider) + assert.ElementsMatch(t, []string{"service"}, cfg.Sources) +} + func restoreEnv(t *testing.T, originalEnv map[string]string) { for k, v := range originalEnv { require.NoError(t, os.Setenv(k, v))