From 42ed86af8c78abd4b9cbacd8f068c84da7704768 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Wed, 3 Jul 2024 18:23:36 -0400 Subject: [PATCH 01/26] Move Advanced NGINX Extensions proposal to Implementable Problem: - Advanced NGINX Extensions proposal https://github.com/nginxinc/nginx-gateway-fabric/issues/2035 is in Provisional state - only problem statement is defined. Solution: - Add the implementation details to the proposal. - Move the enhancement proposal to Implementable Notes: - The goal of Allowing users to customize supported NGINX configuration was moved to a Non-Goal to keep the size of the proposal manageable. - This enhancement proposal can be further split into two - one for SnippetsPolicy and one for SnippetsTemplates CLOSES - https://github.com/nginxinc/nginx-gateway-fabric/issues/815 --- docs/proposals/advanced-nginx-extensions.md | 962 +++++++++++++++++++- 1 file changed, 961 insertions(+), 1 deletion(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index 0f80e170ff..5af1dfc79a 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -12,10 +12,58 @@ the subset will grow. However, it will take time. Additionally, because the numb and parameters is huge, not all of them will be supported that way. As a result, users are not able to implement certain NGINX use cases. To allow them to implement those use cases, we need to bring a new extension mechanism to NGF. +## Table of Contents + + + +- [Enhancement Proposal-2035: Advanced NGINX Extensions](#enhancement-proposal-2035-advanced-nginx-extensions) + - [Summary](#summary) + - [Table of Contents](#table-of-contents) + - [Goals](#goals) + - [Non-Goals](#non-goals) + - [Advanced Extensions](#advanced-extensions) + - [SnippetsPolicy](#snippetspolicy) + - [API](#api) + - [Supported NGINX Contexts](#supported-nginx-contexts) + - [Examples](#examples) + - [Rate-limiting](#rate-limiting) + - [Proxy Buffering](#proxy-buffering) + - [Access Control](#access-control) + - [Third-Party Module](#third-party-module) + - [Inheritance and Conflicts](#inheritance-and-conflicts) + - [Policy or Filter](#policy-or-filter) + - [Personas](#personas) + - [NGINX Config Validation](#nginx-config-validation) + - [Security Considerations](#security-considerations) + - [Upgrades](#upgrades) + - [Prior Arts](#prior-arts) + - [Alternatives](#alternatives) + - [Summary](#summary-1) + - [SnippetsTemplate](#snippetstemplate) + - [API](#api-1) + - [SnippetsTemplate](#snippetstemplate-1) + - [Templates](#templates) + - [How to Use SnippetsTemplate](#how-to-use-snippetstemplate) + - [Examples](#examples-1) + - [Rate Limiting](#rate-limiting-1) + - [Inheritance and Conflicts](#inheritance-and-conflicts-1) + - [Policy or Filter](#policy-or-filter-1) + - [Personas](#personas-1) + - [Validation](#validation) + - [NGINX Values](#nginx-values) + - [Template](#template) + - [Security Considerations](#security-considerations-1) + - [Upgrades](#upgrades-1) + - [Prior Arts](#prior-arts-1) + - [Alternatives](#alternatives-1) + - [Summary](#summary-2) + - [NGINX Features Supported by Proposed Extensions](#nginx-features-supported-by-proposed-extensions) + + + ## Goals - Allow users to insert NGINX configuration not supported via Gateway API resources or NGINX extensions. -- Allow users to customize supported NGINX configuration (for example, add a parameter to an NGINX directive). - Support configuration from modules not loaded in NGINX by default or third-party modules. - Most of the configuration complexity should fall onto the cluster operator persona, not the application developer. - Provide security controls to prevent application developers from injecting arbitrary NGINX configuration. @@ -26,3 +74,915 @@ NGINX use cases. To allow them to implement those use cases, we need to bring a - Support configuration other than NGINX directives. For example, njs configuration files or TLS certificates. - Reimplement already supported features through the new extension mechanism. +- Allow users to customize supported NGINX configuration (for example, add a parameter to an NGINX directive). + +## Advanced Extensions + +This proposal brings two extension mechanisms: + +- [SnippetsPolicy](#snippetspolicy) which allows to quickly bring unsupported NGINX configuration to NGF. However, + because of its implications for reliability and security, SnippetsPolicy is mostly applicable for Cluster operator. +- [SnippetsTemplate](#snippetstemplate) which allows to bring unsupported NGINX configuration to NGF by Application + developers in a safe and uncomplicated manner. However, SnippetsTemplates require some up-front work from Cluster + operator, meaning it cannot be implemented quickly, in contrast with SnippetsPolicy. + +### SnippetsPolicy + +Snippets allow inserting NGINX configuration into various NGINX contexts. + +#### API + +> Field validation rules are intentionally left out of this proposal. + +```go +// SnippetsPolicy is a Direct Attached Policy. It allows inserting NGINX configuration into the generated NGINX +// config that affects resources referenced by the policy. +type SnippetsPolicy struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + // Spec defines the desired state of the SnippetsPolicy. + Spec SnippetsPolicySpec `json:"spec"` + + // Status defines the state of the SnippetsPolicy. + Status gatewayv1alpha2.PolicyStatus `json:"status,omitempty"` +} + +// SnippetsPolicyList contains a list of SnippetsPolicies. +type SnippetsPolicyList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []SnippetsPolicy `json:"items"` +} + +// SnippetsPolicySpec defines the desired state of the SnippetsPolicy. +type SnippetsPolicySpec struct { + // Snippets is a list of NGINX configuration snippets. + // There can only be one snippet per context. + Snippets []Snippet `json:"snippets"` + + // TargetRefs identifies the API object(s) to apply the policy to. + // Objects must be in the same namespace as the policy. + // Objects must be of the same Kinds. + // TO-DO(pleshakov): Remove the line below. + // why same Kinds? Because different kinds allow mutually exclusive snippets. + // + // Support: Gateway, HTTPRoute, GRPCRoute, TLSRoute + // + // For HTTPRoute and GRPCRoute, only http* contexts are supported. + // For TLSRoute, only stream* contexts are supported. + TargetRefs []gatewayv1alpha2.LocalPolicyTargetReference `json:"targetRefs"` +} + +// NginxContext represents the NGINX configuration context. +type NginxContext string + +const ( + // NginxContextMain is the main context of the NGINX configuration. + NginxContextMain NginxContext = "main" + + // NginxContextHTTP is the http context of the NGINX configuration. + NginxContextHTTP NginxContext = "http" + + // NginxContextHTTPServer is the server context of the NGINX configuration. + NginxContextHTTPServer NginxContext = "http.server" + + // NginxContextHTTPServerLocation is the location context of the NGINX configuration. + // TO-DO(pleshakov): Decide if we need to explicitly support the parent location for rules with multiple matches. + // Same problem as in https://github.com/nginxinc/nginx-gateway-fabric/issues/2079 + NginxContextHTTPServerLocation NginxContext = "http.server.location" + + // NginxContextStream is the stream context of the NGINX configuration. + NginxContextStream NginxContext = "stream" + + // NginxContextStreamServer is the server context of the NGINX configuration. + NginxContextStreamServer NginxContext = "stream.server" +) + +// Snippet represents an NGINX configuration snippet. +type Snippet struct { + // Context is the NGINX context to insert the snippet into. + Context NginxContext `json:"context"` + + // Value is the NGINX configuration snippet. + Value string `json:"value"` +} +``` + +#### Supported NGINX Contexts + +SnippetsPolicy supports the following NGINX configuration contexts: + +- `main` +- http module: `http`, `server`, `location` +- stream module: `stream`, `server` + +`upstream` context for both http and stream modules is not included for the following reasons: + +- http module - there are not enough features in https://nginx.org/en/docs/http/ngx_http_upstream_module.html + - There are a few `keepalive`-related directives. But they also require `proxy_set_header Connection "";`, which + is possible to configure with location snippets, but it will conflict with the generated Connection header (see + https://github.com/nginxinc/nginx-gateway-fabric/blob/5968bc348213a8470f6aaaa1a9bd51f2e90523ac/internal/mode/static/nginx/config/servers.go#L39-L40 + and https://github.com/nginxinc/nginx-gateway-fabric/blob/5968bc348213a8470f6aaaa1a9bd51f2e90523ac/internal/mode/static/nginx/config/maps_template.go#L21-L26) + so those keepalive directives won't work. + - Session persistence ([`sticky`](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#sticky)). Those are + valid use cases. But at the same time, they only apply to NGINX Plus. Additionally, + Gateway API started introducing session persistence. + See https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1alpha2.BackendLBPolicy +- stream module - there are not any features in https://nginx.org/en/docs/stream/ngx_stream_upstream_module.html + +Note: because NGF already inserts the `random` load-balancing method, an `upstream` snippet will not be able to +configure +a different method. + +If we choose to introduce upstream snippets, the SnippetsPolicy will need to support targeting a Service, because NGF +generates one upstream per Service. + +#### Examples + +Below are a few examples of using the SnippetsPolicy to bring unsupported NGINX configuration into NGF. + +##### Rate-limiting + +We use NGINX [limit req module](https://nginx.org/en/docs/http/ngx_http_limit_req_module.html) to configure rate +limiting. + +```yaml +apiVersion: gateway.nginx.org/v1alpha1 +kind: SnippetsPolicy +metadata: + name: rate-limit +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: cafe-route + snippets: + - context: http + value: limit_req_zone $binary_remote_addr zone=myzone:10m rate=1r/s; + - context: http.server.location + value: limit_req zone=myzone burst=3; +``` + +As a result, NGF will insert those snippets into the generated configuration for the referenced HTTPRoute: + +```text +# this is http context + +limit_req_zone $binary_remote_addr zone=myzone:10m rate=1r/s; +. . . + +server { + . . . + + location /coffee { + . . . + limit_req zone=myzone burst=3; + . . . + } + + location /tea { + . . . + limit_req zone=myzone burst=3; + . . . + } +``` + +##### Proxy Buffering + +We configure [proxy_buffering](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffering) directive +to disable buffering. + +```yaml +apiVersion: gateway.nginx.org/v1alpha1 +kind: SnippetsPolicy +metadata: + name: buffering +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: cafe-route + snippets: + - context: http.server.location + value: proxy_buffering off; +``` + +As a result, NGF will insert the provided config into the generated locations for the cafe-route HTTPRoute. + +##### Access Control + +We use NGINX [access module](https://nginx.org/en/docs/http/ngx_http_access_module.html) to configure access based +on client IPs. + +```yaml +apiVersion: gateway.nginx.org/v1alpha1 +kind: SnippetsPolicy +metadata: + name: access-control +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: cafe + snippets: + - context: http.server + value: | + allow 10.0.0.0/8; + deny all; +``` + +As a result, NGF will insert the provided NGINX config into the server context of all generated HTTP servers for +the Gateway cafe. + +##### Third-Party Module + +We use the third-party [Brotli module](https://docs.nginx.com/nginx/admin-guide/dynamic-modules/brotli/). + +> nginx container image must include that module -- the user will need to extend the Dockerfile to install +> the module. + +```yaml +apiVersion: gateway.nginx.org/v1alpha1 +kind: SnippetsPolicy +metadata: + name: brotli +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: cafe + snippets: + - context: main + value: load_module modules/ngx_http_brotli_filter_module.so; + - context: http.server + value: brotli on; +``` + +As a result, NGF will: + +- Insert `load_module` into the main context to load the module. +- Configure all `server` blocks belonging to the Gateway cafe to enable the module features. + +### Inheritance and Conflicts + +SnippetsPolicy is general-purpose: it can configure different NGINX features as shown in the +examples before. It is expected that several SnippetPolicies will exist in the cluster, with some of them targeting +the same resources. As a result, implementing inheritance rules or performing conflict resolution is not only +unnecessary, but it will reduce SnippetsPolicy applicability. + +Although a SnippetsPolicy can target different Kinds (like Gateway and HTTPRoute), considering that +(1) each SnippetPolicy is independent of any other and (2) a single SnippetPolicy can only affect resources of the +same Kind, SnippetsPolicy is a [Direct Attached Policy](https://gateway-api.sigs.k8s.io/geps/gep-2648/). + +### Policy or Filter + +Considering that SnippetPolicy can be used to implement authentication and authorization, because of the reasons +mentioned +[here](nginx-extensions.md#authentication) it makes sense to also introduce SnippetsFilter, which will be the same as +SnippetsPolicy but without the `targetRefs` field: + +```yaml +apiVersion: gateway.nginx.org/v1alpha1 +kind: SnippetsFilter +metadata: + name: access-control +spec: + snippets: + - context: http.server + value: | + allow 10.0.0.0/8; + deny all; +``` + +An HTTPRoute can include that filter: + +```yaml +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: my-app +spec: + . . . + rules: + - matches: + - path: + type: PathPrefix + value: / + filters: + - type: ExtensionRef + extensionRef: + group: gateway.nginx.org/v1alpha1 + kind: SnippetsFilter + name: access-control + backendRefs: + - name: headers + port: 80 +``` + +SnippetsFilter can also be used in +GRPCRoute. [TLSRoute](https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1alpha2.TLSRoute) +doesn't support filters, so it needs to be added to the Gateway API first. + +Although SnippetsFilter applicability is limited compared to SnippetsPolicy, SnippetsFilter creates +a natural split of responsibilities between Cluster operator and Application developers: + +- Cluster operator creates a SnippetsFilter. +- Application developers reference the SnippetsFilter to enable it. + +Note that with the SnippetsPolicy, because the targetRef is part of the SnippetsPolicy, it is not possible to have +such a split of responsibilities. + +TO-DO(pleshakov) - Add SnippetsFilter to the GEP after discussions about it. + +### Personas + +The target persona of snippets is Cluster operator. They will create and manage snippets. + +Snippets are not intended for Application developers, because: + +- To create snippets, you need to know about NGINX and how NGF generates NGINX configuration. +- Snippets can easily break NGINX config (if not validated by NGF). See + [Config Validation section](#nginx-config-validation) below. +- Snippets can be used to exploit NGF. (See [Security Considerations section](#security-considerations) below) + +As mentioned in the [Policy or Filter](#policy-or-filter), when snippets are used via SnippetsFilter, Application +developers can still control whether they want to enable or disable snippets by referencing them in an HTTPRoute. + +### NGINX Config Validation + +An invalid snippet can break NGINX config. When this happens, NGINX will continue to use the last valid configuration. +However, any subsequent configuration updates (for example, caused by changes to an HTTPRoute) will not be possible +until the +invalid snippet is removed. + +Before injecting snippets in NGINX config, it is possible to validate snippets +using [crossplane](https://github.com/nginxinc/nginx-go-crossplane), +although some work is necessary to support it -- see https://github.com/nginxinc/nginx-go-crossplane/issues/94. Such +validation will catch cases of using invalid directives or invalid parameters of directives. At the same time, it +will not be a complete validation. For example, it won't catch errors like referencing an undefined variable +in the NGINX config. + +### Security Considerations + +The snippet below exposes the filesystem of the NGINX container including any TLS Secrets and the ServiceAccount Secret, +which NGF uses to access Kubernetes API: + +```yaml +apiVersion: gateway.nginx.org/v1alpha1 +kind: SnippetsPolicy +metadata: + name: expose +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: cafe + snippets: + - context: http.server + value: | + server { + listen 80; + root /; + autoindex on; + } +``` + +As a consequence, creating SnippetsPolicy should only be allowed for the privileged users -- Cluster operator. +As a further precaution, we can disable SnippetsPolicy by default similarly +to [NGINX Ingress Controller](https://docs.nginx.com/nginx-ingress-controller/configuration/security/#snippets). + +It is also possible to add validation of snippets to disallow certain directives (like `root` and `autoindex`) +and certain values of directives. For an example of such extensive NGINX config validation see +[NGINX for Azure](https://docs.nginx.com/nginxaas/azure/overview/overview/) product. + +### Upgrades + +NGF inserts snippets into various parts of the generated configuration. As a result, the configuration +in the snippets must play well with the surrounding configuration, so that it doesn't break that configuration and vice +versa. + +As we develop NGF, that surrounding configuration will expand (because we will implement more NGINX features) and also +might change (for example, due to improvements). As a result, there is a risk that snippets can break after the Cluster +operator upgrades NGF to the next version. Such risk shall be clearly documented in the SnippetsPolicy documentation. + +### Prior Arts + +- NGINX Ingress Controller supports + snippets -- https://docs.nginx.com/nginx-ingress-controller/configuration/security/#snippets + +### Alternatives + +- See the next section - [SnippetsTemplate](#snippetstemplate). + +### Summary + +- Snippets allow Cluster operator to quickly configure NGINX features not available via NGF. +- SnippetsPolicy should be used only by Cluster operator, because of reliability and security implications. +- SnippetsFilter, it is possible to split the responsibility of creating snippets (Cluster operator) and enabling + snippets (Application Developer). + +## SnippetsTemplate + +SnippetsTemplate is similar to SnippetsPolicy: it allows inserting NGINX configuration into various NGINX contexts. +However, in contrast with SnippetsPolicy, it allows Application developers to safely use them without the risk of +breaking NGINX config. At the same time, that safety comes with a cost: creating a SnippetsTemplate is more involved +for Cluster operators compared to SnippetsPolicy. + +### API + +#### SnippetsTemplate + +> Field validation rules are intentionally left out of this proposal. + +```go +// SnippetsTemplate configures an NGINX Gateway Fabric extension based on the templated NGINX configuration snippets. +type SnippetsTemplate struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + // Spec defines the desired state of the SnippetsTemplate. + Spec SnippetsTemplateSpec `json:"spec"` + + // Status defines the state of the SnippetsTemplate. + Status SnippetsTemplateStatus `json:"status,omitempty"` +} + +// SnippetsTemplateList contains a list of SnippetsTemplates. +type SnippetsTemplateList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + + Items []SnippetsTemplate `json:"items"` +} + +// SnippetsTemplateSpec defines the desired state of the SnippetsTemplate. +type SnippetsTemplateSpec struct { + // ValuesCRD is the name of the CRD which provides values for the templates. + ValuesCRD string `json:"valuesCRD"` + + // AllowedTargets is a list of allowed target resources that the CRD can target. + // Only one target is allowed. + AllowedTargets []AllowedTarget `json:"allowedTargets"` + + // Templates is a list of NGINX configuration templates to insert into the generated NGINX config. + // There can only be one template per context. + Templates []Template `json:"templates"` +} + +// AllowedTarget represents an allowed target. +type AllowedTarget struct { + // Group is the group of the target resource. + Group gatewayv1.Group `json:"group"` + + // Kind is kind of the target resource. + Kind gatewayv1.Kind `json:"kind"` +} + +// Template is an NGINX configuration template. +type Template struct { + // Context is the NGINX context to insert the template into. + Context NginxContext `json:"context"` + + // Value is the template. It must be a valid Go template -- https://pkg.go.dev/text/template. + Value string `json:"value"` +} + +// SnippetsTemplateStatus defines the state of the SnippetsTemplate. +type SnippetsTemplateStatus struct { + // Conditions describes the state of the SnippetsTemplate. + Conditions []metav1.Condition `json:"conditions,omitempty"` +} + +// SnippetsTemplateConditionType is a type of condition associated with SnippetsTemplate. +type SnippetsTemplateConditionType string + +// SnippetsTemplateConditionReason is a reason for a SnippetsTemplate condition type. +type SnippetsTemplateConditionReason string + +const ( + // SnippetsTemplateConditionTypeAccepted indicates that the SnippetsTemplate is accepted. + // + // Possible reasons for this condition to be True: + // + // * Accepted + // + // Possible reasons for this condition to be False: + // + // * Invalid + // * CRDInvalid + // * CRDNotFound + // * TargetNotFound + SnippetsTemplateConditionTypeAccepted SnippetsTemplateConditionType = "Accepted" + + // SnippetsTemplateConditionReasonAccepted is used with the Accepted condition type when + // the condition is true. + SnippetsTemplateConditionReasonAccepted SnippetsTemplateConditionReason = "Accepted" + + // SnippetsTemplateConditionReasonInvalid is used with the Accepted condition type when + // SnippetsTemplate is invalid. + SnippetsTemplateConditionReasonInvalid SnippetsTemplateConditionReason = "Invalid" + + // SnippetsTemplateConditionReasonCRDInvalid is used with the Accepted condition type when + // the referenced CRD is invalid. + SnippetsTemplateConditionReasonCRDInvalid SnippetsTemplateConditionReason = "CRDInvalid" + + // SnippetsTemplateConditionReasonCRDNotFound is used with the Accepted condition type when + // the referenced CRD is not found. + SnippetsTemplateConditionReasonCRDNotFound SnippetsTemplateConditionReason = "CRDNotFound" + + // SnippetsTemplateConditionReasonTargetNotFound is used with the Accepted condition type when + // the target is not found. + SnippetsTemplateConditionReasonTargetNotFound SnippetsTemplateConditionReason = "TargetNotFound" +) +``` + +#### Templates + +A template must be a [go template](https://pkg.go.dev/text/template). + +NGF gives a template access to the data and metadata about a Custom resource (corresponds +to `SnippetsTemplate.spec.valuesCRD`) via `TemplateContext` type: + +```go +// TemplateContext provides context for NGINX configuration templates. +type TemplateContext struct { + // Metadata holds the metadata for NGINX configuration templates. + Metadata TemplateMetadata + // Data holds the data for NGINX configuration templates. + Data TemplateData +} + +// TemplateData holds the data for NGINX configuration templates. +type TemplateData struct { + // Spec is the spec of the Policy/Filter of the Kind that corresponds to SnippetsTemplate.spec.valuesCRD. + Spec map[string]interface{} +} + +// TemplateMetadata holds the metadata for NGINX configuration templates. +type TemplateMetadata struct { + // TargetMeta is the metadata of the target resource like HTTPRoute. + TargetMeta metav1.ObjectMeta + // SourceMeta is the metadata of the source resource (Policy/Filter) of the Kind that corresponds to + // SnippetsTemplate.spec.valuesCRD. + SourceMeta metav1.ObjectMeta +} +``` + +If requested by users, NGF can provide additional context. For example, for a template for the `location` context, +it can pass the data from +the [`Location`](https://github.com/nginxinc/nginx-gateway-fabric/blob/7bc0b6e6c5131920ac18f41359dd1eba7f53a8ba/internal/mode/static/nginx/config/http/config.go#L16) +struct, which NGF uses to generate location config. + +### How to Use SnippetsTemplate + +1. Cluster operator comes up with NGINX configuration that configures an NGINX feature needed by Application + developer. +2. Cluster operator thinks about what values Application developer might want to customize in that configuration. +3. Cluster operator creates a CRD that defines fields and validation for those values and creates the CRD in the + cluster. +4. Cluster operator creates a go template that generates the NGINX configuration based on the fields of the CRD. +5. Cluster operator allows Application developer to use the feature by creating a SnippetsTemplate, which binds + the CRD with the template(s). +6. Application developer creates a Custom resource of the CRD to enable that NGINX feature with the desired values. + +Essentially, Cluster operator creates an extension for a particular NGINX feature. Application developers can +use that extension with the ability to further customize it. + +### Examples + +#### Rate Limiting + +First, Cluster operator comes up with NGINX configuration for the rate-limiting feature asked by Application developers: + +```text +```text +# http context +limit_req_zone $binary_remote_addr zone=myzone:10m rate=1r/s; + +server { + . . . + location /somepath { + . . . + limit_req zone=myzone burst=3; + } +``` + +Next, Cluster operator decides that Application developers might want to customize the rate-limiting `rate` and `burst` +values. + +Next, Cluster operator defines the following CRD for those values. Note that the values are validated to prevent invalid +or +malicious values from being propagated into the NGINX config: + +```go +// RateLimitingPolicy is a Direct Attached Policy to configure rate-limiting of client requests. +type RateLimitingPolicy struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + // Spec defines the desired state of the ClientSettingsPolicy. + Spec RateLimitingPolicySpec `json:"spec"` + + // Status defines the state of the ClientSettingsPolicy. + Status gatewayv1alpha2.PolicyStatus `json:"status,omitempty"` +} + +// +kubebuilder:object:root=true + +// RateLimitingPolicyList contains a list of RateLimitingPolicies. +type RateLimitingPolicyList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []ClientSettingsPolicy `json:"items"` +} + +// RateLimitingPolicySpec defines the desired state of RateLimitingPolicy. +type RateLimitingPolicySpec struct { + // The rate of requests permitted per second. + // + // +kubebuilder:validation:Minimum=1 + Rate int32 `json:"rate"` + + // Burst delays excessive requests until their number exceeds the burst value, in which case the request is + // terminated with an error. + // + // +optional + // +kubebuilder:validation:Minimum=1 + Burst *int32 `json:"burst,omitempty"` + + // TargetRefs identifies the API object(s) to apply the policy to. + // Objects must be in the same namespace as the policy. + // Support: HTTPRoute. + // + // +kubebuilder:validation:MaxItems=16 + // +kubebuilder:validation:XValidation:message="TargetRef Kind must be: HTTPRoute.",rule="self.exists(t, t.kind=='HTTPRoute')" + // +kubebuilder:validation:XValidation:message="TargetRef Group must be gateway.networking.k8s.io.",rule="self.all(t, t.group=='gateway.networking.k8s.io')" + //nolint:lll + TargetRefs []gatewayv1alpha2.LocalPolicyTargetReference `json:"targetRefs"` +} +``` + +Next, Cluster operator generates a CRD manifest from that go code and creates the CRD in the cluster. + +Next, Cluster operator creates templates that generate the NGINX configuration based on the fields of the CRD: + +- For http context: + + ```text + {{ $rate := index $.Data "rate" }} + {{ $zoneName := $.Metadata.UID }} + limit_req_zone $binary_remote_addr zone={{ $zoneName }}:10m rate={{ $rate }}r/s; + ``` + +- For location context: + + ```text + {{ $zoneName := $.Metadata.UID }} + {{ $burst := index $.Data "burst }} + limit_req zone={{ $zoneName }}{{ if $burst }}burst={{ $burst }}{{ end }}; + ``` + +Next, Cluster operator allows Application developer to use the feature by creating a SnippetsTemplate, which binds +the CRD with the templates: + +```yaml +apiVersion: gateway.nginx.org/v1alpha1 +kind: SnippetsTemplate +metadata: + name: expose +spec: + valuesCRD: ratelimitingpolicy + allowedTargets: + - group: gateway.networking.k8s.io + kind: HTTPRoute + templates: + - context: http + value: | + {{ $rate := index $.Data "rate" }} + {{ $zoneName := $.Metadata.UID }} + limit_req_zone $binary_remote_addr zone={{ $zoneName }}:10m rate={{ $rate }}r/s; + - context: http.server.location + value: | + {{ $zoneName := $.Metadata.UID }} + {{ $burst := index $.Data "burst }} + limit_req zone={{ $zoneName }}{{ if $burst }}burst={{ $burst }}{{ end }}; +``` + +As a result, Cluster operator created an extension, and now it is ready to be used by Application developers. + +Next, Application developer creates a Custom resource of the CRD to enable that NGINX feature with the desired values: + +```yaml +apiVersion: example.org/v1alpha1 +kind: RateLimitingPolicy +metadata: + name: rate-limit +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: cafe-route + rate: 10 + burst: 5 +``` + +As a result, NGF will insert the config generated by templates into the configuration generated for the +referenced HTTPRoute: + +```text +# this is http context + +limit_req_zone $binary_remote_addr zone=d1fd001c-a784-4964-baa2-100d16aa5540:10m rate=10r/s; +. . . + +server { + # this is server + . . . + + location /coffee { + . . . + limit_req zone=d1fd001c-a784-4964-baa2-100d16aa5540 burst=5; + . . . + } + + location /tea { + . . . + limit_req zone=d1fd001c-a784-4964-baa2-100d16aa5540 burst=5; + . . . + } +``` + +> For brevity, the proposal doesn't include more examples. However, SnippetsTemplate can also implement all examples +> used for SnippetsPolicy. + +### Inheritance and Conflicts + +The CRD that provides values must be a [Direct Attached Policy](https://gateway-api.sigs.k8s.io/geps/gep-2648/). +Specifically, it must +only allow target the same Kinds. + +NGF will perform conflict resolution if the Custom resources of the CRD target the same resources. + +If requested by users, this proposal can be extended to allow the CRD to follow +[Inhereted Policy Attachment](https://gateway-api.sigs.k8s.io/geps/gep-2649/). + +### Policy or Filter + +Similarly to SnippetsPolicy, considering that SnippetsTemplate can be used to implement authentication and +authorization, because of the reasons mentioned +[here](nginx-extensions.md#authentication), it makes sense to also support filters as the values CRD. + +For the rate-limiting example, such CRD remains the same but without the `targetRefs` field. + +```yaml +apiVersion: example.org/v1alpha1 +kind: RateLimitingFilter +metadata: + name: rate-limit +spec: + rate: 10 + burst: 5 +``` + +HTTPRoutes can include that filter: + +```yaml +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: my-app +spec: + . . . + rules: + - matches: + - path: + type: PathPrefix + value: / + filters: + - type: ExtensionRef + extensionRef: + group: example.org/v1alpha1 + kind: RateLimitingFilter + name: rate-limit + backendRefs: + - name: headers + port: 80 +``` + +Similarly, such filters can also be used in +GRPCRoute. [TLSRoute](https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1alpha2.TLSRoute) +doesn't support filters, so it needs to be added to the Gateway API first. + +TO-DO(pleshakov) - Add support for filters to the API section in the GEP after discussions about it. + +### Personas + +- Cluster operator creates a SnippetsTemplate. +- Application developer uses it by creating a Custom resource of the values CRD of the SnippetsTemplate. + +### Validation + +#### NGINX Values + +When Cluster operator defines a values CRD, they should define validation rules to prevent invalid or malicious values +propagating into NGINX config. The Kubernetes API server will perform that validation when an Application developer +creates +a Custom resource of the CRD. + +#### Template + +A template can be invalid (not follow go template or panic when executed). NGF must reject such templates +and also not crash when executing them. + +A template can generate invalid NGINX configuration. When this happens, NGINX will continue to use the last valid +configuration. +However, any subsequent configuration updates (for example, caused by changes to an HTTPRoute) will not be possible +until the invalid configuration is removed. Thus, Cluster operator must carefully design the template. + +### Security Considerations + +As mentioned in the previous section, Cluster operator should define validation rules to prevent invalid/malicious +values from +Custom resources created by Application developers from propagating into the NGINX config. + +However, it is possible that a template generates malicious values. Because of that, only Cluster operator (a privileged +user) should be able to create SnippetsTemplates. + +### Upgrades + +NGF inserts configuration generated from the templates of a SnippetsTemplate into various parts of the configuration +that it already generates. As a result, the configuration generated from the templates must play well with +the surrounding configuration, so that it doesn't break that configuration and vice versa. + +As we develop NGF, that surrounding configuration will expand (because we will implement more NGINX features) and also +might change (for example, due to improvements). As a result, there is a risk that SnippetsTemplate break after +Cluster operator upgrades NGF to the next version. Such risk shall be clearly documented in the SnippetsTemplate +documentation. + +### Prior Arts + +- NGINX Ingress + Controller [Custom annotations](https://docs.nginx.com/nginx-ingress-controller/configuration/ingress-resources/custom-annotations/) +- NGINX Instance + Manager [Config Templates](https://docs.nginx.com/nginx-management-suite/nim/about/templates/config-templates/) + +### Alternatives + +- SnippetsPolicy + +### Summary + +SnippetsTemplate allows Application developers to safely and easily use NGINX configuration created by Cluster operator. +Cluster operator retains full control of that NGINX configuration, but at the same time allows Application +developers to provide custom +values. Kubernetes API will validate such values, which leads to better security and reliability compared to +SnippetsPolicy. + +## NGINX Features Supported by Proposed Extensions + +> Note: the list is from [NGINX Extensions](nginx-extensions.md#prioritized-nginx-features). + +| Features | Supported by SnippetsPolicy/SnippetsTemplate | Requires NGINX Plus | +|--------------------------------------------------------------------------------------------------|------------------------------------------------------------------------|---------------------| +| Log level and format | X (limited, only extra logs on top of the default one) | | +| Passive health checks | | | +| Slow start. Prevent a recently recovered server from being overwhelmed on connections by restart | | X | +| Active health checks | X (if Service is allowed as TargetRef) | X | +| JSON Web Token (JWT) authentication | X | X | +| OpenID Connect (OIDC) authentication. Allow users to authenticate with single-sign-on (SSO) | X (njs files need to be mounted to nginx container FS separately) | X | +| Customize client settings. For example, `client_max_body_size` | X | | +| Upstream keepalives | | | +| Client keepalives | X | | +| TLS settings. For example, TLS protocols and server ciphers | X | | +| OpenTelemetry tracing | X | | +| Connection timeouts | X | | +| Backup server | | | +| Load-balancing method | | | +| Load-balancing method (least time) | | X | +| Limit connections to a server | | | +| Request queuing. Queue requests if servers are unavailable | | X | +| Basic authentication. User/Password | X (password files need to be mounted to nginx container FS separately) | | +| PROXY protocol | | | +| Upstream zone size | | | +| Custom response. Return a custom response for a given path | | | +| External routing. Route to services outside the cluster | | | +| HTTP Strict Transport Security (HSTS) | X | | +| API Key authentication | X (API Keys need to be mounted to nginx container FS separately) | | +| Next upstream retries. NGINX retries by trying the _next_ server in the upstream group | X | | +| Proxy Buffering | X | | +| Pass/hide headers to/from the client | X | | +| Custom error pages | X | | +| Access control | X | | +| Windows Technology LAN Manager (NTLM) | | X | +| Backend client certificate verification | X (CA file needs to be mounted to nginx container FS separately) | | +| NGINX worker settings | X | | +| HTTP/2 client requests | | | +| Set server name size | X | | +| JSON Web Token (JWT) content-based routing | | X | +| QUIC/HTTP3 | | | +| Remote authentication request | X | | +| Timeout for unresponsive clients | X | | +| Rate-limiting | X | | +| Session persistence | | x | +| Cross-Origin Resource Sharing (CORS) | X | | From b6ea660dc1ad48fe7ce8a223c500bb426ae8e796 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Wed, 10 Jul 2024 09:43:45 -0400 Subject: [PATCH 02/26] Fix articles --- docs/proposals/advanced-nginx-extensions.md | 76 ++++++++++----------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index 5af1dfc79a..34e1e896cc 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -65,8 +65,8 @@ NGINX use cases. To allow them to implement those use cases, we need to bring a - Allow users to insert NGINX configuration not supported via Gateway API resources or NGINX extensions. - Support configuration from modules not loaded in NGINX by default or third-party modules. -- Most of the configuration complexity should fall onto the cluster operator persona, not the application developer. -- Provide security controls to prevent application developers from injecting arbitrary NGINX configuration. +- Most of the configuration complexity should fall onto the Cluster operator persona, not the Application developer. +- Provide security controls to prevent Application developers from injecting arbitrary NGINX configuration. - Ensure adequate configuration validation to prevent NGINX outages due to invalid configuration. - Advanced NGINX extensions can be used without source code modification. @@ -81,9 +81,9 @@ NGINX use cases. To allow them to implement those use cases, we need to bring a This proposal brings two extension mechanisms: - [SnippetsPolicy](#snippetspolicy) which allows to quickly bring unsupported NGINX configuration to NGF. However, - because of its implications for reliability and security, SnippetsPolicy is mostly applicable for Cluster operator. + because of its implications for reliability and security, SnippetsPolicy is mostly applicable for the Cluster operator. - [SnippetsTemplate](#snippetstemplate) which allows to bring unsupported NGINX configuration to NGF by Application - developers in a safe and uncomplicated manner. However, SnippetsTemplates require some up-front work from Cluster + developers in a safe and uncomplicated manner. However, SnippetsTemplates require some up-front work from a Cluster operator, meaning it cannot be implemented quickly, in contrast with SnippetsPolicy. ### SnippetsPolicy @@ -385,10 +385,10 @@ GRPCRoute. [TLSRoute](https://gateway-api.sigs.k8s.io/reference/spec/#gateway.ne doesn't support filters, so it needs to be added to the Gateway API first. Although SnippetsFilter applicability is limited compared to SnippetsPolicy, SnippetsFilter creates -a natural split of responsibilities between Cluster operator and Application developers: +a natural split of responsibilities between the Cluster operator and the Application developer: -- Cluster operator creates a SnippetsFilter. -- Application developers reference the SnippetsFilter to enable it. +- The Cluster operator creates a SnippetsFilter. +- The Application developer references the SnippetsFilter to enable it. Note that with the SnippetsPolicy, because the targetRef is part of the SnippetsPolicy, it is not possible to have such a split of responsibilities. @@ -397,7 +397,7 @@ TO-DO(pleshakov) - Add SnippetsFilter to the GEP after discussions about it. ### Personas -The target persona of snippets is Cluster operator. They will create and manage snippets. +The target persona of snippets is the Cluster operator. They will create and manage snippets. Snippets are not intended for Application developers, because: @@ -448,7 +448,7 @@ spec: } ``` -As a consequence, creating SnippetsPolicy should only be allowed for the privileged users -- Cluster operator. +As a consequence, creating SnippetsPolicy should only be allowed for the privileged users -- the Cluster operator. As a further precaution, we can disable SnippetsPolicy by default similarly to [NGINX Ingress Controller](https://docs.nginx.com/nginx-ingress-controller/configuration/security/#snippets). @@ -477,17 +477,17 @@ operator upgrades NGF to the next version. Such risk shall be clearly documented ### Summary -- Snippets allow Cluster operator to quickly configure NGINX features not available via NGF. -- SnippetsPolicy should be used only by Cluster operator, because of reliability and security implications. -- SnippetsFilter, it is possible to split the responsibility of creating snippets (Cluster operator) and enabling - snippets (Application Developer). +- Snippets allow the Cluster operator to quickly configure NGINX features not available via NGF. +- SnippetsPolicy should be used only by the Cluster operator, because of reliability and security implications. +- SnippetsFilter, it is possible to split the responsibility of creating snippets (the Cluster operator) and enabling + snippets (the Application Developer). ## SnippetsTemplate SnippetsTemplate is similar to SnippetsPolicy: it allows inserting NGINX configuration into various NGINX contexts. However, in contrast with SnippetsPolicy, it allows Application developers to safely use them without the risk of breaking NGINX config. At the same time, that safety comes with a cost: creating a SnippetsTemplate is more involved -for Cluster operators compared to SnippetsPolicy. +for the Cluster operator compared to SnippetsPolicy. ### API @@ -636,24 +636,24 @@ struct, which NGF uses to generate location config. ### How to Use SnippetsTemplate -1. Cluster operator comes up with NGINX configuration that configures an NGINX feature needed by Application +1. A Cluster operator comes up with NGINX configuration that configures an NGINX feature needed by an Application developer. -2. Cluster operator thinks about what values Application developer might want to customize in that configuration. -3. Cluster operator creates a CRD that defines fields and validation for those values and creates the CRD in the +2. The Cluster operator thinks about what values the Application developer might want to customize in that configuration. +3. The Cluster operator creates a CRD that defines fields and validation for those values and creates the CRD in the cluster. -4. Cluster operator creates a go template that generates the NGINX configuration based on the fields of the CRD. -5. Cluster operator allows Application developer to use the feature by creating a SnippetsTemplate, which binds +4. The Cluster operator creates a go template that generates the NGINX configuration based on the fields of the CRD. +5. The Cluster operator allows the Application developer to use the feature by creating a SnippetsTemplate, which binds the CRD with the template(s). -6. Application developer creates a Custom resource of the CRD to enable that NGINX feature with the desired values. +6. The Application developer creates a Custom resource of the CRD to enable that NGINX feature with the desired values. -Essentially, Cluster operator creates an extension for a particular NGINX feature. Application developers can +Essentially, the Cluster operator creates an extension for a particular NGINX feature. The Application developer can use that extension with the ability to further customize it. ### Examples #### Rate Limiting -First, Cluster operator comes up with NGINX configuration for the rate-limiting feature asked by Application developers: +First, a Cluster operator comes up with NGINX configuration for the rate-limiting feature asked by an Application developer: ```text ```text @@ -668,10 +668,10 @@ server { } ``` -Next, Cluster operator decides that Application developers might want to customize the rate-limiting `rate` and `burst` +Next, the Cluster operator decides that the Application developer might want to customize the rate-limiting `rate` and `burst` values. -Next, Cluster operator defines the following CRD for those values. Note that the values are validated to prevent invalid +Next, the Cluster operator defines the following CRD for those values. Note that the values are validated to prevent invalid or malicious values from being propagated into the NGINX config: @@ -723,9 +723,9 @@ type RateLimitingPolicySpec struct { } ``` -Next, Cluster operator generates a CRD manifest from that go code and creates the CRD in the cluster. +Next, the Cluster operator generates a CRD manifest from that go code and creates the CRD in the cluster. -Next, Cluster operator creates templates that generate the NGINX configuration based on the fields of the CRD: +Next, the Cluster operator creates templates that generate the NGINX configuration based on the fields of the CRD: - For http context: @@ -743,7 +743,7 @@ Next, Cluster operator creates templates that generate the NGINX configuration b limit_req zone={{ $zoneName }}{{ if $burst }}burst={{ $burst }}{{ end }}; ``` -Next, Cluster operator allows Application developer to use the feature by creating a SnippetsTemplate, which binds +Next, the Cluster operator allows the Application developer to use the feature by creating a SnippetsTemplate, which binds the CRD with the templates: ```yaml @@ -769,9 +769,9 @@ spec: limit_req zone={{ $zoneName }}{{ if $burst }}burst={{ $burst }}{{ end }}; ``` -As a result, Cluster operator created an extension, and now it is ready to be used by Application developers. +As a result, the Cluster operator created an extension, and now it is ready to be used by the Application developer. -Next, Application developer creates a Custom resource of the CRD to enable that NGINX feature with the desired values: +Next, the Application developer creates a Custom resource of the CRD to enable that NGINX feature with the desired values: ```yaml apiVersion: example.org/v1alpha1 @@ -878,14 +878,14 @@ TO-DO(pleshakov) - Add support for filters to the API section in the GEP after d ### Personas -- Cluster operator creates a SnippetsTemplate. -- Application developer uses it by creating a Custom resource of the values CRD of the SnippetsTemplate. +- The Cluster operator creates a SnippetsTemplate. +- The Application developer uses it by creating a Custom resource of the values CRD of the SnippetsTemplate. ### Validation #### NGINX Values -When Cluster operator defines a values CRD, they should define validation rules to prevent invalid or malicious values +When a Cluster operator defines a values CRD, they should define validation rules to prevent invalid or malicious values propagating into NGINX config. The Kubernetes API server will perform that validation when an Application developer creates a Custom resource of the CRD. @@ -898,15 +898,15 @@ and also not crash when executing them. A template can generate invalid NGINX configuration. When this happens, NGINX will continue to use the last valid configuration. However, any subsequent configuration updates (for example, caused by changes to an HTTPRoute) will not be possible -until the invalid configuration is removed. Thus, Cluster operator must carefully design the template. +until the invalid configuration is removed. Thus, a Cluster operator must carefully design the template. ### Security Considerations -As mentioned in the previous section, Cluster operator should define validation rules to prevent invalid/malicious +As mentioned in the previous section, the Cluster operator should define validation rules to prevent invalid/malicious values from Custom resources created by Application developers from propagating into the NGINX config. -However, it is possible that a template generates malicious values. Because of that, only Cluster operator (a privileged +However, it is possible that a template generates malicious values. Because of that, only the Cluster operator (a privileged user) should be able to create SnippetsTemplates. ### Upgrades @@ -917,7 +917,7 @@ the surrounding configuration, so that it doesn't break that configuration and v As we develop NGF, that surrounding configuration will expand (because we will implement more NGINX features) and also might change (for example, due to improvements). As a result, there is a risk that SnippetsTemplate break after -Cluster operator upgrades NGF to the next version. Such risk shall be clearly documented in the SnippetsTemplate +a Cluster operator upgrades NGF to the next version. Such risk shall be clearly documented in the SnippetsTemplate documentation. ### Prior Arts @@ -933,8 +933,8 @@ documentation. ### Summary -SnippetsTemplate allows Application developers to safely and easily use NGINX configuration created by Cluster operator. -Cluster operator retains full control of that NGINX configuration, but at the same time allows Application +SnippetsTemplate allows Application developers to safely and easily use NGINX configuration created by a Cluster operator. +The Cluster operator retains full control of that NGINX configuration, but at the same time allows Application developers to provide custom values. Kubernetes API will validate such values, which leads to better security and reliability compared to SnippetsPolicy. From 56002986e6b6f0c41d387f5df9622072b3ff4268 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Wed, 10 Jul 2024 09:44:40 -0400 Subject: [PATCH 03/26] Remove TOC --- docs/proposals/advanced-nginx-extensions.md | 49 --------------------- 1 file changed, 49 deletions(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index 34e1e896cc..7744f6759a 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -12,55 +12,6 @@ the subset will grow. However, it will take time. Additionally, because the numb and parameters is huge, not all of them will be supported that way. As a result, users are not able to implement certain NGINX use cases. To allow them to implement those use cases, we need to bring a new extension mechanism to NGF. -## Table of Contents - - - -- [Enhancement Proposal-2035: Advanced NGINX Extensions](#enhancement-proposal-2035-advanced-nginx-extensions) - - [Summary](#summary) - - [Table of Contents](#table-of-contents) - - [Goals](#goals) - - [Non-Goals](#non-goals) - - [Advanced Extensions](#advanced-extensions) - - [SnippetsPolicy](#snippetspolicy) - - [API](#api) - - [Supported NGINX Contexts](#supported-nginx-contexts) - - [Examples](#examples) - - [Rate-limiting](#rate-limiting) - - [Proxy Buffering](#proxy-buffering) - - [Access Control](#access-control) - - [Third-Party Module](#third-party-module) - - [Inheritance and Conflicts](#inheritance-and-conflicts) - - [Policy or Filter](#policy-or-filter) - - [Personas](#personas) - - [NGINX Config Validation](#nginx-config-validation) - - [Security Considerations](#security-considerations) - - [Upgrades](#upgrades) - - [Prior Arts](#prior-arts) - - [Alternatives](#alternatives) - - [Summary](#summary-1) - - [SnippetsTemplate](#snippetstemplate) - - [API](#api-1) - - [SnippetsTemplate](#snippetstemplate-1) - - [Templates](#templates) - - [How to Use SnippetsTemplate](#how-to-use-snippetstemplate) - - [Examples](#examples-1) - - [Rate Limiting](#rate-limiting-1) - - [Inheritance and Conflicts](#inheritance-and-conflicts-1) - - [Policy or Filter](#policy-or-filter-1) - - [Personas](#personas-1) - - [Validation](#validation) - - [NGINX Values](#nginx-values) - - [Template](#template) - - [Security Considerations](#security-considerations-1) - - [Upgrades](#upgrades-1) - - [Prior Arts](#prior-arts-1) - - [Alternatives](#alternatives-1) - - [Summary](#summary-2) - - [NGINX Features Supported by Proposed Extensions](#nginx-features-supported-by-proposed-extensions) - - - ## Goals - Allow users to insert NGINX configuration not supported via Gateway API resources or NGINX extensions. From a53950b22dec7f2c5a97a556e8ced784c18df5f0 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Wed, 10 Jul 2024 09:45:59 -0400 Subject: [PATCH 04/26] disable SnippetsPolicy by default --- docs/proposals/advanced-nginx-extensions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index 7744f6759a..2a52676f90 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -400,7 +400,7 @@ spec: ``` As a consequence, creating SnippetsPolicy should only be allowed for the privileged users -- the Cluster operator. -As a further precaution, we can disable SnippetsPolicy by default similarly +As a further precaution, we will disable SnippetsPolicy by default similarly to [NGINX Ingress Controller](https://docs.nginx.com/nginx-ingress-controller/configuration/security/#snippets). It is also possible to add validation of snippets to disallow certain directives (like `root` and `autoindex`) From d8b50625691d504472112251b5ddde9bd7c68b5f Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Wed, 10 Jul 2024 09:47:33 -0400 Subject: [PATCH 05/26] Remove unnecessary SnippetsTemplateConditionReasonTargetNotFound --- docs/proposals/advanced-nginx-extensions.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index 2a52676f90..45cbde3b6b 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -541,10 +541,6 @@ const ( // SnippetsTemplateConditionReasonCRDNotFound is used with the Accepted condition type when // the referenced CRD is not found. SnippetsTemplateConditionReasonCRDNotFound SnippetsTemplateConditionReason = "CRDNotFound" - - // SnippetsTemplateConditionReasonTargetNotFound is used with the Accepted condition type when - // the target is not found. - SnippetsTemplateConditionReasonTargetNotFound SnippetsTemplateConditionReason = "TargetNotFound" ) ``` From e45ff8fb1f29a64c00512f250a2cf69e199bfb86 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Wed, 10 Jul 2024 09:48:04 -0400 Subject: [PATCH 06/26] Remove duplicated ```text --- docs/proposals/advanced-nginx-extensions.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index 45cbde3b6b..ed4168fe23 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -602,7 +602,6 @@ use that extension with the ability to further customize it. First, a Cluster operator comes up with NGINX configuration for the rate-limiting feature asked by an Application developer: -```text ```text # http context limit_req_zone $binary_remote_addr zone=myzone:10m rate=1r/s; From 903fb7bbe06bb42412bd329fa9f3b000797a4245 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Wed, 10 Jul 2024 09:49:25 -0400 Subject: [PATCH 07/26] add that the Cluster operator also creates the values CRD --- docs/proposals/advanced-nginx-extensions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index ed4168fe23..8ac9041354 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -824,7 +824,7 @@ TO-DO(pleshakov) - Add support for filters to the API section in the GEP after d ### Personas -- The Cluster operator creates a SnippetsTemplate. +- The Cluster operator creates a SnippetsTemplate and the values CRD. - The Application developer uses it by creating a Custom resource of the values CRD of the SnippetsTemplate. ### Validation From 443868dbcee94c29e174812c6ad8316cf274fb53 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Wed, 10 Jul 2024 10:25:13 -0400 Subject: [PATCH 08/26] Disable SnippetsTemplates by default --- docs/proposals/advanced-nginx-extensions.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index 8ac9041354..716583f57c 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -853,7 +853,8 @@ values from Custom resources created by Application developers from propagating into the NGINX config. However, it is possible that a template generates malicious values. Because of that, only the Cluster operator (a privileged -user) should be able to create SnippetsTemplates. +user) should be able to create SnippetsTemplates. As a further precaution, we will disable SnippetsTemplate by default +similarly to [SnippetsPolicy](#security-considerations). ### Upgrades From 320e57763f478febdb4dcaa297ae67310af5c541 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Wed, 10 Jul 2024 10:30:53 -0400 Subject: [PATCH 09/26] Clarify supported contexts --- docs/proposals/advanced-nginx-extensions.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index 716583f57c..e2b4bef9ea 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -80,8 +80,11 @@ type SnippetsPolicySpec struct { // // Support: Gateway, HTTPRoute, GRPCRoute, TLSRoute // - // For HTTPRoute and GRPCRoute, only http* contexts are supported. - // For TLSRoute, only stream* contexts are supported. + // Supported contexts depend on the targetRef Kind: + // + // * HTTPRoute and GRPCRoute: http, http.server and http.server.location. + // * TLSRoute: stream and stream.server. + // * Gateway: all contexts. TargetRefs []gatewayv1alpha2.LocalPolicyTargetReference `json:"targetRefs"` } From 195ef6485864104a02e7d7d44c235afe2b578f29 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Wed, 10 Jul 2024 10:51:48 -0400 Subject: [PATCH 10/26] Mention validating SnippetsPolicy and SnippetsTemplate fields --- docs/proposals/advanced-nginx-extensions.md | 31 ++++++++++++++------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index e2b4bef9ea..88405869c7 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -43,7 +43,8 @@ Snippets allow inserting NGINX configuration into various NGINX contexts. #### API -> Field validation rules are intentionally left out of this proposal. +> Specific field validation rules like CEL rules are intentionally left out of this proposal. However, important +> values restrictions are documented in the comments of the types. ```go // SnippetsPolicy is a Direct Attached Policy. It allows inserting NGINX configuration into the generated NGINX @@ -363,12 +364,19 @@ Snippets are not intended for Application developers, because: As mentioned in the [Policy or Filter](#policy-or-filter), when snippets are used via SnippetsFilter, Application developers can still control whether they want to enable or disable snippets by referencing them in an HTTPRoute. -### NGINX Config Validation +### Validation + +#### SnippetsPolicy + +NGF will validate the fields of SnippetsPolicy resources based on the restrictions mentioned in the [API section](#api). + +NGF will not validate the values of snippets. See the next section. + +#### NGINX Values An invalid snippet can break NGINX config. When this happens, NGINX will continue to use the last valid configuration. However, any subsequent configuration updates (for example, caused by changes to an HTTPRoute) will not be possible -until the -invalid snippet is removed. +until the invalid snippet is removed. Before injecting snippets in NGINX config, it is possible to validate snippets using [crossplane](https://github.com/nginxinc/nginx-go-crossplane), @@ -447,7 +455,8 @@ for the Cluster operator compared to SnippetsPolicy. #### SnippetsTemplate -> Field validation rules are intentionally left out of this proposal. +> Specific field validation rules like CEL rules are intentionally left out of this proposal. However, important +> values restrictions are documented in the comments of the types. ```go // SnippetsTemplate configures an NGINX Gateway Fabric extension based on the templated NGINX configuration snippets. @@ -836,19 +845,21 @@ TO-DO(pleshakov) - Add support for filters to the API section in the GEP after d When a Cluster operator defines a values CRD, they should define validation rules to prevent invalid or malicious values propagating into NGINX config. The Kubernetes API server will perform that validation when an Application developer -creates -a Custom resource of the CRD. +creates a Custom resource of the CRD. -#### Template +#### SnippetsTemplate -A template can be invalid (not follow go template or panic when executed). NGF must reject such templates -and also not crash when executing them. +A template defined in SnippetsTemplate can be invalid (not follow go template or panic when executed). NGF must reject +such templates and also not crash when executing them. A template can generate invalid NGINX configuration. When this happens, NGINX will continue to use the last valid configuration. However, any subsequent configuration updates (for example, caused by changes to an HTTPRoute) will not be possible until the invalid configuration is removed. Thus, a Cluster operator must carefully design the template. +NGF will also validate the rest of SnippetsTemplate fields based on the restrictions mentioned in +the [API section](#api-1). + ### Security Considerations As mentioned in the previous section, the Cluster operator should define validation rules to prevent invalid/malicious From 952329bd1f1251d1f48ecc33cc2581d04460cef8 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Thu, 11 Jul 2024 10:58:14 -0400 Subject: [PATCH 11/26] Clarify about snippet/template per context restriction --- docs/proposals/advanced-nginx-extensions.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index 88405869c7..7e28a7c981 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -370,6 +370,9 @@ developers can still control whether they want to enable or disable snippets by NGF will validate the fields of SnippetsPolicy resources based on the restrictions mentioned in the [API section](#api). +We will only allow one snippet per context because it will be easier to comprehend a SnippetsPolicy this way: compare +one `http` snippet with multiple `http` snippets scattered around in the spec. + NGF will not validate the values of snippets. See the next section. #### NGINX Values @@ -860,6 +863,9 @@ until the invalid configuration is removed. Thus, a Cluster operator must carefu NGF will also validate the rest of SnippetsTemplate fields based on the restrictions mentioned in the [API section](#api-1). +We will only allow one template per context because it will be easier to comprehend a SnippetsTemplate this way: compare +one `http` template with multiple `http` templates scattered around in the spec. + ### Security Considerations As mentioned in the previous section, the Cluster operator should define validation rules to prevent invalid/malicious From 1cb71760cf34f913a45ab8144a21b6d98a98a1f4 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Thu, 11 Jul 2024 14:46:07 -0400 Subject: [PATCH 12/26] Add SnippetsFilters --- docs/proposals/advanced-nginx-extensions.md | 225 +++++++++++++------- 1 file changed, 145 insertions(+), 80 deletions(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index 7e28a7c981..3b0c676d55 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -31,15 +31,17 @@ NGINX use cases. To allow them to implement those use cases, we need to bring a This proposal brings two extension mechanisms: -- [SnippetsPolicy](#snippetspolicy) which allows to quickly bring unsupported NGINX configuration to NGF. However, - because of its implications for reliability and security, SnippetsPolicy is mostly applicable for the Cluster operator. +- [Snippets](#snippets) which allow to quickly bring unsupported NGINX configuration to NGF. However, + because of its implications for reliability and security, Snippets are mostly applicable for the Cluster operator. - [SnippetsTemplate](#snippetstemplate) which allows to bring unsupported NGINX configuration to NGF by Application developers in a safe and uncomplicated manner. However, SnippetsTemplates require some up-front work from a Cluster - operator, meaning it cannot be implemented quickly, in contrast with SnippetsPolicy. + operator, meaning it cannot be implemented quickly, in contrast with Snippets. -### SnippetsPolicy +### Snippets -Snippets allow inserting NGINX configuration into various NGINX contexts. +Snippets allow inserting NGINX configuration into various NGINX contexts. They come in two flavours: +- SnippetsPolicy +- SnippetsFilter #### API @@ -76,15 +78,12 @@ type SnippetsPolicySpec struct { // TargetRefs identifies the API object(s) to apply the policy to. // Objects must be in the same namespace as the policy. // Objects must be of the same Kinds. - // TO-DO(pleshakov): Remove the line below. - // why same Kinds? Because different kinds allow mutually exclusive snippets. // - // Support: Gateway, HTTPRoute, GRPCRoute, TLSRoute + // Support: Gateway, HTTPRoute, GRPCRoute // // Supported contexts depend on the targetRef Kind: // // * HTTPRoute and GRPCRoute: http, http.server and http.server.location. - // * TLSRoute: stream and stream.server. // * Gateway: all contexts. TargetRefs []gatewayv1alpha2.LocalPolicyTargetReference `json:"targetRefs"` } @@ -122,6 +121,66 @@ type Snippet struct { // Value is the NGINX configuration snippet. Value string `json:"value"` } + +// SnippetsFilter allows inserting NGINX configuration into the generated NGINX config for HTTPRoute and GRPCRoute +// resources. +// To implement authentication-like features, it is recommended to use SnippetsFilter instead of SnippetsPolicy. +// Since a Filter is referenced from a routing rule, it makes it clear to the Application Developer that authentication +// is applied to the route, whereas due to the discoverability issues around Policies, it wouldn't be clear to the +// Application Developer that authentication exists for their routes. Furthermore, if the referenced Filter does not +// exist, NGINX will return a 500 and the protected path will not be exposed. This isn't the case for a Policy, since +// a missing Policy would have no effect on routes and the protected path will be exposed to all users. +type SnippetsFilter struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + // Spec defines the desired state of the SnippetsFilter. + Spec SnippetsFilterSpec `json:"spec"` + + // Status defines the state of the SnippetsFilter. + Status SnippetsFilterStatus `json:"status,omitempty"` +} + +// SnippetsFilterSpec defines the desired state of the SnippetsFilter. +type SnippetsFilterSpec struct { + // Snippets is a list of NGINX configuration snippets. + // There can only be one snippet per context. + // Allowed contexts: http, http.server, http.server.location. + Snippets []Snippet `json:"snippets"` +} + +// SnippetsFilterStatus defines the state of SnippetsFilter. +type SnippetsFilterStatus struct { + // Conditions describes the state of the SnippetsFilter. + Conditions []metav1.Condition `json:"conditions,omitempty"` +} + +// SnippetsFilterConditionType is a type of condition associated with SnippetsFilter +type SnippetsFilterConditionType string + +// SnippetsFilterConditionReason is a reason for a SnippetsFilter condition type. +type SnippetsFilterConditionReason string + +const ( + // SnippetsFilterConditionTypeAccepted indicates that the SnippetsFilter is accepted. + // + // Possible reasons for this condition to be True: + // + // * Accepted + // + // Possible reasons for this condition to be False: + // + // * Invalid + SnippetsFilterConditionTypeAccepted SnippetsFilterConditionType = "Accepted" + + // SnippetsFilterConditionReasonAccepted is used with the Accepted condition type when + // the condition is true. + SnippetsFilterConditionReasonAccepted SnippetsFilterConditionReason = "Accepted" + + // SnippetsFilterConditionTypeInvalid is used with the Accepted condition type when + // SnippetsFilter is invalid. + SnippetsFilterConditionTypeInvalid SnippetsFilterConditionType = "Invalid" +) ``` #### Supported NGINX Contexts @@ -147,17 +206,25 @@ SnippetsPolicy supports the following NGINX configuration contexts: - stream module - there are not any features in https://nginx.org/en/docs/stream/ngx_stream_upstream_module.html Note: because NGF already inserts the `random` load-balancing method, an `upstream` snippet will not be able to -configure -a different method. +configure a different method. If we choose to introduce upstream snippets, the SnippetsPolicy will need to support targeting a Service, because NGF generates one upstream per Service. +> We don't support routes that correspond the NGINX stream module -- TLSRoute, TCPRoute and UDPRoute. However, +> because we're going to support them in the future, this proposal for SnippetsPolicy includes `stream` and `server` +> contexts of the stream module. + +SnippetsFilter supports `http`, `server` and `location` contexts of the stream module. + +> TLSRoute, TCPRoute and UPDRoute don't support filters. As a result, SnippetsFilter doesn't support stream-related +> contexts. + #### Examples -Below are a few examples of using the SnippetsPolicy to bring unsupported NGINX configuration into NGF. +Below are a few examples of using snippets to bring unsupported NGINX configuration into NGF. -##### Rate-limiting +##### Rate-limiting SnippetPolicy We use NGINX [limit req module](https://nginx.org/en/docs/http/ngx_http_limit_req_module.html) to configure rate limiting. @@ -203,7 +270,7 @@ server { } ``` -##### Proxy Buffering +##### Proxy Buffering SnippetPolicy We configure [proxy_buffering](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffering) directive to disable buffering. @@ -225,7 +292,7 @@ spec: As a result, NGF will insert the provided config into the generated locations for the cafe-route HTTPRoute. -##### Access Control +##### Access Control SnippetPolicy We use NGINX [access module](https://nginx.org/en/docs/http/ngx_http_access_module.html) to configure access based on client IPs. @@ -250,7 +317,55 @@ spec: As a result, NGF will insert the provided NGINX config into the server context of all generated HTTP servers for the Gateway cafe. -##### Third-Party Module +##### Access Control SnippetsFilter + +We use NGINX [access module](https://nginx.org/en/docs/http/ngx_http_access_module.html) to configure access based +on client IPs. + +```yaml +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; +``` + +```yaml +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: coffee +spec: + parentRefs: + - name: gateway + sectionName: http + hostnames: + - "cafe.example.com" + rules: + - matches: + - path: + type: PathPrefix + value: /coffee + filters: + - type: ExtensionRef + extensionRef: + group: gateway.nginx.org/v1alpha1 + kind: SnippetsFilter + name: access-control + backendRefs: + - name: headers + port: 80 +``` + +As a result, NGF will insert the provided NGINX config into all generated locations for the +`/coffee` path. + +##### Third-Party Module SnippetPolicy We use the third-party [Brotli module](https://docs.nginx.com/nginx/admin-guide/dynamic-modules/brotli/). @@ -279,7 +394,7 @@ As a result, NGF will: - Insert `load_module` into the main context to load the module. - Configure all `server` blocks belonging to the Gateway cafe to enable the module features. -### Inheritance and Conflicts +### Inheritance and Conflicts of SnippetsPolicy SnippetsPolicy is general-purpose: it can configure different NGINX features as shown in the examples before. It is expected that several SnippetPolicies will exist in the cluster, with some of them targeting @@ -290,65 +405,14 @@ Although a SnippetsPolicy can target different Kinds (like Gateway and HTTPRoute (1) each SnippetPolicy is independent of any other and (2) a single SnippetPolicy can only affect resources of the same Kind, SnippetsPolicy is a [Direct Attached Policy](https://gateway-api.sigs.k8s.io/geps/gep-2648/). -### Policy or Filter - -Considering that SnippetPolicy can be used to implement authentication and authorization, because of the reasons -mentioned -[here](nginx-extensions.md#authentication) it makes sense to also introduce SnippetsFilter, which will be the same as -SnippetsPolicy but without the `targetRefs` field: - -```yaml -apiVersion: gateway.nginx.org/v1alpha1 -kind: SnippetsFilter -metadata: - name: access-control -spec: - snippets: - - context: http.server - value: | - allow 10.0.0.0/8; - deny all; -``` - -An HTTPRoute can include that filter: - -```yaml -apiVersion: gateway.networking.k8s.io/v1 -kind: HTTPRoute -metadata: - name: my-app -spec: - . . . - rules: - - matches: - - path: - type: PathPrefix - value: / - filters: - - type: ExtensionRef - extensionRef: - group: gateway.nginx.org/v1alpha1 - kind: SnippetsFilter - name: access-control - backendRefs: - - name: headers - port: 80 -``` - -SnippetsFilter can also be used in -GRPCRoute. [TLSRoute](https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1alpha2.TLSRoute) -doesn't support filters, so it needs to be added to the Gateway API first. - -Although SnippetsFilter applicability is limited compared to SnippetsPolicy, SnippetsFilter creates -a natural split of responsibilities between the Cluster operator and the Application developer: - -- The Cluster operator creates a SnippetsFilter. -- The Application developer references the SnippetsFilter to enable it. - -Note that with the SnippetsPolicy, because the targetRef is part of the SnippetsPolicy, it is not possible to have -such a split of responsibilities. +### Why Both Policy and Filter -TO-DO(pleshakov) - Add SnippetsFilter to the GEP after discussions about it. +We introduce both SnippetsPolicy and SnippetsFilter because: +- The usage and error handling of SnippetsFilter is more explicit, as explained [here](nginx-extensions.md#authentication). +- SnippetsFilter creates a natural split of responsibilities between the Cluster operator and the Application developer: + the Cluster operator creates a SnippetsFilter; the Application developer references the SnippetsFilter to enable it. + Note that with the SnippetsPolicy, because the targetRef is part of the SnippetsPolicy, it is not possible to have + such a split of responsibilities. ### Personas @@ -361,12 +425,13 @@ Snippets are not intended for Application developers, because: [Config Validation section](#nginx-config-validation) below. - Snippets can be used to exploit NGF. (See [Security Considerations section](#security-considerations) below) -As mentioned in the [Policy or Filter](#policy-or-filter), when snippets are used via SnippetsFilter, Application -developers can still control whether they want to enable or disable snippets by referencing them in an HTTPRoute. +As mentioned in the [Why Both Policy and Filter](#why-both-policy-and-filter), when snippets are used +via SnippetsFilter, Application developers can still control whether they want to enable or disable snippets by +referencing them in an HTTPRoute. ### Validation -#### SnippetsPolicy +#### SnippetsPolicy/SnippetsFilter NGF will validate the fields of SnippetsPolicy resources based on the restrictions mentioned in the [API section](#api). @@ -413,8 +478,8 @@ spec: } ``` -As a consequence, creating SnippetsPolicy should only be allowed for the privileged users -- the Cluster operator. -As a further precaution, we will disable SnippetsPolicy by default similarly +As a consequence, creating SnippetsPolicy/SnippetsFilter should only be allowed for the privileged users -- the Cluster operator. +As a further precaution, we will disable SnippetsPolicy/SnippetsFilter by default similarly to [NGINX Ingress Controller](https://docs.nginx.com/nginx-ingress-controller/configuration/security/#snippets). It is also possible to add validation of snippets to disallow certain directives (like `root` and `autoindex`) From 1197e01655d8c8295b24601fba98d66a6ee74b9b Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Thu, 11 Jul 2024 18:56:34 -0400 Subject: [PATCH 13/26] Add an alternative: Splitting Snippets from SnippetsPolicy --- docs/proposals/advanced-nginx-extensions.md | 57 ++++++++++++++++++++- 1 file changed, 55 insertions(+), 2 deletions(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index 3b0c676d55..fbe7c80841 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -503,12 +503,65 @@ operator upgrades NGF to the next version. Such risk shall be clearly documented ### Alternatives -- See the next section - [SnippetsTemplate](#snippetstemplate). +#### Splitting Snippets from SnippetsPolicy + +In the example below, SnippetsPolicy reference the snippet, which is defined in a separate resource. + +```yaml +apiVersion: gateway.nginx.org/v1alpha1 +kind: NginxSnippet +metadata: + name: buffering-snippet +spec: + snippets: + - context: http.server.location + value: proxy_buffering off; +--- +apiVersion: gateway.nginx.org/v1alpha1 +kind: SnippetsPolicy +metadata: + name: buffering-snippet-policy +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: cafe-route + snippetRef: + name: buffering-snippet +``` + +This way the Cluster operator is still responsible for creating the snippet, and the Application developer can +apply the snippet to the required target. This way, the Application developer cannot create an unsafe snippet, but +has the full control over the target. + +However, the Application developer can still target a Gateway resource, even though it is managed by the Cluster operator: + +```yaml +apiVersion: gateway.nginx.org/v1alpha1 +kind: SnippetsPolicy +metadata: + name: buffering-snippet-policy +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: my-gateway + snippetRef: + name: buffering-snippet +``` + +As a result, that SnippetPolicy will affect all HTTPRoutes, not only HTTPRoutes of their application. At the same time, +inherited policies like ClientSettingsPolicy has the same issue. + +We're not pursuing this approach for two reasons: +- Splitting snippets is already possible with SnippetsFilter. +- Splitting snippets complicates SnippetsPolicy, because now the Cluster operator needs to manage two resources. + ### Summary - Snippets allow the Cluster operator to quickly configure NGINX features not available via NGF. -- SnippetsPolicy should be used only by the Cluster operator, because of reliability and security implications. +- SnippetsPolicy and should be used only by the Cluster operator, because of reliability and security implications. - SnippetsFilter, it is possible to split the responsibility of creating snippets (the Cluster operator) and enabling snippets (the Application Developer). From 1f6234b5a550325e2c1d6163991ccc1a7774385f Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Fri, 12 Jul 2024 10:32:26 -0400 Subject: [PATCH 14/26] Support Filters as Values CRD --- docs/proposals/advanced-nginx-extensions.md | 83 +++++++++++++-------- 1 file changed, 51 insertions(+), 32 deletions(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index fbe7c80841..e3e123b65c 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -567,10 +567,10 @@ We're not pursuing this approach for two reasons: ## SnippetsTemplate -SnippetsTemplate is similar to SnippetsPolicy: it allows inserting NGINX configuration into various NGINX contexts. -However, in contrast with SnippetsPolicy, it allows Application developers to safely use them without the risk of +SnippetsTemplate is similar to Snippets: it allows inserting NGINX configuration into various NGINX contexts. +However, in contrast with Snippets, it allows Application developers to safely use them without the risk of breaking NGINX config. At the same time, that safety comes with a cost: creating a SnippetsTemplate is more involved -for the Cluster operator compared to SnippetsPolicy. +for the Cluster operator compared to Snippets. ### API @@ -602,20 +602,41 @@ type SnippetsTemplateList struct { // SnippetsTemplateSpec defines the desired state of the SnippetsTemplate. type SnippetsTemplateSpec struct { - // ValuesCRD is the name of the CRD which provides values for the templates. + // ValuesCRD describes the CRD which provides values for the templates. ValuesCRD string `json:"valuesCRD"` - // AllowedTargets is a list of allowed target resources that the CRD can target. - // Only one target is allowed. - AllowedTargets []AllowedTarget `json:"allowedTargets"` - // Templates is a list of NGINX configuration templates to insert into the generated NGINX config. // There can only be one template per context. Templates []Template `json:"templates"` } -// AllowedTarget represents an allowed target. -type AllowedTarget struct { +// ValuesCRD describes the CRD which provides values for the templates. +type ValuesCRD struct { + // Name is the name of the CRD. + Name string `json:"name"` + + // Type is the type of the CRD. + Type ValuesCRDType `json:"type"` + + // AllowedKinds is a list of allowed kinds that can be used with the CRD. + // If the type is 'DirectAttachedPolicy', allowedKinds are the kinds that the policy can target. + // If the type is 'Filter', allowedKinds are the kinds that can reference the filter. + AllowedKinds []AllowedKind `json:"allowedKinds"` +} + +// ValuesCRDType is the type of the CRD which provides values for the templates. +type ValuesCRDType string + +const ( + // ValuesCRDDirectAttachedPolicy corresponds to the DirectAttachedPolicy CRD. + ValuesCRDDirectAttachedPolicy ValuesCRDType = "DirectAttachedPolicy" + + // ValuesCRDFilter corresponds to the Filter CRD. + ValuesCRDFilter ValuesCRDType = "Filter" +) + +// AllowedKind represents an allowed kind. +type AllowedKind struct { // Group is the group of the target resource. Group gatewayv1.Group `json:"group"` @@ -719,8 +740,8 @@ struct, which NGF uses to generate location config. 1. A Cluster operator comes up with NGINX configuration that configures an NGINX feature needed by an Application developer. 2. The Cluster operator thinks about what values the Application developer might want to customize in that configuration. -3. The Cluster operator creates a CRD that defines fields and validation for those values and creates the CRD in the - cluster. +3. The Cluster operator creates a CRD that defines fields and validation for those values, and choose whether the CRD + should be a Policy or a Filter. Then, the operator creates the CRD in the cluster. 4. The Cluster operator creates a go template that generates the NGINX configuration based on the fields of the CRD. 5. The Cluster operator allows the Application developer to use the feature by creating a SnippetsTemplate, which binds the CRD with the template(s). @@ -751,8 +772,8 @@ Next, the Cluster operator decides that the Application developer might want to values. Next, the Cluster operator defines the following CRD for those values. Note that the values are validated to prevent invalid -or -malicious values from being propagated into the NGINX config: +or malicious values from being propagated into the NGINX config. The Cluster operator also decided to use a Policy +for this use case. ```go // RateLimitingPolicy is a Direct Attached Policy to configure rate-limiting of client requests. @@ -893,20 +914,10 @@ server { ``` > For brevity, the proposal doesn't include more examples. However, SnippetsTemplate can also implement all examples -> used for SnippetsPolicy. - -### Inheritance and Conflicts +> used for SnippetsPolicy. Additionally, the values CRD can also be a Filter rather than a Policy (see a short +> example below). -The CRD that provides values must be a [Direct Attached Policy](https://gateway-api.sigs.k8s.io/geps/gep-2648/). -Specifically, it must -only allow target the same Kinds. - -NGF will perform conflict resolution if the Custom resources of the CRD target the same resources. - -If requested by users, this proposal can be extended to allow the CRD to follow -[Inhereted Policy Attachment](https://gateway-api.sigs.k8s.io/geps/gep-2649/). - -### Policy or Filter +### Why Both Policy and Filter Similarly to SnippetsPolicy, considering that SnippetsTemplate can be used to implement authentication and authorization, because of the reasons mentioned @@ -950,10 +961,18 @@ spec: ``` Similarly, such filters can also be used in -GRPCRoute. [TLSRoute](https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1alpha2.TLSRoute) -doesn't support filters, so it needs to be added to the Gateway API first. +GRPCRoute. TLSRoute, TCPRoute and UDPRoute +don't support filters, so it needs to be added to the Gateway API first. -TO-DO(pleshakov) - Add support for filters to the API section in the GEP after discussions about it. +### Inheritance and Conflicts + +If the values CRD is a Policy, it must be a [Direct Attached Policy](https://gateway-api.sigs.k8s.io/geps/gep-2648/). +Specifically, it must only allow target the same Kinds. + +NGF will perform conflict resolution if the Custom resources of the CRD target the same resources. + +If requested by users, this proposal can be extended to allow the CRD to also allow +[Inhereted Policy Attachment](https://gateway-api.sigs.k8s.io/geps/gep-2649/). ### Personas @@ -1022,13 +1041,13 @@ SnippetsTemplate allows Application developers to safely and easily use NGINX co The Cluster operator retains full control of that NGINX configuration, but at the same time allows Application developers to provide custom values. Kubernetes API will validate such values, which leads to better security and reliability compared to -SnippetsPolicy. +SnippetsPolicy/SnippetsFilter. ## NGINX Features Supported by Proposed Extensions > Note: the list is from [NGINX Extensions](nginx-extensions.md#prioritized-nginx-features). -| Features | Supported by SnippetsPolicy/SnippetsTemplate | Requires NGINX Plus | +| Features | Supported by SnippetsPolicy/SnippetsFilter/SnippetsTemplate | Requires NGINX Plus | |--------------------------------------------------------------------------------------------------|------------------------------------------------------------------------|---------------------| | Log level and format | X (limited, only extra logs on top of the default one) | | | Passive health checks | | | From 036afcaef1d994c3db7469a0ffce5203483720f5 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Fri, 12 Jul 2024 10:56:27 -0400 Subject: [PATCH 15/26] Clarify which version of CRD NGF will watch for --- docs/proposals/advanced-nginx-extensions.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index e3e123b65c..c958465386 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -613,6 +613,7 @@ type SnippetsTemplateSpec struct { // ValuesCRD describes the CRD which provides values for the templates. type ValuesCRD struct { // Name is the name of the CRD. + // NGF will watch for resources of the CRD group and kind using the version for which 'storage: true'. Name string `json:"name"` // Type is the type of the CRD. From c37194677e1ad981b11e1e78e8ae6de25b70c482 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Fri, 12 Jul 2024 11:00:35 -0400 Subject: [PATCH 16/26] Will NGF be unmarshaling the CRD referenced by valuesCRD into this type? --- docs/proposals/advanced-nginx-extensions.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index c958465386..2c16280a21 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -736,6 +736,9 @@ it can pass the data from the [`Location`](https://github.com/nginxinc/nginx-gateway-fabric/blob/7bc0b6e6c5131920ac18f41359dd1eba7f53a8ba/internal/mode/static/nginx/config/http/config.go#L16) struct, which NGF uses to generate location config. +NGF will unmarshall the values CRD spec into the `Spec` field of `TemplateData`. This way the template can access +the spec of the Custom resource of the CRD. + ### How to Use SnippetsTemplate 1. A Cluster operator comes up with NGINX configuration that configures an NGINX feature needed by an Application From 739003f288ae09a1dda1f4bab52d43a6171c172b Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Fri, 12 Jul 2024 13:37:34 -0400 Subject: [PATCH 17/26] Update Alternatives Section for SnippetsTemplate --- docs/proposals/advanced-nginx-extensions.md | 76 +++++++++++++++++++-- 1 file changed, 70 insertions(+), 6 deletions(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index 2c16280a21..b463fb13ca 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -854,12 +854,14 @@ the CRD with the templates: apiVersion: gateway.nginx.org/v1alpha1 kind: SnippetsTemplate metadata: - name: expose + name: rate-limiting-template spec: - valuesCRD: ratelimitingpolicy - allowedTargets: - - group: gateway.networking.k8s.io - kind: HTTPRoute + valuesCRD: + name: ratelimitingpolicy + type: DirectAttachedPolicy + allowedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute templates: - context: http value: | @@ -1037,7 +1039,69 @@ documentation. ### Alternatives -- SnippetsPolicy +Instead of asking the Cluster operator to design a CRD, we can provide a ready container CRD. For example: + +```yaml +apiVersion: gateway.nginx.org/v1alpha1 +kind: SnippetsTemplate +metadata: + name: rate-limiting-template +spec: + kind: ClientSettingsPolicy + type: DirectAttachedPolicy + allowedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + templates: + - context: http + value: | + {{ $rate := index $.Data "rate" }} + {{ $zoneName := $.Metadata.UID }} + limit_req_zone $binary_remote_addr zone={{ $zoneName }}:10m rate={{ $rate }}r/s; + - context: http.server.location + value: | + {{ $zoneName := $.Metadata.UID }} + {{ $burst := index $.Data "burst }} + limit_req zone={{ $zoneName }}{{ if $burst }}burst={{ $burst }}{{ end }}; +``` + +```yaml +apiVersion: gateway.nginx.org/v1alpha1 +kind: Values +metadata: + name: rate-limit +spec: + values: # values field is a container + apiVersion: example.com/v1alpha1 + kind: ClientSettingsPolicy # must match kind in SnippetsTemplate.spec.kind + spec: + rate: 10 + burst: 5 + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: cafe-route +``` + +Embedding values is possible by embedding a resource. See https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#rawextension + +> Note: The cluster operator doesn't need to register the ClientSettingsPolicy CRD. However, when embedding a resource +> into a Custom resource field, it is always necessary to provide the apiVersion and kind. + +As a result, NGF will execute the templates from the SnippetsTemplate registered for the ClientSettingsPolicy, +providing the values from Values.spec.values.spec to the templates. + +Pros: +- The Cluster operator doesn't need to design and create a CRD. +- The Cluster operator doesn't need to change NGF RBAC rules to allow for it to watch for the CRD resource type. + +Cons: +- Lack of CRD validation. Kubernetes API will not be able to perform validation of the values. +- More complex validation. Because we still want to support validation, we will need to design a mechanism to allow + the Cluster operator to define a scheme with the structure and validation rules for the values. Because we will not + be relying on the already available validation mechanism (CRD validation), this will result into extra complexity. + +We're not going to pursue this approach because of its cons. ### Summary From 85e55ca7d8b5758214d6b45e932ab9054c58f944 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Fri, 12 Jul 2024 14:00:34 -0400 Subject: [PATCH 18/26] Small fixes and formatting --- docs/proposals/advanced-nginx-extensions.md | 103 ++++++++++---------- 1 file changed, 54 insertions(+), 49 deletions(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index b463fb13ca..86d21dc516 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -35,11 +35,12 @@ This proposal brings two extension mechanisms: because of its implications for reliability and security, Snippets are mostly applicable for the Cluster operator. - [SnippetsTemplate](#snippetstemplate) which allows to bring unsupported NGINX configuration to NGF by Application developers in a safe and uncomplicated manner. However, SnippetsTemplates require some up-front work from a Cluster - operator, meaning it cannot be implemented quickly, in contrast with Snippets. + operator, meaning they cannot be implemented quickly, in contrast with Snippets. ### Snippets -Snippets allow inserting NGINX configuration into various NGINX contexts. They come in two flavours: +Snippets allow inserting NGINX configuration into various NGINX contexts. They come in two flavors: + - SnippetsPolicy - SnippetsFilter @@ -102,8 +103,6 @@ const ( NginxContextHTTPServer NginxContext = "http.server" // NginxContextHTTPServerLocation is the location context of the NGINX configuration. - // TO-DO(pleshakov): Decide if we need to explicitly support the parent location for rules with multiple matches. - // Same problem as in https://github.com/nginxinc/nginx-gateway-fabric/issues/2079 NginxContextHTTPServerLocation NginxContext = "http.server.location" // NginxContextStream is the stream context of the NGINX configuration. @@ -199,9 +198,8 @@ SnippetsPolicy supports the following NGINX configuration contexts: https://github.com/nginxinc/nginx-gateway-fabric/blob/5968bc348213a8470f6aaaa1a9bd51f2e90523ac/internal/mode/static/nginx/config/servers.go#L39-L40 and https://github.com/nginxinc/nginx-gateway-fabric/blob/5968bc348213a8470f6aaaa1a9bd51f2e90523ac/internal/mode/static/nginx/config/maps_template.go#L21-L26) so those keepalive directives won't work. - - Session persistence ([`sticky`](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#sticky)). Those are - valid use cases. But at the same time, they only apply to NGINX Plus. Additionally, - Gateway API started introducing session persistence. + - Session persistence ([`sticky`](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#sticky)). Those are valid use cases. But at the same time, they only apply to + NGINX Plus. Additionally, Gateway API started introducing session persistence. See https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1alpha2.BackendLBPolicy - stream module - there are not any features in https://nginx.org/en/docs/stream/ngx_stream_upstream_module.html @@ -211,13 +209,13 @@ configure a different method. If we choose to introduce upstream snippets, the SnippetsPolicy will need to support targeting a Service, because NGF generates one upstream per Service. -> We don't support routes that correspond the NGINX stream module -- TLSRoute, TCPRoute and UDPRoute. However, +> We don't support routes that correspond to the NGINX stream module -- TLSRoute, TCPRoute, and UDPRoute. However, > because we're going to support them in the future, this proposal for SnippetsPolicy includes `stream` and `server` > contexts of the stream module. -SnippetsFilter supports `http`, `server` and `location` contexts of the stream module. +SnippetsFilter supports `http`, `server`, and `location` contexts of the stream module. -> TLSRoute, TCPRoute and UPDRoute don't support filters. As a result, SnippetsFilter doesn't support stream-related +> TLSRoute, TCPRoute, and UPDRoute don't support filters. As a result, SnippetsFilter doesn't support stream-related > contexts. #### Examples @@ -408,7 +406,9 @@ same Kind, SnippetsPolicy is a [Direct Attached Policy](https://gateway-api.sigs ### Why Both Policy and Filter We introduce both SnippetsPolicy and SnippetsFilter because: -- The usage and error handling of SnippetsFilter is more explicit, as explained [here](nginx-extensions.md#authentication). + +- The usage and error handling of SnippetsFilter is more explicit, as + explained [here](nginx-extensions.md#authentication). - SnippetsFilter creates a natural split of responsibilities between the Cluster operator and the Application developer: the Cluster operator creates a SnippetsFilter; the Application developer references the SnippetsFilter to enable it. Note that with the SnippetsPolicy, because the targetRef is part of the SnippetsPolicy, it is not possible to have @@ -478,8 +478,8 @@ spec: } ``` -As a consequence, creating SnippetsPolicy/SnippetsFilter should only be allowed for the privileged users -- the Cluster operator. -As a further precaution, we will disable SnippetsPolicy/SnippetsFilter by default similarly +As a consequence, creating SnippetsPolicy/SnippetsFilter should only be allowed for the privileged users -- the Cluster +operator. As a further precaution, we will disable SnippetsPolicy/SnippetsFilter by default similarly to [NGINX Ingress Controller](https://docs.nginx.com/nginx-ingress-controller/configuration/security/#snippets). It is also possible to add validation of snippets to disallow certain directives (like `root` and `autoindex`) @@ -531,10 +531,11 @@ spec: ``` This way the Cluster operator is still responsible for creating the snippet, and the Application developer can -apply the snippet to the required target. This way, the Application developer cannot create an unsafe snippet, but -has the full control over the target. +apply the snippet to the required target. This way, the Application developer cannot create an unsafe snippet but +has full control over the target. -However, the Application developer can still target a Gateway resource, even though it is managed by the Cluster operator: +However, the Application developer can still target a Gateway resource, even though it is managed by the Cluster +operator: ```yaml apiVersion: gateway.nginx.org/v1alpha1 @@ -551,17 +552,18 @@ spec: ``` As a result, that SnippetPolicy will affect all HTTPRoutes, not only HTTPRoutes of their application. At the same time, -inherited policies like ClientSettingsPolicy has the same issue. +inherited policies like ClientSettingsPolicy have the same issue. We're not pursuing this approach for two reasons: + - Splitting snippets is already possible with SnippetsFilter. - Splitting snippets complicates SnippetsPolicy, because now the Cluster operator needs to manage two resources. - ### Summary - Snippets allow the Cluster operator to quickly configure NGINX features not available via NGF. -- SnippetsPolicy and should be used only by the Cluster operator, because of reliability and security implications. +- SnippetsPolicy and SnippetsFilter should be used only by the Cluster operator, because of reliability and security + implications. - SnippetsFilter, it is possible to split the responsibility of creating snippets (the Cluster operator) and enabling snippets (the Application Developer). @@ -736,15 +738,16 @@ it can pass the data from the [`Location`](https://github.com/nginxinc/nginx-gateway-fabric/blob/7bc0b6e6c5131920ac18f41359dd1eba7f53a8ba/internal/mode/static/nginx/config/http/config.go#L16) struct, which NGF uses to generate location config. -NGF will unmarshall the values CRD spec into the `Spec` field of `TemplateData`. This way the template can access +NGF will unmarshal the values CRD spec into the `Spec` field of `TemplateData`. This way the template can access the spec of the Custom resource of the CRD. ### How to Use SnippetsTemplate 1. A Cluster operator comes up with NGINX configuration that configures an NGINX feature needed by an Application developer. -2. The Cluster operator thinks about what values the Application developer might want to customize in that configuration. -3. The Cluster operator creates a CRD that defines fields and validation for those values, and choose whether the CRD +2. The Cluster operator thinks about what values the Application developer might want to customize in that + configuration. +3. The Cluster operator creates a CRD that defines fields and validation for those values, and chooses whether the CRD should be a Policy or a Filter. Then, the operator creates the CRD in the cluster. 4. The Cluster operator creates a go template that generates the NGINX configuration based on the fields of the CRD. 5. The Cluster operator allows the Application developer to use the feature by creating a SnippetsTemplate, which binds @@ -758,7 +761,8 @@ use that extension with the ability to further customize it. #### Rate Limiting -First, a Cluster operator comes up with NGINX configuration for the rate-limiting feature asked by an Application developer: +First, a Cluster operator comes up with NGINX configuration for the rate-limiting feature asked by an Application +developer: ```text # http context @@ -772,12 +776,13 @@ server { } ``` -Next, the Cluster operator decides that the Application developer might want to customize the rate-limiting `rate` and `burst` +Next, the Cluster operator decides that the Application developer might want to customize the rate-limiting `rate` +and `burst` values. -Next, the Cluster operator defines the following CRD for those values. Note that the values are validated to prevent invalid -or malicious values from being propagated into the NGINX config. The Cluster operator also decided to use a Policy -for this use case. +Next, the Cluster operator defines the following CRD for those values. Note that the values are validated to prevent +invalid or malicious values from being propagated into the NGINX config. The Cluster operator also decided to use +a Policy for this use case. ```go // RateLimitingPolicy is a Direct Attached Policy to configure rate-limiting of client requests. @@ -847,8 +852,8 @@ Next, the Cluster operator creates templates that generate the NGINX configurati limit_req zone={{ $zoneName }}{{ if $burst }}burst={{ $burst }}{{ end }}; ``` -Next, the Cluster operator allows the Application developer to use the feature by creating a SnippetsTemplate, which binds -the CRD with the templates: +Next, the Cluster operator allows the Application developer to use the feature by creating a SnippetsTemplate, which +binds the CRD with the templates: ```yaml apiVersion: gateway.nginx.org/v1alpha1 @@ -877,7 +882,8 @@ spec: As a result, the Cluster operator created an extension, and now it is ready to be used by the Application developer. -Next, the Application developer creates a Custom resource of the CRD to enable that NGINX feature with the desired values: +Next, the Application developer creates a Custom resource of the CRD to enable that NGINX feature with the desired +values: ```yaml apiVersion: example.org/v1alpha1 @@ -966,9 +972,8 @@ spec: port: 80 ``` -Similarly, such filters can also be used in -GRPCRoute. TLSRoute, TCPRoute and UDPRoute -don't support filters, so it needs to be added to the Gateway API first. +Similarly, such filters can also be used in GRPCRoute. However, TLSRoute, TCPRoute, and UDPRoute don't support filters, +so it needs to be added to the Gateway API first. ### Inheritance and Conflicts @@ -999,9 +1004,8 @@ A template defined in SnippetsTemplate can be invalid (not follow go template or such templates and also not crash when executing them. A template can generate invalid NGINX configuration. When this happens, NGINX will continue to use the last valid -configuration. -However, any subsequent configuration updates (for example, caused by changes to an HTTPRoute) will not be possible -until the invalid configuration is removed. Thus, a Cluster operator must carefully design the template. +configuration. However, any subsequent configuration updates (for example, caused by changes to an HTTPRoute) will not +be possible until the invalid configuration is removed. Thus, a Cluster operator must carefully design the template. NGF will also validate the rest of SnippetsTemplate fields based on the restrictions mentioned in the [API section](#api-1). @@ -1012,12 +1016,11 @@ one `http` template with multiple `http` templates scattered around in the spec. ### Security Considerations As mentioned in the previous section, the Cluster operator should define validation rules to prevent invalid/malicious -values from -Custom resources created by Application developers from propagating into the NGINX config. +values from Custom resources created by Application developers from propagating into the NGINX config. -However, it is possible that a template generates malicious values. Because of that, only the Cluster operator (a privileged -user) should be able to create SnippetsTemplates. As a further precaution, we will disable SnippetsTemplate by default -similarly to [SnippetsPolicy](#security-considerations). +However, it is possible that a template generates malicious values. Because of that, only the Cluster operator (a +privileged user) should be able to create SnippetsTemplates. As a further precaution, we will disable SnippetsTemplate +by default similarly to [SnippetsPolicy](#security-considerations). ### Upgrades @@ -1026,7 +1029,7 @@ that it already generates. As a result, the configuration generated from the tem the surrounding configuration, so that it doesn't break that configuration and vice versa. As we develop NGF, that surrounding configuration will expand (because we will implement more NGINX features) and also -might change (for example, due to improvements). As a result, there is a risk that SnippetsTemplate break after +might change (for example, due to improvements). As a result, there is a risk that a SnippetsTemplate breaks after a Cluster operator upgrades NGF to the next version. Such risk shall be clearly documented in the SnippetsTemplate documentation. @@ -1083,7 +1086,8 @@ spec: name: cafe-route ``` -Embedding values is possible by embedding a resource. See https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#rawextension +Embedding values is possible by embedding a resource. +See https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#rawextension > Note: The cluster operator doesn't need to register the ClientSettingsPolicy CRD. However, when embedding a resource > into a Custom resource field, it is always necessary to provide the apiVersion and kind. @@ -1092,24 +1096,25 @@ As a result, NGF will execute the templates from the SnippetsTemplate registered providing the values from Values.spec.values.spec to the templates. Pros: + - The Cluster operator doesn't need to design and create a CRD. - The Cluster operator doesn't need to change NGF RBAC rules to allow for it to watch for the CRD resource type. Cons: + - Lack of CRD validation. Kubernetes API will not be able to perform validation of the values. - More complex validation. Because we still want to support validation, we will need to design a mechanism to allow the Cluster operator to define a scheme with the structure and validation rules for the values. Because we will not - be relying on the already available validation mechanism (CRD validation), this will result into extra complexity. + be relying on the already available validation mechanism (CRD validation), this will result in extra complexity. We're not going to pursue this approach because of its cons. ### Summary -SnippetsTemplate allows Application developers to safely and easily use NGINX configuration created by a Cluster operator. -The Cluster operator retains full control of that NGINX configuration, but at the same time allows Application -developers to provide custom -values. Kubernetes API will validate such values, which leads to better security and reliability compared to -SnippetsPolicy/SnippetsFilter. +SnippetsTemplate allows Application developers to safely and easily use NGINX configuration created by a Cluster +operator. The Cluster operator retains full control of that NGINX configuration, but at the same time allows Application +developers to provide custom values. Kubernetes API will validate such values, which leads to better security and +reliability compared to SnippetsPolicy/SnippetsFilter. ## NGINX Features Supported by Proposed Extensions From ebdaac2992e425c4613edba5d1bac6cfd9a4572f Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Fri, 12 Jul 2024 14:17:13 -0400 Subject: [PATCH 19/26] Add a note about external location problem --- docs/proposals/advanced-nginx-extensions.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index 86d21dc516..bdd27fd0b1 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -218,6 +218,11 @@ SnippetsFilter supports `http`, `server`, and `location` contexts of the stream > TLSRoute, TCPRoute, and UPDRoute don't support filters. As a result, SnippetsFilter doesn't support stream-related > contexts. +> Snippets for location might share the same problem as mentioned in the issue https://github.com/nginxinc/nginx-gateway-fabric/issues/207, +> depending on the NGINX directives being used in the snippets. This proposal doesn't address the problem but +> anticipates the solution to https://github.com/nginxinc/nginx-gateway-fabric/issues/2079 will also solve the problem +> for Snippets. + #### Examples Below are a few examples of using snippets to bring unsupported NGINX configuration into NGF. From f9402af709b32e92ed8c5b84142498b64715e9b2 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Fri, 12 Jul 2024 14:24:33 -0400 Subject: [PATCH 20/26] Lint fixes --- docs/proposals/advanced-nginx-extensions.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index bdd27fd0b1..768b42e90b 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -217,7 +217,7 @@ SnippetsFilter supports `http`, `server`, and `location` contexts of the stream > TLSRoute, TCPRoute, and UPDRoute don't support filters. As a result, SnippetsFilter doesn't support stream-related > contexts. - +> > Snippets for location might share the same problem as mentioned in the issue https://github.com/nginxinc/nginx-gateway-fabric/issues/207, > depending on the NGINX directives being used in the snippets. This proposal doesn't address the problem but > anticipates the solution to https://github.com/nginxinc/nginx-gateway-fabric/issues/2079 will also solve the problem @@ -427,7 +427,7 @@ Snippets are not intended for Application developers, because: - To create snippets, you need to know about NGINX and how NGF generates NGINX configuration. - Snippets can easily break NGINX config (if not validated by NGF). See - [Config Validation section](#nginx-config-validation) below. + [Config Validation section](#nginx-values) below. - Snippets can be used to exploit NGF. (See [Security Considerations section](#security-considerations) below) As mentioned in the [Why Both Policy and Filter](#why-both-policy-and-filter), when snippets are used From b36d879d74d64b92d604edf21444c6500c127923 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Fri, 12 Jul 2024 18:14:49 -0400 Subject: [PATCH 21/26] Include group in CRD name --- docs/proposals/advanced-nginx-extensions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index 768b42e90b..8dd9a4dd57 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -867,7 +867,7 @@ metadata: name: rate-limiting-template spec: valuesCRD: - name: ratelimitingpolicy + name: ratelimitingpolicy.example.org type: DirectAttachedPolicy allowedKinds: - group: gateway.networking.k8s.io From 62157c526ccf87cf37e63b11f5869f4765f561f8 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Fri, 12 Jul 2024 18:15:26 -0400 Subject: [PATCH 22/26] Fix SnippetsTemplateConditionTypeAccepted doc string --- docs/proposals/advanced-nginx-extensions.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index 8dd9a4dd57..359db2b19e 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -685,7 +685,6 @@ const ( // * Invalid // * CRDInvalid // * CRDNotFound - // * TargetNotFound SnippetsTemplateConditionTypeAccepted SnippetsTemplateConditionType = "Accepted" // SnippetsTemplateConditionReasonAccepted is used with the Accepted condition type when From 523962fc4cd1ee89d66d8d061baed298bde669b4 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Fri, 12 Jul 2024 18:15:54 -0400 Subject: [PATCH 23/26] Fix values CRD --- docs/proposals/advanced-nginx-extensions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index 359db2b19e..d91128ebab 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -610,7 +610,7 @@ type SnippetsTemplateList struct { // SnippetsTemplateSpec defines the desired state of the SnippetsTemplate. type SnippetsTemplateSpec struct { // ValuesCRD describes the CRD which provides values for the templates. - ValuesCRD string `json:"valuesCRD"` + ValuesCRD ValuesCRD `json:"valuesCRD"` // Templates is a list of NGINX configuration templates to insert into the generated NGINX config. // There can only be one template per context. From dc4f1064c4039c79ffeb261f2861e04ced9bfb4f Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Fri, 12 Jul 2024 18:19:48 -0400 Subject: [PATCH 24/26] stream->http --- docs/proposals/advanced-nginx-extensions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index d91128ebab..dd3eeab90f 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -213,7 +213,7 @@ generates one upstream per Service. > because we're going to support them in the future, this proposal for SnippetsPolicy includes `stream` and `server` > contexts of the stream module. -SnippetsFilter supports `http`, `server`, and `location` contexts of the stream module. +SnippetsFilter supports `http`, `server`, and `location` contexts of the http module. > TLSRoute, TCPRoute, and UPDRoute don't support filters. As a result, SnippetsFilter doesn't support stream-related > contexts. From dd2486e156c61a3ce659a9fca8606824117a4439 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Fri, 12 Jul 2024 18:24:43 -0400 Subject: [PATCH 25/26] ClientSettingsPolicy->RateLimitingPolicy --- docs/proposals/advanced-nginx-extensions.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index dd3eeab90f..727b0cd0aa 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -794,10 +794,10 @@ type RateLimitingPolicy struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - // Spec defines the desired state of the ClientSettingsPolicy. + // Spec defines the desired state of the RateLimitingPolicy. Spec RateLimitingPolicySpec `json:"spec"` - // Status defines the state of the ClientSettingsPolicy. + // Status defines the state of the RateLimitingPolicy. Status gatewayv1alpha2.PolicyStatus `json:"status,omitempty"` } @@ -807,7 +807,7 @@ type RateLimitingPolicy struct { type RateLimitingPolicyList struct { metav1.TypeMeta `json:",inline"` metav1.ListMeta `json:"metadata,omitempty"` - Items []ClientSettingsPolicy `json:"items"` + Items []RateLimitingPolicy `json:"items"` } // RateLimitingPolicySpec defines the desired state of RateLimitingPolicy. @@ -1054,7 +1054,7 @@ kind: SnippetsTemplate metadata: name: rate-limiting-template spec: - kind: ClientSettingsPolicy + kind: RateLimitingPolicy type: DirectAttachedPolicy allowedKinds: - group: gateway.networking.k8s.io @@ -1080,7 +1080,7 @@ metadata: spec: values: # values field is a container apiVersion: example.com/v1alpha1 - kind: ClientSettingsPolicy # must match kind in SnippetsTemplate.spec.kind + kind: RateLimitingPolicy # must match kind in SnippetsTemplate.spec.kind spec: rate: 10 burst: 5 @@ -1093,10 +1093,10 @@ spec: Embedding values is possible by embedding a resource. See https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#rawextension -> Note: The cluster operator doesn't need to register the ClientSettingsPolicy CRD. However, when embedding a resource +> Note: The cluster operator doesn't need to register the RateLimitingPolicy CRD. However, when embedding a resource > into a Custom resource field, it is always necessary to provide the apiVersion and kind. -As a result, NGF will execute the templates from the SnippetsTemplate registered for the ClientSettingsPolicy, +As a result, NGF will execute the templates from the SnippetsTemplate registered for the RateLimitingPolicy, providing the values from Values.spec.values.spec to the templates. Pros: From fe597eb75af68a1c89704629f235d9aa429721a5 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Mon, 15 Jul 2024 22:50:17 -0700 Subject: [PATCH 26/26] Change status to implementable --- docs/proposals/advanced-nginx-extensions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/proposals/advanced-nginx-extensions.md b/docs/proposals/advanced-nginx-extensions.md index 727b0cd0aa..5d684664ab 100644 --- a/docs/proposals/advanced-nginx-extensions.md +++ b/docs/proposals/advanced-nginx-extensions.md @@ -1,7 +1,7 @@ # Enhancement Proposal-2035: Advanced NGINX Extensions - Issue: https://github.com/nginxinc/nginx-gateway-fabric/issues/2035 -- Status: Provisional +- Status: Implementable ## Summary