diff --git a/internal/manager/config.go b/internal/manager/config.go index 95006f0660..308a4858a9 100644 --- a/internal/manager/config.go +++ b/internal/manager/config.go @@ -21,6 +21,9 @@ import ( "github.com/kong/kubernetes-ingress-controller/v2/internal/manager/featuregates" ) +// Type override to be used with types.NamespacedName variables to override their type name printed in the help text. +var nnTypeNameOverride = WithTypeNameOverride[mo.Option[types.NamespacedName]]("namespacedName") + // ----------------------------------------------------------------------------- // Controller Manager - Config // ----------------------------------------------------------------------------- @@ -151,7 +154,7 @@ func (c *Config) FlagSet() *pflag.FlagSet { flagSet.StringSliceVar(&c.KongAdminURLs, "kong-admin-url", []string{"http://localhost:8001"}, `Kong Admin URL(s) to connect to in the format "protocol://address:port". `+ `More than 1 URL can be provided, in such case the flag should be used multiple times or a corresponding env variable should use comma delimited addresses.`) - flagSet.Var(NewValidatedValue(&c.KongAdminSvc, namespacedNameFromFlagValue), "kong-admin-svc", + flagSet.Var(NewValidatedValue(&c.KongAdminSvc, namespacedNameFromFlagValue, nnTypeNameOverride), "kong-admin-svc", `Kong Admin API Service namespaced name in "namespace/name" format, to use for Kong Gateway service discovery.`) flagSet.StringSliceVar(&c.KondAdminSvcPortNames, "kong-admin-svc-port-names", []string{"admin", "admin-tls", "kong-admin", "kong-admin-tls"}, "Names of ports on Kong Admin API service to take into account when doing gateway discovery.") @@ -170,7 +173,7 @@ func (c *Config) FlagSet() *pflag.FlagSet { ) // Kubernetes configurations - flagSet.Var(NewValidatedValueWithDefault(&c.GatewayAPIControllerName, gatewayAPIControllerNameFromFlagValue, string(gateway.GetControllerName())), "gateway-api-controller-name", "The controller name to match on Gateway API resources.") + flagSet.Var(NewValidatedValue(&c.GatewayAPIControllerName, gatewayAPIControllerNameFromFlagValue, WithDefault(string(gateway.GetControllerName()))), "gateway-api-controller-name", "The controller name to match on Gateway API resources.") flagSet.StringVar(&c.KubeconfigPath, "kubeconfig", "", "Path to the kubeconfig file.") flagSet.StringVar(&c.IngressClassName, "ingress-class", annotations.DefaultIngressClass, `Name of the ingress class to route through this controller.`) flagSet.StringVar(&c.LeaderElectionID, "election-id", "5b374a9e.konghq.com", `Election id to use for status update.`) @@ -181,12 +184,12 @@ func (c *Config) FlagSet() *pflag.FlagSet { `Namespace(s) to watch for Kubernetes resources. Defaults to all namespaces. To watch multiple namespaces, use a comma-separated list of namespaces.`) // Ingress status - flagSet.Var(NewValidatedValue(&c.PublishService, namespacedNameFromFlagValue), "publish-service", + flagSet.Var(NewValidatedValue(&c.PublishService, namespacedNameFromFlagValue, nnTypeNameOverride), "publish-service", `Service fronting Ingress resources in "namespace/name" format. The controller will update Ingress status information with this Service's endpoints.`) flagSet.StringSliceVar(&c.PublishStatusAddress, "publish-status-address", []string{}, `User-provided addresses in comma-separated string format, for use in lieu of "publish-service" `+ `when that Service lacks useful address information (for example, in bare-metal environments).`) - flagSet.Var(NewValidatedValue(&c.PublishServiceUDP, namespacedNameFromFlagValue), "publish-service-udp", `Service fronting UDP routing resources in `+ + flagSet.Var(NewValidatedValue(&c.PublishServiceUDP, namespacedNameFromFlagValue, nnTypeNameOverride), "publish-service-udp", `Service fronting UDP routing resources in `+ `"namespace/name" format. The controller will update UDP route status information with this Service's `+ `endpoints. If omitted, the same Service will be used for both TCP and UDP routes.`) flagSet.StringSliceVar(&c.PublishStatusAddressUDP, "publish-status-address-udp", []string{}, diff --git a/internal/manager/config_validation.go b/internal/manager/config_validation.go index 0fa6984924..dcd79d6570 100644 --- a/internal/manager/config_validation.go +++ b/internal/manager/config_validation.go @@ -20,6 +20,13 @@ func namespacedNameFromFlagValue(flagValue string) (mo.Option[types.NamespacedNa if len(parts) != 2 { return mo.Option[types.NamespacedName]{}, errors.New("the expected format is namespace/name") } + if parts[0] == "" { + return mo.Option[types.NamespacedName]{}, errors.New("namespace cannot be empty") + } + if parts[1] == "" { + return mo.Option[types.NamespacedName]{}, errors.New("name cannot be empty") + } + return mo.Some(types.NamespacedName{ Namespace: parts[0], Name: parts[1], diff --git a/internal/manager/config_validation_test.go b/internal/manager/config_validation_test.go index 9058650bb4..e3dd827b09 100644 --- a/internal/manager/config_validation_test.go +++ b/internal/manager/config_validation_test.go @@ -55,6 +55,23 @@ func TestConfigValidatedVars(t *testing.T) { ExpectedErrorContains: "the expected format is namespace/name", }, }, + "--kong-admin-svc": { + { + Input: "namespace/servicename", + ExtractValueFn: func(c manager.Config) any { + return c.KongAdminSvc + }, + ExpectedValue: mo.Some(types.NamespacedName{Namespace: "namespace", Name: "servicename"}), + }, + { + Input: "namespace/", + ExpectedErrorContains: "name cannot be empty", + }, + { + Input: "/name", + ExpectedErrorContains: "namespace cannot be empty", + }, + }, } for flag, flagTestCases := range testCasesGroupedByFlag { diff --git a/internal/manager/setup.go b/internal/manager/setup.go index bfd213c9b1..fc45a16b52 100644 --- a/internal/manager/setup.go +++ b/internal/manager/setup.go @@ -211,15 +211,15 @@ func setupDataplaneAddressFinder(mgrc client.Client, c *Config, log logr.Logger) return defaultAddressFinder, udpAddressFinder, nil } -func buildDataplaneAddressFinder(mgrc client.Client, publishStatusAddress []string, publishServiceNn mo.Option[types.NamespacedName]) (*dataplane.AddressFinder, error) { +func buildDataplaneAddressFinder(mgrc client.Client, publishStatusAddress []string, publishServiceNN mo.Option[types.NamespacedName]) (*dataplane.AddressFinder, error) { addressFinder := dataplane.NewAddressFinder() if len(publishStatusAddress) > 0 { addressFinder.SetOverrides(publishStatusAddress) return addressFinder, nil } - if serviceNn, ok := publishServiceNn.Get(); ok { - addressFinder.SetGetter(generateAddressFinderGetter(mgrc, serviceNn)) + if serviceNN, ok := publishServiceNN.Get(); ok { + addressFinder.SetGetter(generateAddressFinderGetter(mgrc, serviceNN)) return addressFinder, nil } @@ -269,8 +269,8 @@ func (c *Config) adminAPIClients(ctx context.Context) ([]*adminapi.Client, error // If kong-admin-svc flag has been specified then use it to get the list // of Kong Admin API endpoints. - if adminSvc, ok := c.KongAdminSvc.Get(); ok { - return c.adminAPIClientFromServiceDiscovery(ctx, httpclient, adminSvc) + if c.KongAdminSvc.IsPresent() { + return c.adminAPIClientFromServiceDiscovery(ctx, httpclient) } // Otherwise fallback to the list of kong admin URLs. @@ -288,12 +288,17 @@ func (c *Config) adminAPIClients(ctx context.Context) ([]*adminapi.Client, error return clients, nil } -func (c *Config) adminAPIClientFromServiceDiscovery(ctx context.Context, httpclient *http.Client, adminSvc types.NamespacedName) ([]*adminapi.Client, error) { +func (c *Config) adminAPIClientFromServiceDiscovery(ctx context.Context, httpclient *http.Client) ([]*adminapi.Client, error) { kubeClient, err := c.GetKubeClient() if err != nil { return nil, err } + kongAdminSvcNN, ok := c.KongAdminSvc.Get() + if !ok { + return nil, errors.New("kong admin service namespaced name not provided") + } + // Retry this as we may encounter an error of getting 0 addresses, // which can mean that Kong instances meant to be configured by this controller // are not yet ready. @@ -303,12 +308,12 @@ func (c *Config) adminAPIClientFromServiceDiscovery(ctx context.Context, httpcli // configuration validation and sending code. var adminAPIs []adminapi.DiscoveredAdminAPI err = retry.Do(func() error { - s, err := adminapi.GetAdminAPIsForService(ctx, kubeClient, adminSvc, sets.New(c.KondAdminSvcPortNames...)) + s, err := adminapi.GetAdminAPIsForService(ctx, kubeClient, kongAdminSvcNN, sets.New(c.KondAdminSvcPortNames...)) if err != nil { return err } if s.Len() == 0 { - return fmt.Errorf("no endpoints for kong admin service: %q", adminSvc) + return fmt.Errorf("no endpoints for kong admin service: %q", kongAdminSvcNN) } adminAPIs = s.UnsortedList() return nil diff --git a/internal/manager/validated.go b/internal/manager/validated.go index d166dd86fc..1b514a7a75 100644 --- a/internal/manager/validated.go +++ b/internal/manager/validated.go @@ -2,32 +2,45 @@ package manager import "fmt" +type ValidatedValueOpt[T any] func(*ValidatedValue[T]) + +// WithDefault sets the default value for the validated variable. +func WithDefault[T any](defaultValue T) ValidatedValueOpt[T] { + return func(v *ValidatedValue[T]) { + *v.variable = defaultValue + } +} + +// WithTypeNameOverride overrides the type name that's printed in the help message. +func WithTypeNameOverride[T any](typeName string) ValidatedValueOpt[T] { + return func(v *ValidatedValue[T]) { + v.typeName = typeName + } +} + // ValidatedValue implements `pflag.Value` interface. It can be used for hooking up arbitrary validation logic to any type. // It should be passed to `pflag.FlagSet.Var()`. type ValidatedValue[T any] struct { origin string variable *T constructor func(string) (T, error) + typeName string } // NewValidatedValue creates a validated variable of type T. Constructor should validate the input and return an error // in case of any failures. If validation passes, constructor should return a value that's to be set in the variable. // The constructor accepts a flagValue that is raw input from user's command line (or an env variable that was bind to // the flag, see: bindEnvVars). -func NewValidatedValue[T any](variable *T, constructor func(flagValue string) (T, error)) ValidatedValue[T] { - return ValidatedValue[T]{ +// It accepts a variadic list of options that can be used e.g. to set the default value or override the type name. +func NewValidatedValue[T any](variable *T, constructor func(flagValue string) (T, error), opts ...ValidatedValueOpt[T]) ValidatedValue[T] { + v := ValidatedValue[T]{ constructor: constructor, variable: variable, } -} - -// NewValidatedValueWithDefault creates a validated variable of type T with a default value. -func NewValidatedValueWithDefault[T any](variable *T, constructor func(flagValue string) (T, error), value T) ValidatedValue[T] { - *variable = value - return ValidatedValue[T]{ - constructor: constructor, - variable: variable, + for _, opt := range opts { + opt(&v) } + return v } func (v ValidatedValue[T]) String() string { @@ -45,6 +58,10 @@ func (v ValidatedValue[T]) Set(s string) error { } func (v ValidatedValue[T]) Type() string { + if v.typeName != "" { + return v.typeName + } + var t T - return fmt.Sprintf("Validated%T", t) + return fmt.Sprintf("%T", t) } diff --git a/internal/manager/validated_test.go b/internal/manager/validated_test.go index 1f6b0956ca..a2b7de49df 100644 --- a/internal/manager/validated_test.go +++ b/internal/manager/validated_test.go @@ -71,12 +71,12 @@ func TestValidatedValue(t *testing.T) { t.Run("with default", func(t *testing.T) { flags := flags() var validatedString string - flags.Var(manager.NewValidatedValueWithDefault(&validatedString, func(s string) (string, error) { + flags.Var(manager.NewValidatedValue(&validatedString, func(s string) (string, error) { if !strings.Contains(s, "magic-token") { return "", errors.New("no magic token passed") } return s, nil - }, "default-value"), "flag-with-default", "") + }, manager.WithDefault("default-value")), "flag-with-default", "") t.Run("empty", func(t *testing.T) { err := flags.Parse(nil) @@ -101,3 +101,31 @@ func TestValidatedValue(t *testing.T) { }) }) } + +func TestValidatedValue_Type(t *testing.T) { + t.Run("string", func(t *testing.T) { + var validatedString string + vv := manager.NewValidatedValue(&validatedString, func(s string) (string, error) { + return s, nil + }) + require.Equal(t, "string", vv.Type()) + }) + + t.Run("struct", func(t *testing.T) { + type customType struct{} + var customTypeVar customType + vv := manager.NewValidatedValue(&customTypeVar, func(s string) (customType, error) { + return customType{}, nil + }) + require.Equal(t, "manager_test.customType", vv.Type()) + }) + + t.Run("overridden type name", func(t *testing.T) { + type customType struct{} + var customTypeVar customType + vv := manager.NewValidatedValue(&customTypeVar, func(s string) (customType, error) { + return customType{}, nil + }, manager.WithTypeNameOverride[customType]("custom-type-override")) + require.Equal(t, "custom-type-override", vv.Type()) + }) +}