Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into feat/udp-publish
Browse files Browse the repository at this point in the history
  • Loading branch information
rainest committed Jan 23, 2023
2 parents f73dd4f + 95c97ce commit 8ab82bd
Show file tree
Hide file tree
Showing 12 changed files with 273 additions and 112 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ _build.debug:

.PHONY: _build.template.debug
_build.template.debug:
go build -o bin/manager-debug -gcflags=all="-N -l" -ldflags " \
go build -o bin/manager-debug -trimpath -gcflags=all="-N -l" -ldflags " \
-X $(REPO_URL)/v2/internal/manager/metadata.Release=$(TAG) \
-X $(REPO_URL)/v2/internal/manager/metadata.Commit=$(COMMIT) \
-X $(REPO_URL)/v2/internal/manager/metadata.Repo=$(REPO_INFO)" ${MAIN}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ require (
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.8.1
golang.org/x/net v0.5.0
google.golang.org/api v0.107.0
google.golang.org/api v0.108.0
k8s.io/api v0.26.1
k8s.io/apiextensions-apiserver v0.26.1
k8s.io/apimachinery v0.26.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1334,8 +1334,8 @@ google.golang.org/api v0.56.0/go.mod h1:38yMfeP1kfjsl8isn0tliTjIb1rJXcQi4UXlbqiv
google.golang.org/api v0.57.0/go.mod h1:dVPlbZyBo2/OjBpmvNdpn2GRm6rPy75jyU7bmhdrMgI=
google.golang.org/api v0.58.0/go.mod h1:cAbP2FsxoGVNwtgNAmmn3y5G1TWAiVYRmg4yku3lv+E=
google.golang.org/api v0.61.0/go.mod h1:xQRti5UdCmoCEqFxcz93fTl338AVqDgyaDRuOZ3hg9I=
google.golang.org/api v0.107.0 h1:I2SlFjD8ZWabaIFOfeEDg3pf0BHJDh6iYQ1ic3Yu/UU=
google.golang.org/api v0.107.0/go.mod h1:2Ts0XTHNVWxypznxWOYUeI4g3WdP9Pk2Qk58+a/O9MY=
google.golang.org/api v0.108.0 h1:WVBc/faN0DkKtR43Q/7+tPny9ZoLZdIiAyG5Q9vFClg=
google.golang.org/api v0.108.0/go.mod h1:2Ts0XTHNVWxypznxWOYUeI4g3WdP9Pk2Qk58+a/O9MY=
google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM=
google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4=
google.golang.org/appengine v1.5.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4=
Expand Down
17 changes: 7 additions & 10 deletions internal/manager/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/kong/go-kong/kong"
"github.com/spf13/pflag"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/clientcmd"
cliflag "k8s.io/component-base/cli/flag"
Expand Down Expand Up @@ -66,8 +67,8 @@ type Config struct {
GatewayAPIControllerName string

// Ingress status
PublishServiceUDP FlagNamespacedName
PublishService FlagNamespacedName
PublishServiceUDP types.NamespacedName
PublishService types.NamespacedName
PublishStatusAddress []string
PublishStatusAddressUDP []string
UpdateStatus bool
Expand Down Expand Up @@ -111,13 +112,9 @@ type Config struct {
// Controller Manager - Config - Methods
// -----------------------------------------------------------------------------

// Validate validates the config. With time this logic may grow to invalidate
// incorrect configurations.
// Validate validates the config. It should be used to validate the config variables' interdependencies.
// When a single variable is to be validated, NewValidatedValue should be used.
func (c *Config) Validate() error {
if !isControllerNameValid(c.GatewayAPIControllerName) {
return fmt.Errorf("--gateway-api-controller-name (%s) is invalid. The expected format is example.com/controller-name", c.GatewayAPIControllerName)
}

return nil
}

Expand Down Expand Up @@ -169,7 +166,7 @@ func (c *Config) FlagSet() *pflag.FlagSet {
)

// Kubernetes configurations
flagSet.StringVar(&c.GatewayAPIControllerName, "gateway-api-controller-name", string(gateway.ControllerName), "The controller name to match on Gateway API resources.")
flagSet.Var(NewValidatedValueWithDefault(&c.GatewayAPIControllerName, gatewayAPIControllerNameFromFlagValue, string(gateway.ControllerName)), "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.`)
Expand All @@ -180,7 +177,7 @@ 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(&c.PublishService, "publish-service",
flagSet.Var(NewValidatedValue(&c.PublishService, namespacedNameFromFlagValue), "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" `+
Expand Down
59 changes: 0 additions & 59 deletions internal/manager/config_test.go

This file was deleted.

29 changes: 29 additions & 0 deletions internal/manager/config_validation.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package manager

import (
"errors"
"strings"

"k8s.io/apimachinery/pkg/types"
)

// This file contains a set of constructors that are used to validate and set validated values in Config.
// They're meant to be used together with ValidatedValue[T] type.

func namespacedNameFromFlagValue(flagValue string) (types.NamespacedName, error) {
parts := strings.SplitN(flagValue, "/", 3)
if len(parts) != 2 {
return types.NamespacedName{}, errors.New("the expected format is namespace/name")
}
return types.NamespacedName{
Namespace: parts[0],
Name: parts[1],
}, nil
}

func gatewayAPIControllerNameFromFlagValue(flagValue string) (string, error) {
if !isControllerNameValid(flagValue) {
return "", errors.New("the expected format is example.com/controller-name")
}
return flagValue, nil
}
77 changes: 77 additions & 0 deletions internal/manager/config_validation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package manager

import (
"fmt"
"testing"

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

"github.com/kong/kubernetes-ingress-controller/v2/internal/controllers/gateway"
)

func TestConfigValidatedVars(t *testing.T) {
type testCase struct {
Input string
ExpectedValue any
ExtractValueFn func(c Config) any
ExpectedErrorContains string
}

testCasesGroupedByFlag := map[string][]testCase{
"--gateway-api-controller-name": {
{
Input: "example.com/controller-name",
ExtractValueFn: func(c Config) any {
return c.GatewayAPIControllerName
},
ExpectedValue: "example.com/controller-name",
},
{
Input: "",
ExtractValueFn: func(c Config) any {
return c.GatewayAPIControllerName
},
ExpectedValue: string(gateway.ControllerName),
},
{
Input: "%invalid_controller_name$",
ExpectedErrorContains: "the expected format is example.com/controller-name",
},
},
"--publish-service": {
{
Input: "namespace/servicename",
ExtractValueFn: func(c Config) any {
return c.PublishService
},
ExpectedValue: types.NamespacedName{Namespace: "namespace", Name: "servicename"},
},
{
Input: "servicename",
ExpectedErrorContains: "the expected format is namespace/name",
},
},
}

for flag, flagTestCases := range testCasesGroupedByFlag {
for _, tc := range flagTestCases {
t.Run(fmt.Sprintf("%s=%s", flag, tc.Input), func(t *testing.T) {
var c Config
var input []string
if tc.Input != "" {
input = []string{flag, tc.Input}
}

err := c.FlagSet().Parse(input)
if tc.ExpectedErrorContains != "" {
require.ErrorContains(t, err, tc.ExpectedErrorContains)
return
}

require.NoError(t, err)
require.Equal(t, tc.ExpectedValue, tc.ExtractValueFn(c))
})
}
}
}
33 changes: 0 additions & 33 deletions internal/manager/flagnamespacedname.go

This file was deleted.

3 changes: 0 additions & 3 deletions internal/manager/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,6 @@ func Run(ctx context.Context, c *Config, diagnostic util.ConfigDumpDiagnostic, d
return err
}

if !isControllerNameValid(c.GatewayAPIControllerName) {
return errors.New("--gateway-api-controller-name is invalid. The expected format is example.com/controller-name")
}
gateway.ControllerName = gatewayv1beta1.GatewayController(c.GatewayAPIControllerName)

setupLog.Info("Starting Enabled Controllers")
Expand Down
6 changes: 3 additions & 3 deletions internal/manager/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ func setupControllerOptions(logger logr.Logger, c *Config, dbmode string) (ctrl.

// 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.NN.String() != "" {
watchNamespaces = append(c.WatchNamespaces, c.PublishService.NN.Namespace)
if c.PublishService.String() != "" {
watchNamespaces = append(c.WatchNamespaces, c.PublishService.Namespace)
}
controllerOpts.NewCache = cache.MultiNamespacedCacheBuilder(watchNamespaces)
}
Expand Down Expand Up @@ -202,7 +202,7 @@ func setupDataplaneAddressFinder(
if overrideAddrs := c.PublishStatusAddress; len(overrideAddrs) > 0 {
dataplaneAddressFinder.SetOverrides(overrideAddrs)
} else if c.PublishService.String() != "" {
publishServiceNn := c.PublishService.NN
publishServiceNn := c.PublishService
dataplaneAddressFinder.SetGetter(func(ctx context.Context) ([]string, error) {
svc := new(corev1.Service)
if err := mgrc.Get(ctx, publishServiceNn, svc); err != nil {
Expand Down
50 changes: 50 additions & 0 deletions internal/manager/validated.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package manager

import "fmt"

// 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)
}

// 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]{
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,
}
}

func (v ValidatedValue[T]) String() string {
return v.origin
}

func (v ValidatedValue[T]) Set(s string) error {
value, err := v.constructor(s)
if err != nil {
return err
}

*v.variable = value
return nil
}

func (v ValidatedValue[T]) Type() string {
var t T
return fmt.Sprintf("Validated%T", t)
}
Loading

0 comments on commit 8ab82bd

Please sign in to comment.