From aaf73c3a8903be0d81674d93748ea557bb2b3331 Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Fri, 16 Sep 2022 15:39:12 -0600 Subject: [PATCH 1/4] Remove unused gateway config CRD --- .../gateway.nginx.org_gatewayconfigs.yaml | 58 ------------------- deploy/manifests/gatewayconfig.yaml | 11 ---- 2 files changed, 69 deletions(-) delete mode 100644 deploy/manifests/crds/gateway.nginx.org_gatewayconfigs.yaml delete mode 100644 deploy/manifests/gatewayconfig.yaml diff --git a/deploy/manifests/crds/gateway.nginx.org_gatewayconfigs.yaml b/deploy/manifests/crds/gateway.nginx.org_gatewayconfigs.yaml deleted file mode 100644 index 02d3753d49..0000000000 --- a/deploy/manifests/crds/gateway.nginx.org_gatewayconfigs.yaml +++ /dev/null @@ -1,58 +0,0 @@ -apiVersion: apiextensions.k8s.io/v1 -kind: CustomResourceDefinition -metadata: - annotations: - controller-gen.kubebuilder.io/version: v0.9.2 - creationTimestamp: null - name: gatewayconfigs.gateway.nginx.org -spec: - group: gateway.nginx.org - names: - kind: GatewayConfig - listKind: GatewayConfigList - plural: gatewayconfigs - shortNames: - - gcfg - singular: gatewayconfig - scope: Cluster - versions: - - name: v1alpha1 - schema: - openAPIV3Schema: - type: object - required: - - spec - properties: - apiVersion: - description: 'APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' - type: string - kind: - description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' - type: string - metadata: - type: object - spec: - type: object - properties: - http: - type: object - properties: - accessLogs: - type: array - items: - type: object - required: - - destination - - format - properties: - destination: - type: string - format: - type: string - worker: - type: object - properties: - processes: - type: integer - served: true - storage: true diff --git a/deploy/manifests/gatewayconfig.yaml b/deploy/manifests/gatewayconfig.yaml deleted file mode 100644 index 2c7fa19e0d..0000000000 --- a/deploy/manifests/gatewayconfig.yaml +++ /dev/null @@ -1,11 +0,0 @@ -apiVersion: gateway.nginx.org/v1alpha1 -kind: GatewayConfig -metadata: - name: nginx -spec: - worker: - processes: 1 - http: - accessLogs: - - format: '$remote_addr - $remote_user [$time_local] "$request" $status $body_bytes_sent "$http_referer" "$http_user_agent" "$http_x_forwarded_for"' - destination: stdout \ No newline at end of file From 4eaef4e41715031c84a97e3a94b534e35bace5af Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Fri, 16 Sep 2022 15:40:29 -0600 Subject: [PATCH 2/4] Modify gateway-ctlr-name format and its validation Removes requirement for namespace in gateway-ctlr-name CLI arg. Expected format is now DOMAIN/PATH. Valid examples include: "k8s-gateway.nginx.org/nginx-gateway-controller", "k8s-gateway.nginx.org/nginx-gateway/nginx-gateway-controller", and "k8s-gateway.nginx.org/nginx-gateway/nginx-gateway-controller/v2". Updated validation of argument to use the regex provided by the kubebuilder annotation for GatewayClass.Spec.controllerName. Updated manifests to use the controller name "k8s-gateway.nginx.org/nginx-gateway-controller". --- cmd/gateway/main.go | 10 +++--- cmd/gateway/setup.go | 45 ++++++++++++------------- cmd/gateway/setup_test.go | 52 +++++++++++++---------------- deploy/manifests/gatewayclass.yaml | 2 +- deploy/manifests/nginx-gateway.yaml | 2 +- docs/cli-args.md | 2 +- 6 files changed, 52 insertions(+), 61 deletions(-) diff --git a/cmd/gateway/main.go b/cmd/gateway/main.go index 0c174ca6eb..dc83e877e3 100644 --- a/cmd/gateway/main.go +++ b/cmd/gateway/main.go @@ -12,8 +12,7 @@ import ( ) const ( - domain string = "k8s-gateway.nginx.org" - namespace string = "nginx-gateway" + domain string = "k8s-gateway.nginx.org" ) var ( @@ -26,13 +25,14 @@ var ( gatewayCtlrName = flag.String( "gateway-ctlr-name", "", - fmt.Sprintf("The name of the Gateway controller. The controller name must be of the form: DOMAIN/NAMESPACE/NAME. The controller's domain is '%s'; the namespace is '%s'", domain, namespace), + fmt.Sprintf("The name of the Gateway controller. The controller name must of the form DOMAIN/PATH. The controller's domain is '%s'", domain), ) gatewayClassName = flag.String( "gatewayclass", "", - "The name of the GatewayClass resource. Every NGINX Gateway must have a unique corresponding GatewayClass resource") + "The name of the GatewayClass resource. Every NGINX Gateway must have a unique corresponding GatewayClass resource", + ) ) func main() { @@ -47,7 +47,7 @@ func main() { MustValidateArguments( flag.CommandLine, - GatewayControllerParam(domain, namespace /* FIXME(f5yacobucci) dynamically set */), + GatewayControllerParam(domain), GatewayClassParam(), ) diff --git a/cmd/gateway/setup.go b/cmd/gateway/setup.go index b40107bba7..86cd7e6345 100644 --- a/cmd/gateway/setup.go +++ b/cmd/gateway/setup.go @@ -4,14 +4,19 @@ import ( "errors" "fmt" "os" + "regexp" "strings" flag "github.com/spf13/pflag" "k8s.io/apimachinery/pkg/util/validation" + + // Adding a dummy import here to remind us to check the controllerNameRegex when we update the Gateway API version. + _ "sigs.k8s.io/gateway-api/apis/v1beta1" ) const ( - errTmpl = "failed validation - flag: '--%s' reason: '%s'\n" + errTmpl = "failed validation - flag: '--%s' reason: '%s'\n" + controllerNameRegex = `^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*\/[A-Za-z0-9\/\-._~%!$&'()*+,;=:]+$` ) type ( @@ -22,13 +27,11 @@ type ( } ) -func GatewayControllerParam(domain string, namespace string) ValidatorContext { +func GatewayControllerParam(domain string) ValidatorContext { name := "gateway-ctlr-name" return ValidatorContext{ name, func(flagset *flag.FlagSet) error { - // FIXME(f5yacobucci) this does not provide the same regex validation as - // GatewayClass.ControllerName. provide equal and then specific validation param, err := flagset.GetString(name) if err != nil { return err @@ -40,34 +43,28 @@ func GatewayControllerParam(domain string, namespace string) ValidatorContext { fields := strings.Split(param, "/") l := len(fields) - if l != 3 { - return errors.New("unsupported path length, must be form DOMAIN/NAMESPACE/NAME") + if l < 2 { + return errors.New("invalid format; must be DOMAIN/PATH") } - for i := len(fields); i > 0; i-- { - switch i { - case 3: - if fields[0] != domain { - return fmt.Errorf("invalid domain: %s", fields[0]) - } - fields = fields[1:] - case 2: - if fields[0] != namespace { - return fmt.Errorf("cross namespace unsupported: %s", fields[0]) - } - fields = fields[1:] - case 1: - if fields[0] == "" { - return errors.New("must provide a name") - } - } + if fields[0] != domain { + return fmt.Errorf("invalid domain: %s; domain must be: %s", fields[0], domain) } - return nil + return validateControllerName(param) }, } } +func validateControllerName(name string) error { + // Regex from: https://github.com/kubernetes-sigs/gateway-api/blob/547122f7f55ac0464685552898c560658fb40073/apis/v1alpha2/shared_types.go#L462 + re := regexp.MustCompile(controllerNameRegex) + if !re.Match([]byte(name)) { + return fmt.Errorf("invalid gateway controller name: %s; expected format is DOMAIN/PATH", name) + } + return nil +} + func GatewayClassParam() ValidatorContext { name := "gatewayclass" return ValidatorContext{ diff --git a/cmd/gateway/setup_test.go b/cmd/gateway/setup_test.go index 9b8aab7aaf..b966ae41ae 100644 --- a/cmd/gateway/setup_test.go +++ b/cmd/gateway/setup_test.go @@ -156,7 +156,7 @@ var _ = Describe("Main", func() { return testCase{ Flag: "gateway-ctlr-name", Value: value, - ValidatorContext: GatewayControllerParam("k8s-gateway.nginx.org", "nginx-gateway"), + ValidatorContext: GatewayControllerParam("k8s-gateway.nginx.org"), ExpError: expError, } } @@ -171,29 +171,32 @@ var _ = Describe("Main", func() { mockFlags = nil }) - It("should parse full gateway-ctlr-name", func() { - t := prepareTestCase( - "k8s-gateway.nginx.org/nginx-gateway/my-gateway", - expectSuccess, - ) - tester(t) - }) // should parse full gateway-ctlr-name - - It("should fail with too many path elements", func() { - t := prepareTestCase( - "k8s-gateway.nginx.org/nginx-gateway/my-gateway/broken", - expectError) - tester(t) - }) // should fail with too many path elements + It("should parse valid gateway-ctlr-name", func() { + table := []testCase{ + prepareTestCase( + "k8s-gateway.nginx.org/my-gateway", + expectSuccess, + ), + prepareTestCase( + "k8s-gateway.nginx.org/nginx-gateway/my-gateway", + expectSuccess, + ), + prepareTestCase( + "k8s-gateway.nginx.org/nginx-gateway/my-gateway/v1", + expectSuccess, + ), + } + runner(table) + }) // should parse valid gateway-ctlr-name It("should fail with too few path elements", func() { table := []testCase{ prepareTestCase( - "nginx-gateway/my-gateway", + "k8s-gateway.nginx.org", expectError, ), prepareTestCase( - "my-gateway", + "k8s-gateway.nginx.org/", expectError, ), } @@ -205,7 +208,7 @@ var _ = Describe("Main", func() { table := []testCase{ prepareTestCase( // bad domain - "invalid-domain/nginx-gateway/my-gateway", + "invalid-domain/my-gateway", expectError, ), prepareTestCase( @@ -213,18 +216,9 @@ var _ = Describe("Main", func() { "/default/my-gateway", expectError, ), - prepareTestCase( - // bad namespace - "k8s-gateway.nginx.org/default/my-gateway", - expectError), - prepareTestCase( - // bad namespace - "k8s-gateway.nginx.org//my-gateway", - expectError, - ), prepareTestCase( // bad name - "k8s-gateway.nginx.org/default/", + "k8s-gateway.nginx.org/", expectError, ), } @@ -266,7 +260,7 @@ var _ = Describe("Main", func() { "$nginx", expectError) tester(t) - }) // should fail with invalid name" + }) // should fail with invalid name }) // gatewayclass validation }) // CLI argument validation }) // end Main diff --git a/deploy/manifests/gatewayclass.yaml b/deploy/manifests/gatewayclass.yaml index aad4511d8b..bfa14bb077 100644 --- a/deploy/manifests/gatewayclass.yaml +++ b/deploy/manifests/gatewayclass.yaml @@ -3,4 +3,4 @@ kind: GatewayClass metadata: name: nginx spec: - controllerName: k8s-gateway.nginx.org/nginx-gateway/gateway + controllerName: k8s-gateway.nginx.org/nginx-gateway-controller diff --git a/deploy/manifests/nginx-gateway.yaml b/deploy/manifests/nginx-gateway.yaml index 6bf89ba1f8..c66b66f8ec 100644 --- a/deploy/manifests/nginx-gateway.yaml +++ b/deploy/manifests/nginx-gateway.yaml @@ -98,7 +98,7 @@ spec: # dropping ALL and adding only CAP_KILL doesn't work # Note: CAP_KILL is needed for sending HUP signal to NGINX main process args: - - --gateway-ctlr-name=k8s-gateway.nginx.org/nginx-gateway/gateway + - --gateway-ctlr-name=k8s-gateway.nginx.org/nginx-gateway-controller - --gatewayclass=nginx - image: nginx:1.23.1 imagePullPolicy: IfNotPresent diff --git a/docs/cli-args.md b/docs/cli-args.md index d308e657ef..a0d7675c5e 100644 --- a/docs/cli-args.md +++ b/docs/cli-args.md @@ -4,5 +4,5 @@ The table below describes the command-line arguments of the `gateway` binary fro | Name | Type | Description | |-|-|-| -|`gateway-ctlr-name` | `string` | The name of the Gateway controller. The controller name must be of the form: `DOMAIN/NAMESPACE/NAME`. The controller's domain is `k8s-gateway.nginx.org`; the namespace is `nginx-ingress`. | +|`gateway-ctlr-name` | `string` | The name of the Gateway controller. The controller name must be of the form: `DOMAIN/PATH`. The controller's domain is `k8s-gateway.nginx.org`. | |`gatewayclass`| `string` | The name of the GatewayClass resource. Every NGINX Gateway must have a unique corresponding GatewayClass resource. | From 1b021cc253b225307b133d0b2dd4fd50b66e092e Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Mon, 19 Sep 2022 16:41:34 -0600 Subject: [PATCH 3/4] Fix typo --- cmd/gateway/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/gateway/main.go b/cmd/gateway/main.go index dc83e877e3..e9b53ae20b 100644 --- a/cmd/gateway/main.go +++ b/cmd/gateway/main.go @@ -25,7 +25,7 @@ var ( gatewayCtlrName = flag.String( "gateway-ctlr-name", "", - fmt.Sprintf("The name of the Gateway controller. The controller name must of the form DOMAIN/PATH. The controller's domain is '%s'", domain), + fmt.Sprintf("The name of the Gateway controller. The controller name must be of the form: DOMAIN/PATH. The controller's domain is '%s'", domain), ) gatewayClassName = flag.String( From 29e6ea0374aa35924cd4ff5d45ec6a229199b714 Mon Sep 17 00:00:00 2001 From: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> Date: Tue, 20 Sep 2022 13:43:14 -0600 Subject: [PATCH 4/4] Update cmd/gateway/setup.go Use re.MatchString instead of re.Match Co-authored-by: Michael Pleshakov --- cmd/gateway/setup.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/gateway/setup.go b/cmd/gateway/setup.go index 86cd7e6345..5084eb9324 100644 --- a/cmd/gateway/setup.go +++ b/cmd/gateway/setup.go @@ -59,7 +59,7 @@ func GatewayControllerParam(domain string) ValidatorContext { func validateControllerName(name string) error { // Regex from: https://github.com/kubernetes-sigs/gateway-api/blob/547122f7f55ac0464685552898c560658fb40073/apis/v1alpha2/shared_types.go#L462 re := regexp.MustCompile(controllerNameRegex) - if !re.Match([]byte(name)) { + if !re.MatchString(name) { return fmt.Errorf("invalid gateway controller name: %s; expected format is DOMAIN/PATH", name) } return nil