From 7670012386022c94d51dc50de5de654bc0c3ad4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Fri, 24 Feb 2023 18:13:59 +0100 Subject: [PATCH] fix(config): correctly check `types.NamespacedName` flags are set --- go.mod | 1 + go.sum | 2 ++ internal/manager/config.go | 7 +++--- internal/manager/config_validation.go | 29 ++++------------------ internal/manager/config_validation_test.go | 7 +++--- internal/manager/controllerdef.go | 6 ++--- internal/manager/run.go | 6 ++--- internal/manager/setup.go | 23 +++++++++-------- 8 files changed, 34 insertions(+), 47 deletions(-) diff --git a/go.mod b/go.mod index 0b705c09e1c..07a1078537b 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 3d2e149a484..258f360d2c7 100644 --- a/go.sum +++ b/go.sum @@ -804,6 +804,8 @@ github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb github.com/ryanuber/columnize v2.1.0+incompatible/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/schollz/closestmatch v2.1.0+incompatible/go.mod h1:RtP1ddjLong6gTkbtmuhtR2uUrrJOpYzYRvbcPAid+g= github.com/sclevine/agouti v3.0.0+incompatible/go.mod h1:b4WX9W9L1sfQKXeJf1mUTLZKJ48R1S7H23Ji7oFO5Bw= github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc= diff --git a/internal/manager/config.go b/internal/manager/config.go index a6d31b6cf8f..95006f06609 100644 --- a/internal/manager/config.go +++ b/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" @@ -52,7 +53,7 @@ type Config struct { MetricsAddr string ProbeAddr string KongAdminURLs []string - KongAdminSvc ValidatedVar[types.NamespacedName] + KongAdminSvc mo.Option[types.NamespacedName] KondAdminSvcPortNames []string ProxySyncSeconds float32 ProxyTimeoutSeconds float32 @@ -68,8 +69,8 @@ type Config struct { GatewayAPIControllerName string // Ingress status - PublishServiceUDP ValidatedVar[types.NamespacedName] - PublishService ValidatedVar[types.NamespacedName] + PublishServiceUDP mo.Option[types.NamespacedName] + PublishService mo.Option[types.NamespacedName] PublishStatusAddress []string PublishStatusAddressUDP []string UpdateStatus bool diff --git a/internal/manager/config_validation.go b/internal/manager/config_validation.go index 51f591d0337..0fa6984924b 100644 --- a/internal/manager/config_validation.go +++ b/internal/manager/config_validation.go @@ -6,40 +6,21 @@ import ( "regexp" "strings" + "github.com/samber/mo" "k8s.io/apimachinery/pkg/types" "github.com/kong/kubernetes-ingress-controller/v2/internal/adminapi" ) -type ValidatedVar[T any] struct { - v *T -} - -func NewValidatedVar[T any](v T) ValidatedVar[T] { - return ValidatedVar[T]{v: &v} -} - -func (v ValidatedVar[T]) Value() T { - if v.v != nil { - return *v.v - } - var t T - return t -} - -func (v ValidatedVar[T]) WasSet() bool { - return v.v != nil -} - // *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) (ValidatedVar[types.NamespacedName], error) { +func namespacedNameFromFlagValue(flagValue string) (mo.Option[types.NamespacedName], error) { parts := strings.SplitN(flagValue, "/", 3) if len(parts) != 2 { - return ValidatedVar[types.NamespacedName]{}, errors.New("the expected format is namespace/name") + return mo.Option[types.NamespacedName]{}, errors.New("the expected format is namespace/name") } - return NewValidatedVar(types.NamespacedName{ + return mo.Some(types.NamespacedName{ Namespace: parts[0], Name: parts[1], }), nil @@ -79,7 +60,7 @@ func (c *Config) validateKonnect() error { return nil } - if !c.KongAdminSvc.WasSet() { + if c.KongAdminSvc.IsAbsent() { return errors.New("--kong-admin-svc has to be set when using --konnect-sync-enabled") } if konnect.Address == "" { diff --git a/internal/manager/config_validation_test.go b/internal/manager/config_validation_test.go index a51eea9cd08..9058650bb4c 100644 --- a/internal/manager/config_validation_test.go +++ b/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,7 +48,7 @@ func TestConfigValidatedVars(t *testing.T) { ExtractValueFn: func(c manager.Config) any { return c.PublishService }, - ExpectedValue: manager.NewValidatedVar(types.NamespacedName{Namespace: "namespace", Name: "servicename"}), + ExpectedValue: mo.Some(types.NamespacedName{Namespace: "namespace", Name: "servicename"}), }, { Input: "servicename", @@ -82,7 +83,7 @@ func TestConfigValidate(t *testing.T) { t.Run("konnect", func(t *testing.T) { validEnabled := func() *manager.Config { return &manager.Config{ - KongAdminSvc: manager.NewValidatedVar(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,7 +154,7 @@ func TestConfigValidate(t *testing.T) { t.Run("enabled with no gateway service discovery enabled", func(t *testing.T) { c := validEnabled() - c.KongAdminSvc = manager.ValidatedVar[types.NamespacedName]{} + c.KongAdminSvc = mo.Option[types.NamespacedName]{} require.ErrorContains(t, c.Validate(), "--kong-admin-svc has to be set when using --konnect-sync-enabled") }) }) diff --git a/internal/manager/controllerdef.go b/internal/manager/controllerdef.go index 23761a88b77..6633ded81f0 100644 --- a/internal/manager/controllerdef.go +++ b/internal/manager/controllerdef.go @@ -91,10 +91,10 @@ func setupControllers( // Kong Gateway Admin API Service discovery // --------------------------------------------------------------------------- { - Enabled: c.KongAdminSvc.WasSet(), + Enabled: c.KongAdminSvc.IsPresent(), Controller: &configuration.KongAdminAPIServiceReconciler{ Client: mgr.GetClient(), - ServiceNN: c.KongAdminSvc.Value(), + 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.Value().String(), + PublishService: c.PublishService.OrEmpty().String(), WatchNamespaces: c.WatchNamespaces, EnableReferenceGrant: referenceGrantsEnabled, CacheSyncTimeout: c.CacheSyncTimeout, diff --git a/internal/manager/run.go b/internal/manager/run.go index 38b0972a8a0..2caeb95fa65 100644 --- a/internal/manager/run.go +++ b/internal/manager/run.go @@ -110,7 +110,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.WasSet() { + 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.Value(), + PublishServiceNN: c.PublishService.OrEmpty(), FeatureGates: featureGates, MeshDetection: len(c.WatchNamespaces) == 0, KonnectSyncEnabled: c.Konnect.ConfigSynchronizationEnabled, - GatewayServiceDiscoveryEnabled: c.KongAdminSvc.WasSet(), + GatewayServiceDiscoveryEnabled: c.KongAdminSvc.IsPresent(), }, ) if err != nil { diff --git a/internal/manager/setup.go b/internal/manager/setup.go index 223718b3e68..e83d6853b6d 100644 --- a/internal/manager/setup.go +++ b/internal/manager/setup.go @@ -12,6 +12,7 @@ import ( "github.com/bombsimon/logrusr/v2" "github.com/go-logr/logr" "github.com/kong/deck/cprint" + "github.com/samber/mo" "github.com/sirupsen/logrus" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" @@ -90,8 +91,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.WasSet() { - watchNamespaces = append(c.WatchNamespaces, c.PublishService.Value().Namespace) + if s, ok := c.PublishService.Get(); ok { + watchNamespaces = append(c.WatchNamespaces, s.Namespace) } controllerOpts.NewCache = cache.MultiNamespacedCacheBuilder(watchNamespaces) } @@ -110,7 +111,7 @@ func leaderElectionEnabled(logger logr.Logger, c *Config, dbmode string) bool { } if dbmode == "off" { - if c.KongAdminSvc.WasSet() { + if c.KongAdminSvc.IsPresent() { logger.Info("DB-less mode detected with service detection, enabling leader election") return true } @@ -222,15 +223,15 @@ func setupDataplaneAddressFinder(mgrc client.Client, c *Config, log logr.Logger) return defaultAddressFinder, udpAddressFinder, nil } -func buildDataplaneAddressFinder(mgrc client.Client, publishStatusAddress []string, publishServiceNn ValidatedVar[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 publishServiceNn.WasSet() { - addressFinder.SetGetter(generateAddressFinderGetter(mgrc, publishServiceNn.Value())) + if serviceNn, ok := publishServiceNn.Get(); ok { + addressFinder.SetGetter(generateAddressFinderGetter(mgrc, serviceNn)) return addressFinder, nil } @@ -280,8 +281,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 c.KongAdminSvc.WasSet() { - return c.adminAPIClientFromServiceDiscovery(ctx, httpclient) + if adminSvc, ok := c.KongAdminSvc.Get(); ok { + return c.adminAPIClientFromServiceDiscovery(ctx, httpclient, adminSvc) } // Otherwise fallback to the list of kong admin URLs. @@ -299,7 +300,7 @@ func (c *Config) adminAPIClients(ctx context.Context) ([]*adminapi.Client, error return clients, nil } -func (c *Config) adminAPIClientFromServiceDiscovery(ctx context.Context, httpclient *http.Client) ([]*adminapi.Client, error) { +func (c *Config) adminAPIClientFromServiceDiscovery(ctx context.Context, httpclient *http.Client, adminSvc types.NamespacedName) ([]*adminapi.Client, error) { kubeClient, err := c.GetKubeClient() if err != nil { return nil, err @@ -314,12 +315,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.Value(), sets.New(c.KondAdminSvcPortNames...)) + s, err := adminapi.GetAdminAPIsForService(ctx, kubeClient, adminSvc, 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.Value()) + return fmt.Errorf("no endpoints for kong admin service: %q", adminSvc) } adminAPIs = s.UnsortedList() return nil