Skip to content

Commit

Permalink
Make static mode compatible with provisioner
Browse files Browse the repository at this point in the history
This commit adds two features for the static mode:
- Support configuring a single Gateway resource to watch.
- Support not reporting the GatewayClass status,
so that it doesn't conflict with the status updates done by
the provisioner.

Needed by nginxinc#634
  • Loading branch information
pleshakov committed May 23, 2023
1 parent 3d6b009 commit 8230e48
Show file tree
Hide file tree
Showing 18 changed files with 615 additions and 147 deletions.
73 changes: 67 additions & 6 deletions cmd/gateway/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"

"github.com/spf13/cobra"
"k8s.io/apimachinery/pkg/types"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"sigs.k8s.io/controller-runtime/pkg/log/zap"

Expand Down Expand Up @@ -36,7 +37,7 @@ var (
)

// stringValidatingValue is a string flag value with custom validation logic.
// stringValidatingValue implements the pflag.Value interface.
// it implements the pflag.Value interface.
type stringValidatingValue struct {
validator func(v string) error
value string
Expand All @@ -58,6 +59,34 @@ func (v *stringValidatingValue) Type() string {
return "string"
}

// namespacedNameValue is a string flag value that represents a namespaced name.
// it implements the pflag.Value interface.
type namespacedNameValue struct {
value types.NamespacedName
}

func (v *namespacedNameValue) String() string {
if (v.value == types.NamespacedName{}) {
// if we don't do that, the default value in the help message will be printed as "/"
return ""
}
return v.value.String()
}

func (v *namespacedNameValue) Set(param string) error {
nsname, err := parseNamespacedResourceName(param)
if err != nil {
return err
}

v.value = nsname
return nil
}

func (v *namespacedNameValue) Type() string {
return "string"
}

func createRootCommand() *cobra.Command {
rootCmd := &cobra.Command{
Use: "gateway",
Expand All @@ -84,7 +113,13 @@ func createRootCommand() *cobra.Command {
}

func createStaticModeCommand() *cobra.Command {
return &cobra.Command{
const gatewayFlag = "gateway"

// flag values
gateway := namespacedNameValue{}
var updateGCStatus bool

cmd := &cobra.Command{
Use: "static-mode",
Short: "Configure NGINX in the scope of a single Gateway resource",
RunE: func(cmd *cobra.Command, args []string) error {
Expand All @@ -100,11 +135,18 @@ func createStaticModeCommand() *cobra.Command {
return fmt.Errorf("error validating POD_IP environment variable: %w", err)
}

var gwNsName *types.NamespacedName
if cmd.Flags().Changed(gatewayFlag) {
gwNsName = &gateway.value
}

conf := config.Config{
GatewayCtlrName: gatewayCtlrName.value,
Logger: logger,
GatewayClassName: gatewayClassName.value,
PodIP: podIP,
GatewayCtlrName: gatewayCtlrName.value,
Logger: logger,
GatewayClassName: gatewayClassName.value,
PodIP: podIP,
GatewayNsName: gwNsName,
UpdateGatewayClassStatus: updateGCStatus,
}

if err := manager.Start(conf); err != nil {
Expand All @@ -114,6 +156,25 @@ func createStaticModeCommand() *cobra.Command {
return nil
},
}

cmd.Flags().Var(
&gateway,
gatewayFlag,
"The namespaced name of the Gateway resource to use. "+
"Must be of the form: NAMESPACE/NAME. "+
"If not specified, the control plane will process all Gateways for the configured GatewayClass. "+
"However, among them, it will choose the oldest resource by creation timestamp. If the timestamps are "+
"equal, it will choose the resource that appears first in alphabetical order by {namespace}/{name}.",
)

cmd.Flags().BoolVar(
&updateGCStatus,
"update-gatewayclass-status",
true,
"Update the status of the GatewayClass resource.",
)

return cmd
}

func createProvisionerModeCommand() *cobra.Command {
Expand Down
107 changes: 88 additions & 19 deletions cmd/gateway/commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,40 @@ import (
"testing"

. "github.com/onsi/gomega"
"github.com/spf13/cobra"
)

type flagTestCase struct {
name string
expectedErrPrefix string
args []string
wantErr bool
}

func testFlag(t *testing.T, cmd *cobra.Command, test flagTestCase) {
g := NewGomegaWithT(t)
// discard any output generated by cobra
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)

// override RunE to avoid executing the command
cmd.RunE = func(cmd *cobra.Command, args []string) error {
return nil
}

cmd.SetArgs(test.args)
err := cmd.Execute()

if test.wantErr {
g.Expect(err).To(HaveOccurred())
g.Expect(err.Error()).To(HavePrefix(test.expectedErrPrefix))
} else {
g.Expect(err).NotTo(HaveOccurred())
}
}

func TestRootCmdFlagValidation(t *testing.T) {
tests := []struct {
name string
expectedErrPrefix string
args []string
wantErr bool
}{
tests := []flagTestCase{
{
name: "valid flags",
args: []string{
Expand Down Expand Up @@ -79,22 +104,66 @@ func TestRootCmdFlagValidation(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
g := NewGomegaWithT(t)

rootCmd := createRootCommand()
// discard any output generated by cobra
rootCmd.SetOut(io.Discard)
rootCmd.SetErr(io.Discard)
testFlag(t, rootCmd, test)
})
}
}

rootCmd.SetArgs(test.args)
err := rootCmd.Execute()
func TestStaticModeCmdFlagValidation(t *testing.T) {
tests := []flagTestCase{
{
name: "valid flags",
args: []string{
"--gateway=nginx-gateway/nginx",
"--update-gatewayclass-status=true",
},
wantErr: false,
},
{
name: "valid flags, not set",
args: nil,
wantErr: false,
},
{
name: "gateway is set to empty string",
args: []string{
"--gateway=",
},
wantErr: true,
expectedErrPrefix: `invalid argument "" for "--gateway" flag: must be set`,
},
{
name: "gateway is invalid",
args: []string{
"--gateway=nginx-gateway", // no namespace
},
wantErr: true,
expectedErrPrefix: `invalid argument "nginx-gateway" for "--gateway" flag: invalid format; ` +
"must be NAMESPACE/NAME",
},
{
name: "update-gatewayclass-status is set to empty string",
args: []string{
"--update-gatewayclass-status=",
},
wantErr: true,
expectedErrPrefix: `invalid argument "" for "--update-gatewayclass-status" flag: strconv.ParseBool`,
},
{
name: "update-gatewayclass-status is invalid",
args: []string{
"--update-gatewayclass-status=invalid", // not a boolean
},
wantErr: true,
expectedErrPrefix: `invalid argument "invalid" for "--update-gatewayclass-status" flag: strconv.ParseBool`,
},
}

if test.wantErr {
g.Expect(err).To(HaveOccurred())
g.Expect(err.Error()).To(HavePrefix(test.expectedErrPrefix))
} else {
g.Expect(err).ToNot(HaveOccurred())
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
cmd := createStaticModeCommand()
testFlag(t, cmd, test)
})
}
}
35 changes: 35 additions & 0 deletions cmd/gateway/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"regexp"
"strings"

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

Expand Down Expand Up @@ -54,6 +55,40 @@ func validateResourceName(value string) error {
return nil
}

func validateNamespaceName(value string) error {
// used by Kubernetes to validate resource namespace names
messages := validation.IsDNS1123Label(value)
if len(messages) > 0 {
msg := strings.Join(messages, "; ")
return fmt.Errorf("invalid format: %s", msg)
}

return nil
}

func parseNamespacedResourceName(value string) (types.NamespacedName, error) {
if value == "" {
return types.NamespacedName{}, errors.New("must be set")
}

parts := strings.Split(value, "/")
if len(parts) != 2 {
return types.NamespacedName{}, errors.New("invalid format; must be NAMESPACE/NAME")
}

if err := validateNamespaceName(parts[0]); err != nil {
return types.NamespacedName{}, fmt.Errorf("invalid namespace name: %w", err)
}
if err := validateResourceName(parts[1]); err != nil {
return types.NamespacedName{}, fmt.Errorf("invalid resource name: %w", err)
}

return types.NamespacedName{
Namespace: parts[0],
Name: parts[1],
}, nil
}

func validateIP(ip string) error {
if ip == "" {
return errors.New("IP address must be set")
Expand Down
Loading

0 comments on commit 8230e48

Please sign in to comment.