Skip to content

Commit

Permalink
fix(config): correctly check types.NamespacedName flags are set
Browse files Browse the repository at this point in the history
  • Loading branch information
czeslavo committed Feb 24, 2023
1 parent ea3753a commit 7670012
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 47 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
7 changes: 4 additions & 3 deletions internal/manager/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
29 changes: 5 additions & 24 deletions internal/manager/config_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 == "" {
Expand Down
7 changes: 4 additions & 3 deletions internal/manager/config_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"testing"

"github.com/samber/mo"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/types"

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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")
})
})
Expand Down
6 changes: 3 additions & 3 deletions internal/manager/controllerdef.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions internal/manager/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down Expand Up @@ -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 {
Expand Down
23 changes: 12 additions & 11 deletions internal/manager/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 7670012

Please sign in to comment.