Skip to content

Commit

Permalink
fix: do not check NamespacedName is set with String()
Browse files Browse the repository at this point in the history
  • Loading branch information
czeslavo committed Feb 24, 2023
1 parent 35c6f41 commit ea3753a
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 38 deletions.
6 changes: 3 additions & 3 deletions internal/manager/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ type Config struct {
MetricsAddr string
ProbeAddr string
KongAdminURLs []string
KongAdminSvc types.NamespacedName
KongAdminSvc ValidatedVar[types.NamespacedName]
KondAdminSvcPortNames []string
ProxySyncSeconds float32
ProxyTimeoutSeconds float32
Expand All @@ -68,8 +68,8 @@ type Config struct {
GatewayAPIControllerName string

// Ingress status
PublishServiceUDP types.NamespacedName
PublishService types.NamespacedName
PublishServiceUDP ValidatedVar[types.NamespacedName]
PublishService ValidatedVar[types.NamespacedName]
PublishStatusAddress []string
PublishStatusAddressUDP []string
UpdateStatus bool
Expand Down
30 changes: 25 additions & 5 deletions internal/manager/config_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,38 @@ import (
"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) (types.NamespacedName, error) {
func namespacedNameFromFlagValue(flagValue string) (ValidatedVar[types.NamespacedName], error) {
parts := strings.SplitN(flagValue, "/", 3)
if len(parts) != 2 {
return types.NamespacedName{}, errors.New("the expected format is namespace/name")
return ValidatedVar[types.NamespacedName]{}, errors.New("the expected format is namespace/name")
}
return types.NamespacedName{
return NewValidatedVar(types.NamespacedName{
Namespace: parts[0],
Name: parts[1],
}, nil
}), nil
}

func gatewayAPIControllerNameFromFlagValue(flagValue string) (string, error) {
Expand Down Expand Up @@ -59,7 +79,7 @@ func (c *Config) validateKonnect() error {
return nil
}

if c.KongAdminSvc.Name == "" || c.KongAdminSvc.Namespace == "" {
if !c.KongAdminSvc.WasSet() {
return errors.New("--kong-admin-svc has to be set when using --konnect-sync-enabled")
}
if konnect.Address == "" {
Expand Down
18 changes: 3 additions & 15 deletions internal/manager/config_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestConfigValidatedVars(t *testing.T) {
ExtractValueFn: func(c manager.Config) any {
return c.PublishService
},
ExpectedValue: types.NamespacedName{Namespace: "namespace", Name: "servicename"},
ExpectedValue: manager.NewValidatedVar(types.NamespacedName{Namespace: "namespace", Name: "servicename"}),
},
{
Input: "servicename",
Expand Down Expand Up @@ -82,7 +82,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: manager.NewValidatedVar(types.NamespacedName{Name: "admin-svc", Namespace: "ns"}),
Konnect: adminapi.KonnectConfig{
ConfigSynchronizationEnabled: true,
RuntimeGroupID: "fbd3036f-0f1c-4e98-b71c-d4cd61213f90",
Expand Down Expand Up @@ -153,19 +153,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.ValidatedVar[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.Name != "",
Enabled: c.KongAdminSvc.WasSet(),
Controller: &configuration.KongAdminAPIServiceReconciler{
Client: mgr.GetClient(),
ServiceNN: c.KongAdminSvc,
ServiceNN: c.KongAdminSvc.Value(),
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.String(),
PublishService: c.PublishService.Value().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.Name != "" {
if c.KongAdminSvc.WasSet() {
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,
PublishServiceNN: c.PublishService.Value(),
FeatureGates: featureGates,
MeshDetection: len(c.WatchNamespaces) == 0,
KonnectSyncEnabled: c.Konnect.ConfigSynchronizationEnabled,
GatewayServiceDiscoveryEnabled: c.KongAdminSvc.String() != "",
GatewayServiceDiscoveryEnabled: c.KongAdminSvc.WasSet(),
},
)
if err != nil {
Expand Down
18 changes: 9 additions & 9 deletions internal/manager/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 c.PublishService.WasSet() {
watchNamespaces = append(c.WatchNamespaces, c.PublishService.Value().Namespace)
}
controllerOpts.NewCache = cache.MultiNamespacedCacheBuilder(watchNamespaces)
}
Expand All @@ -110,7 +110,7 @@ func leaderElectionEnabled(logger logr.Logger, c *Config, dbmode string) bool {
}

if dbmode == "off" {
if c.KongAdminSvc.Name != "" {
if c.KongAdminSvc.WasSet() {
logger.Info("DB-less mode detected with service detection, enabling leader election")
return true
}
Expand Down Expand Up @@ -222,15 +222,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 ValidatedVar[types.NamespacedName]) (*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 publishServiceNn.WasSet() {
addressFinder.SetGetter(generateAddressFinderGetter(mgrc, publishServiceNn.Value()))
return addressFinder, nil
}

Expand Down Expand Up @@ -280,7 +280,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.WasSet() {
return c.adminAPIClientFromServiceDiscovery(ctx, httpclient)
}

Expand Down Expand Up @@ -314,12 +314,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, c.KongAdminSvc.Value(), 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", c.KongAdminSvc.Value())
}
adminAPIs = s.UnsortedList()
return nil
Expand Down

0 comments on commit ea3753a

Please sign in to comment.