From 2459135ed400b6f05d2d02d16b98e31f7fd0faea Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Thu, 5 Sep 2024 16:56:37 -0600 Subject: [PATCH 1/6] Watch SnippetsFilters when feature is enabled --- apis/v1alpha1/snippetsfilter_types.go | 7 +- charts/nginx-gateway-fabric/README.md | 1 + .../templates/clusterrole.yaml | 6 + .../templates/deployment.yaml | 3 + charts/nginx-gateway-fabric/values.yaml | 5 + cmd/gateway/commands.go | 12 + cmd/gateway/commands_test.go | 10 + .../gateway.nginx.org_snippetsfilters.yaml | 4 + .../snippets-filters-nginx-plus/deploy.yaml | 346 ++++++++++++++++++ deploy/snippets-filters/deploy.yaml | 337 +++++++++++++++++ examples/helm/README.md | 2 +- .../snippets-filters-nginx-plus/values.yaml | 12 + examples/helm/snippets-filters/values.yaml | 4 + examples/snippets-filter/README.md | 3 + examples/snippets-filter/snippets-filter.yaml | 10 + internal/mode/static/config/config.go | 2 + internal/mode/static/manager.go | 39 +- internal/mode/static/manager_test.go | 102 +++++- .../mode/static/state/change_processor.go | 6 + .../static/state/conditions/conditions.go | 13 +- internal/mode/static/state/graph/graph.go | 6 + .../mode/static/state/graph/graph_test.go | 26 ++ .../static/state/graph/snippets_filter.go | 94 +++++ .../state/graph/snippets_filter_test.go | 255 +++++++++++++ site/content/reference/api.md | 8 +- site/content/reference/cli-help.md | 13 +- 26 files changed, 1287 insertions(+), 39 deletions(-) create mode 100644 deploy/snippets-filters-nginx-plus/deploy.yaml create mode 100644 deploy/snippets-filters/deploy.yaml create mode 100644 examples/helm/snippets-filters-nginx-plus/values.yaml create mode 100644 examples/helm/snippets-filters/values.yaml create mode 100644 examples/snippets-filter/README.md create mode 100644 examples/snippets-filter/snippets-filter.yaml create mode 100644 internal/mode/static/state/graph/snippets_filter.go create mode 100644 internal/mode/static/state/graph/snippets_filter_test.go diff --git a/apis/v1alpha1/snippetsfilter_types.go b/apis/v1alpha1/snippetsfilter_types.go index c8941fb2f4..8d98e039e2 100644 --- a/apis/v1alpha1/snippetsfilter_types.go +++ b/apis/v1alpha1/snippetsfilter_types.go @@ -38,6 +38,9 @@ type SnippetsFilterSpec struct { // Snippets is a list of NGINX configuration snippets. // There can only be one snippet per context. // Allowed contexts: main, http, http.server, http.server.location. + // +kubebuilder:validation:MaxItems=4 + // +kubebuilder:validation:XValidation:message="Only one snippet allowed per context",rule="self.all(s1, self.exists_one(s2, s1.context == s2.context))" + //nolint:lll Snippets []Snippet `json:"snippets"` } @@ -104,7 +107,7 @@ const ( // the condition is true. SnippetsFilterConditionReasonAccepted SnippetsFilterConditionReason = "Accepted" - // SnippetsFilterConditionTypeInvalid is used with the Accepted condition type when + // SnippetsFilterConditionReasonInvalid is used with the Accepted condition type when // SnippetsFilter is invalid. - SnippetsFilterConditionTypeInvalid SnippetsFilterConditionType = "Invalid" + SnippetsFilterConditionReasonInvalid SnippetsFilterConditionReason = "Invalid" ) diff --git a/charts/nginx-gateway-fabric/README.md b/charts/nginx-gateway-fabric/README.md index 9ca6cb9073..115a9c6ace 100644 --- a/charts/nginx-gateway-fabric/README.md +++ b/charts/nginx-gateway-fabric/README.md @@ -293,6 +293,7 @@ The following table lists the configurable parameters of the NGINX Gateway Fabri | `nginxGateway.replicaCount` | The number of replicas of the NGINX Gateway Fabric Deployment. | int | `1` | | `nginxGateway.resources` | The resource requests and/or limits of the nginx-gateway container. | object | `{}` | | `nginxGateway.securityContext.allowPrivilegeEscalation` | Some environments may need this set to true in order for the control plane to successfully reload NGINX. | bool | `false` | +| `nginxGateway.snippetsFilters.enable` | Enable SnippetsFilters feature. SnippetsFilters allow inserting NGINX configuration into the generated NGINX config for HTTPRoute and GRPCRoute resources. | bool | `false` | | `nodeSelector` | The nodeSelector of the NGINX Gateway Fabric pod. | object | `{}` | | `service.annotations` | The annotations of the NGINX Gateway Fabric service. | object | `{}` | | `service.create` | Creates a service to expose the NGINX Gateway Fabric pods. | bool | `true` | diff --git a/charts/nginx-gateway-fabric/templates/clusterrole.yaml b/charts/nginx-gateway-fabric/templates/clusterrole.yaml index 65c184ef47..e43d1e76cb 100644 --- a/charts/nginx-gateway-fabric/templates/clusterrole.yaml +++ b/charts/nginx-gateway-fabric/templates/clusterrole.yaml @@ -104,6 +104,9 @@ rules: - nginxproxies - clientsettingspolicies - observabilitypolicies + {{- if .Values.nginxGateway.snippetsFilters.enable }} + - snippetsfilters + {{- end }} verbs: - list - watch @@ -113,6 +116,9 @@ rules: - nginxgateways/status - clientsettingspolicies/status - observabilitypolicies/status + {{- if .Values.nginxGateway.snippetsFilters.enable }} + - snippetsfilters/status + {{- end }} verbs: - update {{- if .Values.nginxGateway.leaderElection.enable }} diff --git a/charts/nginx-gateway-fabric/templates/deployment.yaml b/charts/nginx-gateway-fabric/templates/deployment.yaml index 6ce6240c29..948654d33e 100644 --- a/charts/nginx-gateway-fabric/templates/deployment.yaml +++ b/charts/nginx-gateway-fabric/templates/deployment.yaml @@ -75,6 +75,9 @@ spec: {{- if .Values.nginx.usage.insecureSkipVerify }} - --usage-report-skip-verify {{- end }} + {{- if .Values.nginxGateway.snippetsFilters.enable }} + - --snippets-filters + {{- end }} env: - name: POD_IP valueFrom: diff --git a/charts/nginx-gateway-fabric/values.yaml b/charts/nginx-gateway-fabric/values.yaml index 9cfc2064b2..eb604b72bf 100644 --- a/charts/nginx-gateway-fabric/values.yaml +++ b/charts/nginx-gateway-fabric/values.yaml @@ -78,6 +78,11 @@ nginxGateway: # APIs installed from the experimental channel. enable: false + snippetsFilters: + # -- Enable SnippetsFilters feature. SnippetsFilters allow inserting NGINX configuration into the generated NGINX + # config for HTTPRoute and GRPCRoute resources. + enable: false + nginx: image: # -- The NGINX image to use. diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index c95b74b4a9..fe3eb51c47 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -65,6 +65,7 @@ func createStaticModeCommand() *cobra.Command { usageReportServerURLFlag = "usage-report-server-url" usageReportSkipVerifyFlag = "usage-report-skip-verify" usageReportClusterNameFlag = "usage-report-cluster-name" + snippetsFiltersFlag = "snippets-filters" ) // flag values @@ -116,6 +117,8 @@ func createStaticModeCommand() *cobra.Command { usageReportServerURL = stringValidatingValue{ validator: validateURL, } + + snippetsFilters bool ) cmd := &cobra.Command{ @@ -239,6 +242,7 @@ func createStaticModeCommand() *cobra.Command { Names: flagKeys, Values: flagValues, }, + SnippetsFilters: snippetsFilters, } if err := static.StartManager(conf); err != nil { @@ -394,6 +398,14 @@ func createStaticModeCommand() *cobra.Command { "Disable client verification of the NGINX Plus usage reporting server certificate.", ) + cmd.Flags().BoolVar( + &snippetsFilters, + snippetsFiltersFlag, + false, + "Enable SnippetsFilters feature. SnippetsFilters allow inserting NGINX configuration into the "+ + "generated NGINX config for HTTPRoute and GRPCRoute resources.", + ) + return cmd } diff --git a/cmd/gateway/commands_test.go b/cmd/gateway/commands_test.go index 16ec9daa5a..45f6562960 100644 --- a/cmd/gateway/commands_test.go +++ b/cmd/gateway/commands_test.go @@ -166,6 +166,7 @@ func TestStaticModeCmdFlagValidation(t *testing.T) { "--usage-report-secret=default/my-secret", "--usage-report-server-url=https://my-api.com", "--usage-report-cluster-name=my-cluster", + "--snippets-filters", }, wantErr: false, }, @@ -381,6 +382,15 @@ func TestStaticModeCmdFlagValidation(t *testing.T) { wantErr: true, expectedErrPrefix: `invalid argument "$invalid*(#)" for "--usage-report-cluster-name" flag: invalid format`, }, + { + name: "snippets-filters is not a bool", + expectedErrPrefix: `invalid argument "not-a-bool" for "--snippets-filters" flag: strconv.ParseBool:` + + ` parsing "not-a-bool": invalid syntax`, + args: []string{ + "--snippets-filters=not-a-bool", + }, + wantErr: true, + }, } // common flags validation is tested separately diff --git a/config/crd/bases/gateway.nginx.org_snippetsfilters.yaml b/config/crd/bases/gateway.nginx.org_snippetsfilters.yaml index 364989ec7f..6f719d73df 100644 --- a/config/crd/bases/gateway.nginx.org_snippetsfilters.yaml +++ b/config/crd/bases/gateway.nginx.org_snippetsfilters.yaml @@ -73,7 +73,11 @@ spec: - context - value type: object + maxItems: 4 type: array + x-kubernetes-validations: + - message: Only one snippet allowed per context + rule: self.all(s1, self.exists_one(s2, s1.context == s2.context)) required: - snippets type: object diff --git a/deploy/snippets-filters-nginx-plus/deploy.yaml b/deploy/snippets-filters-nginx-plus/deploy.yaml new file mode 100644 index 0000000000..b0f42c3f6a --- /dev/null +++ b/deploy/snippets-filters-nginx-plus/deploy.yaml @@ -0,0 +1,346 @@ +apiVersion: v1 +kind: Namespace +metadata: + name: nginx-gateway +--- +apiVersion: v1 +imagePullSecrets: +- name: nginx-plus-registry-secret +kind: ServiceAccount +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-gateway + namespace: nginx-gateway +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-gateway +rules: +- apiGroups: + - "" + resources: + - namespaces + - services + - secrets + verbs: + - get + - list + - watch +- apiGroups: + - "" + resources: + - pods + verbs: + - get +- apiGroups: + - apps + resources: + - replicasets + verbs: + - get +- apiGroups: + - apps + resources: + - replicasets + verbs: + - list +- apiGroups: + - "" + resources: + - nodes + verbs: + - list +- apiGroups: + - "" + resources: + - events + verbs: + - create + - patch +- apiGroups: + - discovery.k8s.io + resources: + - endpointslices + verbs: + - list + - watch +- apiGroups: + - gateway.networking.k8s.io + resources: + - gatewayclasses + - gateways + - httproutes + - referencegrants + - grpcroutes + verbs: + - list + - watch +- apiGroups: + - gateway.networking.k8s.io + resources: + - httproutes/status + - gateways/status + - gatewayclasses/status + - grpcroutes/status + verbs: + - update +- apiGroups: + - gateway.nginx.org + resources: + - nginxgateways + verbs: + - get + - list + - watch +- apiGroups: + - gateway.nginx.org + resources: + - nginxproxies + - clientsettingspolicies + - observabilitypolicies + - snippetsfilters + verbs: + - list + - watch +- apiGroups: + - gateway.nginx.org + resources: + - nginxgateways/status + - clientsettingspolicies/status + - observabilitypolicies/status + - snippetsfilters/status + verbs: + - update +- apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - create + - get + - update +- apiGroups: + - apiextensions.k8s.io + resources: + - customresourcedefinitions + verbs: + - list + - watch +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-gateway +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: nginx-gateway +subjects: +- kind: ServiceAccount + name: nginx-gateway + namespace: nginx-gateway +--- +apiVersion: v1 +kind: Service +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-gateway + namespace: nginx-gateway +spec: + externalTrafficPolicy: Local + ports: + - name: http + port: 80 + protocol: TCP + targetPort: 80 + - name: https + port: 443 + protocol: TCP + targetPort: 443 + selector: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + type: LoadBalancer +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-gateway + namespace: nginx-gateway +spec: + replicas: 1 + selector: + matchLabels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + template: + metadata: + annotations: + prometheus.io/port: "9113" + prometheus.io/scrape: "true" + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + spec: + containers: + - args: + - static-mode + - --gateway-ctlr-name=gateway.nginx.org/nginx-gateway-controller + - --gatewayclass=nginx + - --config=nginx-gateway-config + - --service=nginx-gateway + - --nginx-plus + - --metrics-port=9113 + - --health-port=8081 + - --leader-election-lock-name=nginx-gateway-leader-election + - --snippets-filters + env: + - name: POD_IP + valueFrom: + fieldRef: + fieldPath: status.podIP + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name + image: ghcr.io/nginxinc/nginx-gateway-fabric:edge + imagePullPolicy: Always + name: nginx-gateway + ports: + - containerPort: 9113 + name: metrics + - containerPort: 8081 + name: health + readinessProbe: + httpGet: + path: /readyz + port: health + initialDelaySeconds: 3 + periodSeconds: 1 + securityContext: + allowPrivilegeEscalation: false + capabilities: + add: + - KILL + drop: + - ALL + readOnlyRootFilesystem: true + runAsGroup: 1001 + runAsUser: 102 + seccompProfile: + type: RuntimeDefault + volumeMounts: + - mountPath: /etc/nginx/conf.d + name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf + - mountPath: /etc/nginx/module-includes + name: module-includes + - mountPath: /etc/nginx/secrets + name: nginx-secrets + - mountPath: /var/run/nginx + name: nginx-run + - mountPath: /etc/nginx/includes + name: nginx-includes + - image: private-registry.nginx.com/nginx-gateway-fabric/nginx-plus:edge + imagePullPolicy: Always + name: nginx + ports: + - containerPort: 80 + name: http + - containerPort: 443 + name: https + securityContext: + capabilities: + add: + - NET_BIND_SERVICE + drop: + - ALL + readOnlyRootFilesystem: true + runAsGroup: 1001 + runAsUser: 101 + seccompProfile: + type: RuntimeDefault + volumeMounts: + - mountPath: /etc/nginx/conf.d + name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf + - mountPath: /etc/nginx/module-includes + name: module-includes + - mountPath: /etc/nginx/secrets + name: nginx-secrets + - mountPath: /var/run/nginx + name: nginx-run + - mountPath: /var/cache/nginx + name: nginx-cache + - mountPath: /etc/nginx/includes + name: nginx-includes + securityContext: + fsGroup: 1001 + runAsNonRoot: true + serviceAccountName: nginx-gateway + shareProcessNamespace: true + terminationGracePeriodSeconds: 30 + volumes: + - emptyDir: {} + name: nginx-conf + - emptyDir: {} + name: nginx-stream-conf + - emptyDir: {} + name: module-includes + - emptyDir: {} + name: nginx-secrets + - emptyDir: {} + name: nginx-run + - emptyDir: {} + name: nginx-cache + - emptyDir: {} + name: nginx-includes +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: GatewayClass +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx +spec: + controllerName: gateway.nginx.org/nginx-gateway-controller +--- +apiVersion: gateway.nginx.org/v1alpha1 +kind: NginxGateway +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-gateway-config + namespace: nginx-gateway +spec: + logging: + level: info diff --git a/deploy/snippets-filters/deploy.yaml b/deploy/snippets-filters/deploy.yaml new file mode 100644 index 0000000000..7cabc5994a --- /dev/null +++ b/deploy/snippets-filters/deploy.yaml @@ -0,0 +1,337 @@ +apiVersion: v1 +kind: Namespace +metadata: + name: nginx-gateway +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-gateway + namespace: nginx-gateway +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-gateway +rules: +- apiGroups: + - "" + resources: + - namespaces + - services + - secrets + verbs: + - get + - list + - watch +- apiGroups: + - "" + resources: + - pods + verbs: + - get +- apiGroups: + - apps + resources: + - replicasets + verbs: + - get +- apiGroups: + - "" + resources: + - nodes + verbs: + - list +- apiGroups: + - "" + resources: + - events + verbs: + - create + - patch +- apiGroups: + - discovery.k8s.io + resources: + - endpointslices + verbs: + - list + - watch +- apiGroups: + - gateway.networking.k8s.io + resources: + - gatewayclasses + - gateways + - httproutes + - referencegrants + - grpcroutes + verbs: + - list + - watch +- apiGroups: + - gateway.networking.k8s.io + resources: + - httproutes/status + - gateways/status + - gatewayclasses/status + - grpcroutes/status + verbs: + - update +- apiGroups: + - gateway.nginx.org + resources: + - nginxgateways + verbs: + - get + - list + - watch +- apiGroups: + - gateway.nginx.org + resources: + - nginxproxies + - clientsettingspolicies + - observabilitypolicies + - snippetsfilters + verbs: + - list + - watch +- apiGroups: + - gateway.nginx.org + resources: + - nginxgateways/status + - clientsettingspolicies/status + - observabilitypolicies/status + - snippetsfilters/status + verbs: + - update +- apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - create + - get + - update +- apiGroups: + - apiextensions.k8s.io + resources: + - customresourcedefinitions + verbs: + - list + - watch +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-gateway +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: nginx-gateway +subjects: +- kind: ServiceAccount + name: nginx-gateway + namespace: nginx-gateway +--- +apiVersion: v1 +kind: Service +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-gateway + namespace: nginx-gateway +spec: + externalTrafficPolicy: Local + ports: + - name: http + port: 80 + protocol: TCP + targetPort: 80 + - name: https + port: 443 + protocol: TCP + targetPort: 443 + selector: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + type: LoadBalancer +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-gateway + namespace: nginx-gateway +spec: + replicas: 1 + selector: + matchLabels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + template: + metadata: + annotations: + prometheus.io/port: "9113" + prometheus.io/scrape: "true" + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + spec: + containers: + - args: + - static-mode + - --gateway-ctlr-name=gateway.nginx.org/nginx-gateway-controller + - --gatewayclass=nginx + - --config=nginx-gateway-config + - --service=nginx-gateway + - --metrics-port=9113 + - --health-port=8081 + - --leader-election-lock-name=nginx-gateway-leader-election + - --snippets-filters + env: + - name: POD_IP + valueFrom: + fieldRef: + fieldPath: status.podIP + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name + image: ghcr.io/nginxinc/nginx-gateway-fabric:edge + imagePullPolicy: Always + name: nginx-gateway + ports: + - containerPort: 9113 + name: metrics + - containerPort: 8081 + name: health + readinessProbe: + httpGet: + path: /readyz + port: health + initialDelaySeconds: 3 + periodSeconds: 1 + securityContext: + allowPrivilegeEscalation: false + capabilities: + add: + - KILL + drop: + - ALL + readOnlyRootFilesystem: true + runAsGroup: 1001 + runAsUser: 102 + seccompProfile: + type: RuntimeDefault + volumeMounts: + - mountPath: /etc/nginx/conf.d + name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf + - mountPath: /etc/nginx/module-includes + name: module-includes + - mountPath: /etc/nginx/secrets + name: nginx-secrets + - mountPath: /var/run/nginx + name: nginx-run + - mountPath: /etc/nginx/includes + name: nginx-includes + - image: ghcr.io/nginxinc/nginx-gateway-fabric/nginx:edge + imagePullPolicy: Always + name: nginx + ports: + - containerPort: 80 + name: http + - containerPort: 443 + name: https + securityContext: + capabilities: + add: + - NET_BIND_SERVICE + drop: + - ALL + readOnlyRootFilesystem: true + runAsGroup: 1001 + runAsUser: 101 + seccompProfile: + type: RuntimeDefault + volumeMounts: + - mountPath: /etc/nginx/conf.d + name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf + - mountPath: /etc/nginx/module-includes + name: module-includes + - mountPath: /etc/nginx/secrets + name: nginx-secrets + - mountPath: /var/run/nginx + name: nginx-run + - mountPath: /var/cache/nginx + name: nginx-cache + - mountPath: /etc/nginx/includes + name: nginx-includes + securityContext: + fsGroup: 1001 + runAsNonRoot: true + serviceAccountName: nginx-gateway + shareProcessNamespace: true + terminationGracePeriodSeconds: 30 + volumes: + - emptyDir: {} + name: nginx-conf + - emptyDir: {} + name: nginx-stream-conf + - emptyDir: {} + name: module-includes + - emptyDir: {} + name: nginx-secrets + - emptyDir: {} + name: nginx-run + - emptyDir: {} + name: nginx-cache + - emptyDir: {} + name: nginx-includes +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: GatewayClass +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx +spec: + controllerName: gateway.nginx.org/nginx-gateway-controller +--- +apiVersion: gateway.nginx.org/v1alpha1 +kind: NginxGateway +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-gateway-config + namespace: nginx-gateway +spec: + logging: + level: info diff --git a/examples/helm/README.md b/examples/helm/README.md index dc8f8b440f..7d66f2ee4a 100644 --- a/examples/helm/README.md +++ b/examples/helm/README.md @@ -8,7 +8,7 @@ This directory contains examples of Helm charts that can be used to deploy NGINX ## Examples -- [Default](./default) - deploys NGINX Gateway Fabric withg NGINX OSS with default configuration. +- [Default](./default) - deploys NGINX Gateway Fabric with NGINX OSS with default configuration. - [NGINX Plus](./nginx-plus) - deploys NGINX Gateway Fabric with NGINX Plus as the data plane. The image is pulled from the NGINX Plus Docker registry, and the `imagePullSecretName` is the name of the secret to use to pull the image. The secret must be created in the same namespace as the NGINX Gateway Fabric deployment. diff --git a/examples/helm/snippets-filters-nginx-plus/values.yaml b/examples/helm/snippets-filters-nginx-plus/values.yaml new file mode 100644 index 0000000000..9cacfdb168 --- /dev/null +++ b/examples/helm/snippets-filters-nginx-plus/values.yaml @@ -0,0 +1,12 @@ +nginxGateway: + name: nginx-gateway + snippetsFilters: + enable: true + +nginx: + plus: true + image: + repository: private-registry.nginx.com/nginx-gateway-fabric/nginx-plus + +serviceAccount: + imagePullSecret: nginx-plus-registry-secret diff --git a/examples/helm/snippets-filters/values.yaml b/examples/helm/snippets-filters/values.yaml new file mode 100644 index 0000000000..898cbf1e74 --- /dev/null +++ b/examples/helm/snippets-filters/values.yaml @@ -0,0 +1,4 @@ +nginxGateway: + name: nginx-gateway + snippetsFilters: + enable: true diff --git a/examples/snippets-filter/README.md b/examples/snippets-filter/README.md new file mode 100644 index 0000000000..f09032d512 --- /dev/null +++ b/examples/snippets-filter/README.md @@ -0,0 +1,3 @@ +# SnippetsFilter + +This directory contains example YAMLs for testing SnippetsFilter. Eventually, this will be converted into a how-to guide. diff --git a/examples/snippets-filter/snippets-filter.yaml b/examples/snippets-filter/snippets-filter.yaml new file mode 100644 index 0000000000..cefe9a6ccb --- /dev/null +++ b/examples/snippets-filter/snippets-filter.yaml @@ -0,0 +1,10 @@ +apiVersion: gateway.nginx.org/v1alpha1 +kind: SnippetsFilter +metadata: + name: access-control +spec: + snippets: + - context: http.server.location + value: | + allow 10.0.0.0/8; + deny all; diff --git a/internal/mode/static/config/config.go b/internal/mode/static/config/config.go index 1a26cf5f03..9424deb5c2 100644 --- a/internal/mode/static/config/config.go +++ b/internal/mode/static/config/config.go @@ -46,6 +46,8 @@ type Config struct { Plus bool // ExperimentalFeatures indicates if experimental features are enabled. ExperimentalFeatures bool + // SnippetsFilters indicates if SnippetsFilters are enabled. + SnippetsFilters bool } // GatewayPodConfig contains information about this Pod. diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 9663f215cf..e07b4e5690 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -241,11 +241,8 @@ func StartManager(cfg config.Config) error { updateGatewayClassStatus: cfg.UpdateGatewayClassStatus, }) - objects, objectLists := prepareFirstEventBatchPreparerArgs( - cfg.GatewayClassName, - cfg.GatewayNsName, - cfg.ExperimentalFeatures, - ) + objects, objectLists := prepareFirstEventBatchPreparerArgs(cfg) + firstBatchPreparer := events.NewFirstEventBatchPreparerImpl(mgr.GetCache(), objects, objectLists) eventLoop := events.NewEventLoop( eventCh, @@ -533,6 +530,19 @@ func registerControllers( } } + if cfg.SnippetsFilters { + cfg.Logger.V(1).Info("Registering controller for SnippetsFilter") + + controllerRegCfgs = append(controllerRegCfgs, + ctlrCfg{ + objectType: &ngfAPI.SnippetsFilter{}, + options: []controller.Option{ + controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}), + }, + }, + ) + } + for _, regCfg := range controllerRegCfgs { name := regCfg.objectType.GetObjectKind().GroupVersionKind().Kind if regCfg.name != "" { @@ -650,13 +660,9 @@ func createUsageWarningJob(cfg config.Config, readyCh <-chan struct{}) *runnable } } -func prepareFirstEventBatchPreparerArgs( - gcName string, - gwNsName *types.NamespacedName, - enableExperimentalFeatures bool, -) ([]client.Object, []client.ObjectList) { +func prepareFirstEventBatchPreparerArgs(cfg config.Config) ([]client.Object, []client.ObjectList) { objects := []client.Object{ - &gatewayv1.GatewayClass{ObjectMeta: metav1.ObjectMeta{Name: gcName}}, + &gatewayv1.GatewayClass{ObjectMeta: metav1.ObjectMeta{Name: cfg.GatewayClassName}}, } partialObjectMetadataList := &metav1.PartialObjectMetadataList{} @@ -682,7 +688,7 @@ func prepareFirstEventBatchPreparerArgs( partialObjectMetadataList, } - if enableExperimentalFeatures { + if cfg.ExperimentalFeatures { objectLists = append( objectLists, &gatewayv1alpha3.BackendTLSPolicyList{}, @@ -691,6 +697,15 @@ func prepareFirstEventBatchPreparerArgs( ) } + if cfg.SnippetsFilters { + objectLists = append( + objectLists, + &ngfAPI.SnippetsFilterList{}, + ) + } + + gwNsName := cfg.GatewayNsName + if gwNsName == nil { objectLists = append(objectLists, &gatewayv1.GatewayList{}) } else { diff --git a/internal/mode/static/manager_test.go b/internal/mode/static/manager_test.go index 041f5062b6..3e4f20b5e9 100644 --- a/internal/mode/static/manager_test.go +++ b/internal/mode/static/manager_test.go @@ -34,15 +34,19 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { ) tests := []struct { - name string - gwNsName *types.NamespacedName expectedObjects []client.Object expectedObjectLists []client.ObjectList - experimentalEnabled bool + name string + cfg config.Config }{ { - name: "gwNsName is nil", - gwNsName: nil, + name: "gwNsName is nil", + cfg: config.Config{ + GatewayClassName: gcName, + GatewayNsName: nil, + ExperimentalFeatures: false, + SnippetsFilters: false, + }, expectedObjects: []client.Object{ &gatewayv1.GatewayClass{ObjectMeta: metav1.ObjectMeta{Name: "nginx"}}, }, @@ -63,9 +67,14 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { }, { name: "gwNsName is not nil", - gwNsName: &types.NamespacedName{ - Namespace: "test", - Name: "my-gateway", + cfg: config.Config{ + GatewayClassName: gcName, + GatewayNsName: &types.NamespacedName{ + Namespace: "test", + Name: "my-gateway", + }, + ExperimentalFeatures: false, + SnippetsFilters: false, }, expectedObjects: []client.Object{ &gatewayv1.GatewayClass{ObjectMeta: metav1.ObjectMeta{Name: "nginx"}}, @@ -87,9 +96,76 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { }, { name: "gwNsName is not nil and experimental enabled", - gwNsName: &types.NamespacedName{ - Namespace: "test", - Name: "my-gateway", + cfg: config.Config{ + GatewayClassName: gcName, + GatewayNsName: &types.NamespacedName{ + Namespace: "test", + Name: "my-gateway", + }, + ExperimentalFeatures: true, + SnippetsFilters: false, + }, + expectedObjects: []client.Object{ + &gatewayv1.GatewayClass{ObjectMeta: metav1.ObjectMeta{Name: "nginx"}}, + &gatewayv1.Gateway{ObjectMeta: metav1.ObjectMeta{Name: "my-gateway", Namespace: "test"}}, + }, + expectedObjectLists: []client.ObjectList{ + &apiv1.ServiceList{}, + &apiv1.SecretList{}, + &apiv1.NamespaceList{}, + &apiv1.ConfigMapList{}, + &discoveryV1.EndpointSliceList{}, + &gatewayv1.HTTPRouteList{}, + &gatewayv1beta1.ReferenceGrantList{}, + &ngfAPI.NginxProxyList{}, + partialObjectMetadataList, + &gatewayv1alpha3.BackendTLSPolicyList{}, + &gatewayv1alpha2.TLSRouteList{}, + &gatewayv1.GRPCRouteList{}, + &ngfAPI.ClientSettingsPolicyList{}, + &ngfAPI.ObservabilityPolicyList{}, + }, + }, + { + name: "gwNsName is not nil and snippets filters enabled", + cfg: config.Config{ + GatewayClassName: gcName, + GatewayNsName: &types.NamespacedName{ + Namespace: "test", + Name: "my-gateway", + }, + ExperimentalFeatures: false, + SnippetsFilters: true, + }, + expectedObjects: []client.Object{ + &gatewayv1.GatewayClass{ObjectMeta: metav1.ObjectMeta{Name: "nginx"}}, + &gatewayv1.Gateway{ObjectMeta: metav1.ObjectMeta{Name: "my-gateway", Namespace: "test"}}, + }, + expectedObjectLists: []client.ObjectList{ + &apiv1.ServiceList{}, + &apiv1.SecretList{}, + &apiv1.NamespaceList{}, + &discoveryV1.EndpointSliceList{}, + &gatewayv1.HTTPRouteList{}, + &gatewayv1beta1.ReferenceGrantList{}, + &ngfAPI.NginxProxyList{}, + partialObjectMetadataList, + &gatewayv1.GRPCRouteList{}, + &ngfAPI.ClientSettingsPolicyList{}, + &ngfAPI.ObservabilityPolicyList{}, + &ngfAPI.SnippetsFilterList{}, + }, + }, + { + name: "gwNsName is not nil, experimental and snippets filters enabled", + cfg: config.Config{ + GatewayClassName: gcName, + GatewayNsName: &types.NamespacedName{ + Namespace: "test", + Name: "my-gateway", + }, + ExperimentalFeatures: true, + SnippetsFilters: true, }, expectedObjects: []client.Object{ &gatewayv1.GatewayClass{ObjectMeta: metav1.ObjectMeta{Name: "nginx"}}, @@ -110,8 +186,8 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { &gatewayv1.GRPCRouteList{}, &ngfAPI.ClientSettingsPolicyList{}, &ngfAPI.ObservabilityPolicyList{}, + &ngfAPI.SnippetsFilterList{}, }, - experimentalEnabled: true, }, } @@ -119,7 +195,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - objects, objectLists := prepareFirstEventBatchPreparerArgs(gcName, test.gwNsName, test.experimentalEnabled) + objects, objectLists := prepareFirstEventBatchPreparerArgs(test.cfg) g.Expect(objects).To(ConsistOf(test.expectedObjects)) g.Expect(objectLists).To(ConsistOf(test.expectedObjectLists)) diff --git a/internal/mode/static/state/change_processor.go b/internal/mode/static/state/change_processor.go index 8605098b57..8e73e82df4 100644 --- a/internal/mode/static/state/change_processor.go +++ b/internal/mode/static/state/change_processor.go @@ -110,6 +110,7 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl { GRPCRoutes: make(map[types.NamespacedName]*v1.GRPCRoute), TLSRoutes: make(map[types.NamespacedName]*v1alpha2.TLSRoute), NGFPolicies: make(map[graph.PolicyKey]policies.Policy), + SnippetsFilters: make(map[types.NamespacedName]*ngfAPI.SnippetsFilter), } processor := &ChangeProcessorImpl{ @@ -218,6 +219,11 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl { store: newObjectStoreMapAdapter(clusterStore.TLSRoutes), predicate: nil, }, + { + gvk: cfg.MustExtractGVK(&ngfAPI.SnippetsFilter{}), + store: newObjectStoreMapAdapter(clusterStore.SnippetsFilters), + predicate: nil, /*TODO(kate-osborn): will add predicate in next PR*/ + }, }, ) diff --git a/internal/mode/static/state/conditions/conditions.go b/internal/mode/static/state/conditions/conditions.go index 026c826787..21f3007264 100644 --- a/internal/mode/static/state/conditions/conditions.go +++ b/internal/mode/static/state/conditions/conditions.go @@ -724,7 +724,7 @@ func NewPolicyNotAcceptedTargetConflict(msg string) conditions.Condition { } // NewPolicyNotAcceptedNginxProxyNotSet returns a Condition that indicates that the Policy is not accepted -// because it relies in the NginxProxy configuration which is missing or invalid. +// because it relies on the NginxProxy configuration which is missing or invalid. func NewPolicyNotAcceptedNginxProxyNotSet(msg string) conditions.Condition { return conditions.Condition{ Type: string(v1alpha2.PolicyConditionAccepted), @@ -733,3 +733,14 @@ func NewPolicyNotAcceptedNginxProxyNotSet(msg string) conditions.Condition { Message: msg, } } + +// NewSnippetsFilterInvalid returns a Condition that indicates that the SnippetsFilter is not accepted because it is +// syntactically or semantically invalid. +func NewSnippetsFilterInvalid(msg string) conditions.Condition { + return conditions.Condition{ + Type: string(ngfAPI.SnippetsFilterConditionTypeAccepted), + Status: metav1.ConditionFalse, + Reason: string(ngfAPI.SnippetsFilterConditionReasonInvalid), + Message: msg, + } +} diff --git a/internal/mode/static/state/graph/graph.go b/internal/mode/static/state/graph/graph.go index 326abbccea..20ea60153a 100644 --- a/internal/mode/static/state/graph/graph.go +++ b/internal/mode/static/state/graph/graph.go @@ -36,6 +36,7 @@ type ClusterState struct { NginxProxies map[types.NamespacedName]*ngfAPI.NginxProxy GRPCRoutes map[types.NamespacedName]*gatewayv1.GRPCRoute NGFPolicies map[PolicyKey]policies.Policy + SnippetsFilters map[types.NamespacedName]*ngfAPI.SnippetsFilter } // Graph is a Graph-like representation of Gateway API resources. @@ -77,6 +78,8 @@ type Graph struct { // GlobalSettings contains global settings from the current state of the graph that may be // needed for policy validation or generation if certain policies rely on those global settings. GlobalSettings *policies.GlobalSettings + // SnippetsFilters holds all the SnippetsFilters. + SnippetsFilters map[types.NamespacedName]*SnippetsFilter } // ProtectedPorts are the ports that may not be configured by a listener with a descriptive name of each port. @@ -215,6 +218,8 @@ func BuildGraph( gw, ) + processedSnippetsFilters := processSnippetsFilters(state.SnippetsFilters) + routes := buildRoutesForGateways( validators.HTTPFieldsValidator, state.HTTPRoutes, @@ -262,6 +267,7 @@ func BuildGraph( NginxProxy: npCfg, NGFPolicies: processedPolicies, GlobalSettings: globalSettings, + SnippetsFilters: processedSnippetsFilters, } g.attachPolicies(controllerName) diff --git a/internal/mode/static/state/graph/graph_test.go b/internal/mode/static/state/graph/graph_test.go index 3312aea694..5776b63571 100644 --- a/internal/mode/static/state/graph/graph_test.go +++ b/internal/mode/static/state/graph/graph_test.go @@ -515,6 +515,26 @@ func TestBuildGraph(t *testing.T) { Valid: true, } + snippetsFilter := &ngfAPI.SnippetsFilter{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-snippet-filter", + Namespace: testNs, + }, + Spec: ngfAPI.SnippetsFilterSpec{ + Snippets: []ngfAPI.Snippet{ + { + Context: ngfAPI.NginxContextMain, + Value: "main snippet", + }, + }, + }, + } + + processedSnippetsFilter := &SnippetsFilter{ + Source: snippetsFilter, + Valid: true, + } + createStateWithGatewayClass := func(gc *gatewayv1.GatewayClass) ClusterState { return ClusterState{ GatewayClasses: map[types.NamespacedName]*gatewayv1.GatewayClass{ @@ -564,6 +584,9 @@ func TestBuildGraph(t *testing.T) { hrPolicyKey: hrPolicy, gwPolicyKey: gwPolicy, }, + SnippetsFilters: map[types.NamespacedName]*ngfAPI.SnippetsFilter{ + client.ObjectKeyFromObject(snippetsFilter): snippetsFilter, + }, } } @@ -810,6 +833,9 @@ func TestBuildGraph(t *testing.T) { NginxProxyValid: true, TelemetryEnabled: true, }, + SnippetsFilters: map[types.NamespacedName]*SnippetsFilter{ + client.ObjectKeyFromObject(snippetsFilter): processedSnippetsFilter, + }, } } diff --git a/internal/mode/static/state/graph/snippets_filter.go b/internal/mode/static/state/graph/snippets_filter.go new file mode 100644 index 0000000000..c02a703b78 --- /dev/null +++ b/internal/mode/static/state/graph/snippets_filter.go @@ -0,0 +1,94 @@ +package graph + +import ( + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" + + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" + staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" +) + +type SnippetsFilter struct { + // Source is the SnippetsFilter. + Source *ngfAPI.SnippetsFilter + // Conditions define the conditions to be reported in the status of the SnippetsFilter. + Conditions []conditions.Condition + // Valid indicates whether the SnippetsFilter is semantically and syntactically valid. + Valid bool +} + +func processSnippetsFilters( + snippetsFilters map[types.NamespacedName]*ngfAPI.SnippetsFilter, +) map[types.NamespacedName]*SnippetsFilter { + if len(snippetsFilters) == 0 { + return nil + } + + processed := make(map[types.NamespacedName]*SnippetsFilter) + + for nsname, sf := range snippetsFilters { + processedSf := &SnippetsFilter{ + Source: sf, + Valid: true, + } + + if cond := validateSnippetsFilter(sf); cond != nil { + processedSf.Valid = false + processedSf.Conditions = []conditions.Condition{*cond} + } + + processed[nsname] = processedSf + } + + return processed +} + +func validateSnippetsFilter(filter *ngfAPI.SnippetsFilter) *conditions.Condition { + var allErrs field.ErrorList + snippetsPath := field.NewPath("spec.snippets") + + usedContexts := make(map[ngfAPI.NginxContext]struct{}) + + for i, snippet := range filter.Spec.Snippets { + ctxPath := snippetsPath.Index(i).Child("context") + + switch snippet.Context { + case ngfAPI.NginxContextMain, + ngfAPI.NginxContextHTTP, + ngfAPI.NginxContextHTTPServer, + ngfAPI.NginxContextHTTPServerLocation: + default: + err := field.NotSupported( + ctxPath, + snippet.Context, + []ngfAPI.NginxContext{ + ngfAPI.NginxContextMain, + ngfAPI.NginxContextHTTP, + ngfAPI.NginxContextHTTPServer, + ngfAPI.NginxContextHTTPServerLocation, + }, + ) + + allErrs = append(allErrs, err) + } + + if _, ok := usedContexts[snippet.Context]; ok { + allErrs = append( + allErrs, + field.Invalid(ctxPath, snippet.Context, "only one snippet is allowed per context"), + ) + + continue + } + + usedContexts[snippet.Context] = struct{}{} + } + + if allErrs != nil { + cond := staticConds.NewSnippetsFilterInvalid(allErrs.ToAggregate().Error()) + return &cond + } + + return nil +} diff --git a/internal/mode/static/state/graph/snippets_filter_test.go b/internal/mode/static/state/graph/snippets_filter_test.go new file mode 100644 index 0000000000..8181b6c648 --- /dev/null +++ b/internal/mode/static/state/graph/snippets_filter_test.go @@ -0,0 +1,255 @@ +package graph + +import ( + "testing" + + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/types" + + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" + staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" +) + +func TestProcessSnippetsFilters(t *testing.T) { + filter1NsName := types.NamespacedName{Namespace: "test", Name: "filter-1"} + filter2NsName := types.NamespacedName{Namespace: "other", Name: "filter-2"} + invalidFilterNsName := types.NamespacedName{Namespace: "default", Name: "invalid"} + + filter1 := &ngfAPI.SnippetsFilter{ + Spec: ngfAPI.SnippetsFilterSpec{ + Snippets: []ngfAPI.Snippet{ + { + Context: ngfAPI.NginxContextMain, + Value: "main snippet", + }, + { + Context: ngfAPI.NginxContextHTTP, + Value: "http snippet", + }, + }, + }, + } + + invalidFilter := &ngfAPI.SnippetsFilter{ + Spec: ngfAPI.SnippetsFilterSpec{ + Snippets: []ngfAPI.Snippet{ + { + Context: ngfAPI.NginxContextMain, + Value: "main snippet", + }, + { + Context: "invalid context", + Value: "invalid snippet", + }, + }, + }, + } + + filter2 := &ngfAPI.SnippetsFilter{ + Spec: ngfAPI.SnippetsFilterSpec{ + Snippets: []ngfAPI.Snippet{ + { + Context: ngfAPI.NginxContextHTTPServerLocation, + Value: "location snippet", + }, + }, + }, + } + + tests := []struct { + snippetsFilters map[types.NamespacedName]*ngfAPI.SnippetsFilter + expProcessedSnippets map[types.NamespacedName]*SnippetsFilter + msg string + }{ + { + msg: "no snippets filters", + snippetsFilters: nil, + expProcessedSnippets: nil, + }, + { + msg: "mix valid and invalid snippets filters", + snippetsFilters: map[types.NamespacedName]*ngfAPI.SnippetsFilter{ + filter1NsName: filter1, + invalidFilterNsName: invalidFilter, + filter2NsName: filter2, + }, + expProcessedSnippets: map[types.NamespacedName]*SnippetsFilter{ + filter1NsName: { + Source: filter1, + Conditions: nil, + Valid: true, + }, + filter2NsName: { + Source: filter2, + Conditions: nil, + Valid: true, + }, + invalidFilterNsName: { + Source: invalidFilter, + Conditions: []conditions.Condition{staticConds.NewSnippetsFilterInvalid( + "spec.snippets[1].context: Unsupported value: \"invalid context\": " + + "supported values: \"main\", \"http\", \"http.server\", \"http.server.location\"", + )}, + Valid: false, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.msg, func(t *testing.T) { + g := NewWithT(t) + + processedSnippetsFilters := processSnippetsFilters(test.snippetsFilters) + g.Expect(processedSnippetsFilters).To(BeEquivalentTo(test.expProcessedSnippets)) + }) + } +} + +func TestValidateSnippetsFilter(t *testing.T) { + tests := []struct { + msg string + filter *ngfAPI.SnippetsFilter + expCond conditions.Condition + }{ + { + msg: "valid filter", + filter: &ngfAPI.SnippetsFilter{ + Spec: ngfAPI.SnippetsFilterSpec{ + Snippets: []ngfAPI.Snippet{ + { + Context: ngfAPI.NginxContextMain, + Value: "main snippet", + }, + { + Context: ngfAPI.NginxContextHTTP, + Value: "http snippet", + }, + }, + }, + }, + expCond: conditions.Condition{}, + }, + { + msg: "invalid filter; invalid snippet context", + filter: &ngfAPI.SnippetsFilter{ + Spec: ngfAPI.SnippetsFilterSpec{ + Snippets: []ngfAPI.Snippet{ + { + Context: ngfAPI.NginxContextMain, + Value: "main snippet", + }, + { + Context: ngfAPI.NginxContextHTTP, + Value: "http snippet", + }, + { + Context: "invalid context", + Value: "invalid", + }, + }, + }, + }, + expCond: staticConds.NewSnippetsFilterInvalid( + "spec.snippets[2].context: Unsupported value: \"invalid context\": " + + "supported values: \"main\", \"http\", \"http.server\", \"http.server.location\"", + ), + }, + { + msg: "invalid filter; multiple invalid snippet contexts", + filter: &ngfAPI.SnippetsFilter{ + Spec: ngfAPI.SnippetsFilterSpec{ + Snippets: []ngfAPI.Snippet{ + { + Context: ngfAPI.NginxContextMain, + Value: "main snippet", + }, + { + Context: "invalid context", + Value: "invalid", + }, + { + Context: "also invalid context", + Value: "invalid too", + }, + }, + }, + }, + expCond: staticConds.NewSnippetsFilterInvalid( + "[spec.snippets[1].context: Unsupported value: \"invalid context\": supported values: " + + "\"main\", \"http\", \"http.server\", \"http.server.location\", spec.snippets[2].context: " + + "Unsupported value: \"also invalid context\": supported values: \"main\", \"http\", " + + "\"http.server\", \"http.server.location\"]", + ), + }, + { + msg: "invalid filter; duplicate contexts", + filter: &ngfAPI.SnippetsFilter{ + Spec: ngfAPI.SnippetsFilterSpec{ + Snippets: []ngfAPI.Snippet{ + { + Context: ngfAPI.NginxContextMain, + Value: "main snippet", + }, + { + Context: ngfAPI.NginxContextHTTP, + Value: "http snippet", + }, + { + Context: ngfAPI.NginxContextMain, + Value: "main again", + }, + }, + }, + }, + expCond: staticConds.NewSnippetsFilterInvalid( + "spec.snippets[2].context: Invalid value: \"main\": only one snippet is allowed per context", + ), + }, + { + msg: "invalid filter; duplicate contexts and invalid context", + filter: &ngfAPI.SnippetsFilter{ + Spec: ngfAPI.SnippetsFilterSpec{ + Snippets: []ngfAPI.Snippet{ + { + Context: ngfAPI.NginxContextMain, + Value: "main snippet", + }, + { + Context: ngfAPI.NginxContextHTTP, + Value: "http snippet", + }, + { + Context: ngfAPI.NginxContextMain, + Value: "main again", + }, + { + Context: "invalid context", + Value: "invalid", + }, + }, + }, + }, + expCond: staticConds.NewSnippetsFilterInvalid( + "[spec.snippets[2].context: Invalid value: \"main\": only one snippet is allowed per context, " + + "spec.snippets[3].context: Unsupported value: \"invalid context\": supported values: \"main\", " + + "\"http\", \"http.server\", \"http.server.location\"]", + ), + }, + } + + for _, test := range tests { + t.Run(test.msg, func(t *testing.T) { + g := NewWithT(t) + + cond := validateSnippetsFilter(test.filter) + if test.expCond != (conditions.Condition{}) { + g.Expect(cond).ToNot(BeNil()) + g.Expect(*cond).To(Equal(test.expCond)) + } else { + g.Expect(cond).To(BeNil()) + } + }) + } +} diff --git a/site/content/reference/api.md b/site/content/reference/api.md index ad4c7c3c67..fa582def32 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -1215,6 +1215,10 @@ string

SnippetsFilterConditionReasonAccepted is used with the Accepted condition type when the condition is true.

+

"Invalid"

+

SnippetsFilterConditionReasonInvalid is used with the Accepted condition type when +SnippetsFilter is invalid.

+

SnippetsFilterConditionType @@ -1241,10 +1245,6 @@ the condition is true.

  • Invalid.
    • -

    "Invalid"

    -

    SnippetsFilterConditionTypeInvalid is used with the Accepted condition type when -SnippetsFilter is invalid.

    -

    SnippetsFilterSpec diff --git a/site/content/reference/cli-help.md b/site/content/reference/cli-help.md index 8f53f5c7e9..375d97ed5b 100644 --- a/site/content/reference/cli-help.md +++ b/site/content/reference/cli-help.md @@ -23,7 +23,7 @@ This command configures NGINX for a single NGINX Gateway Fabric resource. {{< bootstrap-table "table table-bordered table-striped table-responsive" >}} | Name | Type | Description | -| ----------------------------------- | -------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +|-------------------------------------|----------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | _gateway-ctlr-name_ | _string_ | The name of the Gateway controller. The controller name must be in the form: `DOMAIN/PATH`. The controller's domain is `gateway.nginx.org`. | | _gatewayclass_ | _string_ | The name of the GatewayClass resource. Every NGINX Gateway Fabric must have a unique corresponding GatewayClass resource. | | _gateway_ | _string_ | 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. 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}. | @@ -39,11 +39,12 @@ This command configures NGINX for a single NGINX Gateway Fabric resource. | _health-port_ | _int_ | Set the port where the health probe server is exposed. An integer between 1024 - 65535 (Default: `8081`). | | _leader-election-disable_ | _bool_ | Disable leader election, which is used to avoid multiple replicas of the NGINX Gateway Fabric reporting the status of the Gateway API resources. If disabled, all replicas of NGINX Gateway Fabric will update the statuses of the Gateway API resources (Default: `false`). | | _leader-election-lock-name_ | _string_ | The name of the leader election lock. A lease object with this name will be created in the same namespace as the controller (Default: `"nginx-gateway-leader-election-lock"`). | -| _product-telemetry-disable_ | _bool_ | Disable the collection of product telemetry (Default: `false`). | -| _usage-report-secret_ | _string_ | The namespace/name of the Secret containing the credentials for NGINX Plus usage reporting. | -| _usage-report-server-url_ | _string_ | The base server URL of the NGINX Plus usage reporting server. | -| _usage-report-cluster-name_ | _string_ | The display name of the Kubernetes cluster in the NGINX Plus usage reporting server. | -| _usage-report-skip-verify_ | _bool_ | Disable client verification of the NGINX Plus usage reporting server certificate. | +| _product-telemetry-disable_ | _bool_ | Disable the collection of product telemetry (Default: `false`). | +| _usage-report-secret_ | _string_ | The namespace/name of the Secret containing the credentials for NGINX Plus usage reporting. | +| _usage-report-server-url_ | _string_ | The base server URL of the NGINX Plus usage reporting server. | +| _usage-report-cluster-name_ | _string_ | The display name of the Kubernetes cluster in the NGINX Plus usage reporting server. | +| _usage-report-skip-verify_ | _bool_ | Disable client verification of the NGINX Plus usage reporting server certificate. | +| _snippets-filters_ | _bool_ | Enable SnippetsFilters feature. SnippetsFilters allow inserting NGINX configuration into the generated NGINX config for HTTPRoute and GRPCRoute resources. | {{% /bootstrap-table %}} ## Sleep From 3d9ec1956b4ebad3a76f2507ea975eff0e512c7d Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Fri, 6 Sep 2024 12:26:42 -0600 Subject: [PATCH 2/6] Remove duplicate log --- internal/mode/static/manager.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index e07b4e5690..fb536527a2 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -531,8 +531,6 @@ func registerControllers( } if cfg.SnippetsFilters { - cfg.Logger.V(1).Info("Registering controller for SnippetsFilter") - controllerRegCfgs = append(controllerRegCfgs, ctlrCfg{ objectType: &ngfAPI.SnippetsFilter{}, From 4f7d82263afe71da200e21966e7317290e73d37a Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Fri, 6 Sep 2024 15:18:39 -0600 Subject: [PATCH 3/6] Add comment and fix alignment --- internal/mode/static/state/graph/snippets_filter.go | 1 + site/content/reference/cli-help.md | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/mode/static/state/graph/snippets_filter.go b/internal/mode/static/state/graph/snippets_filter.go index c02a703b78..844dd3c2a4 100644 --- a/internal/mode/static/state/graph/snippets_filter.go +++ b/internal/mode/static/state/graph/snippets_filter.go @@ -9,6 +9,7 @@ import ( staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" ) +// SnippetsFilter represents a ngfAPI.SnippetsFilter. type SnippetsFilter struct { // Source is the SnippetsFilter. Source *ngfAPI.SnippetsFilter diff --git a/site/content/reference/cli-help.md b/site/content/reference/cli-help.md index 375d97ed5b..5122212a89 100644 --- a/site/content/reference/cli-help.md +++ b/site/content/reference/cli-help.md @@ -44,7 +44,7 @@ This command configures NGINX for a single NGINX Gateway Fabric resource. | _usage-report-server-url_ | _string_ | The base server URL of the NGINX Plus usage reporting server. | | _usage-report-cluster-name_ | _string_ | The display name of the Kubernetes cluster in the NGINX Plus usage reporting server. | | _usage-report-skip-verify_ | _bool_ | Disable client verification of the NGINX Plus usage reporting server certificate. | -| _snippets-filters_ | _bool_ | Enable SnippetsFilters feature. SnippetsFilters allow inserting NGINX configuration into the generated NGINX config for HTTPRoute and GRPCRoute resources. | +| _snippets-filters_ | _bool_ | Enable SnippetsFilters feature. SnippetsFilters allow inserting NGINX configuration into the generated NGINX config for HTTPRoute and GRPCRoute resources. | {{% /bootstrap-table %}} ## Sleep From 61dffdf0164054385bcfbc05d86dd8ade143fcdb Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Fri, 6 Sep 2024 15:38:05 -0600 Subject: [PATCH 4/6] Update CRD validation --- apis/v1alpha1/snippetsfilter_types.go | 3 +++ config/crd/bases/gateway.nginx.org_snippetsfilters.yaml | 3 +++ 2 files changed, 6 insertions(+) diff --git a/apis/v1alpha1/snippetsfilter_types.go b/apis/v1alpha1/snippetsfilter_types.go index 8d98e039e2..3069f7df4c 100644 --- a/apis/v1alpha1/snippetsfilter_types.go +++ b/apis/v1alpha1/snippetsfilter_types.go @@ -38,6 +38,7 @@ type SnippetsFilterSpec struct { // Snippets is a list of NGINX configuration snippets. // There can only be one snippet per context. // Allowed contexts: main, http, http.server, http.server.location. + // +kubebuilder:validation:MinItems=1 // +kubebuilder:validation:MaxItems=4 // +kubebuilder:validation:XValidation:message="Only one snippet allowed per context",rule="self.all(s1, self.exists_one(s2, s1.context == s2.context))" //nolint:lll @@ -50,12 +51,14 @@ type Snippet struct { Context NginxContext `json:"context"` // Value is the NGINX configuration snippet. + // +kubebuilder:validation:MinLength=1 Value string `json:"value"` } // NginxContext represents the NGINX configuration context. // // +kubebuilder:validation:Enum=main;http;http.server;http.server.location +// +kubebuilder:validation:MinLength=4 type NginxContext string const ( diff --git a/config/crd/bases/gateway.nginx.org_snippetsfilters.yaml b/config/crd/bases/gateway.nginx.org_snippetsfilters.yaml index 6f719d73df..9d9adb76ce 100644 --- a/config/crd/bases/gateway.nginx.org_snippetsfilters.yaml +++ b/config/crd/bases/gateway.nginx.org_snippetsfilters.yaml @@ -65,15 +65,18 @@ spec: - http - http.server - http.server.location + minLength: 4 type: string value: description: Value is the NGINX configuration snippet. + minLength: 1 type: string required: - context - value type: object maxItems: 4 + minItems: 1 type: array x-kubernetes-validations: - message: Only one snippet allowed per context From ef188b403571f91317758c09e6d8f833ad6f3688 Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Fri, 6 Sep 2024 15:57:07 -0600 Subject: [PATCH 5/6] Add more validation and tests --- .../static/state/graph/snippets_filter.go | 16 ++++++++++ .../state/graph/snippets_filter_test.go | 31 +++++++++++++++++-- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/internal/mode/static/state/graph/snippets_filter.go b/internal/mode/static/state/graph/snippets_filter.go index 844dd3c2a4..f3541d7341 100644 --- a/internal/mode/static/state/graph/snippets_filter.go +++ b/internal/mode/static/state/graph/snippets_filter.go @@ -49,9 +49,25 @@ func validateSnippetsFilter(filter *ngfAPI.SnippetsFilter) *conditions.Condition var allErrs field.ErrorList snippetsPath := field.NewPath("spec.snippets") + if len(filter.Spec.Snippets) == 0 { + cond := staticConds.NewSnippetsFilterInvalid( + field.Required(snippetsPath, "at least one snippet must be provided").Error(), + ) + return &cond + } + usedContexts := make(map[ngfAPI.NginxContext]struct{}) for i, snippet := range filter.Spec.Snippets { + valuePath := snippetsPath.Index(i).Child("value") + if snippet.Value == "" { + cond := staticConds.NewSnippetsFilterInvalid( + field.Required(valuePath, "value cannot be empty").Error(), + ) + + return &cond + } + ctxPath := snippetsPath.Index(i).Child("context") switch snippet.Context { diff --git a/internal/mode/static/state/graph/snippets_filter_test.go b/internal/mode/static/state/graph/snippets_filter_test.go index 8181b6c648..9c3116488a 100644 --- a/internal/mode/static/state/graph/snippets_filter_test.go +++ b/internal/mode/static/state/graph/snippets_filter_test.go @@ -131,6 +131,13 @@ func TestValidateSnippetsFilter(t *testing.T) { }, expCond: conditions.Condition{}, }, + { + msg: "empty filter", + filter: &ngfAPI.SnippetsFilter{}, + expCond: staticConds.NewSnippetsFilterInvalid( + "spec.snippets: Required value: at least one snippet must be provided", + ), + }, { msg: "invalid filter; invalid snippet context", filter: &ngfAPI.SnippetsFilter{ @@ -170,7 +177,7 @@ func TestValidateSnippetsFilter(t *testing.T) { Value: "invalid", }, { - Context: "also invalid context", + Context: "", // empty context Value: "invalid too", }, }, @@ -179,7 +186,7 @@ func TestValidateSnippetsFilter(t *testing.T) { expCond: staticConds.NewSnippetsFilterInvalid( "[spec.snippets[1].context: Unsupported value: \"invalid context\": supported values: " + "\"main\", \"http\", \"http.server\", \"http.server.location\", spec.snippets[2].context: " + - "Unsupported value: \"also invalid context\": supported values: \"main\", \"http\", " + + "Unsupported value: \"\": supported values: \"main\", \"http\", " + "\"http.server\", \"http.server.location\"]", ), }, @@ -237,6 +244,26 @@ func TestValidateSnippetsFilter(t *testing.T) { "\"http\", \"http.server\", \"http.server.location\"]", ), }, + { + msg: "invalid filter; empty value", + filter: &ngfAPI.SnippetsFilter{ + Spec: ngfAPI.SnippetsFilterSpec{ + Snippets: []ngfAPI.Snippet{ + { + Context: ngfAPI.NginxContextMain, + Value: "main snippet", + }, + { + Context: ngfAPI.NginxContextMain, + Value: "", // empty value + }, + }, + }, + }, + expCond: staticConds.NewSnippetsFilterInvalid( + "spec.snippets[1].value: Required value: value cannot be empty", + ), + }, } for _, test := range tests { From 69f8ddc7cdeee20b280581c99199c10fc0baa1ad Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Mon, 9 Sep 2024 09:39:04 -0600 Subject: [PATCH 6/6] Remove unnecessary minLength --- apis/v1alpha1/snippetsfilter_types.go | 1 - config/crd/bases/gateway.nginx.org_snippetsfilters.yaml | 1 - 2 files changed, 2 deletions(-) diff --git a/apis/v1alpha1/snippetsfilter_types.go b/apis/v1alpha1/snippetsfilter_types.go index 3069f7df4c..81b1b7e134 100644 --- a/apis/v1alpha1/snippetsfilter_types.go +++ b/apis/v1alpha1/snippetsfilter_types.go @@ -58,7 +58,6 @@ type Snippet struct { // NginxContext represents the NGINX configuration context. // // +kubebuilder:validation:Enum=main;http;http.server;http.server.location -// +kubebuilder:validation:MinLength=4 type NginxContext string const ( diff --git a/config/crd/bases/gateway.nginx.org_snippetsfilters.yaml b/config/crd/bases/gateway.nginx.org_snippetsfilters.yaml index 9d9adb76ce..2d8f01c554 100644 --- a/config/crd/bases/gateway.nginx.org_snippetsfilters.yaml +++ b/config/crd/bases/gateway.nginx.org_snippetsfilters.yaml @@ -65,7 +65,6 @@ spec: - http - http.server - http.server.location - minLength: 4 type: string value: description: Value is the NGINX configuration snippet.