diff --git a/apis/v1alpha1/policy_methods.go b/apis/v1alpha1/policy_methods.go index 02350ef82b..5971010146 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 []gatewayv1.LocalPolicyTargetReference{p.Spec.TargetRef.LocalPolicyTargetReference} +} + +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..4cfb9fa6a7 --- /dev/null +++ b/apis/v1alpha1/snippetspolicy_types.go @@ -0,0 +1,70 @@ +/* +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:resource:shortName=snipol +// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` +// +kubebuilder:printcolumn:name="Accepted",type=string,JSONPath=`.status.conditions[?(@.type=="Accepted")].status` +// +kubebuilder:printcolumn:name="Reason",type=string,JSONPath=`.status.conditions[?(@.type=="Accepted")].reason` + +// SnippetsPolicy provides a way to inject NGINX snippets into the configuration. +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. + // +optional + Status gatewayv1.PolicyStatus `json:"status,omitempty"` +} + +// SnippetsPolicySpec defines the desired state of the SnippetsPolicy. +type SnippetsPolicySpec struct { + // TargetRef is the reference to the Gateway that this policy should be applied to. + TargetRef SnippetsPolicyTargetRef `json:"targetRef"` + + // 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')" + Snippets []Snippet `json:"snippets"` +} + +// SnippetsPolicyTargetRef identifies an API object to apply the policy to. +type SnippetsPolicyTargetRef struct { + gatewayv1.LocalPolicyTargetReference `json:",inline"` +} + +// +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"` +} diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index d07825de2d..71a9f77faa 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -434,6 +434,102 @@ 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 + out.TargetRef = in.TargetRef + 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 *SnippetsPolicyTargetRef) DeepCopyInto(out *SnippetsPolicyTargetRef) { + *out = *in + out.LocalPolicyTargetReference = in.LocalPolicyTargetReference +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SnippetsPolicyTargetRef. +func (in *SnippetsPolicyTargetRef) DeepCopy() *SnippetsPolicyTargetRef { + if in == nil { + return nil + } + out := new(SnippetsPolicyTargetRef) + 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/config/crd/bases/gateway.nginx.org_snippetspolicies.yaml b/config/crd/bases/gateway.nginx.org_snippetspolicies.yaml new file mode 100644 index 0000000000..7ba24b42e8 --- /dev/null +++ b/config/crd/bases/gateway.nginx.org_snippetspolicies.yaml @@ -0,0 +1,419 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.19.0 + name: snippetspolicies.gateway.nginx.org +spec: + group: gateway.nginx.org + names: + kind: SnippetsPolicy + listKind: SnippetsPolicyList + plural: snippetspolicies + shortNames: + - snipol + singular: snippetspolicy + scope: Namespaced + versions: + - additionalPrinterColumns: + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + - jsonPath: .status.conditions[?(@.type=="Accepted")].status + name: Accepted + type: string + - jsonPath: .status.conditions[?(@.type=="Accepted")].reason + name: Reason + type: string + name: v1alpha1 + schema: + openAPIV3Schema: + description: SnippetsPolicy provides a way to inject NGINX snippets into the + configuration. + 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'')' + targetRef: + description: TargetRef is the reference to the Gateway that this policy + should be applied to. + 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 + required: + - snippets + - targetRef + 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/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..9735f40ea3 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.Generator{}, ) files = append(files, g.executeConfigTemplates(conf, policyGenerator)...) @@ -201,11 +203,11 @@ func (g GeneratorImpl) getExecuteFuncs( keepAliveCheck keepAliveChecker, ) []executeFunc { return []executeFunc{ - executeMainConfig, + newExecuteMainConfigFunc(generator), executeEventsConfig, - executeBaseHTTPConfig, - g.newExecuteServersFunc(generator, keepAliveCheck), + newExecuteBaseHTTPConfigFunc(generator), newExecuteUpstreamsFunc(upstreams), + newExecuteServersFunc(g.plus, generator, g.logger, keepAliveCheck), executeSplitClients, executeMaps, executeTelemetry, 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..026bd52fcd --- /dev/null +++ b/internal/controller/nginx/config/policies/snippetspolicy/generator.go @@ -0,0 +1,115 @@ +/* +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 +} + +// 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 + } + + // TargetRef info for file path and comment + gwRef := sp.Spec.TargetRef + 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..b4f659d868 --- /dev/null +++ b/internal/controller/nginx/config/policies/snippetspolicy/generator_test.go @@ -0,0 +1,78 @@ +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{ + TargetRef: v1alpha1.SnippetsPolicyTargetRef{ + LocalPolicyTargetReference: 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;")) + }) +} 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..fc66b76c24 --- /dev/null +++ b/internal/controller/nginx/config/policies/snippetspolicy/validator.go @@ -0,0 +1,97 @@ +/* +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) + + targetRefPath := field.NewPath("spec").Child("targetRef") + supportedKinds := []gatewayv1.Kind{kinds.Gateway} + supportedGroups := []gatewayv1.Group{gatewayv1.GroupName} + + // Validate TargetRef + if err := policies.ValidateTargetRef(sp.Spec.TargetRef.LocalPolicyTargetReference, targetRefPath, 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 +} + +const maxSnippetSize = 2048 // 2KB + +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) + } + + if len(snippet.Value) > maxSnippetSize { + return fmt.Errorf("snippet value for context %q exceeds maximum size of %d bytes", snippet.Context, maxSnippetSize) + } + } + 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..bb223134c5 --- /dev/null +++ b/internal/controller/nginx/config/policies/snippetspolicy/validator_test.go @@ -0,0 +1,157 @@ +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{ + TargetRef: ngfAPI.SnippetsPolicyTargetRef{ + LocalPolicyTargetReference: 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: "invalid target ref - unsupported group", + policy: createModifiedPolicy(func(p *ngfAPI.SnippetsPolicy) *ngfAPI.SnippetsPolicy { + p.Spec.TargetRef.Group = "unsupported.group" + return p + }), + expConditions: []conditions.Condition{ + conditions.NewPolicyInvalid("spec.targetRef.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.TargetRef.Kind = "UnsupportedKind" + return p + }), + expConditions: []conditions.Condition{ + conditions.NewPolicyInvalid("spec.targetRef.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"), + }, + }, + { + name: "snippet too large", + policy: createModifiedPolicy(func(p *ngfAPI.SnippetsPolicy) *ngfAPI.SnippetsPolicy { + p.Spec.Snippets = []ngfAPI.Snippet{ + { + Context: ngfAPI.NginxContextMain, + Value: string(make([]byte, 2049)), + }, + } + return p + }), + expConditions: []conditions.Condition{ + conditions.NewPolicyInvalid("snippet value for context \"main\" exceeds maximum size of 2048 bytes"), + }, + }, + } + + 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/nginx/config/servers.go b/internal/controller/nginx/config/servers.go index 578cf5e2b2..afe9933c3b 100644 --- a/internal/controller/nginx/config/servers.go +++ b/internal/controller/nginx/config/servers.go @@ -11,6 +11,7 @@ import ( "k8s.io/apimachinery/pkg/types" + "github.com/go-logr/logr" "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/shared" @@ -52,26 +53,30 @@ var httpUpgradeHeader = http.Header{ Value: "$http_upgrade", } -func (g GeneratorImpl) newExecuteServersFunc( +func newExecuteServersFunc( + plus bool, generator policies.Generator, + logger logr.Logger, keepAliveCheck keepAliveChecker, ) executeFunc { return func(configuration dataplane.Configuration) []executeResult { - return g.executeServers(configuration, generator, keepAliveCheck) + return executeServers(configuration, plus, generator, logger, keepAliveCheck) } } -func (g GeneratorImpl) executeServers( +func executeServers( conf dataplane.Configuration, + plus bool, generator policies.Generator, + logger logr.Logger, keepAliveCheck keepAliveChecker, ) []executeResult { - servers, httpMatchPairs := createServers(conf, generator, keepAliveCheck) + servers, httpMatchPairs := createServers(conf, plus, generator, logger, keepAliveCheck) serverConfig := http.ServerConfig{ Servers: servers, IPFamily: getIPFamily(conf.BaseHTTPConfig), - Plus: g.plus, + Plus: plus, RewriteClientIP: getRewriteClientIPSettings(conf.BaseHTTPConfig.RewriteClientIPSettings), DisableSNIHostValidation: conf.BaseHTTPConfig.DisableSNIHostValidation, } @@ -116,7 +121,9 @@ func getIPFamily(baseHTTPConfig dataplane.BaseHTTPConfig) shared.IPFamily { func createServers( conf dataplane.Configuration, + plus bool, generator policies.Generator, + logger logr.Logger, keepAliveCheck keepAliveChecker, ) ([]http.Server, httpMatchPairs) { servers := make([]http.Server, 0, len(conf.HTTPServers)+len(conf.SSLServers)) @@ -129,7 +136,7 @@ func createServers( for idx, s := range conf.HTTPServers { serverID := fmt.Sprintf("%d", idx) - httpServer, matchPairs := createServer(s, serverID, generator, keepAliveCheck) + httpServer, matchPairs := createServer(s, serverID, plus, generator, logger, keepAliveCheck) servers = append(servers, httpServer) maps.Copy(finalMatchPairs, matchPairs) } @@ -137,7 +144,7 @@ func createServers( for idx, s := range conf.SSLServers { serverID := fmt.Sprintf("SSL_%d", idx) - sslServer, matchPairs := createSSLServer(s, serverID, generator, keepAliveCheck) + sslServer, matchPairs := createSSLServer(s, serverID, plus, generator, logger, keepAliveCheck) if _, portInUse := sharedTLSPorts[s.Port]; portInUse { sslServer.Listen = getSocketNameHTTPS(s.Port) sslServer.IsSocket = true @@ -152,7 +159,9 @@ func createServers( func createSSLServer( virtualServer dataplane.VirtualServer, serverID string, + plus bool, generator policies.Generator, + logger logr.Logger, keepAliveCheck keepAliveChecker, ) (http.Server, httpMatchPairs) { listen := fmt.Sprint(virtualServer.Port) @@ -163,7 +172,7 @@ func createSSLServer( }, nil } - locs, matchPairs, grpc := createLocations(&virtualServer, serverID, generator, keepAliveCheck) + locs, matchPairs, grpc := createLocations(&virtualServer, serverID, plus, generator, logger, keepAliveCheck) server := http.Server{ ServerName: virtualServer.Hostname, @@ -191,7 +200,9 @@ func createSSLServer( func createServer( virtualServer dataplane.VirtualServer, serverID string, + plus bool, generator policies.Generator, + logger logr.Logger, keepAliveCheck keepAliveChecker, ) (http.Server, httpMatchPairs) { listen := fmt.Sprint(virtualServer.Port) @@ -203,7 +214,7 @@ func createServer( }, nil } - locs, matchPairs, grpc := createLocations(&virtualServer, serverID, generator, keepAliveCheck) + locs, matchPairs, grpc := createLocations(&virtualServer, serverID, plus, generator, logger, keepAliveCheck) server := http.Server{ ServerName: virtualServer.Hostname, @@ -408,7 +419,9 @@ type httpMatchPairs map[string][]routeMatch func createLocations( server *dataplane.VirtualServer, serverID string, + plus bool, generator policies.Generator, + logger logr.Logger, keepAliveCheck keepAliveChecker, ) ([]http.Location, httpMatchPairs, bool) { maxLocs, pathsAndTypes := getMaxLocationCountAndPathMap(server.PathRules) @@ -443,6 +456,8 @@ func createLocations( rule, extLocations, server.Port, + plus, + logger, keepAliveCheck, mirrorPercentage)..., ) @@ -450,8 +465,10 @@ func createLocations( internalLocations, matches := createInternalLocationsForRule( pathRuleIdx, rule, - generator, server.Port, + plus, + generator, + logger, keepAliveCheck, mirrorPercentage, ) @@ -470,12 +487,15 @@ func createLocations( pathRuleIdx, rule, extLocations, - generator, server.Port, + plus, + generator, + logger, keepAliveCheck, mirrorPercentage)..., ) } + } if !rootPathExists { @@ -489,6 +509,8 @@ func updateExternalLocationsForRule( rule dataplane.PathRule, extLocations []http.Location, port int32, + plus bool, + logger logr.Logger, keepAliveCheck keepAliveChecker, mirrorPercentage *float64, ) []http.Location { @@ -498,6 +520,8 @@ func updateExternalLocationsForRule( rule, extLocations, port, + plus, + logger, keepAliveCheck, mirrorPercentage, ) @@ -509,8 +533,10 @@ func updateExternalLocationsForRule( func createInternalLocationsForRule( pathRuleIdx int, rule dataplane.PathRule, - generator policies.Generator, port int32, + plus bool, + generator policies.Generator, + logger logr.Logger, keepAliveCheck keepAliveChecker, mirrorPercentage *float64, ) ([]http.Location, []routeMatch) { @@ -551,6 +577,8 @@ func createInternalLocationsForRule( rule, intLocation, port, + plus, + logger, keepAliveCheck, mirrorPercentage, ) @@ -601,6 +629,8 @@ func createInternalLocationsForRule( rule, intProxyPassLocation, port, + plus, + logger, keepAliveCheck, mirrorPercentage, ) @@ -669,8 +699,10 @@ func createInferenceLocationsForRule( pathRuleIdx int, rule dataplane.PathRule, extLocations []http.Location, - generator policies.Generator, port int32, + plus bool, + generator policies.Generator, + logger logr.Logger, keepAliveCheck keepAliveChecker, mirrorPercentage *float64, ) []http.Location { @@ -742,12 +774,15 @@ func createInferenceLocationsForRule( rule, intProxyPassLocation, port, + plus, + logger, keepAliveCheck, mirrorPercentage, ) locs = append(locs, intProxyPassLocation) if b.EndpointPickerConfig != nil && b.EndpointPickerConfig.EndpointPickerRef != nil { + eppHost, portNum := extractEPPConfig(b) if len(r.BackendGroup.Backends) > 1 { @@ -1035,6 +1070,8 @@ func updateLocation( pathRule dataplane.PathRule, location http.Location, listenerPort int32, + plus bool, + logger logr.Logger, keepAliveCheck keepAliveChecker, mirrorPercentage *float64, ) http.Location { @@ -1186,6 +1223,8 @@ func updateLocations( pathRule dataplane.PathRule, buildLocations []http.Location, listenerPort int32, + plus bool, + logger logr.Logger, keepAliveCheck keepAliveChecker, mirrorPercentage *float64, ) []http.Location { @@ -1197,6 +1236,8 @@ func updateLocations( pathRule, loc, listenerPort, + plus, + logger, keepAliveCheck, mirrorPercentage, ) diff --git a/internal/controller/nginx/config/servers_test.go b/internal/controller/nginx/config/servers_test.go index 59f4303778..ea4117b406 100644 --- a/internal/controller/nginx/config/servers_test.go +++ b/internal/controller/nginx/config/servers_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" "github.com/onsi/gomega/format" "k8s.io/apimachinery/pkg/types" @@ -220,8 +221,7 @@ func TestExecuteServers(t *testing.T) { }, ) - gen := GeneratorImpl{} - results := gen.executeServers(conf, fakeGenerator, alwaysFalseKeepAliveChecker) + results := executeServers(conf, false, fakeGenerator, logr.Discard(), alwaysFalseKeepAliveChecker) g.Expect(results).To(HaveLen(len(expectedResults))) for _, res := range results { @@ -359,9 +359,7 @@ func TestExecuteServers_IPFamily(t *testing.T) { t.Parallel() g := NewWithT(t) - gen := GeneratorImpl{} - results := gen.executeServers(test.config, &policiesfakes.FakeGenerator{}, alwaysFalseKeepAliveChecker) - + results := executeServers(test.config, false, &policiesfakes.FakeGenerator{}, logr.Discard(), alwaysFalseKeepAliveChecker) g.Expect(results).To(HaveLen(2)) serverConf := string(results[0].data) httpMatchConf := string(results[1].data) @@ -478,8 +476,7 @@ func TestExecuteServers_RewriteClientIP(t *testing.T) { t.Parallel() g := NewWithT(t) - gen := GeneratorImpl{} - results := gen.executeServers(test.config, &policiesfakes.FakeGenerator{}, alwaysFalseKeepAliveChecker) + results := executeServers(test.config, false, &policiesfakes.FakeGenerator{}, logr.Discard(), alwaysFalseKeepAliveChecker) g.Expect(results).To(HaveLen(2)) serverConf := string(results[0].data) httpMatchConf := string(results[1].data) @@ -520,8 +517,7 @@ func TestExecuteServers_Plus(t *testing.T) { g := NewWithT(t) - gen := GeneratorImpl{plus: true} - results := gen.executeServers(config, &policiesfakes.FakeGenerator{}, alwaysFalseKeepAliveChecker) + results := executeServers(config, true, &policiesfakes.FakeGenerator{}, logr.Discard(), alwaysFalseKeepAliveChecker) g.Expect(results).To(HaveLen(2)) serverConf := string(results[0].data) @@ -604,8 +600,7 @@ func TestExecuteForDefaultServers(t *testing.T) { t.Parallel() g := NewWithT(t) - gen := GeneratorImpl{} - serverResults := gen.executeServers(tc.conf, &policiesfakes.FakeGenerator{}, alwaysFalseKeepAliveChecker) + serverResults := executeServers(tc.conf, false, &policiesfakes.FakeGenerator{}, logr.Discard(), alwaysFalseKeepAliveChecker) g.Expect(serverResults).To(HaveLen(2)) serverConf := string(serverResults[0].data) httpMatchConf := string(serverResults[1].data) @@ -1898,7 +1893,7 @@ func TestCreateServers(t *testing.T) { } keepAliveCheck := newKeepAliveChecker([]http.Upstream{keepAliveEnabledUpstream}) - result, httpMatchPair := createServers(conf, fakeGenerator, keepAliveCheck) + result, httpMatchPair := createServers(conf, false, fakeGenerator, logr.Discard(), keepAliveCheck) format.MaxLength = 10000 g.Expect(httpMatchPair).To(Equal(allExpMatchPair)) @@ -2119,7 +2114,9 @@ func TestCreateServersConflicts(t *testing.T) { result, _ := createServers( dataplane.Configuration{HTTPServers: httpServers}, + false, &policiesfakes.FakeGenerator{}, + logr.Discard(), alwaysFalseKeepAliveChecker, ) g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty()) @@ -2271,7 +2268,7 @@ func TestCreateServers_Includes(t *testing.T) { conf := dataplane.Configuration{HTTPServers: httpServers, SSLServers: sslServers} - actualServers, matchPairs := createServers(conf, fakeGenerator, alwaysFalseKeepAliveChecker) + actualServers, matchPairs := createServers(conf, false, fakeGenerator, logr.Discard(), alwaysFalseKeepAliveChecker) g.Expect(matchPairs).To(BeEmpty()) g.Expect(actualServers).To(HaveLen(len(expServers))) @@ -2432,7 +2429,7 @@ func TestCreateLocations_Includes(t *testing.T) { }, }) - locations, matches, grpc := createLocations(&httpServer, "1", fakeGenerator, alwaysFalseKeepAliveChecker) + locations, matches, grpc := createLocations(&httpServer, "1", false, fakeGenerator, logr.Discard(), alwaysFalseKeepAliveChecker) g := NewWithT(t) g.Expect(grpc).To(BeFalse()) @@ -2940,7 +2937,9 @@ func TestCreateLocations_InferenceBackends(t *testing.T) { Port: 80, }, "1", + false, &policiesfakes.FakeGenerator{}, + logr.Discard(), alwaysFalseKeepAliveChecker, ) @@ -3131,7 +3130,9 @@ func TestCreateLocationsRootPath(t *testing.T) { Port: 80, }, "1", + false, &policiesfakes.FakeGenerator{}, + logr.Discard(), alwaysFalseKeepAliveChecker, ) g.Expect(locs).To(Equal(test.expLocations)) @@ -3261,7 +3262,9 @@ func TestCreateLocationsPath(t *testing.T) { Port: 80, }, "1", + false, &policiesfakes.FakeGenerator{}, + logr.Discard(), alwaysFalseKeepAliveChecker, ) g.Expect(locs).To(Equal(test.expLocations)) @@ -4918,8 +4921,6 @@ func TestExecuteServers_DisableSNIHostValidation(t *testing.T) { }, Port: 8443, } - gen := GeneratorImpl{} - // Case 1: DisableSNIHostValidation = false (default) confWithValidation := dataplane.Configuration{ SSLServers: []dataplane.VirtualServer{sslServer}, @@ -4927,7 +4928,7 @@ func TestExecuteServers_DisableSNIHostValidation(t *testing.T) { DisableSNIHostValidation: false, }, } - results := gen.executeServers(confWithValidation, &policiesfakes.FakeGenerator{}, alwaysFalseKeepAliveChecker) + results := executeServers(confWithValidation, false, &policiesfakes.FakeGenerator{}, logr.Discard(), alwaysFalseKeepAliveChecker) serverConf := string(results[0].data) g.Expect(serverConf).To(ContainSubstring("if ($ssl_server_name != $host)"), "Expected SNI host validation block to be present when DisableSNIHostValidation is false") @@ -4939,7 +4940,7 @@ func TestExecuteServers_DisableSNIHostValidation(t *testing.T) { DisableSNIHostValidation: true, }, } - results = gen.executeServers(confWithoutValidation, &policiesfakes.FakeGenerator{}, alwaysFalseKeepAliveChecker) + results = executeServers(confWithoutValidation, false, &policiesfakes.FakeGenerator{}, logr.Discard(), alwaysFalseKeepAliveChecker) serverConf = string(results[0].data) g.Expect(serverConf).NotTo(ContainSubstring("if ($ssl_server_name != $host)"), "Expected SNI host validation block to be absent when DisableSNIHostValidation is true") 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..8751b9f30d 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{ + TargetRef: ngfAPIv1alpha1.SnippetsPolicyTargetRef{ + LocalPolicyTargetReference: 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" )