fix(config): correctly check types.NamespacedName flags are set

CHANGELOG.md

@@ -165,6 +165,9 @@ Adding a new version? You'll need three changes:
 - Made Admission Webhook fetch the latest list of Gateways to avoid calling
   outdated services set statically during the setup.
   [#3601](https://github.com/Kong/kubernetes-ingress-controller/pull/3601)
+- Fixed the way configuration flags `KongAdminSvc` and `PublishService` are
+  checked for being set. The old one was always evaluating to `true`.
+  [#3602](https://github.com/Kong/kubernetes-ingress-controller/pull/3602)
 
 ### Under the hood
 

go.mod

@@ -25,6 +25,7 @@ require (
 	github.com/prometheus/client_golang v1.14.0
 	github.com/prometheus/common v0.40.0
 	github.com/samber/lo v1.37.0
+	github.com/samber/mo v1.8.0
 	github.com/sethvargo/go-password v0.2.0
 	github.com/sirupsen/logrus v1.9.0
 	github.com/spf13/cobra v1.6.1

go.sum

@@ -691,6 +691,8 @@ github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQD
 github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts=
 github.com/samber/lo v1.37.0 h1:XjVcB8g6tgUp8rsPsJ2CvhClfImrpL04YpQHXeHPhRw=
 github.com/samber/lo v1.37.0/go.mod h1:9vaz2O4o8oOnK23pd2TrXufcbdbJIa3b6cstBWKpopA=
+github.com/samber/mo v1.8.0 h1:vYjHTfg14JF9tD2NLhpoUsRi9bjyRoYwa4+do0nvbVw=
+github.com/samber/mo v1.8.0/go.mod h1:BfkrCPuYzVG3ZljnZB783WIJIGk1mcZr9c9CPf8tAxs=
 github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc=
 github.com/sergi/go-diff v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
 github.com/sethvargo/go-password v0.2.0 h1:BTDl4CC/gjf/axHMaDQtw507ogrXLci6XRiLc7i/UHI=

internal/manager/config.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"time"
 
+	"github.com/samber/mo"
 	"github.com/spf13/pflag"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/rest"
@@ -20,6 +21,11 @@ import (
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/manager/featuregates"
 )
 
+type OptionalNamespacedName = mo.Option[types.NamespacedName]
+
+// Type override to be used with OptionalNamespacedName variables to override their type name printed in the help text.
+var nnTypeNameOverride = WithTypeNameOverride[OptionalNamespacedName]("namespacedName")
+
 // -----------------------------------------------------------------------------
 // Controller Manager - Config
 // -----------------------------------------------------------------------------
@@ -52,7 +58,7 @@ type Config struct {
 	MetricsAddr           string
 	ProbeAddr             string
 	KongAdminURLs         []string
-	KongAdminSvc          types.NamespacedName
+	KongAdminSvc          OptionalNamespacedName
 	KondAdminSvcPortNames []string
 	ProxySyncSeconds      float32
 	ProxyTimeoutSeconds   float32
@@ -68,8 +74,8 @@ type Config struct {
 	GatewayAPIControllerName string
 
 	// Ingress status
-	PublishServiceUDP       types.NamespacedName
-	PublishService          types.NamespacedName
+	PublishServiceUDP       OptionalNamespacedName
+	PublishService          OptionalNamespacedName
 	PublishStatusAddress    []string
 	PublishStatusAddressUDP []string
 	UpdateStatus            bool
@@ -150,7 +156,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.")
@@ -169,7 +175,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.`)
@@ -180,12 +186,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{},

internal/manager/config_validation.go

@@ -6,6 +6,7 @@ import (
 	"regexp"
 	"strings"
 
+	"github.com/samber/mo"
 	"k8s.io/apimachinery/pkg/types"
 
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/adminapi"
@@ -14,15 +15,22 @@ import (
 // *FromFlagValue functions are used to validate single flag values and set those in Config.
 // They're meant to be used together with ValidatedValue[T] type.
 
-func namespacedNameFromFlagValue(flagValue string) (types.NamespacedName, error) {
+func namespacedNameFromFlagValue(flagValue string) (OptionalNamespacedName, error) {
 	parts := strings.SplitN(flagValue, "/", 3)
 	if len(parts) != 2 {
-		return types.NamespacedName{}, errors.New("the expected format is namespace/name")
+		return OptionalNamespacedName{}, errors.New("the expected format is namespace/name")
 	}
-	return types.NamespacedName{
+	if parts[0] == "" {
+		return OptionalNamespacedName{}, errors.New("namespace cannot be empty")
+	}
+	if parts[1] == "" {
+		return OptionalNamespacedName{}, errors.New("name cannot be empty")
+	}
+
+	return mo.Some(types.NamespacedName{
 		Namespace: parts[0],
 		Name:      parts[1],
-	}, nil
+	}), nil
 }
 
 func gatewayAPIControllerNameFromFlagValue(flagValue string) (string, error) {
@@ -59,7 +67,7 @@ func (c *Config) validateKonnect() error {
 		return nil
 	}
 
-	if c.KongAdminSvc.Name == "" || c.KongAdminSvc.Namespace == "" {
+	if c.KongAdminSvc.IsAbsent() {
 		return errors.New("--kong-admin-svc has to be set when using --konnect-sync-enabled")
 	}
 	if konnect.Address == "" {

internal/manager/config_validation_test.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/samber/mo"
 	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/types"
 
@@ -47,13 +48,30 @@ func TestConfigValidatedVars(t *testing.T) {
 				ExtractValueFn: func(c manager.Config) any {
 					return c.PublishService
 				},
-				ExpectedValue: types.NamespacedName{Namespace: "namespace", Name: "servicename"},
+				ExpectedValue: mo.Some(types.NamespacedName{Namespace: "namespace", Name: "servicename"}),
 			},
 			{
 				Input:                 "servicename",
 				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 {
@@ -82,7 +100,7 @@ func TestConfigValidate(t *testing.T) {
 	t.Run("konnect", func(t *testing.T) {
 		validEnabled := func() *manager.Config {
 			return &manager.Config{
-				KongAdminSvc: types.NamespacedName{Name: "admin-svc", Namespace: "ns"},
+				KongAdminSvc: mo.Some(types.NamespacedName{Name: "admin-svc", Namespace: "ns"}),
 				Konnect: adminapi.KonnectConfig{
 					ConfigSynchronizationEnabled: true,
 					RuntimeGroupID:               "fbd3036f-0f1c-4e98-b71c-d4cd61213f90",
@@ -153,19 +171,7 @@ func TestConfigValidate(t *testing.T) {
 
 		t.Run("enabled with no gateway service discovery enabled", func(t *testing.T) {
 			c := validEnabled()
-			c.KongAdminSvc = types.NamespacedName{}
-			require.ErrorContains(t, c.Validate(), "--kong-admin-svc has to be set when using --konnect-sync-enabled")
-		})
-
-		t.Run("enabled with gateway service discovery without name", func(t *testing.T) {
-			c := validEnabled()
-			c.KongAdminSvc.Name = ""
-			require.ErrorContains(t, c.Validate(), "--kong-admin-svc has to be set when using --konnect-sync-enabled")
-		})
-
-		t.Run("enabled with gateway service discovery without namespace", func(t *testing.T) {
-			c := validEnabled()
-			c.KongAdminSvc.Namespace = ""
+			c.KongAdminSvc = manager.OptionalNamespacedName{}
 			require.ErrorContains(t, c.Validate(), "--kong-admin-svc has to be set when using --konnect-sync-enabled")
 		})
 	})

internal/manager/controllerdef.go

@@ -91,10 +91,10 @@ func setupControllers(
 		// Kong Gateway Admin API Service discovery
 		// ---------------------------------------------------------------------------
 		{
-			Enabled: c.KongAdminSvc.Name != "",
+			Enabled: c.KongAdminSvc.IsPresent(),
 			Controller: &configuration.KongAdminAPIServiceReconciler{
 				Client:            mgr.GetClient(),
-				ServiceNN:         c.KongAdminSvc,
+				ServiceNN:         c.KongAdminSvc.OrEmpty(),
 				PortNames:         sets.New(c.KondAdminSvcPortNames...),
 				Log:               ctrl.Log.WithName("controllers").WithName("KongAdminAPIService"),
 				CacheSyncTimeout:  c.CacheSyncTimeout,
@@ -378,7 +378,7 @@ func setupControllers(
 				Log:                  ctrl.Log.WithName("controllers").WithName(featuregates.GatewayFeature),
 				Scheme:               mgr.GetScheme(),
 				DataplaneClient:      dataplaneClient,
-				PublishService:       c.PublishService.String(),
+				PublishService:       c.PublishService.OrEmpty().String(),
 				WatchNamespaces:      c.WatchNamespaces,
 				EnableReferenceGrant: referenceGrantsEnabled,
 				CacheSyncTimeout:     c.CacheSyncTimeout,

internal/manager/run.go

@@ -105,7 +105,7 @@ func Run(ctx context.Context, c *Config, diagnostic util.ConfigDumpDiagnostic, d
 	if err != nil {
 		return fmt.Errorf("failed to create AdminAPIClientsManager: %w", err)
 	}
-	if c.KongAdminSvc.Name != "" {
+	if c.KongAdminSvc.IsPresent() {
 		setupLog.Info("Running AdminAPIClientsManager notify loop")
 		clientsManager.RunNotifyLoop()
 	}
@@ -211,11 +211,11 @@ func Run(ctx context.Context, c *Config, diagnostic util.ConfigDumpDiagnostic, d
 			kubeconfig,
 			clientsManager,
 			telemetry.ReportValues{
-				PublishServiceNN:               c.PublishService,
+				PublishServiceNN:               c.PublishService.OrEmpty(),
 				FeatureGates:                   featureGates,
 				MeshDetection:                  len(c.WatchNamespaces) == 0,
 				KonnectSyncEnabled:             c.Konnect.ConfigSynchronizationEnabled,
-				GatewayServiceDiscoveryEnabled: c.KongAdminSvc.Name != "",
+				GatewayServiceDiscoveryEnabled: c.KongAdminSvc.IsPresent(),
 			},
 		)
 		if err != nil {

internal/manager/setup.go

@@ -90,8 +90,8 @@ func setupControllerOptions(logger logr.Logger, c *Config, dbmode string, featur
 
 		// if publish service has been provided the namespace for it should be
 		// watched so that controllers can see updates to the service.
-		if c.PublishService.String() != "" {
-			watchNamespaces = append(c.WatchNamespaces, c.PublishService.Namespace)
+		if s, ok := c.PublishService.Get(); ok {
+			watchNamespaces = append(c.WatchNamespaces, s.Namespace)
 		}
 		controllerOpts.NewCache = cache.MultiNamespacedCacheBuilder(watchNamespaces)
 	}
@@ -110,7 +110,7 @@ func leaderElectionEnabled(logger logr.Logger, c *Config, dbmode string) bool {
 	}
 
 	if dbmode == "off" {
-		if c.KongAdminSvc.Name != "" {
+		if c.KongAdminSvc.IsPresent() {
 			logger.Info("DB-less mode detected with service detection, enabling leader election")
 			return true
 		}
@@ -210,15 +210,15 @@ func setupDataplaneAddressFinder(mgrc client.Client, c *Config, log logr.Logger)
 	return defaultAddressFinder, udpAddressFinder, nil
 }
 
-func buildDataplaneAddressFinder(mgrc client.Client, publishStatusAddress []string, publishServiceNn types.NamespacedName) (*dataplane.AddressFinder, error) {
+func buildDataplaneAddressFinder(mgrc client.Client, publishStatusAddress []string, publishServiceNN OptionalNamespacedName) (*dataplane.AddressFinder, error) {
 	addressFinder := dataplane.NewAddressFinder()
 
 	if len(publishStatusAddress) > 0 {
 		addressFinder.SetOverrides(publishStatusAddress)
 		return addressFinder, nil
 	}
-	if publishServiceNn.String() != "" {
-		addressFinder.SetGetter(generateAddressFinderGetter(mgrc, publishServiceNn))
+	if serviceNN, ok := publishServiceNN.Get(); ok {
+		addressFinder.SetGetter(generateAddressFinderGetter(mgrc, serviceNN))
 		return addressFinder, nil
 	}
 
@@ -268,7 +268,7 @@ 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 c.KongAdminSvc.Name != "" {
+	if c.KongAdminSvc.IsPresent() {
 		return c.adminAPIClientFromServiceDiscovery(ctx, httpclient)
 	}
 
@@ -293,6 +293,11 @@ func (c *Config) adminAPIClientFromServiceDiscovery(ctx context.Context, httpcli
 		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.
@@ -302,12 +307,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, c.KongAdminSvc, 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", c.KongAdminSvc)
+			return fmt.Errorf("no endpoints for kong admin service: %q", kongAdminSvcNN)
 		}
 		adminAPIs = s.UnsortedList()
 		return nil