Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make static mode compatible with provisioner #657

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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. "+
kate-osborn marked this conversation as resolved.
Show resolved Hide resolved
"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