diff --git a/apis/v1alpha1/policy_methods.go b/apis/v1alpha1/policy_methods.go index 02350ef82b..61031d68b7 100644 --- a/apis/v1alpha1/policy_methods.go +++ b/apis/v1alpha1/policy_methods.go @@ -31,3 +31,15 @@ func (p *UpstreamSettingsPolicy) GetPolicyStatus() gatewayv1.PolicyStatus { func (p *UpstreamSettingsPolicy) SetPolicyStatus(status gatewayv1.PolicyStatus) { p.Status = status } + +func (p *SnippetsPolicy) GetTargetRefs() []gatewayv1.LocalPolicyTargetReference { + return p.Spec.TargetRefs +} + +func (p *SnippetsPolicy) GetPolicyStatus() gatewayv1.PolicyStatus { + return p.Status +} + +func (p *SnippetsPolicy) SetPolicyStatus(status gatewayv1.PolicyStatus) { + p.Status = status +} diff --git a/apis/v1alpha1/register.go b/apis/v1alpha1/register.go index d01bfcd6e1..7bf78d370c 100644 --- a/apis/v1alpha1/register.go +++ b/apis/v1alpha1/register.go @@ -40,6 +40,8 @@ func addKnownTypes(scheme *runtime.Scheme) error { &SnippetsFilterList{}, &UpstreamSettingsPolicy{}, &UpstreamSettingsPolicyList{}, + &SnippetsPolicy{}, + &SnippetsPolicyList{}, ) // AddToGroupVersion allows the serialization of client types like ListOptions. metav1.AddToGroupVersion(scheme, SchemeGroupVersion) diff --git a/apis/v1alpha1/snippetspolicy_types.go b/apis/v1alpha1/snippetspolicy_types.go new file mode 100644 index 0000000000..bd146dd7c8 --- /dev/null +++ b/apis/v1alpha1/snippetspolicy_types.go @@ -0,0 +1,69 @@ +/* +Copyright 2025 The NGINX Gateway Fabric Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +// +genclient +// +kubebuilder:object:root=true +// +kubebuilder:storageversion +// +kubebuilder:subresource:status +// +kubebuilder:metadata:labels="gateway.networking.k8s.io/policy=direct" +// +kubebuilder:resource:categories=nginx-gateway-fabric,shortName=snippetspolicy +// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` + +// SnippetsPolicy provides a way to inject NGINX snippets into the configuration on Gateway level. +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 current state of the SnippetsPolicy. + Status gatewayv1.PolicyStatus `json:"status,omitempty"` +} + +// +kubebuilder:object:root=true + +// 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 { + // TargetRefs identifies API object(s) to apply the policy to. + // +kubebuilder:validation:MinItems=1 + // +kubebuilder:validation:MaxItems=16 + // +kubebuilder:validation:XValidation:message="TargetRefs Kind must be Gateway",rule="self.all(t, t.kind == 'Gateway')" + // +kubebuilder:validation:XValidation:message="TargetRefs Group must be gateway.networking.k8s.io",rule="self.all(t, t.group == 'gateway.networking.k8s.io')" + //nolint:lll + TargetRefs []gatewayv1.LocalPolicyTargetReference `json:"targetRefs"` + + // Snippets is a list of snippets to be injected into the NGINX configuration. + // +kubebuilder:validation:MaxItems=3 + // +kubebuilder:validation:XValidation:message="Only one snippet allowed per context",rule="self.all(s1, self.exists_one(s2, s1.context == s2.context))" + // +kubebuilder:validation:XValidation:message="http.server.location context is not supported in SnippetsPolicy",rule="!self.exists(s, s.context == 'http.server.location')" + //nolint:lll + Snippets []Snippet `json:"snippets"` +} diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index d07825de2d..e6dfb32c43 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -434,6 +434,90 @@ func (in *SnippetsFilterStatus) DeepCopy() *SnippetsFilterStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SnippetsPolicy) DeepCopyInto(out *SnippetsPolicy) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SnippetsPolicy. +func (in *SnippetsPolicy) DeepCopy() *SnippetsPolicy { + if in == nil { + return nil + } + out := new(SnippetsPolicy) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *SnippetsPolicy) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SnippetsPolicyList) DeepCopyInto(out *SnippetsPolicyList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]SnippetsPolicy, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SnippetsPolicyList. +func (in *SnippetsPolicyList) DeepCopy() *SnippetsPolicyList { + if in == nil { + return nil + } + out := new(SnippetsPolicyList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *SnippetsPolicyList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SnippetsPolicySpec) DeepCopyInto(out *SnippetsPolicySpec) { + *out = *in + if in.TargetRefs != nil { + in, out := &in.TargetRefs, &out.TargetRefs + *out = make([]apisv1.LocalPolicyTargetReference, len(*in)) + copy(*out, *in) + } + if in.Snippets != nil { + in, out := &in.Snippets, &out.Snippets + *out = make([]Snippet, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SnippetsPolicySpec. +func (in *SnippetsPolicySpec) DeepCopy() *SnippetsPolicySpec { + if in == nil { + return nil + } + out := new(SnippetsPolicySpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SpanAttribute) DeepCopyInto(out *SpanAttribute) { *out = *in diff --git a/charts/nginx-gateway-fabric/README.md b/charts/nginx-gateway-fabric/README.md index b7cb221b7f..dcd582df2c 100644 --- a/charts/nginx-gateway-fabric/README.md +++ b/charts/nginx-gateway-fabric/README.md @@ -246,7 +246,7 @@ The following table lists the configurable parameters of the NGINX Gateway Fabri | `nginx.usage.resolver` | The nameserver used to resolve the NGINX Plus usage reporting endpoint. Used with NGINX Instance Manager. | string | `""` | | `nginx.usage.secretName` | The name of the Secret containing the JWT for NGINX Plus usage reporting. Must exist in the same namespace that the NGINX Gateway Fabric control plane is running in (default namespace: nginx-gateway). | string | `"nplus-license"` | | `nginx.usage.skipVerify` | Disable client verification of the NGINX Plus usage reporting server certificate. | bool | `false` | -| `nginxGateway` | The nginxGateway section contains configuration for the NGINX Gateway Fabric control plane deployment. | object | `{"affinity":{},"autoscaling":{"enable":false},"config":{"logging":{"level":"info"}},"configAnnotations":{},"extraVolumeMounts":[],"extraVolumes":[],"gatewayClassAnnotations":{},"gatewayClassName":"nginx","gatewayControllerName":"gateway.nginx.org/nginx-gateway-controller","gwAPIExperimentalFeatures":{"enable":false},"gwAPIInferenceExtension":{"enable":false,"endpointPicker":{"disableTLS":false,"skipVerify":true}},"image":{"pullPolicy":"Always","repository":"ghcr.io/nginx/nginx-gateway-fabric","tag":"edge"},"kind":"deployment","labels":{},"leaderElection":{"enable":true,"lockName":""},"lifecycle":{},"metrics":{"enable":true,"port":9113,"secure":false},"name":"","nodeSelector":{},"podAnnotations":{},"productTelemetry":{"enable":true},"readinessProbe":{"enable":true,"initialDelaySeconds":3,"port":8081},"replicas":1,"resources":{},"service":{"annotations":{},"labels":{}},"serviceAccount":{"annotations":{},"imagePullSecret":"","imagePullSecrets":[],"name":""},"snippetsFilters":{"enable":false},"terminationGracePeriodSeconds":30,"tolerations":[],"topologySpreadConstraints":[]}` | +| `nginxGateway` | The nginxGateway section contains configuration for the NGINX Gateway Fabric control plane deployment. | object | `{"affinity":{},"autoscaling":{"enable":false},"config":{"logging":{"level":"info"}},"configAnnotations":{},"extraVolumeMounts":[],"extraVolumes":[],"gatewayClassAnnotations":{},"gatewayClassName":"nginx","gatewayControllerName":"gateway.nginx.org/nginx-gateway-controller","gwAPIExperimentalFeatures":{"enable":false},"gwAPIInferenceExtension":{"enable":false,"endpointPicker":{"disableTLS":false,"skipVerify":true}},"image":{"pullPolicy":"Always","repository":"ghcr.io/nginx/nginx-gateway-fabric","tag":"edge"},"kind":"deployment","labels":{},"leaderElection":{"enable":true,"lockName":""},"lifecycle":{},"metrics":{"enable":true,"port":9113,"secure":false},"name":"","nodeSelector":{},"podAnnotations":{},"productTelemetry":{"enable":true},"readinessProbe":{"enable":true,"initialDelaySeconds":3,"port":8081},"replicas":1,"resources":{},"service":{"annotations":{},"labels":{}},"serviceAccount":{"annotations":{},"imagePullSecret":"","imagePullSecrets":[],"name":""},"snippetsFilters":{"enable":false},"snippetsPolicies":{"enable":false},"terminationGracePeriodSeconds":30,"tolerations":[],"topologySpreadConstraints":[]}` | | `nginxGateway.affinity` | The affinity of the NGINX Gateway Fabric control plane pod. | object | `{}` | | `nginxGateway.autoscaling` | Autoscaling configuration for the NGINX Gateway Fabric control plane. | object | `{"enable":false}` | | `nginxGateway.autoscaling.enable` | Enable or disable Horizontal Pod Autoscaler for the control plane. | bool | `false` | @@ -290,6 +290,7 @@ The following table lists the configurable parameters of the NGINX Gateway Fabri | `nginxGateway.serviceAccount.imagePullSecrets` | A list of secret names containing docker registry credentials for the control plane. Secrets must exist in the same namespace as the helm release. | list | `[]` | | `nginxGateway.serviceAccount.name` | The name of the service account of the NGINX Gateway Fabric control plane pods. Used for RBAC. | string | Autogenerated if not set or set to "" | | `nginxGateway.snippetsFilters.enable` | Enable SnippetsFilters feature. SnippetsFilters allow inserting NGINX configuration into the generated NGINX config for HTTPRoute and GRPCRoute resources. | bool | `false` | +| `nginxGateway.snippetsPolicies.enable` | Enable SnippetsPolicies feature. SnippetsPolicies allow inserting NGINX configuration into the generated NGINX config for Gateway resources. | bool | `false` | | `nginxGateway.terminationGracePeriodSeconds` | The termination grace period of the NGINX Gateway Fabric control plane pod. | int | `30` | | `nginxGateway.tolerations` | Tolerations for the NGINX Gateway Fabric control plane pod. | list | `[]` | | `nginxGateway.topologySpreadConstraints` | The topology spread constraints for the NGINX Gateway Fabric control plane pod. | list | `[]` | diff --git a/charts/nginx-gateway-fabric/templates/clusterrole.yaml b/charts/nginx-gateway-fabric/templates/clusterrole.yaml index 5d9ea4a5be..813c912ac3 100644 --- a/charts/nginx-gateway-fabric/templates/clusterrole.yaml +++ b/charts/nginx-gateway-fabric/templates/clusterrole.yaml @@ -132,6 +132,9 @@ rules: {{- if .Values.nginxGateway.snippetsFilters.enable }} - snippetsfilters {{- end }} + {{- if .Values.nginxGateway.snippetsPolicies.enable }} + - snippetspolicies + {{- end }} verbs: - list - watch @@ -145,6 +148,9 @@ rules: {{- if .Values.nginxGateway.snippetsFilters.enable }} - snippetsfilters/status {{- end }} + {{- if .Values.nginxGateway.snippetsPolicies.enable }} + - snippetspolicies/status + {{- end }} verbs: - update {{- if .Values.nginxGateway.gwAPIInferenceExtension.enable }} diff --git a/charts/nginx-gateway-fabric/templates/deployment.yaml b/charts/nginx-gateway-fabric/templates/deployment.yaml index 6c79253fe7..0c4383b744 100644 --- a/charts/nginx-gateway-fabric/templates/deployment.yaml +++ b/charts/nginx-gateway-fabric/templates/deployment.yaml @@ -110,6 +110,9 @@ spec: {{- if .Values.nginxGateway.snippetsFilters.enable }} - --snippets-filters {{- end }} + {{- if .Values.nginxGateway.snippetsPolicies.enable }} + - --snippets-policies + {{- end }} {{- if .Capabilities.APIVersions.Has "security.openshift.io/v1/SecurityContextConstraints" }} - --nginx-scc={{ include "nginx-gateway.scc-name" . }}-nginx {{- end}} diff --git a/charts/nginx-gateway-fabric/values.schema.json b/charts/nginx-gateway-fabric/values.schema.json index 85967e4913..7c69e99cd4 100644 --- a/charts/nginx-gateway-fabric/values.schema.json +++ b/charts/nginx-gateway-fabric/values.schema.json @@ -1148,6 +1148,20 @@ "title": "snippetsFilters", "type": "object" }, + "snippetsPolicies": { + "properties": { + "enable": { + "default": false, + "description": "Enable SnippetsPolicies feature. SnippetsPolicies allow inserting NGINX configuration into the generated NGINX\nconfig for Gateway resources.", + "required": [], + "title": "enable", + "type": "boolean" + } + }, + "required": [], + "title": "snippetsPolicies", + "type": "object" + }, "terminationGracePeriodSeconds": { "default": 30, "description": "The termination grace period of the NGINX Gateway Fabric control plane pod.", diff --git a/charts/nginx-gateway-fabric/values.yaml b/charts/nginx-gateway-fabric/values.yaml index 9e8e7de21d..c21860c716 100644 --- a/charts/nginx-gateway-fabric/values.yaml +++ b/charts/nginx-gateway-fabric/values.yaml @@ -232,6 +232,11 @@ nginxGateway: # config for HTTPRoute and GRPCRoute resources. enable: false + snippetsPolicies: + # -- Enable SnippetsPolicies feature. SnippetsPolicies allow inserting NGINX configuration into the generated NGINX + # config for Gateway resources. + enable: false + # -- The nginx section contains the configuration for all NGINX data plane deployments # installed by the NGINX Gateway Fabric control plane. nginx: diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index be458e0dc6..797f64ad9b 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -95,6 +95,7 @@ func createControllerCommand() *cobra.Command { usageReportCASecretFlag = "usage-report-ca-secret" //nolint:gosec // not credentials usageReportEnforceInitialReportFlag = "usage-report-enforce-initial-report" snippetsFiltersFlag = "snippets-filters" + snippetsPoliciesFlag = "snippets-policies" nginxSCCFlag = "nginx-scc" ) @@ -156,7 +157,8 @@ func createControllerCommand() *cobra.Command { disableProductTelemetry bool - snippetsFilters bool + snippetsFilters bool + snippetsPolicies bool plus bool nginxDockerSecrets = stringSliceValidatingValue{ @@ -282,6 +284,7 @@ func createControllerCommand() *cobra.Command { Values: flagValues, }, SnippetsFilters: snippetsFilters, + SnippetsPolicies: snippetsPolicies, NginxDockerSecretNames: nginxDockerSecrets.values, AgentTLSSecretName: agentTLSSecretName.value, NGINXSCCName: nginxSCCName.value, @@ -512,6 +515,14 @@ func createControllerCommand() *cobra.Command { "generated NGINX config for HTTPRoute and GRPCRoute resources.", ) + cmd.Flags().BoolVar( + &snippetsPolicies, + snippetsPoliciesFlag, + false, + "Enable SnippetsPolicies feature. SnippetsPolicies allow inserting NGINX configuration into the "+ + "generated NGINX config for Gateway resources.", + ) + cmd.Flags().Var( &nginxSCCName, nginxSCCFlag, diff --git a/cmd/gateway/commands_test.go b/cmd/gateway/commands_test.go index d31916a745..c893d50545 100644 --- a/cmd/gateway/commands_test.go +++ b/cmd/gateway/commands_test.go @@ -156,6 +156,7 @@ func TestControllerCmdFlagValidation(t *testing.T) { "--usage-report-client-ssl-secret=client-secret", "--usage-report-enforce-initial-report", "--snippets-filters", + "--snippets-policies", "--nginx-scc=nginx-sscc-name", "--nginx-one-dataplane-key-secret=dataplane-key-secret", "--nginx-one-telemetry-endpoint-host=telemetry-endpoint-host", @@ -417,6 +418,15 @@ func TestControllerCmdFlagValidation(t *testing.T) { }, wantErr: true, }, + { + name: "snippets-policies is not a bool", + expectedErrPrefix: `invalid argument "not-a-bool" for "--snippets-policies" flag: strconv.ParseBool:` + + ` parsing "not-a-bool": invalid syntax`, + args: []string{ + "--snippets-policies=not-a-bool", + }, + wantErr: true, + }, { name: "nginx-scc is set to empty string", args: []string{ diff --git a/config/crd/bases/gateway.nginx.org_snippetspolicies.yaml b/config/crd/bases/gateway.nginx.org_snippetspolicies.yaml new file mode 100644 index 0000000000..13203eafa4 --- /dev/null +++ b/config/crd/bases/gateway.nginx.org_snippetspolicies.yaml @@ -0,0 +1,432 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.19.0 + labels: + gateway.networking.k8s.io/policy: direct + name: snippetspolicies.gateway.nginx.org +spec: + group: gateway.nginx.org + names: + categories: + - nginx-gateway-fabric + kind: SnippetsPolicy + listKind: SnippetsPolicyList + plural: snippetspolicies + shortNames: + - snippetspolicy + singular: snippetspolicy + scope: Namespaced + versions: + - additionalPrinterColumns: + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + schema: + openAPIV3Schema: + description: SnippetsPolicy provides a way to inject NGINX snippets into the + configuration on Gateway level. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: Spec defines the desired state of the SnippetsPolicy. + properties: + snippets: + description: Snippets is a list of snippets to be injected into the + NGINX configuration. + items: + description: Snippet represents an NGINX configuration snippet. + properties: + context: + description: Context is the NGINX context to insert the snippet + into. + enum: + - main + - http + - http.server + - http.server.location + type: string + value: + description: Value is the NGINX configuration snippet. + minLength: 1 + type: string + required: + - context + - value + type: object + maxItems: 3 + type: array + x-kubernetes-validations: + - message: Only one snippet allowed per context + rule: self.all(s1, self.exists_one(s2, s1.context == s2.context)) + - message: http.server.location context is not supported in SnippetsPolicy + rule: '!self.exists(s, s.context == ''http.server.location'')' + targetRefs: + description: TargetRefs identifies API object(s) to apply the policy + to. + items: + description: |- + LocalPolicyTargetReference identifies an API object to apply a direct or + inherited policy to. This should be used as part of Policy resources + that can target Gateway API resources. For more information on how this + policy attachment model works, and a sample Policy resource, refer to + the policy attachment documentation for Gateway API. + properties: + group: + description: Group is the group of the target resource. + maxLength: 253 + pattern: ^$|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + kind: + description: Kind is kind of the target resource. + maxLength: 63 + minLength: 1 + pattern: ^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$ + type: string + name: + description: Name is the name of the target resource. + maxLength: 253 + minLength: 1 + type: string + required: + - group + - kind + - name + type: object + maxItems: 16 + minItems: 1 + type: array + x-kubernetes-validations: + - message: TargetRefs Kind must be Gateway + rule: self.all(t, t.kind == 'Gateway') + - message: TargetRefs Group must be gateway.networking.k8s.io + rule: self.all(t, t.group == 'gateway.networking.k8s.io') + required: + - snippets + - targetRefs + type: object + status: + description: Status defines the current state of the SnippetsPolicy. + properties: + ancestors: + description: |- + Ancestors is a list of ancestor resources (usually Gateways) that are + associated with the policy, and the status of the policy with respect to + each ancestor. When this policy attaches to a parent, the controller that + manages the parent and the ancestors MUST add an entry to this list when + the controller first sees the policy and SHOULD update the entry as + appropriate when the relevant ancestor is modified. + + Note that choosing the relevant ancestor is left to the Policy designers; + an important part of Policy design is designing the right object level at + which to namespace this status. + + Note also that implementations MUST ONLY populate ancestor status for + the Ancestor resources they are responsible for. Implementations MUST + use the ControllerName field to uniquely identify the entries in this list + that they are responsible for. + + Note that to achieve this, the list of PolicyAncestorStatus structs + MUST be treated as a map with a composite key, made up of the AncestorRef + and ControllerName fields combined. + + A maximum of 16 ancestors will be represented in this list. An empty list + means the Policy is not relevant for any ancestors. + + If this slice is full, implementations MUST NOT add further entries. + Instead they MUST consider the policy unimplementable and signal that + on any related resources such as the ancestor that would be referenced + here. For example, if this list was full on BackendTLSPolicy, no + additional Gateways would be able to reference the Service targeted by + the BackendTLSPolicy. + items: + description: |- + PolicyAncestorStatus describes the status of a route with respect to an + associated Ancestor. + + Ancestors refer to objects that are either the Target of a policy or above it + in terms of object hierarchy. For example, if a policy targets a Service, the + Policy's Ancestors are, in order, the Service, the HTTPRoute, the Gateway, and + the GatewayClass. Almost always, in this hierarchy, the Gateway will be the most + useful object to place Policy status on, so we recommend that implementations + SHOULD use Gateway as the PolicyAncestorStatus object unless the designers + have a _very_ good reason otherwise. + + In the context of policy attachment, the Ancestor is used to distinguish which + resource results in a distinct application of this policy. For example, if a policy + targets a Service, it may have a distinct result per attached Gateway. + + Policies targeting the same resource may have different effects depending on the + ancestors of those resources. For example, different Gateways targeting the same + Service may have different capabilities, especially if they have different underlying + implementations. + + For example, in BackendTLSPolicy, the Policy attaches to a Service that is + used as a backend in a HTTPRoute that is itself attached to a Gateway. + In this case, the relevant object for status is the Gateway, and that is the + ancestor object referred to in this status. + + Note that a parent is also an ancestor, so for objects where the parent is the + relevant object for status, this struct SHOULD still be used. + + This struct is intended to be used in a slice that's effectively a map, + with a composite key made up of the AncestorRef and the ControllerName. + properties: + ancestorRef: + description: |- + AncestorRef corresponds with a ParentRef in the spec that this + PolicyAncestorStatus struct describes the status of. + properties: + group: + default: gateway.networking.k8s.io + description: |- + Group is the group of the referent. + When unspecified, "gateway.networking.k8s.io" is inferred. + To set the core API group (such as for a "Service" kind referent), + Group must be explicitly set to "" (empty string). + + Support: Core + maxLength: 253 + pattern: ^$|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + kind: + default: Gateway + description: |- + Kind is kind of the referent. + + There are two kinds of parent resources with "Core" support: + + * Gateway (Gateway conformance profile) + * Service (Mesh conformance profile, ClusterIP Services only) + + Support for other resources is Implementation-Specific. + maxLength: 63 + minLength: 1 + pattern: ^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$ + type: string + name: + description: |- + Name is the name of the referent. + + Support: Core + maxLength: 253 + minLength: 1 + type: string + namespace: + description: |- + Namespace is the namespace of the referent. When unspecified, this refers + to the local namespace of the Route. + + Note that there are specific rules for ParentRefs which cross namespace + boundaries. Cross-namespace references are only valid if they are explicitly + allowed by something in the namespace they are referring to. For example: + Gateway has the AllowedRoutes field, and ReferenceGrant provides a + generic way to enable any other kind of cross-namespace reference. + + + ParentRefs from a Route to a Service in the same namespace are "producer" + routes, which apply default routing rules to inbound connections from + any namespace to the Service. + + ParentRefs from a Route to a Service in a different namespace are + "consumer" routes, and these routing rules are only applied to outbound + connections originating from the same namespace as the Route, for which + the intended destination of the connections are a Service targeted as a + ParentRef of the Route. + + + Support: Core + maxLength: 63 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ + type: string + port: + description: |- + Port is the network port this Route targets. It can be interpreted + differently based on the type of parent resource. + + When the parent resource is a Gateway, this targets all listeners + listening on the specified port that also support this kind of Route(and + select this Route). It's not recommended to set `Port` unless the + networking behaviors specified in a Route must apply to a specific port + as opposed to a listener(s) whose port(s) may be changed. When both Port + and SectionName are specified, the name and port of the selected listener + must match both specified values. + + + When the parent resource is a Service, this targets a specific port in the + Service spec. When both Port (experimental) and SectionName are specified, + the name and port of the selected port must match both specified values. + + + Implementations MAY choose to support other parent resources. + Implementations supporting other types of parent resources MUST clearly + document how/if Port is interpreted. + + For the purpose of status, an attachment is considered successful as + long as the parent resource accepts it partially. For example, Gateway + listeners can restrict which Routes can attach to them by Route kind, + namespace, or hostname. If 1 of 2 Gateway listeners accept attachment + from the referencing Route, the Route MUST be considered successfully + attached. If no Gateway listeners accept attachment from this Route, + the Route MUST be considered detached from the Gateway. + + Support: Extended + format: int32 + maximum: 65535 + minimum: 1 + type: integer + sectionName: + description: |- + SectionName is the name of a section within the target resource. In the + following resources, SectionName is interpreted as the following: + + * Gateway: Listener name. When both Port (experimental) and SectionName + are specified, the name and port of the selected listener must match + both specified values. + * Service: Port name. When both Port (experimental) and SectionName + are specified, the name and port of the selected listener must match + both specified values. + + Implementations MAY choose to support attaching Routes to other resources. + If that is the case, they MUST clearly document how SectionName is + interpreted. + + When unspecified (empty string), this will reference the entire resource. + For the purpose of status, an attachment is considered successful if at + least one section in the parent resource accepts it. For example, Gateway + listeners can restrict which Routes can attach to them by Route kind, + namespace, or hostname. If 1 of 2 Gateway listeners accept attachment from + the referencing Route, the Route MUST be considered successfully + attached. If no Gateway listeners accept attachment from this Route, the + Route MUST be considered detached from the Gateway. + + Support: Core + maxLength: 253 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + required: + - name + type: object + conditions: + description: |- + Conditions describes the status of the Policy with respect to the given Ancestor. + + items: + description: Condition contains details for one aspect of + the current state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, + Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + maxItems: 8 + minItems: 1 + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + controllerName: + description: |- + ControllerName is a domain/path string that indicates the name of the + controller that wrote this status. This corresponds with the + controllerName field on GatewayClass. + + Example: "example.net/gateway-controller". + + The format of this field is DOMAIN "/" PATH, where DOMAIN and PATH are + valid Kubernetes names + (https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names). + + Controllers MUST populate this field when writing status. Controllers should ensure that + entries to status populated with their ControllerName are cleaned up when they are no + longer necessary. + maxLength: 253 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*\/[A-Za-z0-9\/\-._~%!$&'()*+,;=:]+$ + type: string + required: + - ancestorRef + - conditions + - controllerName + type: object + maxItems: 16 + type: array + x-kubernetes-list-type: atomic + required: + - ancestors + type: object + required: + - spec + type: object + served: true + storage: true + subresources: + status: {} diff --git a/internal/controller/config/config.go b/internal/controller/config/config.go index 9394bc8c66..99deb4514b 100644 --- a/internal/controller/config/config.go +++ b/internal/controller/config/config.go @@ -52,6 +52,8 @@ type Config struct { InferenceExtension bool // SnippetsFilters indicates if SnippetsFilters are enabled. SnippetsFilters bool + // SnippetsPolicies indicates if SnippetsPolicies are enabled. + SnippetsPolicies bool // EndpointPickerDisableTLS indicates if TLS is disabled for EndpointPicker communication. EndpointPickerDisableTLS bool // EndpointPickerTLSSkipVerify indicates if secure verification is skipped for EndpointPicker communication. diff --git a/internal/controller/manager.go b/internal/controller/manager.go index d4e2114e8a..661641d7f0 100644 --- a/internal/controller/manager.go +++ b/internal/controller/manager.go @@ -49,6 +49,7 @@ import ( "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/clientsettings" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/observability" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/snippetspolicy" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/upstreamsettings" ngxvalidation "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/validation" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/provisioner" @@ -124,7 +125,7 @@ func StartManager(cfg config.Config) error { mustExtractGVK := kinds.NewMustExtractGKV(scheme) genericValidator := ngxvalidation.GenericValidator{} - policyManager := createPolicyManager(mustExtractGVK, genericValidator) + policyManager := createPolicyManager(cfg, mustExtractGVK, genericValidator) plusSecrets, err := createPlusSecretMetadata(cfg, mgr.GetAPIReader()) if err != nil { @@ -321,6 +322,7 @@ func StartManager(cfg config.Config) error { } func createPolicyManager( + cfg config.Config, mustExtractGVK kinds.MustExtractGVK, validator validation.GenericValidator, ) *policies.CompositeValidator { @@ -339,6 +341,13 @@ func createPolicyManager( }, } + if cfg.SnippetsPolicies { + cfgs = append(cfgs, policies.ManagerConfig{ + GVK: mustExtractGVK(&ngfAPIv1alpha1.SnippetsPolicy{}), + Validator: snippetspolicy.NewValidator(), + }) + } + return policies.NewManager(mustExtractGVK, cfgs...) } @@ -586,6 +595,17 @@ func registerControllers( ) } + if cfg.SnippetsPolicies { + controllerRegCfgs = append(controllerRegCfgs, + ctlrCfg{ + objectType: &ngfAPIv1alpha1.SnippetsPolicy{}, + options: []controller.Option{ + controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}), + }, + }, + ) + } + for _, regCfg := range controllerRegCfgs { name := regCfg.objectType.GetObjectKind().GroupVersionKind().Kind if regCfg.name != "" { @@ -791,6 +811,13 @@ func prepareFirstEventBatchPreparerArgs(cfg config.Config) ([]client.Object, []c ) } + if cfg.SnippetsPolicies { + objectLists = append( + objectLists, + &ngfAPIv1alpha1.SnippetsPolicyList{}, + ) + } + objectLists = append(objectLists, &gatewayv1.GatewayList{}) return objects, objectLists diff --git a/internal/controller/manager_test.go b/internal/controller/manager_test.go index 88aee02ac8..fcd32b6a50 100644 --- a/internal/controller/manager_test.go +++ b/internal/controller/manager_test.go @@ -154,6 +154,34 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { &ngfAPIv1alpha1.UpstreamSettingsPolicyList{}, }, }, + { + name: "snippets policies enabled", + cfg: config.Config{ + GatewayClassName: gcName, + SnippetsPolicies: true, + }, + expectedObjects: []client.Object{ + &gatewayv1.GatewayClass{ObjectMeta: metav1.ObjectMeta{Name: "nginx"}}, + }, + expectedObjectLists: []client.ObjectList{ + &apiv1.ServiceList{}, + &apiv1.SecretList{}, + &apiv1.NamespaceList{}, + &discoveryV1.EndpointSliceList{}, + &gatewayv1.HTTPRouteList{}, + &gatewayv1.BackendTLSPolicyList{}, + &apiv1.ConfigMapList{}, + &gatewayv1.GatewayList{}, + &gatewayv1beta1.ReferenceGrantList{}, + &ngfAPIv1alpha2.NginxProxyList{}, + partialObjectMetadataList, + &gatewayv1.GRPCRouteList{}, + &ngfAPIv1alpha1.ClientSettingsPolicyList{}, + &ngfAPIv1alpha2.ObservabilityPolicyList{}, + &ngfAPIv1alpha1.SnippetsPolicyList{}, + &ngfAPIv1alpha1.UpstreamSettingsPolicyList{}, + }, + }, { name: "experimental, inference, and snippets filters enabled", cfg: config.Config{ @@ -186,6 +214,40 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { &ngfAPIv1alpha1.UpstreamSettingsPolicyList{}, }, }, + { + name: "all features enabled", + cfg: config.Config{ + GatewayClassName: gcName, + ExperimentalFeatures: true, + InferenceExtension: true, + SnippetsFilters: true, + SnippetsPolicies: true, + }, + expectedObjects: []client.Object{ + &gatewayv1.GatewayClass{ObjectMeta: metav1.ObjectMeta{Name: "nginx"}}, + }, + expectedObjectLists: []client.ObjectList{ + &apiv1.ServiceList{}, + &apiv1.SecretList{}, + &apiv1.NamespaceList{}, + &apiv1.ConfigMapList{}, + &discoveryV1.EndpointSliceList{}, + &gatewayv1.HTTPRouteList{}, + &gatewayv1.GatewayList{}, + &gatewayv1beta1.ReferenceGrantList{}, + &ngfAPIv1alpha2.NginxProxyList{}, + partialObjectMetadataList, + &inference.InferencePoolList{}, + &gatewayv1.BackendTLSPolicyList{}, + &gatewayv1alpha2.TLSRouteList{}, + &gatewayv1.GRPCRouteList{}, + &ngfAPIv1alpha1.ClientSettingsPolicyList{}, + &ngfAPIv1alpha2.ObservabilityPolicyList{}, + &ngfAPIv1alpha1.SnippetsFilterList{}, + &ngfAPIv1alpha1.SnippetsPolicyList{}, + &ngfAPIv1alpha1.UpstreamSettingsPolicyList{}, + }, + }, } for _, test := range tests { diff --git a/internal/controller/nginx/config/base_http_config.go b/internal/controller/nginx/config/base_http_config.go index 7af0712bde..6df808a835 100644 --- a/internal/controller/nginx/config/base_http_config.go +++ b/internal/controller/nginx/config/base_http_config.go @@ -5,6 +5,7 @@ import ( "net" gotemplate "text/template" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/shared" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" @@ -28,9 +29,18 @@ type httpConfig struct { HTTP2 bool } -func executeBaseHTTPConfig(conf dataplane.Configuration) []executeResult { +func newExecuteBaseHTTPConfigFunc(generator policies.Generator) executeFunc { + return func(conf dataplane.Configuration) []executeResult { + return executeBaseHTTPConfig(conf, generator) + } +} + +func executeBaseHTTPConfig(conf dataplane.Configuration, generator policies.Generator) []executeResult { includes := createIncludesFromSnippets(conf.BaseHTTPConfig.Snippets) + policyIncludes := createIncludesFromPolicyGenerateResult(generator.GenerateForHTTP(conf.Policies)) + includes = append(includes, policyIncludes...) + hc := httpConfig{ HTTP2: conf.BaseHTTPConfig.HTTP2, Includes: includes, diff --git a/internal/controller/nginx/config/base_http_config_test.go b/internal/controller/nginx/config/base_http_config_test.go index 1d9c563dcd..5a5003f5ca 100644 --- a/internal/controller/nginx/config/base_http_config_test.go +++ b/internal/controller/nginx/config/base_http_config_test.go @@ -8,6 +8,7 @@ import ( . "github.com/onsi/gomega" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/policiesfakes" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane" ) @@ -69,7 +70,7 @@ func TestLoggingSettingsTemplate(t *testing.T) { Logging: dataplane.Logging{AccessLog: tt.accessLog}, } - res := executeBaseHTTPConfig(conf) + res := executeBaseHTTPConfig(conf, &policiesfakes.FakeGenerator{}) g.Expect(res).To(HaveLen(1)) httpConfig := string(res[0].data) for _, expectedOutput := range tt.expectedOutputs { @@ -120,7 +121,7 @@ func TestExecuteBaseHttp_HTTP2(t *testing.T) { t.Parallel() g := NewWithT(t) - res := executeBaseHTTPConfig(test.conf) + res := executeBaseHTTPConfig(test.conf, &policiesfakes.FakeGenerator{}) g.Expect(res).To(HaveLen(1)) g.Expect(test.expCount).To(Equal(strings.Count(string(res[0].data), expSubStr))) g.Expect(strings.Count(string(res[0].data), "map $http_host $gw_api_compliant_host {")).To(Equal(1)) @@ -150,7 +151,7 @@ func TestExecuteBaseHttp_Snippets(t *testing.T) { g := NewWithT(t) - res := executeBaseHTTPConfig(conf) + res := executeBaseHTTPConfig(conf, &policiesfakes.FakeGenerator{}) g.Expect(res).To(HaveLen(3)) sort.Slice( @@ -261,7 +262,7 @@ func TestExecuteBaseHttp_NginxReadinessProbePort(t *testing.T) { t.Parallel() g := NewWithT(t) - res := executeBaseHTTPConfig(test.conf) + res := executeBaseHTTPConfig(test.conf, &policiesfakes.FakeGenerator{}) g.Expect(res).To(HaveLen(1)) httpConfig := string(res[0].data) @@ -377,7 +378,7 @@ func TestExecuteBaseHttp_DNSResolver(t *testing.T) { t.Parallel() g := NewWithT(t) - res := executeBaseHTTPConfig(test.conf) + res := executeBaseHTTPConfig(test.conf, &policiesfakes.FakeGenerator{}) g.Expect(res).To(HaveLen(1)) httpConfig := string(res[0].data) @@ -435,7 +436,7 @@ func TestExecuteBaseHttp_GatewaySecretID(t *testing.T) { t.Parallel() g := NewWithT(t) - res := executeBaseHTTPConfig(test.conf) + res := executeBaseHTTPConfig(test.conf, &policiesfakes.FakeGenerator{}) g.Expect(res).To(HaveLen(1)) httpConfig := string(res[0].data) diff --git a/internal/controller/nginx/config/generator.go b/internal/controller/nginx/config/generator.go index 6fa7602c04..787e004561 100644 --- a/internal/controller/nginx/config/generator.go +++ b/internal/controller/nginx/config/generator.go @@ -15,6 +15,7 @@ import ( "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/clientsettings" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/observability" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/snippetspolicy" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/upstreamsettings" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/file" @@ -125,6 +126,7 @@ func (g GeneratorImpl) Generate(conf dataplane.Configuration) []agent.File { policyGenerator := policies.NewCompositeGenerator( clientsettings.NewGenerator(), observability.NewGenerator(conf.Telemetry), + snippetspolicy.NewGenerator(), ) files = append(files, g.executeConfigTemplates(conf, policyGenerator)...) @@ -201,9 +203,9 @@ func (g GeneratorImpl) getExecuteFuncs( keepAliveCheck keepAliveChecker, ) []executeFunc { return []executeFunc{ - executeMainConfig, + newExecuteMainConfigFunc(generator), executeEventsConfig, - executeBaseHTTPConfig, + newExecuteBaseHTTPConfigFunc(generator), g.newExecuteServersFunc(generator, keepAliveCheck), newExecuteUpstreamsFunc(upstreams), executeSplitClients, diff --git a/internal/controller/nginx/config/main_config.go b/internal/controller/nginx/config/main_config.go index bd58eabee7..80a254e684 100644 --- a/internal/controller/nginx/config/main_config.go +++ b/internal/controller/nginx/config/main_config.go @@ -7,6 +7,7 @@ import ( filesHelper "github.com/nginx/agent/v3/pkg/files" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/shared" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/graph" @@ -25,9 +26,18 @@ type mainConfig struct { Conf dataplane.Configuration } -func executeMainConfig(conf dataplane.Configuration) []executeResult { +func newExecuteMainConfigFunc(generator policies.Generator) executeFunc { + return func(conf dataplane.Configuration) []executeResult { + return executeMainConfig(conf, generator) + } +} + +func executeMainConfig(conf dataplane.Configuration, generator policies.Generator) []executeResult { includes := createIncludesFromSnippets(conf.MainSnippets) + policyIncludes := createIncludesFromPolicyGenerateResult(generator.GenerateForMain(conf.Policies)) + includes = append(includes, policyIncludes...) + mc := mainConfig{ Conf: conf, Includes: includes, diff --git a/internal/controller/nginx/config/main_config_test.go b/internal/controller/nginx/config/main_config_test.go index 132084afa7..b9833076ee 100644 --- a/internal/controller/nginx/config/main_config_test.go +++ b/internal/controller/nginx/config/main_config_test.go @@ -6,6 +6,7 @@ import ( . "github.com/onsi/gomega" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/policiesfakes" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane" ) @@ -44,7 +45,7 @@ func TestExecuteMainConfig_Telemetry(t *testing.T) { t.Parallel() g := NewWithT(t) - res := executeMainConfig(test.conf) + res := executeMainConfig(test.conf, &policiesfakes.FakeGenerator{}) g.Expect(res).To(HaveLen(1)) g.Expect(res[0].dest).To(Equal(mainIncludesConfigFile)) if test.expLoadModuleDirective { @@ -67,7 +68,7 @@ func TestExecuteMainConfig_Logging(t *testing.T) { g := NewWithT(t) - res := executeMainConfig(conf) + res := executeMainConfig(conf, &policiesfakes.FakeGenerator{}) g.Expect(res).To(HaveLen(1)) g.Expect(res[0].dest).To(Equal(mainIncludesConfigFile)) @@ -96,7 +97,7 @@ func TestExecuteMainConfig_Snippets(t *testing.T) { g := NewWithT(t) - res := executeMainConfig(conf) + res := executeMainConfig(conf, &policiesfakes.FakeGenerator{}) g.Expect(res).To(HaveLen(4)) // sort results by filename @@ -170,7 +171,7 @@ func TestExecuteMainConfig_WorkerConnections(t *testing.T) { t.Parallel() g := NewWithT(t) - res := executeMainConfig(test.conf) + res := executeMainConfig(test.conf, &policiesfakes.FakeGenerator{}) g.Expect(res).To(HaveLen(1)) g.Expect(res[0].dest).To(Equal(mainIncludesConfigFile)) g.Expect(string(res[0].data)).To(ContainSubstring("error_log stderr")) diff --git a/internal/controller/nginx/config/policies/clientsettings/generator.go b/internal/controller/nginx/config/policies/clientsettings/generator.go index b895f3104a..dcfe05c59e 100644 --- a/internal/controller/nginx/config/policies/clientsettings/generator.go +++ b/internal/controller/nginx/config/policies/clientsettings/generator.go @@ -39,7 +39,9 @@ keepalive_timeout {{ .KeepAlive.Timeout.Server }}; ` // Generator generates nginx configuration based on a clientsettings policy. -type Generator struct{} +type Generator struct { + policies.UnimplementedGenerator +} // NewGenerator returns a new instance of Generator. func NewGenerator() *Generator { diff --git a/internal/controller/nginx/config/policies/generator.go b/internal/controller/nginx/config/policies/generator.go index 5a25df37dc..864fdeb1e7 100644 --- a/internal/controller/nginx/config/policies/generator.go +++ b/internal/controller/nginx/config/policies/generator.go @@ -8,6 +8,10 @@ import ( // //counterfeiter:generate . Generator type Generator interface { + // GenerateForMain generates policy configuration for the main block. + GenerateForMain(policies []Policy) GenerateResultFiles + // GenerateForHTTP generates policy configuration for the http block. + GenerateForHTTP(policies []Policy) GenerateResultFiles // GenerateForServer generates policy configuration for the server block. GenerateForServer(policies []Policy, server http.Server) GenerateResultFiles // GenerateForLocation generates policy configuration for a normal location block. @@ -35,6 +39,28 @@ func NewCompositeGenerator(generators ...Generator) *CompositeGenerator { return &CompositeGenerator{generators: generators} } +// GenerateForMain calls all policy generators for the main block. +func (g *CompositeGenerator) GenerateForMain(policies []Policy) GenerateResultFiles { + var compositeResult GenerateResultFiles + + for _, generator := range g.generators { + compositeResult = append(compositeResult, generator.GenerateForMain(policies)...) + } + + return compositeResult +} + +// GenerateForHTTP calls all policy generators for the http block. +func (g *CompositeGenerator) GenerateForHTTP(policies []Policy) GenerateResultFiles { + var compositeResult GenerateResultFiles + + for _, generator := range g.generators { + compositeResult = append(compositeResult, generator.GenerateForHTTP(policies)...) + } + + return compositeResult +} + // GenerateForServer calls all policy generators for the server block. func (g *CompositeGenerator) GenerateForServer(policies []Policy, server http.Server) GenerateResultFiles { var compositeResult GenerateResultFiles @@ -72,6 +98,14 @@ func (g *CompositeGenerator) GenerateForInternalLocation(policies []Policy) Gene // possible generations, in order to satisfy the Generator interface. type UnimplementedGenerator struct{} +func (u UnimplementedGenerator) GenerateForMain(_ []Policy) GenerateResultFiles { + return nil +} + +func (u UnimplementedGenerator) GenerateForHTTP(_ []Policy) GenerateResultFiles { + return nil +} + func (u UnimplementedGenerator) GenerateForServer(_ []Policy, _ http.Server) GenerateResultFiles { return nil } diff --git a/internal/controller/nginx/config/policies/policiesfakes/fake_generator.go b/internal/controller/nginx/config/policies/policiesfakes/fake_generator.go index e9e1689f3d..8ecd060ccc 100644 --- a/internal/controller/nginx/config/policies/policiesfakes/fake_generator.go +++ b/internal/controller/nginx/config/policies/policiesfakes/fake_generator.go @@ -9,6 +9,17 @@ import ( ) type FakeGenerator struct { + GenerateForHTTPStub func([]policies.Policy) policies.GenerateResultFiles + generateForHTTPMutex sync.RWMutex + generateForHTTPArgsForCall []struct { + arg1 []policies.Policy + } + generateForHTTPReturns struct { + result1 policies.GenerateResultFiles + } + generateForHTTPReturnsOnCall map[int]struct { + result1 policies.GenerateResultFiles + } GenerateForInternalLocationStub func([]policies.Policy) policies.GenerateResultFiles generateForInternalLocationMutex sync.RWMutex generateForInternalLocationArgsForCall []struct { @@ -32,6 +43,17 @@ type FakeGenerator struct { generateForLocationReturnsOnCall map[int]struct { result1 policies.GenerateResultFiles } + GenerateForMainStub func([]policies.Policy) policies.GenerateResultFiles + generateForMainMutex sync.RWMutex + generateForMainArgsForCall []struct { + arg1 []policies.Policy + } + generateForMainReturns struct { + result1 policies.GenerateResultFiles + } + generateForMainReturnsOnCall map[int]struct { + result1 policies.GenerateResultFiles + } GenerateForServerStub func([]policies.Policy, http.Server) policies.GenerateResultFiles generateForServerMutex sync.RWMutex generateForServerArgsForCall []struct { @@ -48,6 +70,72 @@ type FakeGenerator struct { invocationsMutex sync.RWMutex } +func (fake *FakeGenerator) GenerateForHTTP(arg1 []policies.Policy) policies.GenerateResultFiles { + var arg1Copy []policies.Policy + if arg1 != nil { + arg1Copy = make([]policies.Policy, len(arg1)) + copy(arg1Copy, arg1) + } + fake.generateForHTTPMutex.Lock() + ret, specificReturn := fake.generateForHTTPReturnsOnCall[len(fake.generateForHTTPArgsForCall)] + fake.generateForHTTPArgsForCall = append(fake.generateForHTTPArgsForCall, struct { + arg1 []policies.Policy + }{arg1Copy}) + stub := fake.GenerateForHTTPStub + fakeReturns := fake.generateForHTTPReturns + fake.recordInvocation("GenerateForHTTP", []interface{}{arg1Copy}) + fake.generateForHTTPMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeGenerator) GenerateForHTTPCallCount() int { + fake.generateForHTTPMutex.RLock() + defer fake.generateForHTTPMutex.RUnlock() + return len(fake.generateForHTTPArgsForCall) +} + +func (fake *FakeGenerator) GenerateForHTTPCalls(stub func([]policies.Policy) policies.GenerateResultFiles) { + fake.generateForHTTPMutex.Lock() + defer fake.generateForHTTPMutex.Unlock() + fake.GenerateForHTTPStub = stub +} + +func (fake *FakeGenerator) GenerateForHTTPArgsForCall(i int) []policies.Policy { + fake.generateForHTTPMutex.RLock() + defer fake.generateForHTTPMutex.RUnlock() + argsForCall := fake.generateForHTTPArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeGenerator) GenerateForHTTPReturns(result1 policies.GenerateResultFiles) { + fake.generateForHTTPMutex.Lock() + defer fake.generateForHTTPMutex.Unlock() + fake.GenerateForHTTPStub = nil + fake.generateForHTTPReturns = struct { + result1 policies.GenerateResultFiles + }{result1} +} + +func (fake *FakeGenerator) GenerateForHTTPReturnsOnCall(i int, result1 policies.GenerateResultFiles) { + fake.generateForHTTPMutex.Lock() + defer fake.generateForHTTPMutex.Unlock() + fake.GenerateForHTTPStub = nil + if fake.generateForHTTPReturnsOnCall == nil { + fake.generateForHTTPReturnsOnCall = make(map[int]struct { + result1 policies.GenerateResultFiles + }) + } + fake.generateForHTTPReturnsOnCall[i] = struct { + result1 policies.GenerateResultFiles + }{result1} +} + func (fake *FakeGenerator) GenerateForInternalLocation(arg1 []policies.Policy) policies.GenerateResultFiles { var arg1Copy []policies.Policy if arg1 != nil { @@ -181,6 +269,72 @@ func (fake *FakeGenerator) GenerateForLocationReturnsOnCall(i int, result1 polic }{result1} } +func (fake *FakeGenerator) GenerateForMain(arg1 []policies.Policy) policies.GenerateResultFiles { + var arg1Copy []policies.Policy + if arg1 != nil { + arg1Copy = make([]policies.Policy, len(arg1)) + copy(arg1Copy, arg1) + } + fake.generateForMainMutex.Lock() + ret, specificReturn := fake.generateForMainReturnsOnCall[len(fake.generateForMainArgsForCall)] + fake.generateForMainArgsForCall = append(fake.generateForMainArgsForCall, struct { + arg1 []policies.Policy + }{arg1Copy}) + stub := fake.GenerateForMainStub + fakeReturns := fake.generateForMainReturns + fake.recordInvocation("GenerateForMain", []interface{}{arg1Copy}) + fake.generateForMainMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeGenerator) GenerateForMainCallCount() int { + fake.generateForMainMutex.RLock() + defer fake.generateForMainMutex.RUnlock() + return len(fake.generateForMainArgsForCall) +} + +func (fake *FakeGenerator) GenerateForMainCalls(stub func([]policies.Policy) policies.GenerateResultFiles) { + fake.generateForMainMutex.Lock() + defer fake.generateForMainMutex.Unlock() + fake.GenerateForMainStub = stub +} + +func (fake *FakeGenerator) GenerateForMainArgsForCall(i int) []policies.Policy { + fake.generateForMainMutex.RLock() + defer fake.generateForMainMutex.RUnlock() + argsForCall := fake.generateForMainArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeGenerator) GenerateForMainReturns(result1 policies.GenerateResultFiles) { + fake.generateForMainMutex.Lock() + defer fake.generateForMainMutex.Unlock() + fake.GenerateForMainStub = nil + fake.generateForMainReturns = struct { + result1 policies.GenerateResultFiles + }{result1} +} + +func (fake *FakeGenerator) GenerateForMainReturnsOnCall(i int, result1 policies.GenerateResultFiles) { + fake.generateForMainMutex.Lock() + defer fake.generateForMainMutex.Unlock() + fake.GenerateForMainStub = nil + if fake.generateForMainReturnsOnCall == nil { + fake.generateForMainReturnsOnCall = make(map[int]struct { + result1 policies.GenerateResultFiles + }) + } + fake.generateForMainReturnsOnCall[i] = struct { + result1 policies.GenerateResultFiles + }{result1} +} + func (fake *FakeGenerator) GenerateForServer(arg1 []policies.Policy, arg2 http.Server) policies.GenerateResultFiles { var arg1Copy []policies.Policy if arg1 != nil { diff --git a/internal/controller/nginx/config/policies/snippetspolicy/generator.go b/internal/controller/nginx/config/policies/snippetspolicy/generator.go new file mode 100644 index 0000000000..970af8035a --- /dev/null +++ b/internal/controller/nginx/config/policies/snippetspolicy/generator.go @@ -0,0 +1,121 @@ +/* +Copyright 2025 The NGINX Gateway Fabric Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package snippetspolicy + +import ( + "fmt" + "path/filepath" + + "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/http" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" +) + +const ( + mainTemplate = ` +# SnippetsPolicy %s for Gateway %s main context +%s +` + httpTemplate = ` +# SnippetsPolicy %s for Gateway %s http context +%s +` + serverTemplate = ` +# SnippetsPolicy %s for Gateway %s server context in listener %s +%s +` +) + +// Generator generates NGINX configuration snippets for SnippetsPolicies. +type Generator struct { + policies.UnimplementedGenerator +} + +// NewGenerator returns a new instance of Generator. +func NewGenerator() *Generator { + return &Generator{} +} + +// GenerateForMain generates policy configuration for the main block. +func (g *Generator) GenerateForMain(pols []policies.Policy) policies.GenerateResultFiles { + return g.generate(pols, v1alpha1.NginxContextMain, "") +} + +// GenerateForHTTP generates policy configuration for the http block. +func (g *Generator) GenerateForHTTP(pols []policies.Policy) policies.GenerateResultFiles { + return g.generate(pols, v1alpha1.NginxContextHTTP, "") +} + +// GenerateForServer generates policy configuration for the server block. +func (g *Generator) GenerateForServer(pols []policies.Policy, server http.Server) policies.GenerateResultFiles { + return g.generate(pols, v1alpha1.NginxContextHTTPServer, server.Listen) +} + +func (g *Generator) generate(pols []policies.Policy, context v1alpha1.NginxContext, serverID string) policies.GenerateResultFiles { + var files policies.GenerateResultFiles + + for _, policy := range pols { + sp, ok := policy.(*v1alpha1.SnippetsPolicy) + if !ok { + continue + } + + for _, snippet := range sp.Spec.Snippets { + if snippet.Context != context { + continue + } + + for _, gwRef := range sp.Spec.TargetRefs { + // TargetRef info for file path and comment + gwNsName := fmt.Sprintf("%s-%s", sp.GetNamespace(), gwRef.Name) // Assuming local target ref implies same namespace + + policyNsName := fmt.Sprintf("%s/%s", sp.GetNamespace(), sp.GetName()) + + // Build content and filename + var content string + var filename string + + switch context { + case v1alpha1.NginxContextMain: + content = fmt.Sprintf(mainTemplate, policyNsName, gwNsName, snippet.Value) + filename = filepath.Join( + "includes", "policy", gwNsName, + fmt.Sprintf("SnippetsPolicy_main_%s.conf", sp.GetName()), + ) + case v1alpha1.NginxContextHTTP: + content = fmt.Sprintf(httpTemplate, policyNsName, gwNsName, snippet.Value) + filename = filepath.Join( + "includes", "policy", gwNsName, + fmt.Sprintf("SnippetsPolicy_http_%s.conf", sp.GetName()), + ) + case v1alpha1.NginxContextHTTPServer: + content = fmt.Sprintf(serverTemplate, policyNsName, gwNsName, serverID, snippet.Value) + filename = filepath.Join( + "includes", "policy", gwNsName, serverID, + fmt.Sprintf("SnippetsPolicy_server_%s.conf", sp.GetName()), + ) + } + + files = append(files, policies.File{ + Name: filename, + Content: []byte(content), + }) + } + } + } + return files +} diff --git a/internal/controller/nginx/config/policies/snippetspolicy/generator_test.go b/internal/controller/nginx/config/policies/snippetspolicy/generator_test.go new file mode 100644 index 0000000000..3ab1e49be2 --- /dev/null +++ b/internal/controller/nginx/config/policies/snippetspolicy/generator_test.go @@ -0,0 +1,142 @@ +package snippetspolicy_test + +import ( + "testing" + + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + + "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/http" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/snippetspolicy" + "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/kinds" +) + +func TestGenerator(t *testing.T) { + g := &snippetspolicy.Generator{} + + policy := &v1alpha1.SnippetsPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "policy-1", + Namespace: "default", + }, + Spec: v1alpha1.SnippetsPolicySpec{ + TargetRefs: []gatewayv1.LocalPolicyTargetReference{ + { + Group: gatewayv1.GroupName, + Kind: kinds.Gateway, + Name: "gateway-1", + }, + }, + Snippets: []v1alpha1.Snippet{ + { + Context: v1alpha1.NginxContextMain, + Value: "worker_processes 1;", + }, + { + Context: v1alpha1.NginxContextHTTP, + Value: "log_format custom '...';", + }, + { + Context: v1alpha1.NginxContextHTTPServer, + Value: "client_max_body_size 10m;", + }, + }, + }, + } + + pols := []policies.Policy{policy} + + t.Run("GenerateForMain", func(t *testing.T) { + gWithT := NewWithT(t) + files := g.GenerateForMain(pols) + gWithT.Expect(files).To(HaveLen(1)) + gWithT.Expect(files[0].Name).To(Equal("includes/policy/default-gateway-1/SnippetsPolicy_main_policy-1.conf")) + gWithT.Expect(string(files[0].Content)).To(ContainSubstring("worker_processes 1;")) + }) + + t.Run("GenerateForHTTP", func(t *testing.T) { + gWithT := NewWithT(t) + files := g.GenerateForHTTP(pols) + gWithT.Expect(files).To(HaveLen(1)) + gWithT.Expect(files[0].Name).To(Equal("includes/policy/default-gateway-1/SnippetsPolicy_http_policy-1.conf")) + gWithT.Expect(string(files[0].Content)).To(ContainSubstring("log_format custom '...';")) + }) + + t.Run("GenerateForServer", func(t *testing.T) { + gWithT := NewWithT(t) + server := http.Server{ + Listen: "80", + } + files := g.GenerateForServer(pols, server) + gWithT.Expect(files).To(HaveLen(1)) + gWithT.Expect(files[0].Name).To(Equal("includes/policy/default-gateway-1/80/SnippetsPolicy_server_policy-1.conf")) + gWithT.Expect(string(files[0].Content)).To(ContainSubstring("client_max_body_size 10m;")) + }) + + t.Run("GenerateForMain with multiple policies", func(t *testing.T) { + gWithT := NewWithT(t) + policy1 := &v1alpha1.SnippetsPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "p1", Namespace: "default"}, + Spec: v1alpha1.SnippetsPolicySpec{ + TargetRefs: []gatewayv1.LocalPolicyTargetReference{ + { + Name: "gw", + }, + }, + Snippets: []v1alpha1.Snippet{ + {Context: v1alpha1.NginxContextMain, Value: "p1;"}, + }, + }, + } + policy2 := &v1alpha1.SnippetsPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "p2", Namespace: "default"}, + Spec: v1alpha1.SnippetsPolicySpec{ + TargetRefs: []gatewayv1.LocalPolicyTargetReference{ + { + Name: "gw", + }, + }, + Snippets: []v1alpha1.Snippet{ + {Context: v1alpha1.NginxContextMain, Value: "p2;"}, + }, + }, + } + + files := g.GenerateForMain([]policies.Policy{policy1, policy2}) + gWithT.Expect(files).To(HaveLen(2)) + gWithT.Expect(files[0].Name).To(ContainSubstring("p1")) + gWithT.Expect(files[1].Name).To(ContainSubstring("p2")) + }) + + t.Run("GenerateForMain with multiple targets", func(t *testing.T) { + gWithT := NewWithT(t) + policy := &v1alpha1.SnippetsPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "policy-multi", Namespace: "default"}, + Spec: v1alpha1.SnippetsPolicySpec{ + TargetRefs: []gatewayv1.LocalPolicyTargetReference{ + { + Name: "gw1", + }, + { + Name: "gw2", + }, + }, + Snippets: []v1alpha1.Snippet{ + {Context: v1alpha1.NginxContextMain, Value: "data;"}, + }, + }, + } + + files := g.GenerateForMain([]policies.Policy{policy}) + gWithT.Expect(files).To(HaveLen(2)) + gWithT.Expect(files[0].Name).To(ContainSubstring("gw1")) + gWithT.Expect(files[1].Name).To(ContainSubstring("gw2")) + gWithT.Expect(string(files[0].Content)).To(ContainSubstring("data;")) + gWithT.Expect(string(files[1].Content)).To(ContainSubstring("data;")) + gWithT.Expect(string(files[0].Content)).To(ContainSubstring("gw1")) + gWithT.Expect(string(files[1].Content)).To(ContainSubstring("gw2")) + }) +} diff --git a/internal/controller/nginx/config/policies/snippetspolicy/validator.go b/internal/controller/nginx/config/policies/snippetspolicy/validator.go new file mode 100644 index 0000000000..244a52f5fa --- /dev/null +++ b/internal/controller/nginx/config/policies/snippetspolicy/validator.go @@ -0,0 +1,93 @@ +/* +Copyright 2025 The NGINX Gateway Fabric Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package snippetspolicy + +import ( + "fmt" + + "k8s.io/apimachinery/pkg/util/validation/field" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + + ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/conditions" + "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" + "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/kinds" +) + +// Validator validates a SnippetsPolicy. +// Implements policies.Validator interface. +type Validator struct{} + +// NewValidator returns a new instance of Validator. +func NewValidator() *Validator { + return &Validator{} +} + +// Validate validates the spec of a SnippetsPolicy. +func (v *Validator) Validate(policy policies.Policy) []conditions.Condition { + sp := helpers.MustCastObject[*ngfAPI.SnippetsPolicy](policy) + + targetRefsPath := field.NewPath("spec").Child("targetRefs") + supportedKinds := []gatewayv1.Kind{kinds.Gateway} + supportedGroups := []gatewayv1.Group{gatewayv1.GroupName} + + // Validate TargetRef + for i, targetRef := range sp.Spec.TargetRefs { + if err := policies.ValidateTargetRef(targetRef, targetRefsPath.Index(i), supportedGroups, supportedKinds); err != nil { + return []conditions.Condition{conditions.NewPolicyInvalid(err.Error())} + } + } + + // Validate Snippets + if err := validateSnippets(sp.Spec.Snippets); err != nil { + return []conditions.Condition{conditions.NewPolicyInvalid(err.Error())} + } + + return nil +} + +// ValidateGlobalSettings validates a SnippetsPolicy with respect to the NginxProxy global settings. +func (v *Validator) ValidateGlobalSettings( + _ policies.Policy, + _ *policies.GlobalSettings, +) []conditions.Condition { + return nil +} + +// Conflicts returns true if the two SnippetsPolicies conflict. +// SnippetsPolicies are merged by lexicographic order, so they don't inherently conflict +// in a way that prevents them from being applied together (structurally). +// Detailed logical conflicts (e.g. conflicting NGINX directives) are caught by nginx -t. +func (v *Validator) Conflicts(polA, polB policies.Policy) bool { + return false +} + +func validateSnippets(snippets []ngfAPI.Snippet) error { + seenContexts := make(map[ngfAPI.NginxContext]struct{}) + for _, snippet := range snippets { + if _, exists := seenContexts[snippet.Context]; exists { + return fmt.Errorf("duplicate context %q", snippet.Context) + } + seenContexts[snippet.Context] = struct{}{} + + if snippet.Context == ngfAPI.NginxContextHTTPServerLocation { + return fmt.Errorf("context %q is not supported in SnippetsPolicy", snippet.Context) + } + } + return nil +} diff --git a/internal/controller/nginx/config/policies/snippetspolicy/validator_test.go b/internal/controller/nginx/config/policies/snippetspolicy/validator_test.go new file mode 100644 index 0000000000..b519dd3350 --- /dev/null +++ b/internal/controller/nginx/config/policies/snippetspolicy/validator_test.go @@ -0,0 +1,154 @@ +package snippetspolicy_test + +import ( + "testing" + + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + + ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies/snippetspolicy" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/conditions" + "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/kinds" +) + +type policyModFunc func(policy *ngfAPI.SnippetsPolicy) *ngfAPI.SnippetsPolicy + +func createValidPolicy() *ngfAPI.SnippetsPolicy { + return &ngfAPI.SnippetsPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "test-policy", + }, + Spec: ngfAPI.SnippetsPolicySpec{ + TargetRefs: []gatewayv1.LocalPolicyTargetReference{ + { + Group: gatewayv1.GroupName, + Kind: kinds.Gateway, + Name: "test-gateway", + }, + }, + Snippets: []ngfAPI.Snippet{ + { + Context: ngfAPI.NginxContextMain, + Value: "worker_processes 1;", + }, + { + Context: ngfAPI.NginxContextHTTP, + Value: "log_format custom '...';", + }, + }, + }, + Status: gatewayv1.PolicyStatus{}, + } +} + +func createModifiedPolicy(mod policyModFunc) *ngfAPI.SnippetsPolicy { + return mod(createValidPolicy()) +} + +func TestValidator_Validate(t *testing.T) { + t.Parallel() + tests := []struct { + name string + policy *ngfAPI.SnippetsPolicy + expConditions []conditions.Condition + }{ + { + name: "valid policy", + policy: createValidPolicy(), + expConditions: nil, + }, + { + name: "multiple valid target refs", + policy: createModifiedPolicy(func(p *ngfAPI.SnippetsPolicy) *ngfAPI.SnippetsPolicy { + p.Spec.TargetRefs = append(p.Spec.TargetRefs, gatewayv1.LocalPolicyTargetReference{ + Group: gatewayv1.GroupName, + Kind: kinds.Gateway, + Name: "another-gateway", + }) + return p + }), + expConditions: nil, + }, + { + name: "invalid target ref - unsupported group", + policy: createModifiedPolicy(func(p *ngfAPI.SnippetsPolicy) *ngfAPI.SnippetsPolicy { + p.Spec.TargetRefs[0].Group = "unsupported.group" + return p + }), + expConditions: []conditions.Condition{ + conditions.NewPolicyInvalid("spec.targetRefs[0].group: Unsupported value: \"unsupported.group\": supported values: \"gateway.networking.k8s.io\""), + }, + }, + { + name: "invalid target ref - unsupported kind", + policy: createModifiedPolicy(func(p *ngfAPI.SnippetsPolicy) *ngfAPI.SnippetsPolicy { + p.Spec.TargetRefs[0].Kind = "UnsupportedKind" + return p + }), + expConditions: []conditions.Condition{ + conditions.NewPolicyInvalid("spec.targetRefs[0].kind: Unsupported value: \"UnsupportedKind\": supported values: \"Gateway\""), + }, + }, + { + name: "duplicate context", + policy: createModifiedPolicy(func(p *ngfAPI.SnippetsPolicy) *ngfAPI.SnippetsPolicy { + p.Spec.Snippets = append(p.Spec.Snippets, ngfAPI.Snippet{ + Context: ngfAPI.NginxContextMain, + Value: "another snippet;", + }) + return p + }), + expConditions: []conditions.Condition{ + conditions.NewPolicyInvalid("duplicate context \"main\""), + }, + }, + { + name: "unsupported context", + policy: createModifiedPolicy(func(p *ngfAPI.SnippetsPolicy) *ngfAPI.SnippetsPolicy { + p.Spec.Snippets = []ngfAPI.Snippet{ + { + Context: ngfAPI.NginxContextHTTPServerLocation, + Value: "location snippet", + }, + } + return p + }), + expConditions: []conditions.Condition{ + conditions.NewPolicyInvalid("context \"http.server.location\" is not supported in SnippetsPolicy"), + }, + }, + } + + v := snippetspolicy.NewValidator() + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + conds := v.Validate(test.policy) + g.Expect(conds).To(Equal(test.expConditions)) + }) + } +} + +func TestValidator_ValidateGlobalSettings(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + v := snippetspolicy.NewValidator() + + g.Expect(v.ValidateGlobalSettings(nil, nil)).To(BeNil()) +} + +func TestValidator_Conflicts(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + v := snippetspolicy.NewValidator() + + g.Expect(v.Conflicts(nil, nil)).To(BeFalse()) +} diff --git a/internal/controller/state/change_processor.go b/internal/controller/state/change_processor.go index d661903b8c..5804d9b86f 100644 --- a/internal/controller/state/change_processor.go +++ b/internal/controller/state/change_processor.go @@ -66,6 +66,8 @@ type ChangeProcessorConfig struct { GatewayClassName string // ExperimentalFeatures indicates if experimental features are enabled. ExperimentalFeatures bool + // SnippetsPolicies indicates if SnippetsPolicies are enabled. + SnippetsPolicies bool } // ChangeProcessorImpl is an implementation of ChangeProcessor. @@ -127,105 +129,115 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl { // Use this object store for all NGF policies commonPolicyObjectStore := newNGFPolicyObjectStore(clusterStore.NGFPolicies, cfg.MustExtractGVK) + trackingUpdaterCfg := []changeTrackingUpdaterObjectTypeCfg{ + { + gvk: cfg.MustExtractGVK(&v1.GatewayClass{}), + store: newObjectStoreMapAdapter(clusterStore.GatewayClasses), + predicate: nil, + }, + { + gvk: cfg.MustExtractGVK(&v1.Gateway{}), + store: newObjectStoreMapAdapter(clusterStore.Gateways), + predicate: nil, + }, + { + gvk: cfg.MustExtractGVK(&v1.HTTPRoute{}), + store: newObjectStoreMapAdapter(clusterStore.HTTPRoutes), + predicate: nil, + }, + { + gvk: cfg.MustExtractGVK(&v1beta1.ReferenceGrant{}), + store: newObjectStoreMapAdapter(clusterStore.ReferenceGrants), + predicate: nil, + }, + { + gvk: cfg.MustExtractGVK(&v1.BackendTLSPolicy{}), + store: newObjectStoreMapAdapter(clusterStore.BackendTLSPolicies), + predicate: nil, + }, + { + gvk: cfg.MustExtractGVK(&v1.GRPCRoute{}), + store: newObjectStoreMapAdapter(clusterStore.GRPCRoutes), + predicate: nil, + }, + { + gvk: cfg.MustExtractGVK(&apiv1.Namespace{}), + store: newObjectStoreMapAdapter(clusterStore.Namespaces), + predicate: funcPredicate{stateChanged: isReferenced}, + }, + { + gvk: cfg.MustExtractGVK(&apiv1.Service{}), + store: newObjectStoreMapAdapter(clusterStore.Services), + predicate: funcPredicate{stateChanged: isReferenced}, + }, + { + gvk: cfg.MustExtractGVK(&inference.InferencePool{}), + store: newObjectStoreMapAdapter(clusterStore.InferencePools), + predicate: funcPredicate{stateChanged: isReferenced}, + }, + { + gvk: cfg.MustExtractGVK(&discoveryV1.EndpointSlice{}), + store: nil, + predicate: funcPredicate{stateChanged: isReferenced}, + }, + { + gvk: cfg.MustExtractGVK(&apiv1.Secret{}), + store: newObjectStoreMapAdapter(clusterStore.Secrets), + predicate: funcPredicate{stateChanged: isReferenced}, + }, + { + gvk: cfg.MustExtractGVK(&apiv1.ConfigMap{}), + store: newObjectStoreMapAdapter(clusterStore.ConfigMaps), + predicate: funcPredicate{stateChanged: isReferenced}, + }, + { + gvk: cfg.MustExtractGVK(&apiext.CustomResourceDefinition{}), + store: newObjectStoreMapAdapter(clusterStore.CRDMetadata), + predicate: annotationChangedPredicate{annotation: consts.BundleVersionAnnotation}, + }, + { + gvk: cfg.MustExtractGVK(&ngfAPIv1alpha2.NginxProxy{}), + store: newObjectStoreMapAdapter(clusterStore.NginxProxies), + predicate: funcPredicate{stateChanged: isReferenced}, + }, + { + gvk: cfg.MustExtractGVK(&ngfAPIv1alpha1.ClientSettingsPolicy{}), + store: commonPolicyObjectStore, + predicate: funcPredicate{stateChanged: isNGFPolicyRelevant}, + }, + { + gvk: cfg.MustExtractGVK(&ngfAPIv1alpha2.ObservabilityPolicy{}), + store: commonPolicyObjectStore, + predicate: funcPredicate{stateChanged: isNGFPolicyRelevant}, + }, + { + gvk: cfg.MustExtractGVK(&ngfAPIv1alpha1.UpstreamSettingsPolicy{}), + store: commonPolicyObjectStore, + predicate: funcPredicate{stateChanged: isNGFPolicyRelevant}, + }, + { + gvk: cfg.MustExtractGVK(&v1alpha2.TLSRoute{}), + store: newObjectStoreMapAdapter(clusterStore.TLSRoutes), + predicate: nil, + }, + { + gvk: cfg.MustExtractGVK(&ngfAPIv1alpha1.SnippetsFilter{}), + store: newObjectStoreMapAdapter(clusterStore.SnippetsFilters), + predicate: nil, // we always want to write status to SnippetsFilters so we don't filter them out + }, + } + + if cfg.SnippetsPolicies { + trackingUpdaterCfg = append(trackingUpdaterCfg, changeTrackingUpdaterObjectTypeCfg{ + gvk: cfg.MustExtractGVK(&ngfAPIv1alpha1.SnippetsPolicy{}), + store: commonPolicyObjectStore, + predicate: funcPredicate{stateChanged: isNGFPolicyRelevant}, + }) + } + trackingUpdater := newChangeTrackingUpdater( cfg.MustExtractGVK, - []changeTrackingUpdaterObjectTypeCfg{ - { - gvk: cfg.MustExtractGVK(&v1.GatewayClass{}), - store: newObjectStoreMapAdapter(clusterStore.GatewayClasses), - predicate: nil, - }, - { - gvk: cfg.MustExtractGVK(&v1.Gateway{}), - store: newObjectStoreMapAdapter(clusterStore.Gateways), - predicate: nil, - }, - { - gvk: cfg.MustExtractGVK(&v1.HTTPRoute{}), - store: newObjectStoreMapAdapter(clusterStore.HTTPRoutes), - predicate: nil, - }, - { - gvk: cfg.MustExtractGVK(&v1beta1.ReferenceGrant{}), - store: newObjectStoreMapAdapter(clusterStore.ReferenceGrants), - predicate: nil, - }, - { - gvk: cfg.MustExtractGVK(&v1.BackendTLSPolicy{}), - store: newObjectStoreMapAdapter(clusterStore.BackendTLSPolicies), - predicate: nil, - }, - { - gvk: cfg.MustExtractGVK(&v1.GRPCRoute{}), - store: newObjectStoreMapAdapter(clusterStore.GRPCRoutes), - predicate: nil, - }, - { - gvk: cfg.MustExtractGVK(&apiv1.Namespace{}), - store: newObjectStoreMapAdapter(clusterStore.Namespaces), - predicate: funcPredicate{stateChanged: isReferenced}, - }, - { - gvk: cfg.MustExtractGVK(&apiv1.Service{}), - store: newObjectStoreMapAdapter(clusterStore.Services), - predicate: funcPredicate{stateChanged: isReferenced}, - }, - { - gvk: cfg.MustExtractGVK(&inference.InferencePool{}), - store: newObjectStoreMapAdapter(clusterStore.InferencePools), - predicate: funcPredicate{stateChanged: isReferenced}, - }, - { - gvk: cfg.MustExtractGVK(&discoveryV1.EndpointSlice{}), - store: nil, - predicate: funcPredicate{stateChanged: isReferenced}, - }, - { - gvk: cfg.MustExtractGVK(&apiv1.Secret{}), - store: newObjectStoreMapAdapter(clusterStore.Secrets), - predicate: funcPredicate{stateChanged: isReferenced}, - }, - { - gvk: cfg.MustExtractGVK(&apiv1.ConfigMap{}), - store: newObjectStoreMapAdapter(clusterStore.ConfigMaps), - predicate: funcPredicate{stateChanged: isReferenced}, - }, - { - gvk: cfg.MustExtractGVK(&apiext.CustomResourceDefinition{}), - store: newObjectStoreMapAdapter(clusterStore.CRDMetadata), - predicate: annotationChangedPredicate{annotation: consts.BundleVersionAnnotation}, - }, - { - gvk: cfg.MustExtractGVK(&ngfAPIv1alpha2.NginxProxy{}), - store: newObjectStoreMapAdapter(clusterStore.NginxProxies), - predicate: funcPredicate{stateChanged: isReferenced}, - }, - { - gvk: cfg.MustExtractGVK(&ngfAPIv1alpha1.ClientSettingsPolicy{}), - store: commonPolicyObjectStore, - predicate: funcPredicate{stateChanged: isNGFPolicyRelevant}, - }, - { - gvk: cfg.MustExtractGVK(&ngfAPIv1alpha2.ObservabilityPolicy{}), - store: commonPolicyObjectStore, - predicate: funcPredicate{stateChanged: isNGFPolicyRelevant}, - }, - { - gvk: cfg.MustExtractGVK(&ngfAPIv1alpha1.UpstreamSettingsPolicy{}), - store: commonPolicyObjectStore, - predicate: funcPredicate{stateChanged: isNGFPolicyRelevant}, - }, - { - gvk: cfg.MustExtractGVK(&v1alpha2.TLSRoute{}), - store: newObjectStoreMapAdapter(clusterStore.TLSRoutes), - predicate: nil, - }, - { - gvk: cfg.MustExtractGVK(&ngfAPIv1alpha1.SnippetsFilter{}), - store: newObjectStoreMapAdapter(clusterStore.SnippetsFilters), - predicate: nil, // we always want to write status to SnippetsFilters so we don't filter them out - }, - }, + trackingUpdaterCfg, ) processor.getAndResetClusterStateChanged = trackingUpdater.getAndResetChangedStatus diff --git a/internal/controller/state/change_processor_test.go b/internal/controller/state/change_processor_test.go index 17af9a4a84..125dbd71be 100644 --- a/internal/controller/state/change_processor_test.go +++ b/internal/controller/state/change_processor_test.go @@ -426,6 +426,7 @@ var _ = Describe("ChangeProcessor", func() { Logger: logr.Discard(), Validators: createAlwaysValidValidators(), MustExtractGVK: kinds.NewMustExtractGKV(createScheme()), + SnippetsPolicies: true, }) }) @@ -2941,13 +2942,14 @@ var _ = Describe("ChangeProcessor", func() { Describe("NGF Policy resource changes", Ordered, func() { var ( - gw *v1.Gateway - route *v1.HTTPRoute - svc *apiv1.Service - csp, cspUpdated *ngfAPIv1alpha1.ClientSettingsPolicy - obs, obsUpdated *ngfAPIv1alpha2.ObservabilityPolicy - usp, uspUpdated *ngfAPIv1alpha1.UpstreamSettingsPolicy - cspKey, obsKey, uspKey graph.PolicyKey + gw *v1.Gateway + route *v1.HTTPRoute + svc *apiv1.Service + csp, cspUpdated *ngfAPIv1alpha1.ClientSettingsPolicy + obs, obsUpdated *ngfAPIv1alpha2.ObservabilityPolicy + usp, uspUpdated *ngfAPIv1alpha1.UpstreamSettingsPolicy + snip, snipUpdated *ngfAPIv1alpha1.SnippetsPolicy + cspKey, obsKey, uspKey, snipKey graph.PolicyKey ) BeforeAll(func() { @@ -3069,6 +3071,43 @@ var _ = Describe("ChangeProcessor", func() { Version: "v1alpha1", }, } + + snip = &ngfAPIv1alpha1.SnippetsPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "snip", + Namespace: "test", + }, + Spec: ngfAPIv1alpha1.SnippetsPolicySpec{ + TargetRefs: []v1.LocalPolicyTargetReference{ + { + Group: v1.GroupName, + Kind: kinds.Gateway, + Name: "gw", + }, + }, + Snippets: []ngfAPIv1alpha1.Snippet{ + { + Context: ngfAPIv1alpha1.NginxContextMain, + Value: "worker_processes 1;", + }, + }, + }, + } + + snipUpdated = snip.DeepCopy() + snipUpdated.Spec.Snippets = append(snipUpdated.Spec.Snippets, ngfAPIv1alpha1.Snippet{ + Context: ngfAPIv1alpha1.NginxContextHTTP, + Value: "keepalive_timeout 65s;", + }) + + snipKey = graph.PolicyKey{ + NsName: types.NamespacedName{Name: "snip", Namespace: "test"}, + GVK: schema.GroupVersionKind{ + Group: ngfAPIv1alpha1.GroupName, + Kind: kinds.SnippetsPolicy, + Version: "v1alpha1", + }, + } }) /* @@ -3082,6 +3121,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(csp) processor.CaptureUpsertChange(obs) processor.CaptureUpsertChange(usp) + processor.CaptureUpsertChange(snip) Expect(processor.Process()).To(BeNil()) }) @@ -3107,6 +3147,12 @@ var _ = Describe("ChangeProcessor", func() { Expect(graph).ToNot(BeNil()) Expect(graph.NGFPolicies).To(HaveKey(uspKey)) Expect(graph.NGFPolicies[uspKey].Source).To(Equal(usp)) + + processor.CaptureUpsertChange(snip) + graph = processor.Process() + Expect(graph).ToNot(BeNil()) + Expect(graph.NGFPolicies).To(HaveKey(snipKey)) + Expect(graph.NGFPolicies[snipKey].Source).To(Equal(snip)) }) }) When("the policy is updated", func() { @@ -3115,6 +3161,13 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(obsUpdated) processor.CaptureUpsertChange(uspUpdated) + snipUpdated := snip.DeepCopy() + snipUpdated.Spec.Snippets = append(snipUpdated.Spec.Snippets, ngfAPIv1alpha1.Snippet{ + Context: ngfAPIv1alpha1.NginxContextHTTP, + Value: "keepalive_timeout 65s;", + }) + processor.CaptureUpsertChange(snipUpdated) + graph := processor.Process() Expect(graph).ToNot(BeNil()) Expect(graph.NGFPolicies).To(HaveKey(cspKey)) @@ -3123,6 +3176,8 @@ var _ = Describe("ChangeProcessor", func() { Expect(graph.NGFPolicies[obsKey].Source).To(Equal(obsUpdated)) Expect(graph.NGFPolicies).To(HaveKey(uspKey)) Expect(graph.NGFPolicies[uspKey].Source).To(Equal(uspUpdated)) + Expect(graph.NGFPolicies).To(HaveKey(snipKey)) + Expect(graph.NGFPolicies[snipKey].Source).To(Equal(snipUpdated)) }) }) When("the policy is deleted", func() { @@ -3130,6 +3185,7 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureDeleteChange(&ngfAPIv1alpha1.ClientSettingsPolicy{}, client.ObjectKeyFromObject(csp)) processor.CaptureDeleteChange(&ngfAPIv1alpha2.ObservabilityPolicy{}, client.ObjectKeyFromObject(obs)) processor.CaptureDeleteChange(&ngfAPIv1alpha1.UpstreamSettingsPolicy{}, client.ObjectKeyFromObject(usp)) + processor.CaptureDeleteChange(&ngfAPIv1alpha1.SnippetsPolicy{}, client.ObjectKeyFromObject(snip)) graph := processor.Process() Expect(graph).ToNot(BeNil()) diff --git a/internal/controller/state/conditions/conditions.go b/internal/controller/state/conditions/conditions.go index 8e186fe1a4..de1c30ab0e 100644 --- a/internal/controller/state/conditions/conditions.go +++ b/internal/controller/state/conditions/conditions.go @@ -127,6 +127,10 @@ const ( // ObservabilityPolicy is applied to a HTTPRoute, or GRPCRoute. ObservabilityPolicyAffected v1.PolicyConditionType = "ObservabilityPolicyAffected" + // SnippetsPolicyAffected is used with the "PolicyAffected" condition when a + // SnippetsPolicy is applied to a Gateway. + SnippetsPolicyAffected v1.PolicyConditionType = "SnippetsPolicyAffected" + // PolicyAffectedReason is used with the "PolicyAffected" condition when a // ObservabilityPolicy or ClientSettingsPolicy is applied to Gateways or Routes. PolicyAffectedReason v1.PolicyConditionReason = "PolicyAffected" @@ -1145,6 +1149,17 @@ func NewClientSettingsPolicyAffected() Condition { } } +// NewSnippetsPolicyAffected returns a Condition that indicates that a SnippetsPolicy +// is applied to the resource. +func NewSnippetsPolicyAffected() Condition { + return Condition{ + Type: string(SnippetsPolicyAffected), + Status: metav1.ConditionTrue, + Reason: string(PolicyAffectedReason), + Message: "The SnippetsPolicy is applied to the resource", + } +} + // NewBackendTLSPolicyResolvedRefs returns a Condition that indicates that all CACertificateRefs // in the BackendTLSPolicy are resolved. func NewBackendTLSPolicyResolvedRefs() Condition { diff --git a/internal/controller/state/dataplane/configuration.go b/internal/controller/state/dataplane/configuration.go index def60605b6..dbd7086ef4 100644 --- a/internal/controller/state/dataplane/configuration.go +++ b/internal/controller/state/dataplane/configuration.go @@ -99,6 +99,7 @@ func BuildConfiguration( Logging: buildLogging(gateway), NginxPlus: nginxPlus, MainSnippets: buildSnippetsForContext(gatewaySnippetsFilters, ngfAPIv1alpha1.NginxContextMain), + Policies: buildPolicies(gateway, gateway.Policies), AuxiliarySecrets: buildAuxiliarySecrets(g.PlusSecrets), WorkerConnections: buildWorkerConnections(gateway), } diff --git a/internal/controller/state/dataplane/types.go b/internal/controller/state/dataplane/types.go index 9e1beeaa0f..8a73e6c7a4 100644 --- a/internal/controller/state/dataplane/types.go +++ b/internal/controller/state/dataplane/types.go @@ -43,6 +43,8 @@ type Configuration struct { BackendGroups []BackendGroup // MainSnippets holds all the snippets that apply to the main context. MainSnippets []Snippet + // Policies holds the policies attached to the Gateway. + Policies []policies.Policy // Upstreams holds all unique http Upstreams. Upstreams []Upstream // NginxPlus specifies NGINX Plus additional settings. diff --git a/internal/controller/state/graph/policies.go b/internal/controller/state/graph/policies.go index 2f1d8ffd47..8bae773510 100644 --- a/internal/controller/state/graph/policies.go +++ b/internal/controller/state/graph/policies.go @@ -637,5 +637,10 @@ func addStatusToTargetRefs(policyKind string, conditionsList *[]conditions.Condi return } *conditionsList = append(*conditionsList, conditions.NewClientSettingsPolicyAffected()) + case kinds.SnippetsPolicy: + if conditions.HasMatchingCondition(*conditionsList, conditions.NewSnippetsPolicyAffected()) { + return + } + *conditionsList = append(*conditionsList, conditions.NewSnippetsPolicyAffected()) } } diff --git a/internal/controller/state/graph/policies_test.go b/internal/controller/state/graph/policies_test.go index 251ab5aeb8..0a3637c58c 100644 --- a/internal/controller/state/graph/policies_test.go +++ b/internal/controller/state/graph/policies_test.go @@ -1660,6 +1660,7 @@ func TestAddPolicyAffectedStatusOnTargetRefs(t *testing.T) { cspGVK := schema.GroupVersionKind{Group: "Group", Version: "Version", Kind: "ClientSettingsPolicy"} opGVK := schema.GroupVersionKind{Group: "Group", Version: "Version", Kind: "ObservabilityPolicy"} + snipGVK := schema.GroupVersionKind{Group: "Group", Version: "Version", Kind: "SnippetsPolicy"} gw1Ref := createTestRef(kinds.Gateway, v1.GroupName, "gw1") gw1TargetRef := createTestPolicyTargetRef( @@ -1676,6 +1677,11 @@ func TestAddPolicyAffectedStatusOnTargetRefs(t *testing.T) { kinds.Gateway, types.NamespacedName{Namespace: testNs, Name: "gw3"}, ) + gwSnipRef := createTestRef(kinds.Gateway, v1.GroupName, "gw-snip") + gwSnipTargetRef := createTestPolicyTargetRef( + kinds.Gateway, + types.NamespacedName{Namespace: testNs, Name: "gw-snip"}, + ) hr1Ref := createTestRef(kinds.HTTPRoute, v1.GroupName, "hr1") hr1TargetRef := createTestPolicyTargetRef( @@ -1751,14 +1757,24 @@ func TestAddPolicyAffectedStatusOnTargetRefs(t *testing.T) { Source: createTestPolicy(opGVK, "observabilityPolicy1", gw2Ref), TargetRefs: []PolicyTargetRef{gw2TargetRef}, }, + createTestPolicyKey(snipGVK, "snippetsPolicy1"): { + Source: createTestPolicy(snipGVK, "snippetsPolicy1", gwSnipRef), + TargetRefs: []PolicyTargetRef{gwSnipTargetRef}, + }, }, - gws: createGatewayMap(types.NamespacedName{Namespace: testNs, Name: "gw2"}), + gws: createGatewayMap( + types.NamespacedName{Namespace: testNs, Name: "gw2"}, + types.NamespacedName{Namespace: testNs, Name: "gw-snip"}, + ), routes: nil, expectedConditions: map[types.NamespacedName][]conditions.Condition{ {Namespace: testNs, Name: "gw2"}: { conditions.NewClientSettingsPolicyAffected(), conditions.NewObservabilityPolicyAffected(), }, + {Namespace: testNs, Name: "gw-snip"}: { + conditions.NewSnippetsPolicyAffected(), + }, }, }, { @@ -1849,6 +1865,9 @@ func TestAddPolicyAffectedStatusOnTargetRefs(t *testing.T) { {Namespace: testNs, Name: "gr2"}: { conditions.NewObservabilityPolicyAffected(), }, + {Namespace: testNs, Name: "gw-snip"}: { + conditions.NewSnippetsPolicyAffected(), + }, }, }, { diff --git a/internal/framework/kinds/kinds.go b/internal/framework/kinds/kinds.go index b59b06df96..5cfccb6e5e 100644 --- a/internal/framework/kinds/kinds.go +++ b/internal/framework/kinds/kinds.go @@ -47,6 +47,8 @@ const ( NginxProxy = "NginxProxy" // SnippetsFilter is the SnippetsFilter kind. SnippetsFilter = "SnippetsFilter" + // SnippetsPolicy is the SnippetsPolicy kind. + SnippetsPolicy = "SnippetsPolicy" // UpstreamSettingsPolicy is the UpstreamSettingsPolicy kind. UpstreamSettingsPolicy = "UpstreamSettingsPolicy" ) diff --git a/tests/suite/manifests/snippets-policy/cafe.yaml b/tests/suite/manifests/snippets-policy/cafe.yaml new file mode 100644 index 0000000000..972b1e7e99 --- /dev/null +++ b/tests/suite/manifests/snippets-policy/cafe.yaml @@ -0,0 +1,50 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: cafe +spec: + parentRefs: + - name: gateway + hostnames: + - "cafe.example.com" + rules: + - matches: + - path: + type: PathPrefix + value: /coffee + backendRefs: + - name: coffee + port: 80 +--- +apiVersion: v1 +kind: Service +metadata: + name: coffee +spec: + ports: + - port: 80 + targetPort: 80 + protocol: TCP + name: http + selector: + app: coffee +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: coffee +spec: + replicas: 1 + selector: + matchLabels: + app: coffee + template: + metadata: + labels: + app: coffee + spec: + containers: + - name: coffee + image: nginxdemos/nginx-hello:plain-text + ports: + - containerPort: 80 diff --git a/tests/suite/manifests/snippets-policy/gateway-2.yaml b/tests/suite/manifests/snippets-policy/gateway-2.yaml new file mode 100644 index 0000000000..ac7603def7 --- /dev/null +++ b/tests/suite/manifests/snippets-policy/gateway-2.yaml @@ -0,0 +1,10 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: gateway-2 +spec: + gatewayClassName: nginx + listeners: + - name: http + port: 8080 + protocol: HTTP diff --git a/tests/suite/manifests/snippets-policy/gateway.yaml b/tests/suite/manifests/snippets-policy/gateway.yaml new file mode 100644 index 0000000000..9d402bd5a1 --- /dev/null +++ b/tests/suite/manifests/snippets-policy/gateway.yaml @@ -0,0 +1,10 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: gateway +spec: + gatewayClassName: nginx + listeners: + - name: http + port: 80 + protocol: HTTP diff --git a/tests/suite/manifests/snippets-policy/invalid-context-sp.yaml b/tests/suite/manifests/snippets-policy/invalid-context-sp.yaml new file mode 100644 index 0000000000..43f6d44b3d --- /dev/null +++ b/tests/suite/manifests/snippets-policy/invalid-context-sp.yaml @@ -0,0 +1,12 @@ +apiVersion: gateway.nginx.org/v1alpha1 +kind: SnippetsPolicy +metadata: + name: invalid-context-sp +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway + snippets: + - context: http + value: "worker_priority 0;" diff --git a/tests/suite/manifests/snippets-policy/invalid-duplicate-sp.yaml b/tests/suite/manifests/snippets-policy/invalid-duplicate-sp.yaml new file mode 100644 index 0000000000..64daa65bc5 --- /dev/null +++ b/tests/suite/manifests/snippets-policy/invalid-duplicate-sp.yaml @@ -0,0 +1,12 @@ +apiVersion: gateway.nginx.org/v1alpha1 +kind: SnippetsPolicy +metadata: + name: invalid-duplicate-sp +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway + snippets: + - context: main + value: "worker_processes 1;" diff --git a/tests/suite/manifests/snippets-policy/multi-target-sp.yaml b/tests/suite/manifests/snippets-policy/multi-target-sp.yaml new file mode 100644 index 0000000000..513e283d06 --- /dev/null +++ b/tests/suite/manifests/snippets-policy/multi-target-sp.yaml @@ -0,0 +1,15 @@ +apiVersion: gateway.nginx.org/v1alpha1 +kind: SnippetsPolicy +metadata: + name: multi-target-sp +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 + snippets: + - context: main + value: "env MULTI_TARGET;" diff --git a/tests/suite/manifests/snippets-policy/valid-sp.yaml b/tests/suite/manifests/snippets-policy/valid-sp.yaml new file mode 100644 index 0000000000..e053b2d90d --- /dev/null +++ b/tests/suite/manifests/snippets-policy/valid-sp.yaml @@ -0,0 +1,16 @@ +apiVersion: gateway.nginx.org/v1alpha1 +kind: SnippetsPolicy +metadata: + name: valid-sp +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway + snippets: + - context: main + value: "worker_priority 0;" + - context: http + value: "aio off;" + - context: http.server + value: "auth_delay 0s;" diff --git a/tests/suite/snippets_policy_test.go b/tests/suite/snippets_policy_test.go new file mode 100644 index 0000000000..14abf446ab --- /dev/null +++ b/tests/suite/snippets_policy_test.go @@ -0,0 +1,249 @@ +package main + +import ( + "context" + "fmt" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + core "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + v1 "sigs.k8s.io/gateway-api/apis/v1" + + ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" + "github.com/nginx/nginx-gateway-fabric/v2/tests/framework" +) + +var _ = Describe("SnippetsPolicy", Ordered, Label("functional", "snippets-policy"), func() { + var ( + files = []string{ + "snippets-policy/cafe.yaml", + "snippets-policy/gateway.yaml", + "snippets-policy/gateway-2.yaml", + } + + namespace = "snippets-policy" + + nginxPodName string + gatewayNsName = types.NamespacedName{Name: "gateway", Namespace: namespace} + ) + + BeforeAll(func() { + ns := &core.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespace, + }, + } + + Expect(resourceManager.Apply([]client.Object{ns})).To(Succeed()) + Expect(resourceManager.ApplyFromFiles(files, namespace)).To(Succeed()) + Expect(resourceManager.WaitForAppsToBeReady(namespace)).To(Succeed()) + + nginxPodNames, err := resourceManager.GetReadyNginxPodNames( + namespace, + timeoutConfig.GetStatusTimeout, + ) + Expect(err).ToNot(HaveOccurred()) + Expect(nginxPodNames).To(HaveLen(1)) + + nginxPodName = nginxPodNames[0] + + setUpPortForward(nginxPodName, namespace) + }) + + AfterAll(func() { + cleanUpPortForward() + + Expect(resourceManager.DeleteNamespace(namespace)).To(Succeed()) + }) + + When("SnippetsPolicies are applied to the resources", func() { + snippetsPolicy := []string{ + "snippets-policy/valid-sp.yaml", + "snippets-policy/multi-target-sp.yaml", + } + + BeforeAll(func() { + Expect(resourceManager.ApplyFromFiles(snippetsPolicy, namespace)).To(Succeed()) + Expect(resourceManager.WaitForAppsToBeReady(namespace)).To(Succeed()) + }) + + AfterAll(func() { + framework.AddNginxLogsAndEventsToReport(resourceManager, namespace) + Expect(resourceManager.DeleteFromFiles(snippetsPolicy, namespace)).To(Succeed()) + }) + + Specify("snippetsPolicies are accepted", func() { + snippetsPolicyNames := []string{ + "valid-sp", + "multi-target-sp", + } + + for _, name := range snippetsPolicyNames { + nsname := types.NamespacedName{Name: name, Namespace: namespace} + + Eventually(checkForSnippetsPolicyToBeAccepted). + WithArguments(nsname). + WithTimeout(timeoutConfig.GetStatusTimeout). + WithPolling(500*time.Millisecond). + Should(Succeed(), fmt.Sprintf("%s was not accepted", name)) + } + }) + + Context("verify working traffic", func() { + It("should return a 200 response for HTTPRoute", func() { + port := 80 + if portFwdPort != 0 { + port = portFwdPort + } + baseURL := fmt.Sprintf("http://cafe.example.com:%d%s", port, "/coffee") + + Eventually( + func() error { + return expectRequestToSucceed(baseURL, address, "URI: /coffee") + }). + WithTimeout(timeoutConfig.RequestTimeout). + WithPolling(500 * time.Millisecond). + Should(Succeed()) + }) + }) + + Context("nginx directives", func() { + var conf *framework.Payload + + // gwNsName := fmt.Sprintf("%s-%s", namespace, "gateway") + // mainContext := fmt.Sprintf("%s%s/SnippetsPolicy_main_", snippetsPolicyFilePrefix, gwNsName) + // httpContext := fmt.Sprintf("%s%s/SnippetsPolicy_http_", snippetsPolicyFilePrefix, gwNsName) + // serverContext := fmt.Sprintf("%s%s/http:80/SnippetsPolicy_server_", snippetsPolicyFilePrefix, gwNsName) + + BeforeAll(func() { + var err error + conf, err = resourceManager.GetNginxConfig(nginxPodName, namespace, "") + Expect(err).ToNot(HaveOccurred()) + }) + + DescribeTable("are set properly for", + func(expCfgs []framework.ExpectedNginxField) { + for _, expCfg := range expCfgs { + Expect(framework.ValidateNginxFieldExists(conf, expCfg)).To(Succeed()) + } + }, + Entry("SnippetsPolicy", []framework.ExpectedNginxField{ + { + Directive: "worker_priority", + Value: "0", + File: "SnippetsPolicy_main_valid-sp.conf", + }, + { + Directive: "aio", + Value: "off", + File: "SnippetsPolicy_http_valid-sp.conf", + }, + { + Directive: "auth_delay", + Value: "0s", + File: "SnippetsPolicy_server_valid-sp.conf", + }, + }), + Entry("multi-target-sp", []framework.ExpectedNginxField{ + { + Directive: "env", + Value: "MULTI_TARGET;", + File: "snippets-policy-gateway/SnippetsPolicy_main_multi-target-sp.conf", + }, + { + Directive: "env", + Value: "MULTI_TARGET;", + File: "snippets-policy-gateway-2/SnippetsPolicy_main_multi-target-sp.conf", + }, + }), + ) + }) + }) + + When("SnippetsPolicy is invalid", func() { + Specify("if directives already present in the config are used", func() { + files := []string{"snippets-policy/invalid-duplicate-sp.yaml"} + + Expect(resourceManager.ApplyFromFiles(files, namespace)).To(Succeed()) + + Eventually(checkGatewayToHaveGatewayNotProgrammedCond). + WithArguments(gatewayNsName). + WithTimeout(timeoutConfig.GetStatusTimeout). + WithPolling(500 * time.Millisecond). + Should(Succeed()) + + Expect(resourceManager.DeleteFromFiles(files, namespace)).To(Succeed()) + }) + + Specify("if directives are provided in the wrong context", func() { + files := []string{"snippets-policy/invalid-context-sp.yaml"} + + Expect(resourceManager.ApplyFromFiles(files, namespace)).To(Succeed()) + + Eventually(checkGatewayToHaveGatewayNotProgrammedCond). + WithArguments(gatewayNsName). + WithTimeout(timeoutConfig.GetStatusTimeout). + WithPolling(500 * time.Millisecond). + Should(Succeed()) + + Expect(resourceManager.DeleteFromFiles(files, namespace)).To(Succeed()) + }) + }) +}) + +func checkForSnippetsPolicyToBeAccepted(snippetsPolicyNsNames types.NamespacedName) error { + ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.GetTimeout) + defer cancel() + + GinkgoWriter.Printf( + "Checking for SnippetsPolicy %q to have the condition Accepted/True/Accepted\n", + snippetsPolicyNsNames, + ) + + var sp ngfAPI.SnippetsPolicy + var err error + + if err = resourceManager.Get(ctx, snippetsPolicyNsNames, &sp); err != nil { + return err + } + + if len(sp.Status.Ancestors) == 0 { + return fmt.Errorf("snippetsPolicy has no ancestors") + } + + if len(sp.Status.Ancestors[0].Conditions) == 0 { + return fmt.Errorf("snippetsPolicy ancestor has no conditions") + } + + condition := sp.Status.Ancestors[0].Conditions[0] + if condition.Type != string(v1.PolicyConditionAccepted) { + wrongTypeErr := fmt.Errorf("expected condition type to be Accepted, got %s", condition.Type) + GinkgoWriter.Printf("ERROR: %v\n", wrongTypeErr) + + return wrongTypeErr + } + + if condition.Status != metav1.ConditionTrue { + wrongStatusErr := fmt.Errorf("expected condition status to be %s, got %s", metav1.ConditionTrue, condition.Status) + GinkgoWriter.Printf("ERROR: %v\n", wrongStatusErr) + + return wrongStatusErr + } + + if condition.Reason != string(v1.PolicyReasonAccepted) { + wrongReasonErr := fmt.Errorf( + "expected condition reason to be %s, got %s", + v1.PolicyReasonAccepted, + condition.Reason, + ) + GinkgoWriter.Printf("ERROR: %v\n", wrongReasonErr) + + return wrongReasonErr + } + + return nil +}