From 0a8a78ac10faa57bdf88ac579e468b4afae85eaa Mon Sep 17 00:00:00 2001
From: bjee19 <139261241+bjee19@users.noreply.github.com>
Date: Tue, 10 Sep 2024 10:17:02 -0700
Subject: [PATCH 1/7] Add UpstreamSettingsPolicy CRD (#2515)
Problem: Users want to configure the behavior of the connection between NGINX and their upstream applications.
Solution: Add the UpstreamSettingsPolicy CRD, which is a direct policy that will attach to a Service that is referenced in an HTTPRoute or GRPCRoute.
Testing: Tested that validation works.
---
apis/v1alpha1/register.go | 2 +
apis/v1alpha1/upstreamsettingspolicy_types.go | 97 ++++
apis/v1alpha1/zz_generated.deepcopy.go | 124 +++++
...ay.nginx.org_upstreamsettingspolicies.yaml | 444 ++++++++++++++++++
docs/proposals/upstream-settings.md | 2 +-
examples/upstream-settings-policy/README.md | 4 +
.../upstream-settings-policy.yaml | 15 +
site/content/reference/api.md | 280 ++++++++++-
8 files changed, 965 insertions(+), 3 deletions(-)
create mode 100644 apis/v1alpha1/upstreamsettingspolicy_types.go
create mode 100644 config/crd/bases/gateway.nginx.org_upstreamsettingspolicies.yaml
create mode 100644 examples/upstream-settings-policy/README.md
create mode 100644 examples/upstream-settings-policy/upstream-settings-policy.yaml
diff --git a/apis/v1alpha1/register.go b/apis/v1alpha1/register.go
index f9970f4b4c..0d18c29eaa 100644
--- a/apis/v1alpha1/register.go
+++ b/apis/v1alpha1/register.go
@@ -42,6 +42,8 @@ func addKnownTypes(scheme *runtime.Scheme) error {
&ClientSettingsPolicyList{},
&SnippetsFilter{},
&SnippetsFilterList{},
+ &UpstreamSettingsPolicy{},
+ &UpstreamSettingsPolicyList{},
)
// AddToGroupVersion allows the serialization of client types like ListOptions.
metav1.AddToGroupVersion(scheme, SchemeGroupVersion)
diff --git a/apis/v1alpha1/upstreamsettingspolicy_types.go b/apis/v1alpha1/upstreamsettingspolicy_types.go
new file mode 100644
index 0000000000..f3276d0f69
--- /dev/null
+++ b/apis/v1alpha1/upstreamsettingspolicy_types.go
@@ -0,0 +1,97 @@
+package v1alpha1
+
+import (
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+ gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
+)
+
+// +genclient
+// +kubebuilder:object:root=true
+// +kubebuilder:storageversion
+// +kubebuilder:subresource:status
+// +kubebuilder:resource:categories=nginx-gateway-fabric,scope=Namespaced,shortName=uspolicy
+// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
+// +kubebuilder:metadata:labels="gateway.networking.k8s.io/policy=direct"
+
+// UpstreamSettingsPolicy is a Direct Attached Policy. It provides a way to configure the behavior of
+// the connection between NGINX and the upstream applications.
+type UpstreamSettingsPolicy struct {
+ metav1.TypeMeta `json:",inline"`
+ metav1.ObjectMeta `json:"metadata,omitempty"`
+
+ // Spec defines the desired state of the UpstreamSettingsPolicy.
+ Spec UpstreamSettingsPolicySpec `json:"spec"`
+
+ // Status defines the state of the UpstreamSettingsPolicy.
+ Status gatewayv1alpha2.PolicyStatus `json:"status,omitempty"`
+}
+
+// +kubebuilder:object:root=true
+
+// UpstreamSettingsPolicyList contains a list of UpstreamSettingsPolicies.
+type UpstreamSettingsPolicyList struct {
+ metav1.TypeMeta `json:",inline"`
+ metav1.ListMeta `json:"metadata,omitempty"`
+ Items []UpstreamSettingsPolicy `json:"items"`
+}
+
+// UpstreamSettingsPolicySpec defines the desired state of the UpstreamSettingsPolicy.
+type UpstreamSettingsPolicySpec struct {
+ // ZoneSize is the size of the shared memory zone used by the upstream. This memory zone is used to share
+ // the upstream configuration between nginx worker processes. The more servers that an upstream has,
+ // the larger memory zone is required.
+ // Default: OSS: 512k, Plus: 1m.
+ // Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#zone
+ //
+ // +optional
+ ZoneSize *Size `json:"zoneSize,omitempty"`
+
+ // KeepAlive defines the keep-alive settings.
+ //
+ // +optional
+ KeepAlive *UpstreamKeepAlive `json:"keepAlive,omitempty"`
+
+ // TargetRefs identifies API object(s) to apply the policy to.
+ // Objects must be in the same namespace as the policy.
+ // Support: Service
+ //
+ // +kubebuilder:validation:MinItems=1
+ // +kubebuilder:validation:MaxItems=16
+ // +kubebuilder:validation:XValidation:message="TargetRefs Kind must be: Service",rule="self.all(t, t.kind=='Service')"
+ // +kubebuilder:validation:XValidation:message="TargetRefs Group must be core",rule="self.exists(t, t.group=='') || self.exists(t, t.group=='core')"
+ //nolint:lll
+ TargetRefs []gatewayv1alpha2.LocalPolicyTargetReference `json:"targetRefs"`
+}
+
+// UpstreamKeepAlive defines the keep-alive settings for upstreams.
+type UpstreamKeepAlive struct {
+ // Connections sets the maximum number of idle keep-alive connections to upstream servers that are preserved
+ // in the cache of each nginx worker process. When this number is exceeded, the least recently used
+ // connections are closed.
+ // Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive
+ //
+ // +optional
+ // +kubebuilder:validation:Minimum=1
+ Connections *int32 `json:"connections,omitempty"`
+
+ // Requests sets the maximum number of requests that can be served through one keep-alive connection.
+ // After the maximum number of requests are made, the connection is closed.
+ // Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive_requests
+ //
+ // +optional
+ // +kubebuilder:validation:Minimum=0
+ Requests *int32 `json:"requests,omitempty"`
+
+ // Time defines the maximum time during which requests can be processed through one keep-alive connection.
+ // After this time is reached, the connection is closed following the subsequent request processing.
+ // Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive_time
+ //
+ // +optional
+ Time *Duration `json:"time,omitempty"`
+
+ // Timeout defines the keep-alive timeout for upstreams.
+ // Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive_timeout
+ //
+ // +optional
+ Timeout *Duration `json:"timeout,omitempty"`
+}
diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go
index aa249ed430..9624b658aa 100644
--- a/apis/v1alpha1/zz_generated.deepcopy.go
+++ b/apis/v1alpha1/zz_generated.deepcopy.go
@@ -785,3 +785,127 @@ func (in *Tracing) DeepCopy() *Tracing {
in.DeepCopyInto(out)
return out
}
+
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *UpstreamKeepAlive) DeepCopyInto(out *UpstreamKeepAlive) {
+ *out = *in
+ if in.Connections != nil {
+ in, out := &in.Connections, &out.Connections
+ *out = new(int32)
+ **out = **in
+ }
+ if in.Requests != nil {
+ in, out := &in.Requests, &out.Requests
+ *out = new(int32)
+ **out = **in
+ }
+ if in.Time != nil {
+ in, out := &in.Time, &out.Time
+ *out = new(Duration)
+ **out = **in
+ }
+ if in.Timeout != nil {
+ in, out := &in.Timeout, &out.Timeout
+ *out = new(Duration)
+ **out = **in
+ }
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new UpstreamKeepAlive.
+func (in *UpstreamKeepAlive) DeepCopy() *UpstreamKeepAlive {
+ if in == nil {
+ return nil
+ }
+ out := new(UpstreamKeepAlive)
+ in.DeepCopyInto(out)
+ return out
+}
+
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *UpstreamSettingsPolicy) DeepCopyInto(out *UpstreamSettingsPolicy) {
+ *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 UpstreamSettingsPolicy.
+func (in *UpstreamSettingsPolicy) DeepCopy() *UpstreamSettingsPolicy {
+ if in == nil {
+ return nil
+ }
+ out := new(UpstreamSettingsPolicy)
+ in.DeepCopyInto(out)
+ return out
+}
+
+// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object.
+func (in *UpstreamSettingsPolicy) 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 *UpstreamSettingsPolicyList) DeepCopyInto(out *UpstreamSettingsPolicyList) {
+ *out = *in
+ out.TypeMeta = in.TypeMeta
+ in.ListMeta.DeepCopyInto(&out.ListMeta)
+ if in.Items != nil {
+ in, out := &in.Items, &out.Items
+ *out = make([]UpstreamSettingsPolicy, len(*in))
+ for i := range *in {
+ (*in)[i].DeepCopyInto(&(*out)[i])
+ }
+ }
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new UpstreamSettingsPolicyList.
+func (in *UpstreamSettingsPolicyList) DeepCopy() *UpstreamSettingsPolicyList {
+ if in == nil {
+ return nil
+ }
+ out := new(UpstreamSettingsPolicyList)
+ in.DeepCopyInto(out)
+ return out
+}
+
+// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object.
+func (in *UpstreamSettingsPolicyList) 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 *UpstreamSettingsPolicySpec) DeepCopyInto(out *UpstreamSettingsPolicySpec) {
+ *out = *in
+ if in.ZoneSize != nil {
+ in, out := &in.ZoneSize, &out.ZoneSize
+ *out = new(Size)
+ **out = **in
+ }
+ if in.KeepAlive != nil {
+ in, out := &in.KeepAlive, &out.KeepAlive
+ *out = new(UpstreamKeepAlive)
+ (*in).DeepCopyInto(*out)
+ }
+ if in.TargetRefs != nil {
+ in, out := &in.TargetRefs, &out.TargetRefs
+ *out = make([]v1alpha2.LocalPolicyTargetReference, len(*in))
+ copy(*out, *in)
+ }
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new UpstreamSettingsPolicySpec.
+func (in *UpstreamSettingsPolicySpec) DeepCopy() *UpstreamSettingsPolicySpec {
+ if in == nil {
+ return nil
+ }
+ out := new(UpstreamSettingsPolicySpec)
+ in.DeepCopyInto(out)
+ return out
+}
diff --git a/config/crd/bases/gateway.nginx.org_upstreamsettingspolicies.yaml b/config/crd/bases/gateway.nginx.org_upstreamsettingspolicies.yaml
new file mode 100644
index 0000000000..dbe0462862
--- /dev/null
+++ b/config/crd/bases/gateway.nginx.org_upstreamsettingspolicies.yaml
@@ -0,0 +1,444 @@
+---
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+ annotations:
+ controller-gen.kubebuilder.io/version: v0.16.2
+ labels:
+ gateway.networking.k8s.io/policy: direct
+ name: upstreamsettingspolicies.gateway.nginx.org
+spec:
+ group: gateway.nginx.org
+ names:
+ categories:
+ - nginx-gateway-fabric
+ kind: UpstreamSettingsPolicy
+ listKind: UpstreamSettingsPolicyList
+ plural: upstreamsettingspolicies
+ shortNames:
+ - uspolicy
+ singular: upstreamsettingspolicy
+ scope: Namespaced
+ versions:
+ - additionalPrinterColumns:
+ - jsonPath: .metadata.creationTimestamp
+ name: Age
+ type: date
+ name: v1alpha1
+ schema:
+ openAPIV3Schema:
+ description: |-
+ UpstreamSettingsPolicy is a Direct Attached Policy. It provides a way to configure the behavior of
+ the connection between NGINX and the upstream applications.
+ 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 UpstreamSettingsPolicy.
+ properties:
+ keepAlive:
+ description: KeepAlive defines the keep-alive settings.
+ properties:
+ connections:
+ description: |-
+ Connections sets the maximum number of idle keep-alive connections to upstream servers that are preserved
+ in the cache of each nginx worker process. When this number is exceeded, the least recently used
+ connections are closed.
+ Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive
+ format: int32
+ minimum: 1
+ type: integer
+ requests:
+ description: |-
+ Requests sets the maximum number of requests that can be served through one keep-alive connection.
+ After the maximum number of requests are made, the connection is closed.
+ Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive_requests
+ format: int32
+ minimum: 0
+ type: integer
+ time:
+ description: |-
+ Time defines the maximum time during which requests can be processed through one keep-alive connection.
+ After this time is reached, the connection is closed following the subsequent request processing.
+ Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive_time
+ pattern: ^\d{1,4}(ms|s)?$
+ type: string
+ timeout:
+ description: |-
+ Timeout defines the keep-alive timeout for upstreams.
+ Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive_timeout
+ pattern: ^\d{1,4}(ms|s)?$
+ type: string
+ type: object
+ targetRefs:
+ description: |-
+ TargetRefs identifies API object(s) to apply the policy to.
+ Objects must be in the same namespace as the policy.
+ Support: Service
+ 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: Service'
+ rule: self.all(t, t.kind=='Service')
+ - message: TargetRefs Group must be core
+ rule: self.exists(t, t.group=='') || self.exists(t, t.group=='core')
+ zoneSize:
+ description: |-
+ ZoneSize is the size of the shared memory zone used by the upstream. This memory zone is used to share
+ the upstream configuration between nginx worker processes. The more servers that an upstream has,
+ the larger memory zone is required.
+ Default: OSS: 512k, Plus: 1m.
+ Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#zone
+ pattern: ^\d{1,4}(k|m|g)?$
+ type: string
+ required:
+ - targetRefs
+ type: object
+ status:
+ description: Status defines the state of the UpstreamSettingsPolicy.
+ 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
+ - controllerName
+ type: object
+ maxItems: 16
+ type: array
+ required:
+ - ancestors
+ type: object
+ required:
+ - spec
+ type: object
+ served: true
+ storage: true
+ subresources:
+ status: {}
diff --git a/docs/proposals/upstream-settings.md b/docs/proposals/upstream-settings.md
index a56b3346f3..7928220c5b 100644
--- a/docs/proposals/upstream-settings.md
+++ b/docs/proposals/upstream-settings.md
@@ -89,7 +89,7 @@ type UpstreamKeepAlive struct {
// Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive
//
// +optional
- Connections *int32 `json"connections,omitempty"`
+ Connections *int32 `json:"connections,omitempty"`
// Requests sets the maximum number of requests that can be served through one keep-alive connection.
// After the maximum number of requests are made, the connection is closed.
diff --git a/examples/upstream-settings-policy/README.md b/examples/upstream-settings-policy/README.md
new file mode 100644
index 0000000000..a6ad17a087
--- /dev/null
+++ b/examples/upstream-settings-policy/README.md
@@ -0,0 +1,4 @@
+# UpstreamSettingsPolicy
+
+This directory contains example YAMLs for testing the UpstreamSettingsPolicy CRD. Eventually, this will be converted
+into a how-to-guide.
diff --git a/examples/upstream-settings-policy/upstream-settings-policy.yaml b/examples/upstream-settings-policy/upstream-settings-policy.yaml
new file mode 100644
index 0000000000..e49ed1dffd
--- /dev/null
+++ b/examples/upstream-settings-policy/upstream-settings-policy.yaml
@@ -0,0 +1,15 @@
+apiVersion: gateway.nginx.org/v1alpha1
+kind: UpstreamSettingsPolicy
+metadata:
+ name: upstream-settings-policy
+spec:
+ zoneSize: 512k
+ targetRefs:
+ - group: core
+ kind: Service
+ name: service
+ keepAlive:
+ connections: 32
+ requests: 1001
+ time: 300s
+ timeout: 60s
diff --git a/site/content/reference/api.md b/site/content/reference/api.md
index f74d7f984f..74ea08d3c8 100644
--- a/site/content/reference/api.md
+++ b/site/content/reference/api.md
@@ -27,6 +27,8 @@ Resource Types:
ObservabilityPolicy
SnippetsFilter
+
+UpstreamSettingsPolicy
ClientSettingsPolicy
@@ -576,6 +578,131 @@ SnippetsFilterStatus
+UpstreamSettingsPolicy
+
+
+
+
UpstreamSettingsPolicy is a Direct Attached Policy. It provides a way to configure the behavior of
+the connection between NGINX and the upstream applications.
+
+
Address
@@ -974,7 +1101,8 @@ longer necessary.
ClientBody,
ClientKeepAlive,
ClientKeepAliveTimeout,
-TelemetryExporter)
+TelemetryExporter,
+UpstreamKeepAlive)
Duration is a string value representing a duration in time.
@@ -1521,7 +1649,8 @@ IP address in the X-Forwarded-For HTTP header.
(Appears on:
-ClientBody)
+ClientBody,
+UpstreamSettingsPolicySpec)
Size is a string value representing a size. Size can be specified in bytes, kilobytes (k), megabytes (m),
@@ -2018,6 +2147,153 @@ Examples of invalid names: some-$value, quoted-“value”-name, unescap
+
UpstreamKeepAlive
+
+
+
+(Appears on:
+UpstreamSettingsPolicySpec)
+
+
+
UpstreamKeepAlive defines the keep-alive settings for upstreams.
+
+
+UpstreamSettingsPolicySpec
+
+
+
+(Appears on:
+UpstreamSettingsPolicy)
+
+
+
UpstreamSettingsPolicySpec defines the desired state of the UpstreamSettingsPolicy.
+
+
Generated with gen-crd-api-reference-docs
From 6d12c4c3c956296d45fe5befe2864fc98fe5ff52 Mon Sep 17 00:00:00 2001
From: bjee19 <139261241+bjee19@users.noreply.github.com>
Date: Wed, 11 Dec 2024 15:40:54 -0800
Subject: [PATCH 2/7] Add NGINX configuration for UpstreamSettingsPolicy
(#2877)
Translate data plane intermediary UpstreamSettingsPolicy configuration into NGINX configuration.
Problem: I want the data plane configuration generated from my UpstreamSettingsPolicy to be translated
into NGINX Configuration.
Solution: Translate the data plane UpstreamSettingsPolicy configuration into NGINX configuration.
Testing: Unit tests.
---
apis/v1alpha1/policy_methods.go | 12 +
...ay.nginx.org_upstreamsettingspolicies.yaml | 6 +-
.../mode/static/nginx/config/generator.go | 17 +-
.../mode/static/nginx/config/http/config.go | 13 +-
.../policies/upstreamsettings/processor.go | 67 +++
.../upstreamsettings/processor_test.go | 334 ++++++++++++
internal/mode/static/nginx/config/servers.go | 215 ++++----
.../mode/static/nginx/config/servers_test.go | 431 ++++++++++------
.../mode/static/nginx/config/upstreams.go | 54 +-
.../static/nginx/config/upstreams_template.go | 32 ++
.../static/nginx/config/upstreams_test.go | 479 +++++++++++++++++-
internal/mode/static/state/dataplane/types.go | 2 +
12 files changed, 1395 insertions(+), 267 deletions(-)
create mode 100644 internal/mode/static/nginx/config/policies/upstreamsettings/processor.go
create mode 100644 internal/mode/static/nginx/config/policies/upstreamsettings/processor_test.go
diff --git a/apis/v1alpha1/policy_methods.go b/apis/v1alpha1/policy_methods.go
index 97e7074a62..ad399c0897 100644
--- a/apis/v1alpha1/policy_methods.go
+++ b/apis/v1alpha1/policy_methods.go
@@ -31,3 +31,15 @@ func (p *ObservabilityPolicy) GetPolicyStatus() v1alpha2.PolicyStatus {
func (p *ObservabilityPolicy) SetPolicyStatus(status v1alpha2.PolicyStatus) {
p.Status = status
}
+
+func (p *UpstreamSettingsPolicy) GetTargetRefs() []v1alpha2.LocalPolicyTargetReference {
+ return p.Spec.TargetRefs
+}
+
+func (p *UpstreamSettingsPolicy) GetPolicyStatus() v1alpha2.PolicyStatus {
+ return p.Status
+}
+
+func (p *UpstreamSettingsPolicy) SetPolicyStatus(status v1alpha2.PolicyStatus) {
+ p.Status = status
+}
diff --git a/config/crd/bases/gateway.nginx.org_upstreamsettingspolicies.yaml b/config/crd/bases/gateway.nginx.org_upstreamsettingspolicies.yaml
index dbe0462862..992af79932 100644
--- a/config/crd/bases/gateway.nginx.org_upstreamsettingspolicies.yaml
+++ b/config/crd/bases/gateway.nginx.org_upstreamsettingspolicies.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
- controller-gen.kubebuilder.io/version: v0.16.2
+ controller-gen.kubebuilder.io/version: v0.16.5
labels:
gateway.networking.k8s.io/policy: direct
name: upstreamsettingspolicies.gateway.nginx.org
@@ -76,13 +76,13 @@ spec:
Time defines the maximum time during which requests can be processed through one keep-alive connection.
After this time is reached, the connection is closed following the subsequent request processing.
Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive_time
- pattern: ^\d{1,4}(ms|s)?$
+ pattern: ^[0-9]{1,4}(ms|s|m|h)?$
type: string
timeout:
description: |-
Timeout defines the keep-alive timeout for upstreams.
Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive_timeout
- pattern: ^\d{1,4}(ms|s)?$
+ pattern: ^[0-9]{1,4}(ms|s|m|h)?$
type: string
type: object
targetRefs:
diff --git a/internal/mode/static/nginx/config/generator.go b/internal/mode/static/nginx/config/generator.go
index 714ade9dd3..a408cffd6a 100644
--- a/internal/mode/static/nginx/config/generator.go
+++ b/internal/mode/static/nginx/config/generator.go
@@ -8,9 +8,11 @@ import (
"github.com/go-logr/logr"
ngfConfig "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/http"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/clientsettings"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/observability"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/upstreamsettings"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane"
)
@@ -152,7 +154,10 @@ func (g GeneratorImpl) executeConfigTemplates(
) []file.File {
fileBytes := make(map[string][]byte)
- for _, execute := range g.getExecuteFuncs(generator) {
+ httpUpstreams := g.createUpstreams(conf.Upstreams, upstreamsettings.NewProcessor())
+ keepAliveCheck := newKeepAliveChecker(httpUpstreams)
+
+ for _, execute := range g.getExecuteFuncs(generator, httpUpstreams, keepAliveCheck) {
results := execute(conf)
for _, res := range results {
fileBytes[res.dest] = append(fileBytes[res.dest], res.data...)
@@ -177,12 +182,16 @@ func (g GeneratorImpl) executeConfigTemplates(
return files
}
-func (g GeneratorImpl) getExecuteFuncs(generator policies.Generator) []executeFunc {
+func (g GeneratorImpl) getExecuteFuncs(
+ generator policies.Generator,
+ upstreams []http.Upstream,
+ keepAliveCheck keepAliveChecker,
+) []executeFunc {
return []executeFunc{
executeMainConfig,
executeBaseHTTPConfig,
- g.newExecuteServersFunc(generator),
- g.executeUpstreams,
+ g.newExecuteServersFunc(generator, keepAliveCheck),
+ newExecuteUpstreamsFunc(upstreams),
executeSplitClients,
executeMaps,
executeTelemetry,
diff --git a/internal/mode/static/nginx/config/http/config.go b/internal/mode/static/nginx/config/http/config.go
index 6d063dc8a7..6a8e77ff4d 100644
--- a/internal/mode/static/nginx/config/http/config.go
+++ b/internal/mode/static/nginx/config/http/config.go
@@ -1,6 +1,8 @@
package http
-import "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared"
+import (
+ "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared"
+)
const (
InternalRoutePathPrefix = "/_ngf-internal"
@@ -85,9 +87,18 @@ type Upstream struct {
Name string
ZoneSize string // format: 512k, 1m
StateFile string
+ KeepAlive UpstreamKeepAlive
Servers []UpstreamServer
}
+// UpstreamKeepAlive holds the keepalive configuration for an HTTP upstream.
+type UpstreamKeepAlive struct {
+ Time string
+ Timeout string
+ Connections int32
+ Requests int32
+}
+
// UpstreamServer holds all configuration for an HTTP upstream server.
type UpstreamServer struct {
Address string
diff --git a/internal/mode/static/nginx/config/policies/upstreamsettings/processor.go b/internal/mode/static/nginx/config/policies/upstreamsettings/processor.go
new file mode 100644
index 0000000000..5df29eed64
--- /dev/null
+++ b/internal/mode/static/nginx/config/policies/upstreamsettings/processor.go
@@ -0,0 +1,67 @@
+package upstreamsettings
+
+import (
+ ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/http"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies"
+)
+
+// Processor processes UpstreamSettingsPolicies.
+type Processor struct{}
+
+// UpstreamSettings contains settings from UpstreamSettingsPolicy.
+type UpstreamSettings struct {
+ // ZoneSize is the zone size setting.
+ ZoneSize string
+ // KeepAlive contains the keepalive settings.
+ KeepAlive http.UpstreamKeepAlive
+}
+
+// NewProcessor returns a new Processor.
+func NewProcessor() Processor {
+ return Processor{}
+}
+
+// Process processes policies into an UpstreamSettings object. The policies are already validated and are guaranteed
+// to not contain overlapping settings. This method merges all fields in the policies into a single UpstreamSettings
+// object.
+func (g Processor) Process(pols []policies.Policy) UpstreamSettings {
+ return processPolicies(pols)
+}
+
+func processPolicies(pols []policies.Policy) UpstreamSettings {
+ upstreamSettings := UpstreamSettings{}
+
+ for _, pol := range pols {
+ usp, ok := pol.(*ngfAPI.UpstreamSettingsPolicy)
+ if !ok {
+ continue
+ }
+
+ // we can assume that there will be no instance of two or more policies setting the same
+ // field for the same service
+ if usp.Spec.ZoneSize != nil {
+ upstreamSettings.ZoneSize = string(*usp.Spec.ZoneSize)
+ }
+
+ if usp.Spec.KeepAlive != nil {
+ if usp.Spec.KeepAlive.Connections != nil {
+ upstreamSettings.KeepAlive.Connections = *usp.Spec.KeepAlive.Connections
+ }
+
+ if usp.Spec.KeepAlive.Requests != nil {
+ upstreamSettings.KeepAlive.Requests = *usp.Spec.KeepAlive.Requests
+ }
+
+ if usp.Spec.KeepAlive.Time != nil {
+ upstreamSettings.KeepAlive.Time = string(*usp.Spec.KeepAlive.Time)
+ }
+
+ if usp.Spec.KeepAlive.Timeout != nil {
+ upstreamSettings.KeepAlive.Timeout = string(*usp.Spec.KeepAlive.Timeout)
+ }
+ }
+ }
+
+ return upstreamSettings
+}
diff --git a/internal/mode/static/nginx/config/policies/upstreamsettings/processor_test.go b/internal/mode/static/nginx/config/policies/upstreamsettings/processor_test.go
new file mode 100644
index 0000000000..b7c785376f
--- /dev/null
+++ b/internal/mode/static/nginx/config/policies/upstreamsettings/processor_test.go
@@ -0,0 +1,334 @@
+package upstreamsettings
+
+import (
+ "testing"
+
+ . "github.com/onsi/gomega"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+ ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/http"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies"
+)
+
+func TestProcess(t *testing.T) {
+ t.Parallel()
+
+ tests := []struct {
+ name string
+ expUpstreamSettings UpstreamSettings
+ policies []policies.Policy
+ }{
+ {
+ name: "all fields populated",
+ policies: []policies.Policy{
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ ZoneSize: helpers.GetPointer[ngfAPI.Size]("2m"),
+ KeepAlive: helpers.GetPointer(ngfAPI.UpstreamKeepAlive{
+ Connections: helpers.GetPointer(int32(1)),
+ Requests: helpers.GetPointer(int32(1)),
+ Time: helpers.GetPointer[ngfAPI.Duration]("5s"),
+ Timeout: helpers.GetPointer[ngfAPI.Duration]("10s"),
+ }),
+ },
+ },
+ },
+ expUpstreamSettings: UpstreamSettings{
+ ZoneSize: "2m",
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 1,
+ Requests: 1,
+ Time: "5s",
+ Timeout: "10s",
+ },
+ },
+ },
+ {
+ name: "zone size set",
+ policies: []policies.Policy{
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ ZoneSize: helpers.GetPointer[ngfAPI.Size]("2m"),
+ },
+ },
+ },
+ expUpstreamSettings: UpstreamSettings{
+ ZoneSize: "2m",
+ },
+ },
+ {
+ name: "keep alive connections set",
+ policies: []policies.Policy{
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ KeepAlive: helpers.GetPointer(ngfAPI.UpstreamKeepAlive{
+ Connections: helpers.GetPointer(int32(1)),
+ }),
+ },
+ },
+ },
+ expUpstreamSettings: UpstreamSettings{
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 1,
+ },
+ },
+ },
+ {
+ name: "keep alive requests set",
+ policies: []policies.Policy{
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ KeepAlive: helpers.GetPointer(ngfAPI.UpstreamKeepAlive{
+ Requests: helpers.GetPointer(int32(1)),
+ }),
+ },
+ },
+ },
+ expUpstreamSettings: UpstreamSettings{
+ KeepAlive: http.UpstreamKeepAlive{
+ Requests: 1,
+ },
+ },
+ },
+ {
+ name: "keep alive time set",
+ policies: []policies.Policy{
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ KeepAlive: helpers.GetPointer(ngfAPI.UpstreamKeepAlive{
+ Time: helpers.GetPointer[ngfAPI.Duration]("5s"),
+ }),
+ },
+ },
+ },
+ expUpstreamSettings: UpstreamSettings{
+ KeepAlive: http.UpstreamKeepAlive{
+ Time: "5s",
+ },
+ },
+ },
+ {
+ name: "keep alive timeout set",
+ policies: []policies.Policy{
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ KeepAlive: helpers.GetPointer(ngfAPI.UpstreamKeepAlive{
+ Timeout: helpers.GetPointer[ngfAPI.Duration]("10s"),
+ }),
+ },
+ },
+ },
+ expUpstreamSettings: UpstreamSettings{
+ KeepAlive: http.UpstreamKeepAlive{
+ Timeout: "10s",
+ },
+ },
+ },
+ {
+ name: "no fields populated",
+ policies: []policies.Policy{
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{},
+ },
+ },
+ expUpstreamSettings: UpstreamSettings{},
+ },
+ {
+ name: "multiple UpstreamSettingsPolicies",
+ policies: []policies.Policy{
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp-zonesize",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ ZoneSize: helpers.GetPointer[ngfAPI.Size]("2m"),
+ },
+ },
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp-keepalive-connections",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ KeepAlive: helpers.GetPointer(ngfAPI.UpstreamKeepAlive{
+ Connections: helpers.GetPointer(int32(1)),
+ }),
+ },
+ },
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp-keepalive-requests",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ KeepAlive: helpers.GetPointer(ngfAPI.UpstreamKeepAlive{
+ Requests: helpers.GetPointer(int32(1)),
+ }),
+ },
+ },
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp-keepalive-time",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ KeepAlive: helpers.GetPointer(ngfAPI.UpstreamKeepAlive{
+ Time: helpers.GetPointer[ngfAPI.Duration]("5s"),
+ }),
+ },
+ },
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp-keepalive-timeout",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ KeepAlive: helpers.GetPointer(ngfAPI.UpstreamKeepAlive{
+ Timeout: helpers.GetPointer[ngfAPI.Duration]("10s"),
+ }),
+ },
+ },
+ },
+ expUpstreamSettings: UpstreamSettings{
+ ZoneSize: "2m",
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 1,
+ Requests: 1,
+ Time: "5s",
+ Timeout: "10s",
+ },
+ },
+ },
+ {
+ name: "multiple UpstreamSettingsPolicies along with other policies",
+ policies: []policies.Policy{
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp-zonesize",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ ZoneSize: helpers.GetPointer[ngfAPI.Size]("2m"),
+ },
+ },
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp-keepalive-connections",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ KeepAlive: helpers.GetPointer(ngfAPI.UpstreamKeepAlive{
+ Connections: helpers.GetPointer(int32(1)),
+ }),
+ },
+ },
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp-keepalive-requests",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ KeepAlive: helpers.GetPointer(ngfAPI.UpstreamKeepAlive{
+ Requests: helpers.GetPointer(int32(1)),
+ }),
+ },
+ },
+ &ngfAPI.ClientSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "client-settings-policy",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.ClientSettingsPolicySpec{
+ Body: &ngfAPI.ClientBody{
+ MaxSize: helpers.GetPointer[ngfAPI.Size]("1m"),
+ },
+ },
+ },
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp-keepalive-time",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ KeepAlive: helpers.GetPointer(ngfAPI.UpstreamKeepAlive{
+ Time: helpers.GetPointer[ngfAPI.Duration]("5s"),
+ }),
+ },
+ },
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp-keepalive-timeout",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ KeepAlive: helpers.GetPointer(ngfAPI.UpstreamKeepAlive{
+ Timeout: helpers.GetPointer[ngfAPI.Duration]("10s"),
+ }),
+ },
+ },
+ &ngfAPI.ObservabilityPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "observability-policy",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.ObservabilityPolicySpec{
+ Tracing: &ngfAPI.Tracing{
+ Strategy: ngfAPI.TraceStrategyRatio,
+ Ratio: helpers.GetPointer(int32(1)),
+ },
+ },
+ },
+ },
+ expUpstreamSettings: UpstreamSettings{
+ ZoneSize: "2m",
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 1,
+ Requests: 1,
+ Time: "5s",
+ Timeout: "10s",
+ },
+ },
+ },
+ }
+
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ t.Parallel()
+ g := NewWithT(t)
+ processor := NewProcessor()
+
+ g.Expect(processor.Process(test.policies)).To(Equal(test.expUpstreamSettings))
+ })
+ }
+}
diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go
index 33ea858f31..e7ccef05fc 100644
--- a/internal/mode/static/nginx/config/servers.go
+++ b/internal/mode/static/nginx/config/servers.go
@@ -23,82 +23,41 @@ const (
rootPath = "/"
)
-// httpBaseHeaders contains the constant headers set in each HTTP server block.
-var httpBaseHeaders = []http.Header{
- {
- Name: "Host",
- Value: "$gw_api_compliant_host",
- },
- {
- Name: "X-Forwarded-For",
- Value: "$proxy_add_x_forwarded_for",
- },
- {
- Name: "Upgrade",
- Value: "$http_upgrade",
- },
- {
- Name: "Connection",
- Value: "$connection_upgrade",
- },
- {
- Name: "X-Real-IP",
- Value: "$remote_addr",
- },
- {
- Name: "X-Forwarded-Proto",
- Value: "$scheme",
- },
- {
- Name: "X-Forwarded-Host",
- Value: "$host",
- },
- {
- Name: "X-Forwarded-Port",
- Value: "$server_port",
- },
-}
-
-// grpcBaseHeaders contains the constant headers set in each gRPC server block.
-var grpcBaseHeaders = []http.Header{
- {
- Name: "Host",
- Value: "$gw_api_compliant_host",
- },
- {
- Name: "X-Forwarded-For",
- Value: "$proxy_add_x_forwarded_for",
- },
- {
- Name: "Authority",
- Value: "$gw_api_compliant_host",
- },
- {
- Name: "X-Real-IP",
- Value: "$remote_addr",
- },
- {
- Name: "X-Forwarded-Proto",
- Value: "$scheme",
- },
- {
- Name: "X-Forwarded-Host",
- Value: "$host",
- },
- {
- Name: "X-Forwarded-Port",
- Value: "$server_port",
- },
-}
-
-func (g GeneratorImpl) newExecuteServersFunc(generator policies.Generator) executeFunc {
+var grpcAuthorityHeader = http.Header{
+ Name: "Authority",
+ Value: "$gw_api_compliant_host",
+}
+
+var httpConnectionHeader = http.Header{
+ Name: "Connection",
+ Value: "$connection_upgrade",
+}
+
+var unsetHTTPConnectionHeader = http.Header{
+ Name: "Connection",
+ Value: "",
+}
+
+var httpUpgradeHeader = http.Header{
+ Name: "Upgrade",
+ Value: "$http_upgrade",
+}
+
+func (g GeneratorImpl) newExecuteServersFunc(
+ generator policies.Generator,
+ keepAliveCheck keepAliveChecker,
+) executeFunc {
return func(configuration dataplane.Configuration) []executeResult {
- return g.executeServers(configuration, generator)
+ return g.executeServers(configuration, generator, keepAliveCheck)
}
}
-func (g GeneratorImpl) executeServers(conf dataplane.Configuration, generator policies.Generator) []executeResult {
- servers, httpMatchPairs := createServers(conf, generator)
+func (g GeneratorImpl) executeServers(
+ conf dataplane.Configuration,
+ generator policies.Generator,
+ keepAliveCheck keepAliveChecker,
+) []executeResult {
+ servers, httpMatchPairs := createServers(conf, generator, keepAliveCheck)
serverConfig := http.ServerConfig{
Servers: servers,
@@ -145,7 +104,11 @@ func getIPFamily(baseHTTPConfig dataplane.BaseHTTPConfig) shared.IPFamily {
return shared.IPFamily{IPv4: true, IPv6: true}
}
-func createServers(conf dataplane.Configuration, generator policies.Generator) ([]http.Server, httpMatchPairs) {
+func createServers(
+ conf dataplane.Configuration,
+ generator policies.Generator,
+ keepAliveCheck keepAliveChecker,
+) ([]http.Server, httpMatchPairs) {
servers := make([]http.Server, 0, len(conf.HTTPServers)+len(conf.SSLServers))
finalMatchPairs := make(httpMatchPairs)
sharedTLSPorts := make(map[int32]struct{})
@@ -156,7 +119,7 @@ func createServers(conf dataplane.Configuration, generator policies.Generator) (
for idx, s := range conf.HTTPServers {
serverID := fmt.Sprintf("%d", idx)
- httpServer, matchPairs := createServer(s, serverID, generator)
+ httpServer, matchPairs := createServer(s, serverID, generator, keepAliveCheck)
servers = append(servers, httpServer)
maps.Copy(finalMatchPairs, matchPairs)
}
@@ -164,7 +127,7 @@ func createServers(conf dataplane.Configuration, generator policies.Generator) (
for idx, s := range conf.SSLServers {
serverID := fmt.Sprintf("SSL_%d", idx)
- sslServer, matchPairs := createSSLServer(s, serverID, generator)
+ sslServer, matchPairs := createSSLServer(s, serverID, generator, keepAliveCheck)
if _, portInUse := sharedTLSPorts[s.Port]; portInUse {
sslServer.Listen = getSocketNameHTTPS(s.Port)
sslServer.IsSocket = true
@@ -180,6 +143,7 @@ func createSSLServer(
virtualServer dataplane.VirtualServer,
serverID string,
generator policies.Generator,
+ keepAliveCheck keepAliveChecker,
) (http.Server, httpMatchPairs) {
listen := fmt.Sprint(virtualServer.Port)
if virtualServer.IsDefault {
@@ -189,7 +153,7 @@ func createSSLServer(
}, nil
}
- locs, matchPairs, grpc := createLocations(&virtualServer, serverID, generator)
+ locs, matchPairs, grpc := createLocations(&virtualServer, serverID, generator, keepAliveCheck)
server := http.Server{
ServerName: virtualServer.Hostname,
@@ -218,6 +182,7 @@ func createServer(
virtualServer dataplane.VirtualServer,
serverID string,
generator policies.Generator,
+ keepAliveCheck keepAliveChecker,
) (http.Server, httpMatchPairs) {
listen := fmt.Sprint(virtualServer.Port)
@@ -228,7 +193,7 @@ func createServer(
}, nil
}
- locs, matchPairs, grpc := createLocations(&virtualServer, serverID, generator)
+ locs, matchPairs, grpc := createLocations(&virtualServer, serverID, generator, keepAliveCheck)
server := http.Server{
ServerName: virtualServer.Hostname,
@@ -264,6 +229,7 @@ func createLocations(
server *dataplane.VirtualServer,
serverID string,
generator policies.Generator,
+ keepAliveCheck keepAliveChecker,
) ([]http.Location, httpMatchPairs, bool) {
maxLocs, pathsAndTypes := getMaxLocationCountAndPathMap(server.PathRules)
locs := make([]http.Location, 0, maxLocs)
@@ -292,7 +258,15 @@ func createLocations(
if !needsInternalLocations(rule) {
for _, r := range rule.MatchRules {
- extLocations = updateLocations(r.Filters, extLocations, r, server.Port, rule.Path, rule.GRPC)
+ extLocations = updateLocations(
+ r.Filters,
+ extLocations,
+ r,
+ server.Port,
+ rule.Path,
+ rule.GRPC,
+ keepAliveCheck,
+ )
}
locs = append(locs, extLocations...)
@@ -314,6 +288,7 @@ func createLocations(
server.Port,
rule.Path,
rule.GRPC,
+ keepAliveCheck,
)
internalLocations = append(internalLocations, intLocation)
@@ -450,6 +425,7 @@ func updateLocation(
listenerPort int32,
path string,
grpc bool,
+ keepAliveCheck keepAliveChecker,
) http.Location {
if filters.InvalidFilter != nil {
location.Return = &http.Return{Code: http.StatusInternalServerError}
@@ -465,7 +441,16 @@ func updateLocation(
}
rewrites := createRewritesValForRewriteFilter(filters.RequestURLRewrite, path)
- proxySetHeaders := generateProxySetHeaders(&matchRule.Filters, grpc)
+
+ extraHeaders := make([]http.Header, 0, 3)
+ if grpc {
+ extraHeaders = append(extraHeaders, grpcAuthorityHeader)
+ } else {
+ extraHeaders = append(extraHeaders, httpUpgradeHeader)
+ extraHeaders = append(extraHeaders, getConnectionHeader(keepAliveCheck, matchRule.BackendGroup.Backends))
+ }
+
+ proxySetHeaders := generateProxySetHeaders(&matchRule.Filters, createBaseProxySetHeaders(extraHeaders...))
responseHeaders := generateResponseHeaders(&matchRule.Filters)
if rewrites != nil {
@@ -502,11 +487,12 @@ func updateLocations(
listenerPort int32,
path string,
grpc bool,
+ keepAliveCheck keepAliveChecker,
) []http.Location {
updatedLocations := make([]http.Location, len(buildLocations))
for i, loc := range buildLocations {
- updatedLocations[i] = updateLocation(filters, loc, matchRule, listenerPort, path, grpc)
+ updatedLocations[i] = updateLocation(filters, loc, matchRule, listenerPort, path, grpc, keepAliveCheck)
}
return updatedLocations
@@ -760,32 +746,26 @@ func createMatchLocation(path string, grpc bool) http.Location {
return loc
}
-func generateProxySetHeaders(filters *dataplane.HTTPFilters, grpc bool) []http.Header {
- var headers []http.Header
- if !grpc {
- headers = make([]http.Header, len(httpBaseHeaders))
- copy(headers, httpBaseHeaders)
- } else {
- headers = make([]http.Header, len(grpcBaseHeaders))
- copy(headers, grpcBaseHeaders)
- }
-
+func generateProxySetHeaders(
+ filters *dataplane.HTTPFilters,
+ baseHeaders []http.Header,
+) []http.Header {
if filters != nil && filters.RequestURLRewrite != nil && filters.RequestURLRewrite.Hostname != nil {
- for i, header := range headers {
+ for i, header := range baseHeaders {
if header.Name == "Host" {
- headers[i].Value = *filters.RequestURLRewrite.Hostname
+ baseHeaders[i].Value = *filters.RequestURLRewrite.Hostname
break
}
}
}
if filters == nil || filters.RequestHeaderModifiers == nil {
- return headers
+ return baseHeaders
}
headerFilter := filters.RequestHeaderModifiers
- headerLen := len(headerFilter.Add) + len(headerFilter.Set) + len(headerFilter.Remove) + len(headers)
+ headerLen := len(headerFilter.Add) + len(headerFilter.Set) + len(headerFilter.Remove) + len(baseHeaders)
proxySetHeaders := make([]http.Header, 0, headerLen)
if len(headerFilter.Add) > 0 {
addHeaders := createHeadersWithVarName(headerFilter.Add)
@@ -803,7 +783,7 @@ func generateProxySetHeaders(filters *dataplane.HTTPFilters, grpc bool) []http.H
})
}
- return append(proxySetHeaders, headers...)
+ return append(proxySetHeaders, baseHeaders...)
}
func generateResponseHeaders(filters *dataplane.HTTPFilters) http.ResponseHeaders {
@@ -887,3 +867,48 @@ func getRewriteClientIPSettings(rewriteIPConfig dataplane.RewriteClientIPSetting
ProxyProtocol: proxyProtocol,
}
}
+
+func createBaseProxySetHeaders(extraHeaders ...http.Header) []http.Header {
+ baseHeaders := []http.Header{
+ {
+ Name: "Host",
+ Value: "$gw_api_compliant_host",
+ },
+ {
+ Name: "X-Forwarded-For",
+ Value: "$proxy_add_x_forwarded_for",
+ },
+ {
+ Name: "X-Real-IP",
+ Value: "$remote_addr",
+ },
+ {
+ Name: "X-Forwarded-Proto",
+ Value: "$scheme",
+ },
+ {
+ Name: "X-Forwarded-Host",
+ Value: "$host",
+ },
+ {
+ Name: "X-Forwarded-Port",
+ Value: "$server_port",
+ },
+ }
+
+ baseHeaders = append(baseHeaders, extraHeaders...)
+
+ return baseHeaders
+}
+
+func getConnectionHeader(keepAliveCheck keepAliveChecker, backends []dataplane.Backend) http.Header {
+ for _, backend := range backends {
+ if keepAliveCheck(backend.UpstreamName) {
+ // if keep-alive settings are enabled on any upstream, the connection header value
+ // must be empty for the location
+ return unsetHTTPConnectionHeader
+ }
+ }
+
+ return httpConnectionHeader
+}
diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go
index 112991b521..24b40d0f60 100644
--- a/internal/mode/static/nginx/config/servers_test.go
+++ b/internal/mode/static/nginx/config/servers_test.go
@@ -17,6 +17,12 @@ import (
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane"
)
+var (
+ httpBaseHeaders = createBaseProxySetHeaders(httpUpgradeHeader, httpConnectionHeader)
+ grpcBaseHeaders = createBaseProxySetHeaders(grpcAuthorityHeader)
+ alwaysFalseKeepAliveChecker = func(_ string) bool { return false }
+)
+
func TestExecuteServers(t *testing.T) {
t.Parallel()
@@ -182,7 +188,7 @@ func TestExecuteServers(t *testing.T) {
)
gen := GeneratorImpl{}
- results := gen.executeServers(conf, fakeGenerator)
+ results := gen.executeServers(conf, fakeGenerator, alwaysFalseKeepAliveChecker)
g.Expect(results).To(HaveLen(len(expectedResults)))
for _, res := range results {
@@ -321,7 +327,8 @@ func TestExecuteServers_IPFamily(t *testing.T) {
g := NewWithT(t)
gen := GeneratorImpl{}
- results := gen.executeServers(test.config, &policiesfakes.FakeGenerator{})
+ results := gen.executeServers(test.config, &policiesfakes.FakeGenerator{}, alwaysFalseKeepAliveChecker)
+
g.Expect(results).To(HaveLen(2))
serverConf := string(results[0].data)
httpMatchConf := string(results[1].data)
@@ -439,7 +446,7 @@ func TestExecuteServers_RewriteClientIP(t *testing.T) {
g := NewWithT(t)
gen := GeneratorImpl{}
- results := gen.executeServers(test.config, &policiesfakes.FakeGenerator{})
+ results := gen.executeServers(test.config, &policiesfakes.FakeGenerator{}, alwaysFalseKeepAliveChecker)
g.Expect(results).To(HaveLen(2))
serverConf := string(results[0].data)
httpMatchConf := string(results[1].data)
@@ -481,7 +488,7 @@ func TestExecuteServers_Plus(t *testing.T) {
g := NewWithT(t)
gen := GeneratorImpl{plus: true}
- results := gen.executeServers(config, &policiesfakes.FakeGenerator{})
+ results := gen.executeServers(config, &policiesfakes.FakeGenerator{}, alwaysFalseKeepAliveChecker)
g.Expect(results).To(HaveLen(2))
serverConf := string(results[0].data)
@@ -565,7 +572,7 @@ func TestExecuteForDefaultServers(t *testing.T) {
g := NewWithT(t)
gen := GeneratorImpl{}
- serverResults := gen.executeServers(tc.conf, &policiesfakes.FakeGenerator{})
+ serverResults := gen.executeServers(tc.conf, &policiesfakes.FakeGenerator{}, alwaysFalseKeepAliveChecker)
g.Expect(serverResults).To(HaveLen(2))
serverConf := string(serverResults[0].data)
httpMatchConf := string(serverResults[1].data)
@@ -649,6 +656,18 @@ func TestCreateServers(t *testing.T) {
},
}
+ keepAliveGroup := dataplane.BackendGroup{
+ Source: hrNsName,
+ RuleIdx: 4,
+ Backends: []dataplane.Backend{
+ {
+ UpstreamName: "test_keep_alive_80",
+ Valid: true,
+ Weight: 1,
+ },
+ },
+ }
+
filterGroup1 := dataplane.BackendGroup{Source: hrNsName, RuleIdx: 3}
filterGroup2 := dataplane.BackendGroup{Source: hrNsName, RuleIdx: 4}
@@ -969,6 +988,16 @@ func TestCreateServers(t *testing.T) {
},
},
},
+ {
+ Path: "/keep-alive-enabled",
+ PathType: dataplane.PathTypeExact,
+ MatchRules: []dataplane.MatchRule{
+ {
+ Match: dataplane.Match{},
+ BackendGroup: keepAliveGroup,
+ },
+ },
+ },
}
conf := dataplane.Configuration{
@@ -1072,14 +1101,6 @@ func TestCreateServers(t *testing.T) {
Name: "X-Forwarded-For",
Value: "$proxy_add_x_forwarded_for",
},
- {
- Name: "Upgrade",
- Value: "$http_upgrade",
- },
- {
- Name: "Connection",
- Value: "$connection_upgrade",
- },
{
Name: "X-Real-IP",
Value: "$remote_addr",
@@ -1096,6 +1117,14 @@ func TestCreateServers(t *testing.T) {
Name: "X-Forwarded-Port",
Value: "$server_port",
},
+ {
+ Name: "Upgrade",
+ Value: "$http_upgrade",
+ },
+ {
+ Name: "Connection",
+ Value: "$connection_upgrade",
+ },
}
externalIncludes := []shared.Include{
@@ -1343,44 +1372,12 @@ func TestCreateServers(t *testing.T) {
{
Path: "/proxy-set-headers/",
ProxyPass: "http://test_foo_80$request_uri",
- ProxySetHeaders: []http.Header{
+ ProxySetHeaders: append([]http.Header{
{
Name: "my-header",
Value: "${my_header_header_var}some-value-123",
},
- {
- Name: "Host",
- Value: "$gw_api_compliant_host",
- },
- {
- Name: "X-Forwarded-For",
- Value: "$proxy_add_x_forwarded_for",
- },
- {
- Name: "Upgrade",
- Value: "$http_upgrade",
- },
- {
- Name: "Connection",
- Value: "$connection_upgrade",
- },
- {
- Name: "X-Real-IP",
- Value: "$remote_addr",
- },
- {
- Name: "X-Forwarded-Proto",
- Value: "$scheme",
- },
- {
- Name: "X-Forwarded-Host",
- Value: "$host",
- },
- {
- Name: "X-Forwarded-Port",
- Value: "$server_port",
- },
- },
+ }, httpBaseHeaders...),
ResponseHeaders: http.ResponseHeaders{
Add: []http.Header{
{
@@ -1397,44 +1394,12 @@ func TestCreateServers(t *testing.T) {
{
Path: "= /proxy-set-headers",
ProxyPass: "http://test_foo_80$request_uri",
- ProxySetHeaders: []http.Header{
+ ProxySetHeaders: append([]http.Header{
{
Name: "my-header",
Value: "${my_header_header_var}some-value-123",
},
- {
- Name: "Host",
- Value: "$gw_api_compliant_host",
- },
- {
- Name: "X-Forwarded-For",
- Value: "$proxy_add_x_forwarded_for",
- },
- {
- Name: "Upgrade",
- Value: "$http_upgrade",
- },
- {
- Name: "Connection",
- Value: "$connection_upgrade",
- },
- {
- Name: "X-Real-IP",
- Value: "$remote_addr",
- },
- {
- Name: "X-Forwarded-Proto",
- Value: "$scheme",
- },
- {
- Name: "X-Forwarded-Host",
- Value: "$host",
- },
- {
- Name: "X-Forwarded-Port",
- Value: "$server_port",
- },
- },
+ }, httpBaseHeaders...),
ResponseHeaders: http.ResponseHeaders{
Add: []http.Header{
{
@@ -1489,6 +1454,13 @@ func TestCreateServers(t *testing.T) {
Type: http.InternalLocationType,
Includes: internalIncludes,
},
+ {
+ Path: "= /keep-alive-enabled",
+ ProxyPass: "http://test_keep_alive_80$request_uri",
+ ProxySetHeaders: createBaseProxySetHeaders(httpUpgradeHeader, unsetHTTPConnectionHeader),
+ Type: http.ExternalLocationType,
+ Includes: externalIncludes,
+ },
}
}
@@ -1541,7 +1513,15 @@ func TestCreateServers(t *testing.T) {
},
})
- result, httpMatchPair := createServers(conf, fakeGenerator)
+ keepAliveEnabledUpstream := http.Upstream{
+ Name: "test_keep_alive_80",
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 1,
+ },
+ }
+ keepAliveCheck := newKeepAliveChecker([]http.Upstream{keepAliveEnabledUpstream})
+
+ result, httpMatchPair := createServers(conf, fakeGenerator, keepAliveCheck)
g.Expect(httpMatchPair).To(Equal(allExpMatchPair))
g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty())
@@ -1762,6 +1742,7 @@ func TestCreateServersConflicts(t *testing.T) {
result, _ := createServers(
dataplane.Configuration{HTTPServers: httpServers},
&policiesfakes.FakeGenerator{},
+ alwaysFalseKeepAliveChecker,
)
g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty())
})
@@ -1912,7 +1893,7 @@ func TestCreateServers_Includes(t *testing.T) {
conf := dataplane.Configuration{HTTPServers: httpServers, SSLServers: sslServers}
- actualServers, matchPairs := createServers(conf, fakeGenerator)
+ actualServers, matchPairs := createServers(conf, fakeGenerator, alwaysFalseKeepAliveChecker)
g.Expect(matchPairs).To(BeEmpty())
g.Expect(actualServers).To(HaveLen(len(expServers)))
@@ -2073,7 +2054,7 @@ func TestCreateLocations_Includes(t *testing.T) {
},
})
- locations, matches, grpc := createLocations(&httpServer, "1", fakeGenerator)
+ locations, matches, grpc := createLocations(&httpServer, "1", fakeGenerator, alwaysFalseKeepAliveChecker)
g := NewWithT(t)
g.Expect(grpc).To(BeFalse())
@@ -2256,10 +2237,15 @@ func TestCreateLocationsRootPath(t *testing.T) {
t.Parallel()
g := NewWithT(t)
- locs, httpMatchPair, grpc := createLocations(&dataplane.VirtualServer{
- PathRules: test.pathRules,
- Port: 80,
- }, "1", &policiesfakes.FakeGenerator{})
+ locs, httpMatchPair, grpc := createLocations(
+ &dataplane.VirtualServer{
+ PathRules: test.pathRules,
+ Port: 80,
+ },
+ "1",
+ &policiesfakes.FakeGenerator{},
+ alwaysFalseKeepAliveChecker,
+ )
g.Expect(locs).To(Equal(test.expLocations))
g.Expect(httpMatchPair).To(BeEmpty())
g.Expect(grpc).To(Equal(test.grpc))
@@ -2897,7 +2883,7 @@ func TestGenerateProxySetHeaders(t *testing.T) {
filters *dataplane.HTTPFilters
msg string
expectedHeaders []http.Header
- GRPC bool
+ baseHeaders []http.Header
}{
{
msg: "header filter",
@@ -2918,7 +2904,7 @@ func TestGenerateProxySetHeaders(t *testing.T) {
Remove: []string{"my-header"},
},
},
- expectedHeaders: []http.Header{
+ expectedHeaders: append([]http.Header{
{
Name: "Authorization",
Value: "${authorization_header_var}my-auth",
@@ -2931,39 +2917,8 @@ func TestGenerateProxySetHeaders(t *testing.T) {
Name: "my-header",
Value: "",
},
- {
- Name: "Host",
- Value: "$gw_api_compliant_host",
- },
- {
- Name: "X-Forwarded-For",
- Value: "$proxy_add_x_forwarded_for",
- },
- {
- Name: "Upgrade",
- Value: "$http_upgrade",
- },
- {
- Name: "Connection",
- Value: "$connection_upgrade",
- },
- {
- Name: "X-Real-IP",
- Value: "$remote_addr",
- },
- {
- Name: "X-Forwarded-Proto",
- Value: "$scheme",
- },
- {
- Name: "X-Forwarded-Host",
- Value: "$host",
- },
- {
- Name: "X-Forwarded-Port",
- Value: "$server_port",
- },
- },
+ }, httpBaseHeaders...),
+ baseHeaders: httpBaseHeaders,
},
{
msg: "with url rewrite hostname",
@@ -2993,14 +2948,6 @@ func TestGenerateProxySetHeaders(t *testing.T) {
Name: "X-Forwarded-For",
Value: "$proxy_add_x_forwarded_for",
},
- {
- Name: "Upgrade",
- Value: "$http_upgrade",
- },
- {
- Name: "Connection",
- Value: "$connection_upgrade",
- },
{
Name: "X-Real-IP",
Value: "$remote_addr",
@@ -3017,11 +2964,19 @@ func TestGenerateProxySetHeaders(t *testing.T) {
Name: "X-Forwarded-Port",
Value: "$server_port",
},
+ {
+ Name: "Upgrade",
+ Value: "$http_upgrade",
+ },
+ {
+ Name: "Connection",
+ Value: "$connection_upgrade",
+ },
},
+ baseHeaders: createBaseProxySetHeaders(httpUpgradeHeader, httpConnectionHeader),
},
{
- msg: "header filter with gRPC",
- GRPC: true,
+ msg: "header filter with gRPC",
filters: &dataplane.HTTPFilters{
RequestHeaderModifiers: &dataplane.HTTPHeaderFilter{
Add: []dataplane.HTTPHeader{
@@ -3039,7 +2994,7 @@ func TestGenerateProxySetHeaders(t *testing.T) {
Remove: []string{"my-header"},
},
},
- expectedHeaders: []http.Header{
+ expectedHeaders: append([]http.Header{
{
Name: "Authorization",
Value: "${authorization_header_var}my-auth",
@@ -3052,33 +3007,207 @@ func TestGenerateProxySetHeaders(t *testing.T) {
Name: "my-header",
Value: "",
},
+ }, grpcBaseHeaders...),
+ baseHeaders: grpcBaseHeaders,
+ },
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.msg, func(t *testing.T) {
+ t.Parallel()
+ g := NewWithT(t)
+
+ headers := generateProxySetHeaders(tc.filters, tc.baseHeaders)
+ g.Expect(headers).To(Equal(tc.expectedHeaders))
+ })
+ }
+}
+
+func TestCreateBaseProxySetHeaders(t *testing.T) {
+ t.Parallel()
+
+ expBaseHeaders := []http.Header{
+ {
+ Name: "Host",
+ Value: "$gw_api_compliant_host",
+ },
+ {
+ Name: "X-Forwarded-For",
+ Value: "$proxy_add_x_forwarded_for",
+ },
+ {
+ Name: "X-Real-IP",
+ Value: "$remote_addr",
+ },
+ {
+ Name: "X-Forwarded-Proto",
+ Value: "$scheme",
+ },
+ {
+ Name: "X-Forwarded-Host",
+ Value: "$host",
+ },
+ {
+ Name: "X-Forwarded-Port",
+ Value: "$server_port",
+ },
+ }
+
+ tests := []struct {
+ msg string
+ additionalHeaders []http.Header
+ expBaseHeaders []http.Header
+ }{
+ {
+ msg: "no additional headers",
+ additionalHeaders: []http.Header{},
+ expBaseHeaders: expBaseHeaders,
+ },
+ {
+ msg: "single additional headers",
+ additionalHeaders: []http.Header{
+ grpcAuthorityHeader,
+ },
+ expBaseHeaders: append(expBaseHeaders, grpcAuthorityHeader),
+ },
+ {
+ msg: "multiple additional headers",
+ additionalHeaders: []http.Header{
+ httpConnectionHeader,
+ httpUpgradeHeader,
+ },
+ expBaseHeaders: append(expBaseHeaders, httpConnectionHeader, httpUpgradeHeader),
+ },
+ {
+ msg: "unset connection header and upgrade header",
+ additionalHeaders: []http.Header{
+ unsetHTTPConnectionHeader,
+ httpUpgradeHeader,
+ },
+ expBaseHeaders: append(expBaseHeaders, unsetHTTPConnectionHeader, httpUpgradeHeader),
+ },
+ }
+
+ for _, test := range tests {
+ t.Run(test.msg, func(t *testing.T) {
+ t.Parallel()
+ g := NewWithT(t)
+
+ result := createBaseProxySetHeaders(test.additionalHeaders...)
+ g.Expect(result).To(Equal(test.expBaseHeaders))
+ })
+ }
+}
+
+func TestGetConnectionHeader(t *testing.T) {
+ t.Parallel()
+
+ tests := []struct {
+ msg string
+ upstreams []http.Upstream
+ expConnectionHeader http.Header
+ backends []dataplane.Backend
+ }{
+ {
+ msg: "no upstreams with keepAlive enabled",
+ upstreams: []http.Upstream{
+ {
+ Name: "upstream1",
+ },
{
- Name: "Host",
- Value: "$gw_api_compliant_host",
+ Name: "upstream2",
},
{
- Name: "X-Forwarded-For",
- Value: "$proxy_add_x_forwarded_for",
+ Name: "upstream3",
},
+ },
+ backends: []dataplane.Backend{
{
- Name: "Authority",
- Value: "$gw_api_compliant_host",
+ UpstreamName: "upstream1",
},
{
- Name: "X-Real-IP",
- Value: "$remote_addr",
+ UpstreamName: "upstream2",
},
{
- Name: "X-Forwarded-Proto",
- Value: "$scheme",
+ UpstreamName: "upstream3",
},
+ },
+ expConnectionHeader: httpConnectionHeader,
+ },
+ {
+ msg: "upstream with keepAlive enabled",
+ upstreams: []http.Upstream{
{
- Name: "X-Forwarded-Host",
- Value: "$host",
+ Name: "upstream",
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 1,
+ },
},
+ },
+ backends: []dataplane.Backend{
{
- Name: "X-Forwarded-Port",
- Value: "$server_port",
+ UpstreamName: "upstream",
+ },
+ },
+ expConnectionHeader: unsetHTTPConnectionHeader,
+ },
+ {
+ msg: "multiple upstreams with keepAlive enabled",
+ upstreams: []http.Upstream{
+ {
+ Name: "upstream1",
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 1,
+ },
+ },
+ {
+ Name: "upstream2",
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 2,
+ Requests: 1,
+ },
+ },
+ {
+ Name: "upstream3",
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 3,
+ Time: "5s",
+ },
+ },
+ },
+ backends: []dataplane.Backend{
+ {
+ UpstreamName: "upstream1",
+ },
+ {
+ UpstreamName: "upstream2",
+ },
+ {
+ UpstreamName: "upstream3",
+ },
+ },
+ expConnectionHeader: unsetHTTPConnectionHeader,
+ },
+ {
+ msg: "mix of upstreams with keepAlive enabled and disabled",
+ expConnectionHeader: unsetHTTPConnectionHeader,
+ upstreams: []http.Upstream{
+ {
+ Name: "upstream1",
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 1,
+ },
+ },
+ {
+ Name: "upstream2",
+ },
+ },
+ backends: []dataplane.Backend{
+ {
+ UpstreamName: "upstream1",
+ },
+ {
+ UpstreamName: "upstream2",
},
},
},
@@ -3089,8 +3218,10 @@ func TestGenerateProxySetHeaders(t *testing.T) {
t.Parallel()
g := NewWithT(t)
- headers := generateProxySetHeaders(tc.filters, tc.GRPC)
- g.Expect(headers).To(Equal(tc.expectedHeaders))
+ keepAliveCheck := newKeepAliveChecker(tc.upstreams)
+
+ connectionHeader := getConnectionHeader(keepAliveCheck, tc.backends)
+ g.Expect(connectionHeader).To(Equal(tc.expConnectionHeader))
})
}
}
diff --git a/internal/mode/static/nginx/config/upstreams.go b/internal/mode/static/nginx/config/upstreams.go
index 51af6f4f4b..bcee82c68f 100644
--- a/internal/mode/static/nginx/config/upstreams.go
+++ b/internal/mode/static/nginx/config/upstreams.go
@@ -6,11 +6,15 @@ import (
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/http"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/upstreamsettings"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/stream"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane"
)
-var upstreamsTemplate = gotemplate.Must(gotemplate.New("upstreams").Parse(upstreamsTemplateText))
+var (
+ upstreamsTemplate = gotemplate.Must(gotemplate.New("upstreams").Parse(upstreamsTemplateText))
+ streamUpstreamsTemplate = gotemplate.Must(gotemplate.New("streamUpstreams").Parse(streamUpstreamsTemplateText))
+)
const (
// nginx503Server is used as a backend for services that cannot be resolved (have no IP address).
@@ -31,9 +35,32 @@ const (
stateDir = "/var/lib/nginx/state"
)
-func (g GeneratorImpl) executeUpstreams(conf dataplane.Configuration) []executeResult {
- upstreams := g.createUpstreams(conf.Upstreams)
+// keepAliveChecker takes an upstream name and returns if it has keep alive settings enabled.
+type keepAliveChecker func(upstreamName string) bool
+
+func newKeepAliveChecker(upstreams []http.Upstream) keepAliveChecker {
+ upstreamMap := make(map[string]http.Upstream)
+
+ for _, upstream := range upstreams {
+ upstreamMap[upstream.Name] = upstream
+ }
+
+ return func(upstreamName string) bool {
+ if upstream, exists := upstreamMap[upstreamName]; exists {
+ return upstream.KeepAlive.Connections != 0
+ }
+
+ return false
+ }
+}
+
+func newExecuteUpstreamsFunc(upstreams []http.Upstream) executeFunc {
+ return func(_ dataplane.Configuration) []executeResult {
+ return executeUpstreams(upstreams)
+ }
+}
+func executeUpstreams(upstreams []http.Upstream) []executeResult {
result := executeResult{
dest: httpConfigFile,
data: helpers.MustExecuteTemplate(upstreamsTemplate, upstreams),
@@ -47,7 +74,7 @@ func (g GeneratorImpl) executeStreamUpstreams(conf dataplane.Configuration) []ex
result := executeResult{
dest: streamConfigFile,
- data: helpers.MustExecuteTemplate(upstreamsTemplate, upstreams),
+ data: helpers.MustExecuteTemplate(streamUpstreamsTemplate, upstreams),
}
return []executeResult{result}
@@ -92,12 +119,15 @@ func (g GeneratorImpl) createStreamUpstream(up dataplane.Upstream) stream.Upstre
}
}
-func (g GeneratorImpl) createUpstreams(upstreams []dataplane.Upstream) []http.Upstream {
+func (g GeneratorImpl) createUpstreams(
+ upstreams []dataplane.Upstream,
+ processor upstreamsettings.Processor,
+) []http.Upstream {
// capacity is the number of upstreams + 1 for the invalid backend ref upstream
ups := make([]http.Upstream, 0, len(upstreams)+1)
for _, u := range upstreams {
- ups = append(ups, g.createUpstream(u))
+ ups = append(ups, g.createUpstream(u, processor))
}
ups = append(ups, createInvalidBackendRefUpstream())
@@ -105,14 +135,23 @@ func (g GeneratorImpl) createUpstreams(upstreams []dataplane.Upstream) []http.Up
return ups
}
-func (g GeneratorImpl) createUpstream(up dataplane.Upstream) http.Upstream {
+func (g GeneratorImpl) createUpstream(
+ up dataplane.Upstream,
+ processor upstreamsettings.Processor,
+) http.Upstream {
var stateFile string
+ upstreamPolicySettings := processor.Process(up.Policies)
+
zoneSize := ossZoneSize
if g.plus {
zoneSize = plusZoneSize
stateFile = fmt.Sprintf("%s/%s.conf", stateDir, up.Name)
}
+ if upstreamPolicySettings.ZoneSize != "" {
+ zoneSize = upstreamPolicySettings.ZoneSize
+ }
+
if len(up.Endpoints) == 0 {
return http.Upstream{
Name: up.Name,
@@ -142,6 +181,7 @@ func (g GeneratorImpl) createUpstream(up dataplane.Upstream) http.Upstream {
ZoneSize: zoneSize,
StateFile: stateFile,
Servers: upstreamServers,
+ KeepAlive: upstreamPolicySettings.KeepAlive,
}
}
diff --git a/internal/mode/static/nginx/config/upstreams_template.go b/internal/mode/static/nginx/config/upstreams_template.go
index 40d5740ad0..e4bcaa69ad 100644
--- a/internal/mode/static/nginx/config/upstreams_template.go
+++ b/internal/mode/static/nginx/config/upstreams_template.go
@@ -5,6 +5,8 @@ package config
// NGINX Plus needs 1m to support roughly the same amount of http servers (556 upstream servers).
// For stream upstream servers, 512k will support 576 in OSS and 1m will support 991 in NGINX Plus
// https://github.com/nginxinc/nginx-gateway-fabric/issues/483
+//
+// if the keepalive directive is present, it is necessary to activate the load balancing method before the directive.
const upstreamsTemplateText = `
{{ range $u := . }}
upstream {{ $u.Name }} {
@@ -20,6 +22,36 @@ upstream {{ $u.Name }} {
server {{ $server.Address }};
{{- end }}
{{- end }}
+ {{ if $u.KeepAlive.Connections -}}
+ keepalive {{ $u.KeepAlive.Connections }};
+ {{- end }}
+ {{ if $u.KeepAlive.Requests -}}
+ keepalive_requests {{ $u.KeepAlive.Requests }};
+ {{- end }}
+ {{ if $u.KeepAlive.Time -}}
+ keepalive_time {{ $u.KeepAlive.Time }};
+ {{- end }}
+ {{ if $u.KeepAlive.Timeout -}}
+ keepalive_timeout {{ $u.KeepAlive.Timeout }};
+ {{- end }}
+}
+{{ end -}}
+`
+
+const streamUpstreamsTemplateText = `
+{{ range $u := . }}
+upstream {{ $u.Name }} {
+ random two least_conn;
+ {{ if $u.ZoneSize -}}
+ zone {{ $u.Name }} {{ $u.ZoneSize }};
+ {{- end }}
+ {{- if $u.StateFile }}
+ state {{ $u.StateFile }};
+ {{- else }}
+ {{ range $server := $u.Servers }}
+ server {{ $server.Address }};
+ {{- end }}
+ {{- end }}
}
{{ end -}}
`
diff --git a/internal/mode/static/nginx/config/upstreams_test.go b/internal/mode/static/nginx/config/upstreams_test.go
index f2e5b1071b..9d6cfdf208 100644
--- a/internal/mode/static/nginx/config/upstreams_test.go
+++ b/internal/mode/static/nginx/config/upstreams_test.go
@@ -4,8 +4,13 @@ import (
"testing"
. "github.com/onsi/gomega"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+ ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/http"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/upstreamsettings"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/stream"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/resolver"
@@ -47,6 +52,32 @@ func TestExecuteUpstreams(t *testing.T) {
},
},
},
+ {
+ Name: "up5-usp",
+ Endpoints: []resolver.Endpoint{
+ {
+ Address: "12.0.0.0",
+ Port: 80,
+ },
+ },
+ Policies: []policies.Policy{
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ ZoneSize: helpers.GetPointer[ngfAPI.Size]("2m"),
+ KeepAlive: helpers.GetPointer(ngfAPI.UpstreamKeepAlive{
+ Connections: helpers.GetPointer(int32(1)),
+ Requests: helpers.GetPointer(int32(1)),
+ Time: helpers.GetPointer[ngfAPI.Duration]("5s"),
+ Timeout: helpers.GetPointer[ngfAPI.Duration]("10s"),
+ }),
+ },
+ },
+ },
+ },
}
expectedSubStrings := []string{
@@ -54,21 +85,32 @@ func TestExecuteUpstreams(t *testing.T) {
"upstream up2",
"upstream up3",
"upstream up4-ipv6",
+ "upstream up5-usp",
"upstream invalid-backend-ref",
+
"server 10.0.0.0:80;",
"server 11.0.0.0:80;",
"server [2001:db8::1]:80",
+ "server 12.0.0.0:80;",
"server unix:/var/run/nginx/nginx-503-server.sock;",
+
+ "keepalive 1;",
+ "keepalive_requests 1;",
+ "keepalive_time 5s;",
+ "keepalive_timeout 10s;",
+ "zone up5-usp 2m;",
}
- upstreamResults := gen.executeUpstreams(dataplane.Configuration{Upstreams: stateUpstreams})
+ upstreams := gen.createUpstreams(stateUpstreams, upstreamsettings.NewProcessor())
+
+ upstreamResults := executeUpstreams(upstreams)
g := NewWithT(t)
g.Expect(upstreamResults).To(HaveLen(1))
- upstreams := string(upstreamResults[0].data)
+ nginxUpstreams := string(upstreamResults[0].data)
g.Expect(upstreamResults[0].dest).To(Equal(httpConfigFile))
for _, expSubString := range expectedSubStrings {
- g.Expect(upstreams).To(ContainSubstring(expSubString))
+ g.Expect(nginxUpstreams).To(ContainSubstring(expSubString))
}
}
@@ -116,6 +158,32 @@ func TestCreateUpstreams(t *testing.T) {
},
},
},
+ {
+ Name: "up5-usp",
+ Endpoints: []resolver.Endpoint{
+ {
+ Address: "12.0.0.0",
+ Port: 80,
+ },
+ },
+ Policies: []policies.Policy{
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ ZoneSize: helpers.GetPointer[ngfAPI.Size]("2m"),
+ KeepAlive: helpers.GetPointer(ngfAPI.UpstreamKeepAlive{
+ Connections: helpers.GetPointer(int32(1)),
+ Requests: helpers.GetPointer(int32(1)),
+ Time: helpers.GetPointer[ngfAPI.Duration]("5s"),
+ Timeout: helpers.GetPointer[ngfAPI.Duration]("10s"),
+ }),
+ },
+ },
+ },
+ },
}
expUpstreams := []http.Upstream{
@@ -161,6 +229,21 @@ func TestCreateUpstreams(t *testing.T) {
},
},
},
+ {
+ Name: "up5-usp",
+ ZoneSize: "2m",
+ Servers: []http.UpstreamServer{
+ {
+ Address: "12.0.0.0:80",
+ },
+ },
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 1,
+ Requests: 1,
+ Time: "5s",
+ Timeout: "10s",
+ },
+ },
{
Name: invalidBackendRef,
Servers: []http.UpstreamServer{
@@ -172,7 +255,7 @@ func TestCreateUpstreams(t *testing.T) {
}
g := NewWithT(t)
- result := gen.createUpstreams(stateUpstreams)
+ result := gen.createUpstreams(stateUpstreams, upstreamsettings.NewProcessor())
g.Expect(result).To(Equal(expUpstreams))
}
@@ -181,8 +264,8 @@ func TestCreateUpstream(t *testing.T) {
gen := GeneratorImpl{}
tests := []struct {
msg string
- stateUpstream dataplane.Upstream
expectedUpstream http.Upstream
+ stateUpstream dataplane.Upstream
}{
{
stateUpstream: dataplane.Upstream{
@@ -273,13 +356,183 @@ func TestCreateUpstream(t *testing.T) {
},
msg: "endpoint ipv6",
},
+ {
+ stateUpstream: dataplane.Upstream{
+ Name: "single upstreamSettingsPolicy",
+ Endpoints: []resolver.Endpoint{
+ {
+ Address: "10.0.0.1",
+ Port: 80,
+ },
+ },
+ Policies: []policies.Policy{
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ ZoneSize: helpers.GetPointer[ngfAPI.Size]("2m"),
+ KeepAlive: helpers.GetPointer(ngfAPI.UpstreamKeepAlive{
+ Connections: helpers.GetPointer(int32(1)),
+ Requests: helpers.GetPointer(int32(1)),
+ Time: helpers.GetPointer[ngfAPI.Duration]("5s"),
+ Timeout: helpers.GetPointer[ngfAPI.Duration]("10s"),
+ }),
+ },
+ },
+ },
+ },
+ expectedUpstream: http.Upstream{
+ Name: "single upstreamSettingsPolicy",
+ ZoneSize: "2m",
+ Servers: []http.UpstreamServer{
+ {
+ Address: "10.0.0.1:80",
+ },
+ },
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 1,
+ Requests: 1,
+ Time: "5s",
+ Timeout: "10s",
+ },
+ },
+ msg: "single upstreamSettingsPolicy",
+ },
+ {
+ stateUpstream: dataplane.Upstream{
+ Name: "multiple upstreamSettingsPolicies",
+ Endpoints: []resolver.Endpoint{
+ {
+ Address: "10.0.0.1",
+ Port: 80,
+ },
+ },
+ Policies: []policies.Policy{
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp1",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ ZoneSize: helpers.GetPointer[ngfAPI.Size]("2m"),
+ KeepAlive: helpers.GetPointer(ngfAPI.UpstreamKeepAlive{
+ Time: helpers.GetPointer[ngfAPI.Duration]("5s"),
+ Timeout: helpers.GetPointer[ngfAPI.Duration]("10s"),
+ }),
+ },
+ },
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp2",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ KeepAlive: helpers.GetPointer(ngfAPI.UpstreamKeepAlive{
+ Connections: helpers.GetPointer(int32(1)),
+ Requests: helpers.GetPointer(int32(1)),
+ }),
+ },
+ },
+ },
+ },
+ expectedUpstream: http.Upstream{
+ Name: "multiple upstreamSettingsPolicies",
+ ZoneSize: "2m",
+ Servers: []http.UpstreamServer{
+ {
+ Address: "10.0.0.1:80",
+ },
+ },
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 1,
+ Requests: 1,
+ Time: "5s",
+ Timeout: "10s",
+ },
+ },
+ msg: "multiple upstreamSettingsPolicies",
+ },
+ {
+ stateUpstream: dataplane.Upstream{
+ Name: "empty upstreamSettingsPolicies",
+ Endpoints: []resolver.Endpoint{
+ {
+ Address: "10.0.0.1",
+ Port: 80,
+ },
+ },
+ Policies: []policies.Policy{
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp1",
+ Namespace: "test",
+ },
+ },
+ },
+ },
+ expectedUpstream: http.Upstream{
+ Name: "empty upstreamSettingsPolicies",
+ ZoneSize: ossZoneSize,
+ Servers: []http.UpstreamServer{
+ {
+ Address: "10.0.0.1:80",
+ },
+ },
+ },
+ msg: "empty upstreamSettingsPolicies",
+ },
+ {
+ stateUpstream: dataplane.Upstream{
+ Name: "upstreamSettingsPolicy with only keep alive settings",
+ Endpoints: []resolver.Endpoint{
+ {
+ Address: "10.0.0.1",
+ Port: 80,
+ },
+ },
+ Policies: []policies.Policy{
+ &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp1",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ KeepAlive: helpers.GetPointer(ngfAPI.UpstreamKeepAlive{
+ Connections: helpers.GetPointer(int32(1)),
+ Requests: helpers.GetPointer(int32(1)),
+ Time: helpers.GetPointer[ngfAPI.Duration]("5s"),
+ Timeout: helpers.GetPointer[ngfAPI.Duration]("10s"),
+ }),
+ },
+ },
+ },
+ },
+ expectedUpstream: http.Upstream{
+ Name: "upstreamSettingsPolicy with only keep alive settings",
+ ZoneSize: ossZoneSize,
+ Servers: []http.UpstreamServer{
+ {
+ Address: "10.0.0.1:80",
+ },
+ },
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 1,
+ Requests: 1,
+ Time: "5s",
+ Timeout: "10s",
+ },
+ },
+ msg: "upstreamSettingsPolicy with only keep alive settings",
+ },
}
for _, test := range tests {
t.Run(test.msg, func(t *testing.T) {
t.Parallel()
g := NewWithT(t)
- result := gen.createUpstream(test.stateUpstream)
+ result := gen.createUpstream(test.stateUpstream, upstreamsettings.NewProcessor())
g.Expect(result).To(Equal(test.expectedUpstream))
})
}
@@ -339,7 +592,7 @@ func TestCreateUpstreamPlus(t *testing.T) {
t.Run(test.msg, func(t *testing.T) {
t.Parallel()
g := NewWithT(t)
- result := gen.createUpstream(test.stateUpstream)
+ result := gen.createUpstream(test.stateUpstream, upstreamsettings.NewProcessor())
g.Expect(result).To(Equal(test.expectedUpstream))
})
}
@@ -537,3 +790,215 @@ func TestCreateStreamUpstreamPlus(t *testing.T) {
g := NewWithT(t)
g.Expect(result).To(Equal(expectedUpstream))
}
+
+func TestKeepAliveChecker(t *testing.T) {
+ t.Parallel()
+
+ tests := []struct {
+ msg string
+ upstreams []http.Upstream
+ expKeepAliveEnabled []bool
+ }{
+ {
+ msg: "upstream with all keepAlive fields set",
+ upstreams: []http.Upstream{
+ {
+ Name: "upAllKeepAliveFieldsSet",
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 1,
+ Requests: 1,
+ Time: "5s",
+ Timeout: "10s",
+ },
+ },
+ },
+ expKeepAliveEnabled: []bool{
+ true,
+ },
+ },
+ {
+ msg: "upstream with keepAlive connection field set",
+ upstreams: []http.Upstream{
+ {
+ Name: "upKeepAliveConnectionsSet",
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 1,
+ },
+ },
+ },
+ expKeepAliveEnabled: []bool{
+ true,
+ },
+ },
+ {
+ msg: "upstream with keepAlive requests field set",
+ upstreams: []http.Upstream{
+ {
+ Name: "upKeepAliveRequestsSet",
+ KeepAlive: http.UpstreamKeepAlive{
+ Requests: 1,
+ },
+ },
+ },
+ expKeepAliveEnabled: []bool{
+ false,
+ },
+ },
+ {
+ msg: "upstream with keepAlive time field set",
+ upstreams: []http.Upstream{
+ {
+ Name: "upKeepAliveTimeSet",
+ KeepAlive: http.UpstreamKeepAlive{
+ Time: "5s",
+ },
+ },
+ },
+ expKeepAliveEnabled: []bool{
+ false,
+ },
+ },
+ {
+ msg: "upstream with keepAlive timeout field set",
+ upstreams: []http.Upstream{
+ {
+ Name: "upKeepAliveTimeoutSet",
+ KeepAlive: http.UpstreamKeepAlive{
+ Timeout: "10s",
+ },
+ },
+ },
+ expKeepAliveEnabled: []bool{
+ false,
+ },
+ },
+ {
+ msg: "upstream with no keepAlive fields set",
+ upstreams: []http.Upstream{
+ {
+ Name: "upNoKeepAliveFieldsSet",
+ },
+ },
+ expKeepAliveEnabled: []bool{
+ false,
+ },
+ },
+ {
+ msg: "upstream with keepAlive fields set to empty values",
+ upstreams: []http.Upstream{
+ {
+ Name: "upKeepAliveFieldsEmpty",
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 0,
+ Requests: 0,
+ Time: "",
+ Timeout: "",
+ },
+ },
+ },
+ expKeepAliveEnabled: []bool{
+ false,
+ },
+ },
+ {
+ msg: "multiple upstreams with keepAlive fields set",
+ upstreams: []http.Upstream{
+ {
+ Name: "upstream1",
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 1,
+ Requests: 1,
+ Time: "5s",
+ Timeout: "10s",
+ },
+ },
+ {
+ Name: "upstream2",
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 1,
+ Requests: 1,
+ Time: "5s",
+ Timeout: "10s",
+ },
+ },
+ {
+ Name: "upstream3",
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 1,
+ Requests: 1,
+ Time: "5s",
+ Timeout: "10s",
+ },
+ },
+ },
+ expKeepAliveEnabled: []bool{
+ true,
+ true,
+ true,
+ },
+ },
+ {
+ msg: "mix of keepAlive enabled upstreams and disabled upstreams",
+ upstreams: []http.Upstream{
+ {
+ Name: "upstream1",
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 1,
+ Requests: 1,
+ Time: "5s",
+ Timeout: "10s",
+ },
+ },
+ {
+ Name: "upstream2",
+ },
+ {
+ Name: "upstream3",
+ KeepAlive: http.UpstreamKeepAlive{
+ Connections: 1,
+ Requests: 1,
+ Time: "5s",
+ Timeout: "10s",
+ },
+ },
+ },
+ expKeepAliveEnabled: []bool{
+ true,
+ false,
+ true,
+ },
+ },
+ {
+ msg: "all upstreams without keepAlive fields set",
+ upstreams: []http.Upstream{
+ {
+ Name: "upstream1",
+ },
+ {
+ Name: "upstream2",
+ },
+ {
+ Name: "upstream3",
+ },
+ },
+ expKeepAliveEnabled: []bool{
+ false,
+ false,
+ false,
+ },
+ },
+ }
+
+ for _, test := range tests {
+ t.Run(test.msg, func(t *testing.T) {
+ t.Parallel()
+ g := NewWithT(t)
+
+ keepAliveCheck := newKeepAliveChecker(test.upstreams)
+
+ for index, upstream := range test.upstreams {
+ g.Expect(keepAliveCheck(upstream.Name)).To(Equal(test.expKeepAliveEnabled[index]))
+ }
+ })
+ }
+}
diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go
index f9ed79b762..3d98d1d229 100644
--- a/internal/mode/static/state/dataplane/types.go
+++ b/internal/mode/static/state/dataplane/types.go
@@ -111,6 +111,8 @@ type Upstream struct {
ErrorMsg string
// Endpoints are the endpoints of the Upstream.
Endpoints []resolver.Endpoint
+ // Policies contains the list of policies that are applied to this Upstream.
+ Policies []policies.Policy
}
// SSL is the SSL configuration for a server.
From 31f8ccb0e8ff0a820aff83c98ad70537aa01f23e Mon Sep 17 00:00:00 2001
From: Kate Osborn <50597707+kate-osborn@users.noreply.github.com>
Date: Fri, 13 Dec 2024 14:05:32 -0700
Subject: [PATCH 3/7] Watch UpstreamSettingsPolicies and translate into
dataplane configuration (#2887)
Problem: As a userI want NGF to take my configuration for an UpstreamSettingsPolicy
and transform it into data plane configuration within NGF, so that NGF can then translate
those settings into NGINX configuration, and so that NGF maintains an abstraction layer
between data plane configuration and the specific data plane NGF uses.
Solution: Add controller to watch UpstreamSettingsPolicies, and store them in the cluster state
as generic NGF Policies. Update the graph to validate and process these policies and attach
them to the relevant Services. When building the dataplane configuration, store the policies
on the relevant http upstreams.
---
.../templates/clusterrole.yaml | 2 +
config/crd/kustomization.yaml | 1 +
deploy/aws-nlb/deploy.yaml | 2 +
deploy/azure/deploy.yaml | 2 +
deploy/crds.yaml | 444 ++++++++++++++++++
deploy/default/deploy.yaml | 2 +
deploy/experimental-nginx-plus/deploy.yaml | 2 +
deploy/experimental/deploy.yaml | 2 +
deploy/nginx-plus/deploy.yaml | 2 +
deploy/nodeport/deploy.yaml | 2 +
deploy/openshift/deploy.yaml | 2 +
.../snippets-filters-nginx-plus/deploy.yaml | 2 +
deploy/snippets-filters/deploy.yaml | 2 +
.../upstream-settings-policy/cafe-routes.yaml | 37 ++
examples/upstream-settings-policy/cafe.yaml | 65 +++
.../upstream-settings-policy/gateway.yaml | 11 +
.../upstream-settings-policy.yaml | 2 +-
internal/framework/kinds/kinds.go | 12 +-
internal/mode/static/manager.go | 12 +
internal/mode/static/manager_test.go | 5 +
.../policies/clientsettings/validator.go | 4 +-
.../policies/observability/validator.go | 4 +-
.../static/nginx/config/policies/policy.go | 9 +-
.../policies/upstreamsettings/validator.go | 125 +++++
.../upstreamsettings/validator_test.go | 266 +++++++++++
.../mode/static/state/change_processor.go | 5 +
.../static/state/change_processor_test.go | 127 +++--
.../static/state/dataplane/configuration.go | 16 +-
.../state/dataplane/configuration_test.go | 63 ++-
internal/mode/static/state/dataplane/types.go | 2 +-
internal/mode/static/state/graph/graph.go | 22 +-
.../mode/static/state/graph/graph_test.go | 46 +-
internal/mode/static/state/graph/policies.go | 95 +++-
.../mode/static/state/graph/policies_test.go | 350 ++++++++++----
internal/mode/static/state/graph/service.go | 52 +-
.../mode/static/state/graph/service_test.go | 143 +++---
.../mode/static/telemetry/collector_test.go | 4 +-
37 files changed, 1699 insertions(+), 245 deletions(-)
create mode 100644 examples/upstream-settings-policy/cafe-routes.yaml
create mode 100644 examples/upstream-settings-policy/cafe.yaml
create mode 100644 examples/upstream-settings-policy/gateway.yaml
create mode 100644 internal/mode/static/nginx/config/policies/upstreamsettings/validator.go
create mode 100644 internal/mode/static/nginx/config/policies/upstreamsettings/validator_test.go
diff --git a/charts/nginx-gateway-fabric/templates/clusterrole.yaml b/charts/nginx-gateway-fabric/templates/clusterrole.yaml
index cbb163ae1d..9ee1be4254 100644
--- a/charts/nginx-gateway-fabric/templates/clusterrole.yaml
+++ b/charts/nginx-gateway-fabric/templates/clusterrole.yaml
@@ -104,6 +104,7 @@ rules:
- nginxproxies
- clientsettingspolicies
- observabilitypolicies
+ - upstreamsettingspolicies
{{- if .Values.nginxGateway.snippetsFilters.enable }}
- snippetsfilters
{{- end }}
@@ -116,6 +117,7 @@ rules:
- nginxgateways/status
- clientsettingspolicies/status
- observabilitypolicies/status
+ - upstreamsettingspolicies/status
{{- if .Values.nginxGateway.snippetsFilters.enable }}
- snippetsfilters/status
{{- end }}
diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml
index 0258b9baaa..b067d64141 100644
--- a/config/crd/kustomization.yaml
+++ b/config/crd/kustomization.yaml
@@ -6,3 +6,4 @@ resources:
- bases/gateway.nginx.org_nginxproxies.yaml
- bases/gateway.nginx.org_observabilitypolicies.yaml
- bases/gateway.nginx.org_snippetsfilters.yaml
+ - bases/gateway.nginx.org_upstreamsettingspolicies.yaml
diff --git a/deploy/aws-nlb/deploy.yaml b/deploy/aws-nlb/deploy.yaml
index 6e4ff4865f..018cf36107 100644
--- a/deploy/aws-nlb/deploy.yaml
+++ b/deploy/aws-nlb/deploy.yaml
@@ -98,6 +98,7 @@ rules:
- nginxproxies
- clientsettingspolicies
- observabilitypolicies
+ - upstreamsettingspolicies
verbs:
- list
- watch
@@ -107,6 +108,7 @@ rules:
- nginxgateways/status
- clientsettingspolicies/status
- observabilitypolicies/status
+ - upstreamsettingspolicies/status
verbs:
- update
- apiGroups:
diff --git a/deploy/azure/deploy.yaml b/deploy/azure/deploy.yaml
index 06ba7a02ae..a8dc451b7e 100644
--- a/deploy/azure/deploy.yaml
+++ b/deploy/azure/deploy.yaml
@@ -98,6 +98,7 @@ rules:
- nginxproxies
- clientsettingspolicies
- observabilitypolicies
+ - upstreamsettingspolicies
verbs:
- list
- watch
@@ -107,6 +108,7 @@ rules:
- nginxgateways/status
- clientsettingspolicies/status
- observabilitypolicies/status
+ - upstreamsettingspolicies/status
verbs:
- update
- apiGroups:
diff --git a/deploy/crds.yaml b/deploy/crds.yaml
index 83fe910a61..5a9d708c60 100644
--- a/deploy/crds.yaml
+++ b/deploy/crds.yaml
@@ -1482,3 +1482,447 @@ spec:
storage: true
subresources:
status: {}
+---
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+ annotations:
+ controller-gen.kubebuilder.io/version: v0.16.5
+ labels:
+ gateway.networking.k8s.io/policy: direct
+ name: upstreamsettingspolicies.gateway.nginx.org
+spec:
+ group: gateway.nginx.org
+ names:
+ categories:
+ - nginx-gateway-fabric
+ kind: UpstreamSettingsPolicy
+ listKind: UpstreamSettingsPolicyList
+ plural: upstreamsettingspolicies
+ shortNames:
+ - uspolicy
+ singular: upstreamsettingspolicy
+ scope: Namespaced
+ versions:
+ - additionalPrinterColumns:
+ - jsonPath: .metadata.creationTimestamp
+ name: Age
+ type: date
+ name: v1alpha1
+ schema:
+ openAPIV3Schema:
+ description: |-
+ UpstreamSettingsPolicy is a Direct Attached Policy. It provides a way to configure the behavior of
+ the connection between NGINX and the upstream applications.
+ 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 UpstreamSettingsPolicy.
+ properties:
+ keepAlive:
+ description: KeepAlive defines the keep-alive settings.
+ properties:
+ connections:
+ description: |-
+ Connections sets the maximum number of idle keep-alive connections to upstream servers that are preserved
+ in the cache of each nginx worker process. When this number is exceeded, the least recently used
+ connections are closed.
+ Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive
+ format: int32
+ minimum: 1
+ type: integer
+ requests:
+ description: |-
+ Requests sets the maximum number of requests that can be served through one keep-alive connection.
+ After the maximum number of requests are made, the connection is closed.
+ Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive_requests
+ format: int32
+ minimum: 0
+ type: integer
+ time:
+ description: |-
+ Time defines the maximum time during which requests can be processed through one keep-alive connection.
+ After this time is reached, the connection is closed following the subsequent request processing.
+ Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive_time
+ pattern: ^[0-9]{1,4}(ms|s|m|h)?$
+ type: string
+ timeout:
+ description: |-
+ Timeout defines the keep-alive timeout for upstreams.
+ Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive_timeout
+ pattern: ^[0-9]{1,4}(ms|s|m|h)?$
+ type: string
+ type: object
+ targetRefs:
+ description: |-
+ TargetRefs identifies API object(s) to apply the policy to.
+ Objects must be in the same namespace as the policy.
+ Support: Service
+ 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: Service'
+ rule: self.all(t, t.kind=='Service')
+ - message: TargetRefs Group must be core
+ rule: self.exists(t, t.group=='') || self.exists(t, t.group=='core')
+ zoneSize:
+ description: |-
+ ZoneSize is the size of the shared memory zone used by the upstream. This memory zone is used to share
+ the upstream configuration between nginx worker processes. The more servers that an upstream has,
+ the larger memory zone is required.
+ Default: OSS: 512k, Plus: 1m.
+ Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#zone
+ pattern: ^\d{1,4}(k|m|g)?$
+ type: string
+ required:
+ - targetRefs
+ type: object
+ status:
+ description: Status defines the state of the UpstreamSettingsPolicy.
+ 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
+ - controllerName
+ type: object
+ maxItems: 16
+ type: array
+ required:
+ - ancestors
+ type: object
+ required:
+ - spec
+ type: object
+ served: true
+ storage: true
+ subresources:
+ status: {}
diff --git a/deploy/default/deploy.yaml b/deploy/default/deploy.yaml
index 7771ea7a3f..69853e77d3 100644
--- a/deploy/default/deploy.yaml
+++ b/deploy/default/deploy.yaml
@@ -98,6 +98,7 @@ rules:
- nginxproxies
- clientsettingspolicies
- observabilitypolicies
+ - upstreamsettingspolicies
verbs:
- list
- watch
@@ -107,6 +108,7 @@ rules:
- nginxgateways/status
- clientsettingspolicies/status
- observabilitypolicies/status
+ - upstreamsettingspolicies/status
verbs:
- update
- apiGroups:
diff --git a/deploy/experimental-nginx-plus/deploy.yaml b/deploy/experimental-nginx-plus/deploy.yaml
index 6e37bdbba6..9f30307439 100644
--- a/deploy/experimental-nginx-plus/deploy.yaml
+++ b/deploy/experimental-nginx-plus/deploy.yaml
@@ -111,6 +111,7 @@ rules:
- nginxproxies
- clientsettingspolicies
- observabilitypolicies
+ - upstreamsettingspolicies
verbs:
- list
- watch
@@ -120,6 +121,7 @@ rules:
- nginxgateways/status
- clientsettingspolicies/status
- observabilitypolicies/status
+ - upstreamsettingspolicies/status
verbs:
- update
- apiGroups:
diff --git a/deploy/experimental/deploy.yaml b/deploy/experimental/deploy.yaml
index e6dd9e8f24..f3114d3b55 100644
--- a/deploy/experimental/deploy.yaml
+++ b/deploy/experimental/deploy.yaml
@@ -103,6 +103,7 @@ rules:
- nginxproxies
- clientsettingspolicies
- observabilitypolicies
+ - upstreamsettingspolicies
verbs:
- list
- watch
@@ -112,6 +113,7 @@ rules:
- nginxgateways/status
- clientsettingspolicies/status
- observabilitypolicies/status
+ - upstreamsettingspolicies/status
verbs:
- update
- apiGroups:
diff --git a/deploy/nginx-plus/deploy.yaml b/deploy/nginx-plus/deploy.yaml
index 65d5e68bee..4561a6edb9 100644
--- a/deploy/nginx-plus/deploy.yaml
+++ b/deploy/nginx-plus/deploy.yaml
@@ -106,6 +106,7 @@ rules:
- nginxproxies
- clientsettingspolicies
- observabilitypolicies
+ - upstreamsettingspolicies
verbs:
- list
- watch
@@ -115,6 +116,7 @@ rules:
- nginxgateways/status
- clientsettingspolicies/status
- observabilitypolicies/status
+ - upstreamsettingspolicies/status
verbs:
- update
- apiGroups:
diff --git a/deploy/nodeport/deploy.yaml b/deploy/nodeport/deploy.yaml
index 36d5ff08ec..19bae58059 100644
--- a/deploy/nodeport/deploy.yaml
+++ b/deploy/nodeport/deploy.yaml
@@ -98,6 +98,7 @@ rules:
- nginxproxies
- clientsettingspolicies
- observabilitypolicies
+ - upstreamsettingspolicies
verbs:
- list
- watch
@@ -107,6 +108,7 @@ rules:
- nginxgateways/status
- clientsettingspolicies/status
- observabilitypolicies/status
+ - upstreamsettingspolicies/status
verbs:
- update
- apiGroups:
diff --git a/deploy/openshift/deploy.yaml b/deploy/openshift/deploy.yaml
index d66e286be5..8dd460e35a 100644
--- a/deploy/openshift/deploy.yaml
+++ b/deploy/openshift/deploy.yaml
@@ -98,6 +98,7 @@ rules:
- nginxproxies
- clientsettingspolicies
- observabilitypolicies
+ - upstreamsettingspolicies
verbs:
- list
- watch
@@ -107,6 +108,7 @@ rules:
- nginxgateways/status
- clientsettingspolicies/status
- observabilitypolicies/status
+ - upstreamsettingspolicies/status
verbs:
- update
- apiGroups:
diff --git a/deploy/snippets-filters-nginx-plus/deploy.yaml b/deploy/snippets-filters-nginx-plus/deploy.yaml
index efc9f843ad..176cf08c0a 100644
--- a/deploy/snippets-filters-nginx-plus/deploy.yaml
+++ b/deploy/snippets-filters-nginx-plus/deploy.yaml
@@ -106,6 +106,7 @@ rules:
- nginxproxies
- clientsettingspolicies
- observabilitypolicies
+ - upstreamsettingspolicies
- snippetsfilters
verbs:
- list
@@ -116,6 +117,7 @@ rules:
- nginxgateways/status
- clientsettingspolicies/status
- observabilitypolicies/status
+ - upstreamsettingspolicies/status
- snippetsfilters/status
verbs:
- update
diff --git a/deploy/snippets-filters/deploy.yaml b/deploy/snippets-filters/deploy.yaml
index 743b8f7fdf..6c177233bd 100644
--- a/deploy/snippets-filters/deploy.yaml
+++ b/deploy/snippets-filters/deploy.yaml
@@ -98,6 +98,7 @@ rules:
- nginxproxies
- clientsettingspolicies
- observabilitypolicies
+ - upstreamsettingspolicies
- snippetsfilters
verbs:
- list
@@ -108,6 +109,7 @@ rules:
- nginxgateways/status
- clientsettingspolicies/status
- observabilitypolicies/status
+ - upstreamsettingspolicies/status
- snippetsfilters/status
verbs:
- update
diff --git a/examples/upstream-settings-policy/cafe-routes.yaml b/examples/upstream-settings-policy/cafe-routes.yaml
new file mode 100644
index 0000000000..67927335cb
--- /dev/null
+++ b/examples/upstream-settings-policy/cafe-routes.yaml
@@ -0,0 +1,37 @@
+apiVersion: gateway.networking.k8s.io/v1
+kind: HTTPRoute
+metadata:
+ name: coffee
+spec:
+ parentRefs:
+ - name: gateway
+ sectionName: http
+ hostnames:
+ - "cafe.example.com"
+ rules:
+ - matches:
+ - path:
+ type: PathPrefix
+ value: /coffee
+ backendRefs:
+ - name: coffee
+ port: 80
+---
+apiVersion: gateway.networking.k8s.io/v1
+kind: HTTPRoute
+metadata:
+ name: tea
+spec:
+ parentRefs:
+ - name: gateway
+ sectionName: http
+ hostnames:
+ - "cafe.example.com"
+ rules:
+ - matches:
+ - path:
+ type: Exact
+ value: /tea
+ backendRefs:
+ - name: tea
+ port: 80
diff --git a/examples/upstream-settings-policy/cafe.yaml b/examples/upstream-settings-policy/cafe.yaml
new file mode 100644
index 0000000000..2d03ae59ff
--- /dev/null
+++ b/examples/upstream-settings-policy/cafe.yaml
@@ -0,0 +1,65 @@
+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: 8080
+---
+apiVersion: v1
+kind: Service
+metadata:
+ name: coffee
+spec:
+ ports:
+ - port: 80
+ targetPort: 8080
+ protocol: TCP
+ name: http
+ selector:
+ app: coffee
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+ name: tea
+spec:
+ replicas: 1
+ selector:
+ matchLabels:
+ app: tea
+ template:
+ metadata:
+ labels:
+ app: tea
+ spec:
+ containers:
+ - name: tea
+ image: nginxdemos/nginx-hello:plain-text
+ ports:
+ - containerPort: 8080
+---
+apiVersion: v1
+kind: Service
+metadata:
+ name: tea
+spec:
+ ports:
+ - port: 80
+ targetPort: 8080
+ protocol: TCP
+ name: http
+ selector:
+ app: tea
diff --git a/examples/upstream-settings-policy/gateway.yaml b/examples/upstream-settings-policy/gateway.yaml
new file mode 100644
index 0000000000..e6507f613b
--- /dev/null
+++ b/examples/upstream-settings-policy/gateway.yaml
@@ -0,0 +1,11 @@
+apiVersion: gateway.networking.k8s.io/v1
+kind: Gateway
+metadata:
+ name: gateway
+spec:
+ gatewayClassName: nginx
+ listeners:
+ - name: http
+ port: 80
+ protocol: HTTP
+ hostname: "*.example.com"
diff --git a/examples/upstream-settings-policy/upstream-settings-policy.yaml b/examples/upstream-settings-policy/upstream-settings-policy.yaml
index e49ed1dffd..95e8a34e6d 100644
--- a/examples/upstream-settings-policy/upstream-settings-policy.yaml
+++ b/examples/upstream-settings-policy/upstream-settings-policy.yaml
@@ -7,7 +7,7 @@ spec:
targetRefs:
- group: core
kind: Service
- name: service
+ name: coffee
keepAlive:
connections: 32
requests: 1001
diff --git a/internal/framework/kinds/kinds.go b/internal/framework/kinds/kinds.go
index 7700ad39ce..baeda6f9ee 100644
--- a/internal/framework/kinds/kinds.go
+++ b/internal/framework/kinds/kinds.go
@@ -11,9 +11,9 @@ import (
// Gateway API Kinds.
const (
- // Gateway is the Gateway Kind.
+ // Gateway is the Gateway kind.
Gateway = "Gateway"
- // GatewayClass is the GatewayClass Kind.
+ // GatewayClass is the GatewayClass kind.
GatewayClass = "GatewayClass"
// HTTPRoute is the HTTPRoute kind.
HTTPRoute = "HTTPRoute"
@@ -23,6 +23,12 @@ const (
TLSRoute = "TLSRoute"
)
+// Core API Kinds.
+const (
+ // Service is the Service kind.
+ Service = "Service"
+)
+
// NGINX Gateway Fabric kinds.
const (
// ClientSettingsPolicy is the ClientSettingsPolicy kind.
@@ -33,6 +39,8 @@ const (
NginxProxy = "NginxProxy"
// SnippetsFilter is the SnippetsFilter kind.
SnippetsFilter = "SnippetsFilter"
+ // UpstreamSettingsPolicy is the UpstreamSettingsPolicy kind.
+ UpstreamSettingsPolicy = "UpstreamSettingsPolicy"
)
// MustExtractGVK is a function that extracts the GroupVersionKind (GVK) of a client.object.
diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go
index bc94e61346..afb0560179 100644
--- a/internal/mode/static/manager.go
+++ b/internal/mode/static/manager.go
@@ -53,6 +53,7 @@ import (
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/clientsettings"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/observability"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/upstreamsettings"
ngxvalidation "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/validation"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file"
ngxruntime "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime"
@@ -320,6 +321,10 @@ func createPolicyManager(
GVK: mustExtractGVK(&ngfAPI.ObservabilityPolicy{}),
Validator: observability.NewValidator(validator),
},
+ {
+ GVK: mustExtractGVK(&ngfAPI.UpstreamSettingsPolicy{}),
+ Validator: upstreamsettings.NewValidator(validator),
+ },
}
return policies.NewManager(mustExtractGVK, cfgs...)
@@ -501,6 +506,12 @@ func registerControllers(
controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}),
},
},
+ {
+ objectType: &ngfAPI.UpstreamSettingsPolicy{},
+ options: []controller.Option{
+ controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}),
+ },
+ },
}
if cfg.ExperimentalFeatures {
@@ -737,6 +748,7 @@ func prepareFirstEventBatchPreparerArgs(cfg config.Config) ([]client.Object, []c
&gatewayv1.GRPCRouteList{},
&ngfAPI.ClientSettingsPolicyList{},
&ngfAPI.ObservabilityPolicyList{},
+ &ngfAPI.UpstreamSettingsPolicyList{},
partialObjectMetadataList,
}
diff --git a/internal/mode/static/manager_test.go b/internal/mode/static/manager_test.go
index 5a61a75854..0f2d802d32 100644
--- a/internal/mode/static/manager_test.go
+++ b/internal/mode/static/manager_test.go
@@ -67,6 +67,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) {
partialObjectMetadataList,
&ngfAPI.ClientSettingsPolicyList{},
&ngfAPI.ObservabilityPolicyList{},
+ &ngfAPI.UpstreamSettingsPolicyList{},
},
},
{
@@ -96,6 +97,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) {
partialObjectMetadataList,
&ngfAPI.ClientSettingsPolicyList{},
&ngfAPI.ObservabilityPolicyList{},
+ &ngfAPI.UpstreamSettingsPolicyList{},
},
},
{
@@ -128,6 +130,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) {
&gatewayv1.GRPCRouteList{},
&ngfAPI.ClientSettingsPolicyList{},
&ngfAPI.ObservabilityPolicyList{},
+ &ngfAPI.UpstreamSettingsPolicyList{},
},
},
{
@@ -158,6 +161,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) {
&ngfAPI.ClientSettingsPolicyList{},
&ngfAPI.ObservabilityPolicyList{},
&ngfAPI.SnippetsFilterList{},
+ &ngfAPI.UpstreamSettingsPolicyList{},
},
},
{
@@ -191,6 +195,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) {
&ngfAPI.ClientSettingsPolicyList{},
&ngfAPI.ObservabilityPolicyList{},
&ngfAPI.SnippetsFilterList{},
+ &ngfAPI.UpstreamSettingsPolicyList{},
},
},
}
diff --git a/internal/mode/static/nginx/config/policies/clientsettings/validator.go b/internal/mode/static/nginx/config/policies/clientsettings/validator.go
index 60ce55c7a9..79a390ce9b 100644
--- a/internal/mode/static/nginx/config/policies/clientsettings/validator.go
+++ b/internal/mode/static/nginx/config/policies/clientsettings/validator.go
@@ -30,7 +30,9 @@ func (v *Validator) Validate(policy policies.Policy, _ *policies.GlobalSettings)
targetRefPath := field.NewPath("spec").Child("targetRef")
supportedKinds := []gatewayv1.Kind{kinds.Gateway, kinds.HTTPRoute, kinds.GRPCRoute}
- if err := policies.ValidateTargetRef(csp.Spec.TargetRef, targetRefPath, supportedKinds); err != nil {
+ supportedGroups := []gatewayv1.Group{gatewayv1.GroupName}
+
+ if err := policies.ValidateTargetRef(csp.Spec.TargetRef, targetRefPath, supportedGroups, supportedKinds); err != nil {
return []conditions.Condition{staticConds.NewPolicyInvalid(err.Error())}
}
diff --git a/internal/mode/static/nginx/config/policies/observability/validator.go b/internal/mode/static/nginx/config/policies/observability/validator.go
index 3fa7ea2289..b93902d958 100644
--- a/internal/mode/static/nginx/config/policies/observability/validator.go
+++ b/internal/mode/static/nginx/config/policies/observability/validator.go
@@ -45,8 +45,10 @@ func (v *Validator) Validate(
targetRefPath := field.NewPath("spec").Child("targetRefs")
supportedKinds := []gatewayv1.Kind{kinds.HTTPRoute, kinds.GRPCRoute}
+ supportedGroups := []gatewayv1.Group{gatewayv1.GroupName}
+
for _, ref := range obs.Spec.TargetRefs {
- if err := policies.ValidateTargetRef(ref, targetRefPath, supportedKinds); err != nil {
+ if err := policies.ValidateTargetRef(ref, targetRefPath, supportedGroups, supportedKinds); err != nil {
return []conditions.Condition{staticConds.NewPolicyInvalid(err.Error())}
}
}
diff --git a/internal/mode/static/nginx/config/policies/policy.go b/internal/mode/static/nginx/config/policies/policy.go
index c26f3bec7b..b818f5a216 100644
--- a/internal/mode/static/nginx/config/policies/policy.go
+++ b/internal/mode/static/nginx/config/policies/policy.go
@@ -24,9 +24,9 @@ type Policy interface {
// GlobalSettings contains global settings from the current state of the graph that may be
// needed for policy validation or generation if certain policies rely on those global settings.
type GlobalSettings struct {
- // NginxProxyValid is whether or not the NginxProxy resource is valid.
+ // NginxProxyValid is whether the NginxProxy resource is valid.
NginxProxyValid bool
- // TelemetryEnabled is whether or not telemetry is enabled in the NginxProxy resource.
+ // TelemetryEnabled is whether telemetry is enabled in the NginxProxy resource.
TelemetryEnabled bool
}
@@ -34,15 +34,16 @@ type GlobalSettings struct {
func ValidateTargetRef(
ref v1alpha2.LocalPolicyTargetReference,
basePath *field.Path,
+ groups []gatewayv1.Group,
supportedKinds []gatewayv1.Kind,
) error {
- if ref.Group != gatewayv1.GroupName {
+ if !slices.Contains(groups, ref.Group) {
path := basePath.Child("group")
return field.NotSupported(
path,
ref.Group,
- []string{gatewayv1.GroupName},
+ groups,
)
}
diff --git a/internal/mode/static/nginx/config/policies/upstreamsettings/validator.go b/internal/mode/static/nginx/config/policies/upstreamsettings/validator.go
new file mode 100644
index 0000000000..9fccebd166
--- /dev/null
+++ b/internal/mode/static/nginx/config/policies/upstreamsettings/validator.go
@@ -0,0 +1,125 @@
+package upstreamsettings
+
+import (
+ "k8s.io/apimachinery/pkg/util/validation/field"
+ gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
+
+ ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies"
+ staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation"
+)
+
+// Validator validates an UpstreamSettingsPolicy.
+// Implements policies.Validator interface.
+type Validator struct {
+ genericValidator validation.GenericValidator
+}
+
+// NewValidator returns a new Validator.
+func NewValidator(genericValidator validation.GenericValidator) Validator {
+ return Validator{genericValidator: genericValidator}
+}
+
+// Validate validates the spec of an UpstreamsSettingsPolicy.
+func (v Validator) Validate(policy policies.Policy, _ *policies.GlobalSettings) []conditions.Condition {
+ usp := helpers.MustCastObject[*ngfAPI.UpstreamSettingsPolicy](policy)
+
+ targetRefsPath := field.NewPath("spec").Child("targetRefs")
+ supportedKinds := []gatewayv1.Kind{kinds.Service}
+ supportedGroups := []gatewayv1.Group{"", "core"}
+
+ for i, ref := range usp.Spec.TargetRefs {
+ indexedPath := targetRefsPath.Index(i)
+ if err := policies.ValidateTargetRef(ref, indexedPath, supportedGroups, supportedKinds); err != nil {
+ return []conditions.Condition{staticConds.NewPolicyInvalid(err.Error())}
+ }
+ }
+
+ if err := v.validateSettings(usp.Spec); err != nil {
+ return []conditions.Condition{staticConds.NewPolicyInvalid(err.Error())}
+ }
+
+ return nil
+}
+
+// Conflicts returns true if the two UpstreamsSettingsPolicies conflict.
+func (v Validator) Conflicts(polA, polB policies.Policy) bool {
+ cspA := helpers.MustCastObject[*ngfAPI.UpstreamSettingsPolicy](polA)
+ cspB := helpers.MustCastObject[*ngfAPI.UpstreamSettingsPolicy](polB)
+
+ return conflicts(cspA.Spec, cspB.Spec)
+}
+
+func conflicts(a, b ngfAPI.UpstreamSettingsPolicySpec) bool {
+ if a.ZoneSize != nil && b.ZoneSize != nil {
+ return true
+ }
+
+ if a.KeepAlive != nil && b.KeepAlive != nil {
+ if a.KeepAlive.Connections != nil && b.KeepAlive.Connections != nil {
+ return true
+ }
+ if a.KeepAlive.Requests != nil && b.KeepAlive.Requests != nil {
+ return true
+ }
+
+ if a.KeepAlive.Time != nil && b.KeepAlive.Time != nil {
+ return true
+ }
+
+ if a.KeepAlive.Timeout != nil && b.KeepAlive.Timeout != nil {
+ return true
+ }
+ }
+
+ return false
+}
+
+// validateSettings performs validation on fields in the spec that are vulnerable to code injection.
+// For all other fields, we rely on the CRD validation.
+func (v Validator) validateSettings(spec ngfAPI.UpstreamSettingsPolicySpec) error {
+ var allErrs field.ErrorList
+ fieldPath := field.NewPath("spec")
+
+ if spec.ZoneSize != nil {
+ if err := v.genericValidator.ValidateNginxSize(string(*spec.ZoneSize)); err != nil {
+ path := fieldPath.Child("zoneSize")
+ allErrs = append(allErrs, field.Invalid(path, spec.ZoneSize, err.Error()))
+ }
+ }
+
+ if spec.KeepAlive != nil {
+ allErrs = append(allErrs, v.validateUpstreamKeepAlive(*spec.KeepAlive, fieldPath.Child("keepAlive"))...)
+ }
+
+ return allErrs.ToAggregate()
+}
+
+func (v Validator) validateUpstreamKeepAlive(
+ keepAlive ngfAPI.UpstreamKeepAlive,
+ fieldPath *field.Path,
+) field.ErrorList {
+ var allErrs field.ErrorList
+
+ if keepAlive.Time != nil {
+ if err := v.genericValidator.ValidateNginxDuration(string(*keepAlive.Time)); err != nil {
+ path := fieldPath.Child("time")
+
+ allErrs = append(allErrs, field.Invalid(path, *keepAlive.Time, err.Error()))
+ }
+ }
+
+ if keepAlive.Timeout != nil {
+ if err := v.genericValidator.ValidateNginxDuration(string(*keepAlive.Timeout)); err != nil {
+ path := fieldPath.Child("timeout")
+
+ allErrs = append(allErrs, field.Invalid(path, *keepAlive.Timeout, err.Error()))
+ }
+ }
+
+ return allErrs
+}
diff --git a/internal/mode/static/nginx/config/policies/upstreamsettings/validator_test.go b/internal/mode/static/nginx/config/policies/upstreamsettings/validator_test.go
new file mode 100644
index 0000000000..3e303a8229
--- /dev/null
+++ b/internal/mode/static/nginx/config/policies/upstreamsettings/validator_test.go
@@ -0,0 +1,266 @@
+package upstreamsettings_test
+
+import (
+ "testing"
+
+ . "github.com/onsi/gomega"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+ "sigs.k8s.io/gateway-api/apis/v1alpha2"
+
+ ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/policiesfakes"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/upstreamsettings"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/validation"
+ staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions"
+)
+
+type policyModFunc func(policy *ngfAPI.UpstreamSettingsPolicy) *ngfAPI.UpstreamSettingsPolicy
+
+func createValidPolicy() *ngfAPI.UpstreamSettingsPolicy {
+ return &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Namespace: "default",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ TargetRefs: []v1alpha2.LocalPolicyTargetReference{
+ {
+ Group: "core",
+ Kind: kinds.Service,
+ Name: "svc",
+ },
+ },
+ ZoneSize: helpers.GetPointer[ngfAPI.Size]("1k"),
+ KeepAlive: &ngfAPI.UpstreamKeepAlive{
+ Requests: helpers.GetPointer[int32](900),
+ Time: helpers.GetPointer[ngfAPI.Duration]("50s"),
+ Timeout: helpers.GetPointer[ngfAPI.Duration]("30s"),
+ Connections: helpers.GetPointer[int32](100),
+ },
+ },
+ Status: v1alpha2.PolicyStatus{},
+ }
+}
+
+func createModifiedPolicy(mod policyModFunc) *ngfAPI.UpstreamSettingsPolicy {
+ return mod(createValidPolicy())
+}
+
+func TestValidator_Validate(t *testing.T) {
+ t.Parallel()
+ tests := []struct {
+ name string
+ policy *ngfAPI.UpstreamSettingsPolicy
+ expConditions []conditions.Condition
+ }{
+ {
+ name: "invalid target ref; unsupported group",
+ policy: createModifiedPolicy(func(p *ngfAPI.UpstreamSettingsPolicy) *ngfAPI.UpstreamSettingsPolicy {
+ p.Spec.TargetRefs = append(
+ p.Spec.TargetRefs,
+ v1alpha2.LocalPolicyTargetReference{
+ Group: "Unsupported",
+ Kind: kinds.Service,
+ Name: "svc",
+ })
+ return p
+ }),
+ expConditions: []conditions.Condition{
+ staticConds.NewPolicyInvalid("spec.targetRefs[1].group: Unsupported value: \"Unsupported\": " +
+ "supported values: \"\", \"core\""),
+ },
+ },
+ {
+ name: "invalid target ref; unsupported kind",
+ policy: createModifiedPolicy(func(p *ngfAPI.UpstreamSettingsPolicy) *ngfAPI.UpstreamSettingsPolicy {
+ p.Spec.TargetRefs = append(
+ p.Spec.TargetRefs,
+ v1alpha2.LocalPolicyTargetReference{
+ Group: "",
+ Kind: "Unsupported",
+ Name: "svc",
+ })
+ return p
+ }),
+ expConditions: []conditions.Condition{
+ staticConds.NewPolicyInvalid("spec.targetRefs[1].kind: Unsupported value: \"Unsupported\": " +
+ "supported values: \"Service\""),
+ },
+ },
+ {
+ name: "invalid zone size",
+ policy: createModifiedPolicy(func(p *ngfAPI.UpstreamSettingsPolicy) *ngfAPI.UpstreamSettingsPolicy {
+ p.Spec.ZoneSize = helpers.GetPointer[ngfAPI.Size]("invalid")
+ return p
+ }),
+ expConditions: []conditions.Condition{
+ staticConds.NewPolicyInvalid("spec.zoneSize: Invalid value: \"invalid\": ^\\d{1,4}(k|m|g)?$ " +
+ "(e.g. '1024', or '8k', or '20m', or '1g', regex used for validation is 'must contain a number. " +
+ "May be followed by 'k', 'm', or 'g', otherwise bytes are assumed')"),
+ },
+ },
+ {
+ name: "invalid durations",
+ policy: createModifiedPolicy(func(p *ngfAPI.UpstreamSettingsPolicy) *ngfAPI.UpstreamSettingsPolicy {
+ p.Spec.KeepAlive.Time = helpers.GetPointer[ngfAPI.Duration]("invalid")
+ p.Spec.KeepAlive.Timeout = helpers.GetPointer[ngfAPI.Duration]("invalid")
+ return p
+ }),
+ expConditions: []conditions.Condition{
+ staticConds.NewPolicyInvalid(
+ "[spec.keepAlive.time: Invalid value: \"invalid\": ^[0-9]{1,4}(ms|s|m|h)? " +
+ "(e.g. '5ms', or '10s', or '500m', or '1000h', regex used for validation is " +
+ "'must contain an, at most, four digit number followed by 'ms', 's', 'm', or 'h''), " +
+ "spec.keepAlive.timeout: Invalid value: \"invalid\": ^[0-9]{1,4}(ms|s|m|h)? " +
+ "(e.g. '5ms', or '10s', or '500m', or '1000h', regex used for validation is " +
+ "'must contain an, at most, four digit number followed by 'ms', 's', 'm', or 'h'')]"),
+ },
+ },
+ {
+ name: "valid",
+ policy: createValidPolicy(),
+ expConditions: nil,
+ },
+ }
+
+ v := upstreamsettings.NewValidator(validation.GenericValidator{})
+
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ t.Parallel()
+ g := NewWithT(t)
+
+ conds := v.Validate(test.policy, nil)
+ g.Expect(conds).To(Equal(test.expConditions))
+ })
+ }
+}
+
+func TestValidator_ValidatePanics(t *testing.T) {
+ t.Parallel()
+ v := upstreamsettings.NewValidator(nil)
+
+ validate := func() {
+ _ = v.Validate(&policiesfakes.FakePolicy{}, nil)
+ }
+
+ g := NewWithT(t)
+
+ g.Expect(validate).To(Panic())
+}
+
+func TestValidator_Conflicts(t *testing.T) {
+ t.Parallel()
+ tests := []struct {
+ polA *ngfAPI.UpstreamSettingsPolicy
+ polB *ngfAPI.UpstreamSettingsPolicy
+ name string
+ conflicts bool
+ }{
+ {
+ name: "no conflicts",
+ polA: &ngfAPI.UpstreamSettingsPolicy{
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ ZoneSize: helpers.GetPointer[ngfAPI.Size]("10m"),
+ KeepAlive: &ngfAPI.UpstreamKeepAlive{
+ Requests: helpers.GetPointer[int32](900),
+ Time: helpers.GetPointer[ngfAPI.Duration]("50s"),
+ },
+ },
+ },
+ polB: &ngfAPI.UpstreamSettingsPolicy{
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ KeepAlive: &ngfAPI.UpstreamKeepAlive{
+ Timeout: helpers.GetPointer[ngfAPI.Duration]("30s"),
+ Connections: helpers.GetPointer[int32](50),
+ },
+ },
+ },
+ conflicts: false,
+ },
+ {
+ name: "zone max size conflicts",
+ polA: createValidPolicy(),
+ polB: &ngfAPI.UpstreamSettingsPolicy{
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ ZoneSize: helpers.GetPointer[ngfAPI.Size]("10m"),
+ },
+ },
+ conflicts: true,
+ },
+ {
+ name: "keepalive requests conflicts",
+ polA: createValidPolicy(),
+ polB: &ngfAPI.UpstreamSettingsPolicy{
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ KeepAlive: &ngfAPI.UpstreamKeepAlive{
+ Requests: helpers.GetPointer[int32](900),
+ },
+ },
+ },
+ conflicts: true,
+ },
+ {
+ name: "keepalive connections conflicts",
+ polA: createValidPolicy(),
+ polB: &ngfAPI.UpstreamSettingsPolicy{
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ KeepAlive: &ngfAPI.UpstreamKeepAlive{
+ Connections: helpers.GetPointer[int32](900),
+ },
+ },
+ },
+ conflicts: true,
+ },
+ {
+ name: "keepalive time conflicts",
+ polA: createValidPolicy(),
+ polB: &ngfAPI.UpstreamSettingsPolicy{
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ KeepAlive: &ngfAPI.UpstreamKeepAlive{
+ Time: helpers.GetPointer[ngfAPI.Duration]("50s"),
+ },
+ },
+ },
+ conflicts: true,
+ },
+ {
+ name: "keepalive timeout conflicts",
+ polA: createValidPolicy(),
+ polB: &ngfAPI.UpstreamSettingsPolicy{
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ KeepAlive: &ngfAPI.UpstreamKeepAlive{
+ Timeout: helpers.GetPointer[ngfAPI.Duration]("30s"),
+ },
+ },
+ },
+ conflicts: true,
+ },
+ }
+
+ v := upstreamsettings.NewValidator(nil)
+
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ t.Parallel()
+ g := NewWithT(t)
+
+ g.Expect(v.Conflicts(test.polA, test.polB)).To(Equal(test.conflicts))
+ })
+ }
+}
+
+func TestValidator_ConflictsPanics(t *testing.T) {
+ t.Parallel()
+ v := upstreamsettings.NewValidator(nil)
+
+ conflicts := func() {
+ _ = v.Conflicts(&policiesfakes.FakePolicy{}, &policiesfakes.FakePolicy{})
+ }
+
+ g := NewWithT(t)
+
+ g.Expect(conflicts).To(Panic())
+}
diff --git a/internal/mode/static/state/change_processor.go b/internal/mode/static/state/change_processor.go
index cb35491209..82edbcc7b1 100644
--- a/internal/mode/static/state/change_processor.go
+++ b/internal/mode/static/state/change_processor.go
@@ -216,6 +216,11 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl {
store: commonPolicyObjectStore,
predicate: funcPredicate{stateChanged: isNGFPolicyRelevant},
},
+ {
+ gvk: cfg.MustExtractGVK(&ngfAPI.UpstreamSettingsPolicy{}),
+ store: commonPolicyObjectStore,
+ predicate: funcPredicate{stateChanged: isNGFPolicyRelevant},
+ },
{
gvk: cfg.MustExtractGVK(&v1alpha2.TLSRoute{}),
store: newObjectStoreMapAdapter(clusterStore.TLSRoutes),
diff --git a/internal/mode/static/state/change_processor_test.go b/internal/mode/static/state/change_processor_test.go
index 334335c804..03d86a377b 100644
--- a/internal/mode/static/state/change_processor_test.go
+++ b/internal/mode/static/state/change_processor_test.go
@@ -265,13 +265,11 @@ func createHTTPBackendRef(
}
}
-func createTLSBackendRef(
- name v1.ObjectName,
- namespace v1.Namespace,
-) v1.BackendRef {
+func createTLSBackendRef(name, namespace string) v1.BackendRef {
kindSvc := v1.Kind("Service")
+ ns := v1.Namespace(namespace)
return v1.BackendRef{
- BackendObjectReference: createBackendRefObj(&kindSvc, name, &namespace),
+ BackendObjectReference: createBackendRefObj(&kindSvc, v1.ObjectName(name), &ns),
}
}
@@ -437,6 +435,7 @@ var _ = Describe("ChangeProcessor", func() {
gatewayAPICRD, gatewayAPICRDUpdated *metav1.PartialObjectMetadata
httpRouteKey1, httpRouteKey2, grpcRouteKey1, grpcRouteKey2 graph.RouteKey
trKey1, trKey2 graph.L4RouteKey
+ refSvc, refGRPCSvc, refTLSSvc types.NamespacedName
)
processAndValidateGraph := func(expGraph *graph.Graph) {
@@ -450,12 +449,16 @@ var _ = Describe("ChangeProcessor", func() {
gcUpdated = gc.DeepCopy()
gcUpdated.Generation++
+ refSvc = types.NamespacedName{Namespace: "service-ns", Name: "service"}
+ refGRPCSvc = types.NamespacedName{Namespace: "grpc-service-ns", Name: "grpc-service"}
+ refTLSSvc = types.NamespacedName{Namespace: "tls-service-ns", Name: "tls-service"}
+
crossNsHTTPBackendRef := v1.HTTPBackendRef{
BackendRef: v1.BackendRef{
BackendObjectReference: v1.BackendObjectReference{
Kind: helpers.GetPointer[v1.Kind]("Service"),
- Name: "service",
- Namespace: helpers.GetPointer[v1.Namespace]("service-ns"),
+ Name: v1.ObjectName(refSvc.Name),
+ Namespace: helpers.GetPointer(v1.Namespace(refSvc.Namespace)),
Port: helpers.GetPointer[v1.PortNumber](80),
},
},
@@ -465,8 +468,8 @@ var _ = Describe("ChangeProcessor", func() {
BackendRef: v1.BackendRef{
BackendObjectReference: v1.BackendObjectReference{
Kind: helpers.GetPointer[v1.Kind]("Service"),
- Name: "grpc-service",
- Namespace: helpers.GetPointer[v1.Namespace]("grpc-service-ns"),
+ Name: v1.ObjectName(refGRPCSvc.Name),
+ Namespace: helpers.GetPointer(v1.Namespace(refGRPCSvc.Namespace)),
Port: helpers.GetPointer[v1.PortNumber](80),
},
},
@@ -486,7 +489,7 @@ var _ = Describe("ChangeProcessor", func() {
gr2 = createGRPCRoute("gr-2", "gateway-2", "bar.example.com")
grpcRouteKey2 = graph.CreateRouteKey(gr2)
- tlsBackendRef := createTLSBackendRef("tls-service", "tls-service-ns")
+ tlsBackendRef := createTLSBackendRef(refTLSSvc.Name, refTLSSvc.Namespace)
tr1 = createTLSRoute("tr-1", "gateway-1", "foo.tls.com", tlsBackendRef)
trKey1 = graph.CreateRouteKeyL4(tr1)
tr1Updated = tr1.DeepCopy()
@@ -666,7 +669,7 @@ var _ = Describe("ChangeProcessor", func() {
{
BackendRefs: []graph.BackendRef{
{
- SvcNsName: types.NamespacedName{Namespace: "service-ns", Name: "service"},
+ SvcNsName: refSvc,
Weight: 1,
},
},
@@ -761,7 +764,7 @@ var _ = Describe("ChangeProcessor", func() {
{
BackendRefs: []graph.BackendRef{
{
- SvcNsName: types.NamespacedName{Namespace: "grpc-service-ns", Name: "grpc-service"},
+ SvcNsName: refGRPCSvc,
Weight: 1,
},
},
@@ -841,7 +844,7 @@ var _ = Describe("ChangeProcessor", func() {
Spec: graph.L4RouteSpec{
Hostnames: tr1.Spec.Hostnames,
BackendRef: graph.BackendRef{
- SvcNsName: types.NamespacedName{Namespace: "tls-service-ns", Name: "tls-service"},
+ SvcNsName: refTLSSvc,
Valid: false,
},
},
@@ -869,7 +872,7 @@ var _ = Describe("ChangeProcessor", func() {
Spec: graph.L4RouteSpec{
Hostnames: tr2.Spec.Hostnames,
BackendRef: graph.BackendRef{
- SvcNsName: types.NamespacedName{Namespace: "tls-service-ns", Name: "tls-service"},
+ SvcNsName: refTLSSvc,
Valid: false,
},
},
@@ -935,19 +938,10 @@ var _ = Describe("ChangeProcessor", func() {
L4Routes: map[graph.L4RouteKey]*graph.L4Route{trKey1: expRouteTR1},
Routes: map[graph.RouteKey]*graph.L7Route{httpRouteKey1: expRouteHR1, grpcRouteKey1: expRouteGR1},
ReferencedSecrets: map[types.NamespacedName]*graph.Secret{},
- ReferencedServices: map[types.NamespacedName]struct{}{
- {
- Namespace: "service-ns",
- Name: "service",
- }: {},
- {
- Namespace: "grpc-service-ns",
- Name: "grpc-service",
- }: {},
- {
- Namespace: "tls-service-ns",
- Name: "tls-service",
- }: {},
+ ReferencedServices: map[types.NamespacedName]*graph.ReferencedService{
+ refSvc: {},
+ refTLSSvc: {},
+ refGRPCSvc: {},
},
}
})
@@ -1181,7 +1175,7 @@ var _ = Describe("ChangeProcessor", func() {
"Backend ref to Service grpc-service-ns/grpc-service not permitted by any ReferenceGrant",
),
}
- delete(expGraph.ReferencedServices, types.NamespacedName{Namespace: "grpc-service-ns", Name: "grpc-service"})
+ delete(expGraph.ReferencedServices, refGRPCSvc)
expRouteGR1.Spec.Rules[0].BackendRefs[0].SvcNsName = types.NamespacedName{}
// no ref grant exists yet for tr1
@@ -1190,7 +1184,7 @@ var _ = Describe("ChangeProcessor", func() {
"Backend ref to Service tls-service-ns/tls-service not permitted by any ReferenceGrant",
),
}
- delete(expGraph.ReferencedServices, types.NamespacedName{Namespace: "tls-service-ns", Name: "tls-service"})
+ delete(expGraph.ReferencedServices, refTLSSvc)
expRouteTR1.Spec.BackendRef.SvcNsName = types.NamespacedName{}
expGraph.ReferencedSecrets[client.ObjectKeyFromObject(diffNsTLSSecret)] = &graph.Secret{
@@ -2350,11 +2344,13 @@ var _ = Describe("ChangeProcessor", func() {
Describe("NGF Policy resource changes", Ordered, func() {
var (
- gw *v1.Gateway
- route *v1.HTTPRoute
- csp, cspUpdated *ngfAPI.ClientSettingsPolicy
- obs, obsUpdated *ngfAPI.ObservabilityPolicy
- cspKey, obsKey graph.PolicyKey
+ gw *v1.Gateway
+ route *v1.HTTPRoute
+ svc *apiv1.Service
+ csp, cspUpdated *ngfAPI.ClientSettingsPolicy
+ obs, obsUpdated *ngfAPI.ObservabilityPolicy
+ usp, uspUpdated *ngfAPI.UpstreamSettingsPolicy
+ cspKey, obsKey, uspKey graph.PolicyKey
)
BeforeAll(func() {
@@ -2365,7 +2361,28 @@ var _ = Describe("ChangeProcessor", func() {
Expect(newGraph.NGFPolicies).To(BeEmpty())
gw = createGateway("gw", createHTTPListener())
- route = createHTTPRoute("hr-1", "gw", "foo.example.com", v1.HTTPBackendRef{})
+ route = createHTTPRoute(
+ "hr-1",
+ "gw",
+ "foo.example.com",
+ v1.HTTPBackendRef{
+ BackendRef: v1.BackendRef{
+ BackendObjectReference: v1.BackendObjectReference{
+ Group: helpers.GetPointer[v1.Group](""),
+ Kind: helpers.GetPointer[v1.Kind](kinds.Service),
+ Name: "svc",
+ Port: helpers.GetPointer[v1.PortNumber](80),
+ },
+ },
+ },
+ )
+
+ svc = &apiv1.Service{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "svc",
+ Namespace: "test",
+ },
+ }
csp = &ngfAPI.ClientSettingsPolicy{
ObjectMeta: metav1.ObjectMeta{
@@ -2426,6 +2443,35 @@ var _ = Describe("ChangeProcessor", func() {
Version: "v1alpha1",
},
}
+
+ usp = &ngfAPI.UpstreamSettingsPolicy{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "usp",
+ Namespace: "test",
+ },
+ Spec: ngfAPI.UpstreamSettingsPolicySpec{
+ ZoneSize: helpers.GetPointer[ngfAPI.Size]("10m"),
+ TargetRefs: []v1alpha2.LocalPolicyTargetReference{
+ {
+ Group: "core",
+ Kind: kinds.Service,
+ Name: "svc",
+ },
+ },
+ },
+ }
+
+ uspUpdated = usp.DeepCopy()
+ uspUpdated.Spec.ZoneSize = helpers.GetPointer[ngfAPI.Size]("20m")
+
+ uspKey = graph.PolicyKey{
+ NsName: types.NamespacedName{Name: "usp", Namespace: "test"},
+ GVK: schema.GroupVersionKind{
+ Group: ngfAPI.GroupName,
+ Kind: kinds.UpstreamSettingsPolicy,
+ Version: "v1alpha1",
+ },
+ }
})
/*
@@ -2438,6 +2484,7 @@ var _ = Describe("ChangeProcessor", func() {
It("reports no changes", func() {
processor.CaptureUpsertChange(csp)
processor.CaptureUpsertChange(obs)
+ processor.CaptureUpsertChange(usp)
changed, _ := processor.Process()
Expect(changed).To(Equal(state.NoChange))
@@ -2458,12 +2505,19 @@ var _ = Describe("ChangeProcessor", func() {
Expect(changed).To(Equal(state.ClusterStateChange))
Expect(graph.NGFPolicies).To(HaveKey(obsKey))
Expect(graph.NGFPolicies[obsKey].Source).To(Equal(obs))
+
+ processor.CaptureUpsertChange(svc)
+ changed, graph = processor.Process()
+ Expect(changed).To(Equal(state.ClusterStateChange))
+ Expect(graph.NGFPolicies).To(HaveKey(uspKey))
+ Expect(graph.NGFPolicies[uspKey].Source).To(Equal(usp))
})
})
When("the policy is updated", func() {
It("captures changes for a policy", func() {
processor.CaptureUpsertChange(cspUpdated)
processor.CaptureUpsertChange(obsUpdated)
+ processor.CaptureUpsertChange(uspUpdated)
changed, graph := processor.Process()
Expect(changed).To(Equal(state.ClusterStateChange))
@@ -2471,12 +2525,15 @@ var _ = Describe("ChangeProcessor", func() {
Expect(graph.NGFPolicies[cspKey].Source).To(Equal(cspUpdated))
Expect(graph.NGFPolicies).To(HaveKey(obsKey))
Expect(graph.NGFPolicies[obsKey].Source).To(Equal(obsUpdated))
+ Expect(graph.NGFPolicies).To(HaveKey(uspKey))
+ Expect(graph.NGFPolicies[uspKey].Source).To(Equal(uspUpdated))
})
})
When("the policy is deleted", func() {
It("removes the policy from the graph", func() {
processor.CaptureDeleteChange(&ngfAPI.ClientSettingsPolicy{}, client.ObjectKeyFromObject(csp))
processor.CaptureDeleteChange(&ngfAPI.ObservabilityPolicy{}, client.ObjectKeyFromObject(obs))
+ processor.CaptureDeleteChange(&ngfAPI.UpstreamSettingsPolicy{}, client.ObjectKeyFromObject(usp))
changed, graph := processor.Process()
Expect(changed).To(Equal(state.ClusterStateChange))
diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go
index f533b67a1b..cafcbc0db8 100644
--- a/internal/mode/static/state/dataplane/configuration.go
+++ b/internal/mode/static/state/dataplane/configuration.go
@@ -41,12 +41,19 @@ func BuildConfiguration(
httpServers, sslServers := buildServers(g)
backendGroups := buildBackendGroups(append(httpServers, sslServers...))
+ upstreams := buildUpstreams(
+ ctx,
+ g.Gateway.Listeners,
+ serviceResolver,
+ g.ReferencedServices,
+ baseHTTPConfig.IPFamily,
+ )
config := Configuration{
HTTPServers: httpServers,
SSLServers: sslServers,
TLSPassthroughServers: buildPassthroughServers(g),
- Upstreams: buildUpstreams(ctx, g.Gateway.Listeners, serviceResolver, baseHTTPConfig.IPFamily),
+ Upstreams: upstreams,
StreamUpstreams: buildStreamUpstreams(ctx, g.Gateway.Listeners, serviceResolver, baseHTTPConfig.IPFamily),
BackendGroups: backendGroups,
SSLKeyPairs: buildSSLKeyPairs(g.ReferencedSecrets, g.Gateway.Listeners),
@@ -602,6 +609,7 @@ func buildUpstreams(
ctx context.Context,
listeners []*graph.Listener,
svcResolver resolver.ServiceResolver,
+ referencedServices map[types.NamespacedName]*graph.ReferencedService,
ipFamily IPFamilyType,
) []Upstream {
// There can be duplicate upstreams if multiple routes reference the same upstream.
@@ -642,10 +650,16 @@ func buildUpstreams(
errMsg = err.Error()
}
+ var upstreamPolicies []policies.Policy
+ if graphSvc, exists := referencedServices[br.SvcNsName]; exists {
+ upstreamPolicies = buildPolicies(graphSvc.Policies)
+ }
+
uniqueUpstreams[upstreamName] = Upstream{
Name: upstreamName,
Endpoints: eps,
ErrorMsg: errMsg,
+ Policies: upstreamPolicies,
}
}
}
diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go
index 7bed89e5d2..037bcd7d9d 100644
--- a/internal/mode/static/state/dataplane/configuration_test.go
+++ b/internal/mode/static/state/dataplane/configuration_test.go
@@ -81,6 +81,7 @@ func getNormalGraph() *graph.Graph {
Routes: map[graph.RouteKey]*graph.L7Route{},
ReferencedSecrets: map[types.NamespacedName]*graph.Secret{},
ReferencedCaCertConfigMaps: map[types.NamespacedName]*graph.CaCertConfigMap{},
+ ReferencedServices: map[types.NamespacedName]*graph.ReferencedService{},
}
}
@@ -2804,6 +2805,13 @@ func TestBuildUpstreams(t *testing.T) {
},
}
+ policyEndpoints := []resolver.Endpoint{
+ {
+ Address: "16.0.0.0",
+ Port: 80,
+ },
+ }
+
createBackendRefs := func(serviceNames ...string) []graph.BackendRef {
var backends []graph.BackendRef
for _, name := range serviceNames {
@@ -2836,6 +2844,8 @@ func TestBuildUpstreams(t *testing.T) {
invalidHRRefs := createBackendRefs("abc")
+ refsWithPolicies := createBackendRefs("policies")
+
routes := map[graph.RouteKey]*graph.L7Route{
{NamespacedName: types.NamespacedName{Name: "hr1", Namespace: "test"}}: {
Valid: true,
@@ -2893,6 +2903,15 @@ func TestBuildUpstreams(t *testing.T) {
},
}
+ routesWithPolicies := map[graph.RouteKey]*graph.L7Route{
+ {NamespacedName: types.NamespacedName{Name: "policies", Namespace: "test"}}: {
+ Valid: true,
+ Spec: graph.L7RouteSpec{
+ Rules: refsToValidRules(refsWithPolicies),
+ },
+ },
+ }
+
listeners := []*graph.Listener{
{
Name: "invalid-listener",
@@ -2919,6 +2938,41 @@ func TestBuildUpstreams(t *testing.T) {
Valid: true,
Routes: routes3,
},
+ {
+ Name: "listener-5",
+ Valid: true,
+ Routes: routesWithPolicies,
+ },
+ }
+
+ validPolicy1 := &policiesfakes.FakePolicy{}
+ validPolicy2 := &policiesfakes.FakePolicy{}
+ invalidPolicy := &policiesfakes.FakePolicy{}
+
+ referencedServices := map[types.NamespacedName]*graph.ReferencedService{
+ {Name: "bar", Namespace: "test"}: {},
+ {Name: "baz", Namespace: "test"}: {},
+ {Name: "baz2", Namespace: "test"}: {},
+ {Name: "foo", Namespace: "test"}: {},
+ {Name: "empty-endpoints", Namespace: "test"}: {},
+ {Name: "nil-endpoints", Namespace: "test"}: {},
+ {Name: "ipv6-endpoints", Namespace: "test"}: {},
+ {Name: "policies", Namespace: "test"}: {
+ Policies: []*graph.Policy{
+ {
+ Valid: true,
+ Source: validPolicy1,
+ },
+ {
+ Valid: false,
+ Source: invalidPolicy,
+ },
+ {
+ Valid: true,
+ Source: validPolicy2,
+ },
+ },
+ },
}
emptyEndpointsErrMsg := "empty endpoints error"
@@ -2955,6 +3009,11 @@ func TestBuildUpstreams(t *testing.T) {
Name: "test_ipv6-endpoints_80",
Endpoints: ipv6Endpoints,
},
+ {
+ Name: "test_policies_80",
+ Endpoints: policyEndpoints,
+ Policies: []policies.Policy{validPolicy1, validPolicy2},
+ },
}
fakeResolver := &resolverfakes.FakeServiceResolver{}
@@ -2981,6 +3040,8 @@ func TestBuildUpstreams(t *testing.T) {
return abcEndpoints, nil
case "ipv6-endpoints":
return ipv6Endpoints, nil
+ case "policies":
+ return policyEndpoints, nil
default:
return nil, fmt.Errorf("unexpected service %s", svcNsName.Name)
}
@@ -2988,7 +3049,7 @@ func TestBuildUpstreams(t *testing.T) {
g := NewWithT(t)
- upstreams := buildUpstreams(context.TODO(), listeners, fakeResolver, Dual)
+ upstreams := buildUpstreams(context.TODO(), listeners, fakeResolver, referencedServices, Dual)
g.Expect(upstreams).To(ConsistOf(expUpstreams))
}
diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go
index 3d98d1d229..128990617b 100644
--- a/internal/mode/static/state/dataplane/types.go
+++ b/internal/mode/static/state/dataplane/types.go
@@ -111,7 +111,7 @@ type Upstream struct {
ErrorMsg string
// Endpoints are the endpoints of the Upstream.
Endpoints []resolver.Endpoint
- // Policies contains the list of policies that are applied to this Upstream.
+ // Policies holds all the valid policies that apply to the Upstream.
Policies []policies.Policy
}
diff --git a/internal/mode/static/state/graph/graph.go b/internal/mode/static/state/graph/graph.go
index 73b2ecac68..0b56ec1018 100644
--- a/internal/mode/static/state/graph/graph.go
+++ b/internal/mode/static/state/graph/graph.go
@@ -66,9 +66,8 @@ type Graph struct {
ReferencedSecrets map[types.NamespacedName]*Secret
// ReferencedNamespaces includes Namespaces with labels that match the Gateway Listener's label selector.
ReferencedNamespaces map[types.NamespacedName]*v1.Namespace
- // ReferencedServices includes the NamespacedNames of all the Services that are referenced by at least one HTTPRoute.
- // Storing the whole resource is not necessary, compared to the similar maps above.
- ReferencedServices map[types.NamespacedName]struct{}
+ // ReferencedServices includes the NamespacedNames of all the Services that are referenced by at least one Route.
+ ReferencedServices map[types.NamespacedName]*ReferencedService
// ReferencedCaCertConfigMaps includes ConfigMaps that have been referenced by any BackendTLSPolicies.
ReferencedCaCertConfigMaps map[types.NamespacedName]*CaCertConfigMap
// BackendTLSPolicies holds BackendTLSPolicy resources.
@@ -155,8 +154,18 @@ func (g *Graph) IsNGFPolicyRelevant(
}
for _, ref := range policy.GetTargetRefs() {
- if ref.Group == gatewayv1.GroupName && g.gatewayAPIResourceExist(ref, policy.GetNamespace()) {
- return true
+ switch ref.Group {
+ case gatewayv1.GroupName:
+ if g.gatewayAPIResourceExist(ref, policy.GetNamespace()) {
+ return true
+ }
+ case "", "core":
+ if ref.Kind == kinds.Service {
+ svcNsName := types.NamespacedName{Namespace: policy.GetNamespace(), Name: string(ref.Name)}
+ if _, exists := g.ReferencedServices[svcNsName]; exists {
+ return true
+ }
+ }
}
}
@@ -249,7 +258,7 @@ func BuildGraph(
referencedNamespaces := buildReferencedNamespaces(state.Namespaces, gw)
- referencedServices := buildReferencedServices(routes, l4routes)
+ referencedServices := buildReferencedServices(routes, l4routes, gw)
// policies must be processed last because they rely on the state of the other resources in the graph
processedPolicies := processPolicies(
@@ -257,6 +266,7 @@ func BuildGraph(
validators.PolicyValidator,
processedGws,
routes,
+ referencedServices,
globalSettings,
)
diff --git a/internal/mode/static/state/graph/graph_test.go b/internal/mode/static/state/graph/graph_test.go
index 627e308bc0..f2b71d05f8 100644
--- a/internal/mode/static/state/graph/graph_test.go
+++ b/internal/mode/static/state/graph/graph_test.go
@@ -33,7 +33,6 @@ func TestBuildGraph(t *testing.T) {
const (
gcName = "my-class"
controllerName = "my.controller"
- testNS = "test"
)
protectedPorts := ProtectedPorts{
@@ -894,7 +893,7 @@ func TestBuildGraph(t *testing.T) {
ReferencedNamespaces: map[types.NamespacedName]*v1.Namespace{
client.ObjectKeyFromObject(ns): ns,
},
- ReferencedServices: map[types.NamespacedName]struct{}{
+ ReferencedServices: map[types.NamespacedName]*ReferencedService{
client.ObjectKeyFromObject(svc): {},
client.ObjectKeyFromObject(svc1): {},
},
@@ -1157,7 +1156,7 @@ func TestIsReferenced(t *testing.T) {
ReferencedNamespaces: map[types.NamespacedName]*v1.Namespace{
client.ObjectKeyFromObject(nsInGraph): nsInGraph,
},
- ReferencedServices: map[types.NamespacedName]struct{}{
+ ReferencedServices: map[types.NamespacedName]*ReferencedService{
client.ObjectKeyFromObject(serviceInGraph): {},
},
ReferencedCaCertConfigMaps: map[types.NamespacedName]*CaCertConfigMap{
@@ -1359,6 +1358,7 @@ func TestIsNGFPolicyRelevant(t *testing.T) {
Source: &policiesfakes.FakePolicy{},
},
},
+ ReferencedServices: nil,
}
}
@@ -1469,6 +1469,46 @@ func TestIsNGFPolicyRelevant(t *testing.T) {
nsname: types.NamespacedName{Namespace: "test", Name: "nil-gw-source"},
expRelevant: false,
},
+ {
+ name: "relevant; policy references a Service that is referenced by a route, group core is inferred",
+ graph: getModifiedGraph(func(g *Graph) *Graph {
+ g.ReferencedServices = map[types.NamespacedName]*ReferencedService{
+ {Namespace: "test", Name: "ref-service"}: {},
+ }
+
+ return g
+ }),
+ policy: getPolicy(createTestRef(kinds.Service, "", "ref-service")),
+ nsname: types.NamespacedName{Namespace: "test", Name: "policy-for-svc"},
+ expRelevant: true,
+ },
+ {
+ name: "relevant; policy references a Service that is referenced by a route, group core is explicit",
+ graph: getModifiedGraph(func(g *Graph) *Graph {
+ g.ReferencedServices = map[types.NamespacedName]*ReferencedService{
+ {Namespace: "test", Name: "ref-service"}: {},
+ }
+
+ return g
+ }),
+ policy: getPolicy(createTestRef(kinds.Service, "core", "ref-service")),
+ nsname: types.NamespacedName{Namespace: "test", Name: "policy-for-svc"},
+ expRelevant: true,
+ },
+ {
+ name: "irrelevant; policy references a Service that is not referenced by a route, group core is inferred",
+ graph: getGraph(),
+ policy: getPolicy(createTestRef(kinds.Service, "", "not-ref-service")),
+ nsname: types.NamespacedName{Namespace: "test", Name: "policy-for-not-ref-svc"},
+ expRelevant: false,
+ },
+ {
+ name: "irrelevant; policy references a Service that is not referenced by a route, group core is explicit",
+ graph: getGraph(),
+ policy: getPolicy(createTestRef(kinds.Service, "core", "not-ref-service")),
+ nsname: types.NamespacedName{Namespace: "test", Name: "policy-for-not-ref-svc"},
+ expRelevant: false,
+ },
}
for _, test := range tests {
diff --git a/internal/mode/static/state/graph/policies.go b/internal/mode/static/state/graph/policies.go
index a4b794c5e1..9ce3e7eb23 100644
--- a/internal/mode/static/state/graph/policies.go
+++ b/internal/mode/static/state/graph/policies.go
@@ -21,7 +21,7 @@ import (
type Policy struct {
// Source is the corresponding Policy resource.
Source policies.Policy
- // Ancestors is list of ancestor objects of the Policy. Used in status.
+ // Ancestors is a list of ancestor objects of the Policy. Used in status.
Ancestors []PolicyAncestor
// TargetRefs are the resources that the Policy targets.
TargetRefs []PolicyTargetRef
@@ -63,6 +63,7 @@ const (
gatewayGroupKind = v1.GroupName + "/" + kinds.Gateway
hrGroupKind = v1.GroupName + "/" + kinds.HTTPRoute
grpcGroupKind = v1.GroupName + "/" + kinds.GRPCRoute
+ serviceGroupKind = "core" + "/" + kinds.Service
)
// attachPolicies attaches the graph's processed policies to the resources they target. It modifies the graph in place.
@@ -83,11 +84,42 @@ func (g *Graph) attachPolicies(ctlrName string) {
}
attachPolicyToRoute(policy, route, ctlrName)
+ case kinds.Service:
+ svc, exists := g.ReferencedServices[ref.Nsname]
+ if !exists {
+ continue
+ }
+
+ attachPolicyToService(policy, svc, g.Gateway, ctlrName)
}
}
}
}
+func attachPolicyToService(
+ policy *Policy,
+ svc *ReferencedService,
+ gw *Gateway,
+ ctlrName string,
+) {
+ if ngfPolicyAncestorsFull(policy, ctlrName) {
+ return
+ }
+
+ ancestor := PolicyAncestor{
+ Ancestor: createParentReference(v1.GroupName, kinds.Gateway, client.ObjectKeyFromObject(gw.Source)),
+ }
+
+ if !gw.Valid {
+ ancestor.Conditions = []conditions.Condition{staticConds.NewPolicyTargetNotFound("Parent Gateway is invalid")}
+ policy.Ancestors = append(policy.Ancestors, ancestor)
+ return
+ }
+
+ policy.Ancestors = append(policy.Ancestors, ancestor)
+ svc.Policies = append(svc.Policies, policy)
+}
+
func attachPolicyToRoute(policy *Policy, route *L7Route, ctlrName string) {
kind := v1.Kind(kinds.HTTPRoute)
if route.RouteType == RouteTypeGRPC {
@@ -158,6 +190,7 @@ func processPolicies(
validator validation.PolicyValidator,
gateways processedGateways,
routes map[RouteKey]*L7Route,
+ services map[types.NamespacedName]*ReferencedService,
globalSettings *policies.GlobalSettings,
) map[PolicyKey]*Policy {
if len(pols) == 0 || gateways.Winner == nil {
@@ -174,9 +207,8 @@ func processPolicies(
for _, ref := range policy.GetTargetRefs() {
refNsName := types.NamespacedName{Name: string(ref.Name), Namespace: policy.GetNamespace()}
- refGroupKind := fmt.Sprintf("%s/%s", ref.Group, ref.Kind)
- switch refGroupKind {
+ switch refGroupKind(ref.Group, ref.Kind) {
case gatewayGroupKind:
if !gatewayExists(refNsName, gateways.Winner, gateways.Ignored) {
continue
@@ -187,6 +219,10 @@ func processPolicies(
} else {
targetedRoutes[client.ObjectKeyFromObject(route.Source)] = route
}
+ case serviceGroupKind:
+ if _, exists := services[refNsName]; !exists {
+ continue
+ }
default:
continue
}
@@ -203,22 +239,8 @@ func processPolicies(
continue
}
- for _, targetedRoute := range targetedRoutes {
- // We need to check if this route referenced in the policy has an overlapping
- // hostname:port/path with any other route that isn't referenced by this policy.
- // If so, deny the policy.
- hostPortPaths := buildHostPortPaths(targetedRoute)
-
- for _, route := range routes {
- if _, ok := targetedRoutes[client.ObjectKeyFromObject(route.Source)]; ok {
- continue
- }
-
- if cond := checkForRouteOverlap(route, hostPortPaths); cond != nil {
- conds = append(conds, *cond)
- }
- }
- }
+ overlapConds := checkTargetRoutesForOverlap(targetedRoutes, routes)
+ conds = append(conds, overlapConds...)
conds = append(conds, validator.Validate(policy, globalSettings)...)
@@ -236,6 +258,32 @@ func processPolicies(
return processedPolicies
}
+func checkTargetRoutesForOverlap(
+ targetedRoutes map[types.NamespacedName]*L7Route,
+ graphRoutes map[RouteKey]*L7Route,
+) []conditions.Condition {
+ var conds []conditions.Condition
+
+ for _, targetedRoute := range targetedRoutes {
+ // We need to check if this route referenced in the policy has an overlapping
+ // hostname:port/path with any other route that isn't referenced by this policy.
+ // If so, deny the policy.
+ hostPortPaths := buildHostPortPaths(targetedRoute)
+
+ for _, route := range graphRoutes {
+ if _, ok := targetedRoutes[client.ObjectKeyFromObject(route.Source)]; ok {
+ continue
+ }
+
+ if cond := checkForRouteOverlap(route, hostPortPaths); cond != nil {
+ conds = append(conds, *cond)
+ }
+ }
+ }
+
+ return conds
+}
+
// checkForRouteOverlap checks if any route references the same hostname:port/path combination
// as a route referenced in a policy.
func checkForRouteOverlap(route *L7Route, hostPortPaths map[string]string) *conditions.Condition {
@@ -355,3 +403,12 @@ func markConflictedPolicies(pols map[PolicyKey]*Policy, validator validation.Pol
}
}
}
+
+// refGroupKind formats the group and kind as a string.
+func refGroupKind(group v1.Group, kind v1.Kind) string {
+ if group == "" {
+ return fmt.Sprintf("core/%s", kind)
+ }
+
+ return fmt.Sprintf("%s/%s", group, kind)
+}
diff --git a/internal/mode/static/state/graph/policies_test.go b/internal/mode/static/state/graph/policies_test.go
index 340d3f10f0..2c11ac2229 100644
--- a/internal/mode/static/state/graph/policies_test.go
+++ b/internal/mode/static/state/graph/policies_test.go
@@ -15,7 +15,7 @@ import (
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies"
- policiesfakes "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/policiesfakes"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/policiesfakes"
staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation"
)
@@ -24,7 +24,9 @@ var testNs = "test"
func TestAttachPolicies(t *testing.T) {
t.Parallel()
+
policyGVK := schema.GroupVersionKind{Group: "Group", Version: "Version", Kind: "Policy"}
+
createPolicy := func(targetRefsNames []string, refKind v1.Kind) *Policy {
targetRefs := make([]PolicyTargetRef, 0, len(targetRefsNames))
for _, name := range targetRefsNames {
@@ -48,18 +50,6 @@ func TestAttachPolicies(t *testing.T) {
}
}
- createGateway := func(name string) *Gateway {
- return &Gateway{
- Source: &v1.Gateway{
- ObjectMeta: metav1.ObjectMeta{
- Name: name,
- Namespace: testNs,
- },
- },
- Valid: true,
- }
- }
-
createRoutesForGraph := func(routes map[string]RouteType) map[RouteKey]*L7Route {
routesMap := make(map[RouteKey]*L7Route, len(routes))
for routeName, routeType := range routes {
@@ -84,94 +74,137 @@ func TestAttachPolicies(t *testing.T) {
return routesMap
}
- expectNoPolicyAttachment := func(g *WithT, graph *Graph) {
+ expectNoGatewayPolicyAttachment := func(g *WithT, graph *Graph) {
if graph.Gateway != nil {
g.Expect(graph.Gateway.Policies).To(BeNil())
}
+ }
+ expectNoRoutePolicyAttachment := func(g *WithT, graph *Graph) {
for _, r := range graph.Routes {
g.Expect(r.Policies).To(BeNil())
}
}
- expectPolicyAttachment := func(g *WithT, graph *Graph) {
+ expectNoSvcPolicyAttachment := func(g *WithT, graph *Graph) {
+ for _, r := range graph.ReferencedServices {
+ g.Expect(r.Policies).To(BeNil())
+ }
+ }
+
+ expectGatewayPolicyAttachment := func(g *WithT, graph *Graph) {
if graph.Gateway != nil {
g.Expect(graph.Gateway.Policies).To(HaveLen(1))
}
+ }
+ expectRoutePolicyAttachment := func(g *WithT, graph *Graph) {
for _, r := range graph.Routes {
g.Expect(r.Policies).To(HaveLen(1))
}
}
- expectGatewayPolicyAttachment := func(g *WithT, graph *Graph) {
- if graph.Gateway != nil {
- g.Expect(graph.Gateway.Policies).To(HaveLen(1))
+ expectSvcPolicyAttachment := func(g *WithT, graph *Graph) {
+ for _, r := range graph.ReferencedServices {
+ g.Expect(r.Policies).To(HaveLen(1))
}
+ }
- for _, r := range graph.Routes {
- g.Expect(r.Policies).To(BeNil())
+ expectNoAttachmentList := []func(g *WithT, graph *Graph){
+ expectNoGatewayPolicyAttachment,
+ expectNoSvcPolicyAttachment,
+ expectNoRoutePolicyAttachment,
+ }
+
+ expectAllAttachmentList := []func(g *WithT, graph *Graph){
+ expectGatewayPolicyAttachment,
+ expectSvcPolicyAttachment,
+ expectRoutePolicyAttachment,
+ }
+
+ getPolicies := func() map[PolicyKey]*Policy {
+ return map[PolicyKey]*Policy{
+ createTestPolicyKey(policyGVK, "gw-policy1"): createPolicy([]string{"gateway", "gateway1"}, kinds.Gateway),
+ createTestPolicyKey(policyGVK, "route-policy1"): createPolicy(
+ []string{"hr1-route", "hr2-route"},
+ kinds.HTTPRoute,
+ ),
+ createTestPolicyKey(policyGVK, "grpc-route-policy1"): createPolicy([]string{"grpc-route"}, kinds.GRPCRoute),
+ createTestPolicyKey(policyGVK, "svc-policy"): createPolicy([]string{"svc-1"}, kinds.Service),
+ }
+ }
+
+ getRoutes := func() map[RouteKey]*L7Route {
+ return createRoutesForGraph(
+ map[string]RouteType{
+ "hr1-route": RouteTypeHTTP,
+ "hr2-route": RouteTypeHTTP,
+ "grpc-route": RouteTypeGRPC,
+ },
+ )
+ }
+
+ getGateway := func() *Gateway {
+ return &Gateway{
+ Source: &v1.Gateway{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "gateway",
+ Namespace: testNs,
+ },
+ },
+ Valid: true,
+ }
+ }
+
+ getServices := func() map[types.NamespacedName]*ReferencedService {
+ return map[types.NamespacedName]*ReferencedService{
+ {Namespace: testNs, Name: "svc-1"}: {},
}
}
tests := []struct {
gateway *Gateway
routes map[RouteKey]*L7Route
+ svcs map[types.NamespacedName]*ReferencedService
ngfPolicies map[PolicyKey]*Policy
- expect func(g *WithT, graph *Graph)
name string
+ expects []func(g *WithT, graph *Graph)
}{
{
- name: "nil Gateway",
- routes: createRoutesForGraph(
- map[string]RouteType{
- "hr1-route": RouteTypeHTTP,
- "hr2-route": RouteTypeHTTP,
- "grpc-route": RouteTypeGRPC,
- },
- ),
- ngfPolicies: map[PolicyKey]*Policy{
- createTestPolicyKey(policyGVK, "gw-policy"): createPolicy([]string{"gateway", "gateway1"}, kinds.Gateway),
- createTestPolicyKey(policyGVK, "route-policy"): createPolicy(
- []string{"hr1-route", "hr2-route"},
- kinds.HTTPRoute,
- ),
- createTestPolicyKey(policyGVK, "grpc-route-policy"): createPolicy([]string{"grpc-route"}, kinds.GRPCRoute),
- },
- expect: expectNoPolicyAttachment,
+ name: "nil Gateway; no policies attach",
+ routes: getRoutes(),
+ ngfPolicies: getPolicies(),
+ expects: expectNoAttachmentList,
},
{
- name: "nil routes",
- gateway: createGateway("gateway"),
- ngfPolicies: map[PolicyKey]*Policy{
- createTestPolicyKey(policyGVK, "gw-policy1"): createPolicy([]string{"gateway", "gateway1"}, kinds.Gateway),
- createTestPolicyKey(policyGVK, "route-policy1"): createPolicy(
- []string{"hr1-route", "hr2-route"},
- kinds.HTTPRoute,
- ),
- createTestPolicyKey(policyGVK, "grpc-route-policy1"): createPolicy([]string{"grpc-route"}, kinds.GRPCRoute),
+ name: "nil Routes; gateway and service policies attach",
+ gateway: getGateway(),
+ svcs: getServices(),
+ ngfPolicies: getPolicies(),
+ expects: []func(g *WithT, graph *Graph){
+ expectGatewayPolicyAttachment,
+ expectSvcPolicyAttachment,
+ expectNoRoutePolicyAttachment,
},
- expect: expectGatewayPolicyAttachment,
},
{
- name: "normal",
- routes: createRoutesForGraph(
- map[string]RouteType{
- "hr-1": RouteTypeHTTP,
- "hr-2": RouteTypeHTTP,
- "grpc-1": RouteTypeGRPC,
- },
- ),
- ngfPolicies: map[PolicyKey]*Policy{
- createTestPolicyKey(policyGVK, "gw-policy2"): createPolicy([]string{"gateway2", "gateway3"}, kinds.Gateway),
- createTestPolicyKey(policyGVK, "route-policy2"): createPolicy(
- []string{"hr-1", "hr-2"},
- kinds.HTTPRoute,
- ),
- createTestPolicyKey(policyGVK, "grpc-route-policy2"): createPolicy([]string{"grpc-1"}, kinds.GRPCRoute),
+ name: "nil ReferencedServices; gateway and route policies attach",
+ routes: getRoutes(),
+ ngfPolicies: getPolicies(),
+ gateway: getGateway(),
+ expects: []func(g *WithT, graph *Graph){
+ expectGatewayPolicyAttachment,
+ expectRoutePolicyAttachment,
+ expectNoSvcPolicyAttachment,
},
- gateway: createGateway("gateway2"),
- expect: expectPolicyAttachment,
+ },
+ {
+ name: "all policies attach",
+ routes: getRoutes(),
+ svcs: getServices(),
+ ngfPolicies: getPolicies(),
+ gateway: getGateway(),
+ expects: expectAllAttachmentList,
},
}
@@ -181,13 +214,16 @@ func TestAttachPolicies(t *testing.T) {
g := NewWithT(t)
graph := &Graph{
- Gateway: test.gateway,
- Routes: test.routes,
- NGFPolicies: test.ngfPolicies,
+ Gateway: test.gateway,
+ Routes: test.routes,
+ ReferencedServices: test.svcs,
+ NGFPolicies: test.ngfPolicies,
}
graph.attachPolicies("nginx-gateway")
- test.expect(g, graph)
+ for _, expect := range test.expects {
+ expect(g, graph)
+ }
})
}
}
@@ -360,15 +396,6 @@ func TestAttachPolicyToGateway(t *testing.T) {
}
}
- getGatewayParentRef := func(gwNsName types.NamespacedName) v1.ParentReference {
- return v1.ParentReference{
- Group: helpers.GetPointer[v1.Group](v1.GroupName),
- Kind: helpers.GetPointer[v1.Kind]("Gateway"),
- Namespace: (*v1.Namespace)(&gwNsName.Namespace),
- Name: v1.ObjectName(gwNsName.Name),
- }
- }
-
tests := []struct {
policy *Policy
gw *Gateway
@@ -508,6 +535,83 @@ func TestAttachPolicyToGateway(t *testing.T) {
}
}
+func TestAttachPolicyToService(t *testing.T) {
+ t.Parallel()
+
+ gwNsname := types.NamespacedName{Namespace: testNs, Name: "gateway"}
+
+ getGateway := func(valid bool) *Gateway {
+ return &Gateway{
+ Source: &v1.Gateway{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: gwNsname.Name,
+ Namespace: gwNsname.Namespace,
+ },
+ },
+ Valid: valid,
+ }
+ }
+
+ tests := []struct {
+ policy *Policy
+ svc *ReferencedService
+ gw *Gateway
+ name string
+ expAncestors []PolicyAncestor
+ expAttached bool
+ }{
+ {
+ name: "attachment",
+ policy: &Policy{Source: &policiesfakes.FakePolicy{}},
+ svc: &ReferencedService{},
+ gw: getGateway(true /*valid*/),
+ expAttached: true,
+ expAncestors: []PolicyAncestor{
+ {
+ Ancestor: getGatewayParentRef(gwNsname),
+ },
+ },
+ },
+ {
+ name: "no attachment; gateway is invalid",
+ policy: &Policy{Source: &policiesfakes.FakePolicy{}},
+ svc: &ReferencedService{},
+ gw: getGateway(false /*invalid*/),
+ expAttached: false,
+ expAncestors: []PolicyAncestor{
+ {
+ Ancestor: getGatewayParentRef(gwNsname),
+ Conditions: []conditions.Condition{staticConds.NewPolicyTargetNotFound("Parent Gateway is invalid")},
+ },
+ },
+ },
+ {
+ name: "no attachment; max ancestor",
+ policy: &Policy{Source: createTestPolicyWithAncestors(16)},
+ svc: &ReferencedService{},
+ gw: getGateway(true /*valid*/),
+ expAttached: false,
+ expAncestors: nil,
+ },
+ }
+
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ t.Parallel()
+ g := NewWithT(t)
+
+ attachPolicyToService(test.policy, test.svc, test.gw, "ctlr")
+ if test.expAttached {
+ g.Expect(test.svc.Policies).To(HaveLen(1))
+ } else {
+ g.Expect(test.svc.Policies).To(BeEmpty())
+ }
+
+ g.Expect(test.policy.Ancestors).To(BeEquivalentTo(test.expAncestors))
+ })
+ }
+}
+
func TestProcessPolicies(t *testing.T) {
t.Parallel()
policyGVK := schema.GroupVersionKind{Group: "Group", Version: "Version", Kind: "MyPolicy"}
@@ -518,6 +622,7 @@ func TestProcessPolicies(t *testing.T) {
grpcRef := createTestRef(kinds.GRPCRoute, v1.GroupName, "grpc")
gatewayRef := createTestRef(kinds.Gateway, v1.GroupName, "gw")
ignoredGatewayRef := createTestRef(kinds.Gateway, v1.GroupName, "ignored")
+ svcRef := createTestRef(kinds.Service, "core", "svc")
// These refs reference objects that do not belong to NGF.
// Policies that contain these refs should NOT be processed.
@@ -525,6 +630,7 @@ func TestProcessPolicies(t *testing.T) {
hrWrongGroup := createTestRef(kinds.HTTPRoute, "WrongGroup", "hr")
gatewayWrongGroupRef := createTestRef(kinds.Gateway, "WrongGroup", "gw")
nonNGFGatewayRef := createTestRef(kinds.Gateway, v1.GroupName, "not-ours")
+ svcDoesNotExistRef := createTestRef(kinds.Service, "core", "dne")
pol1, pol1Key := createTestPolicyAndKey(policyGVK, "pol1", hrRef)
pol2, pol2Key := createTestPolicyAndKey(policyGVK, "pol2", grpcRef)
@@ -534,6 +640,8 @@ func TestProcessPolicies(t *testing.T) {
pol6, pol6Key := createTestPolicyAndKey(policyGVK, "pol6", hrWrongGroup)
pol7, pol7Key := createTestPolicyAndKey(policyGVK, "pol7", gatewayWrongGroupRef)
pol8, pol8Key := createTestPolicyAndKey(policyGVK, "pol8", nonNGFGatewayRef)
+ pol9, pol9Key := createTestPolicyAndKey(policyGVK, "pol9", svcDoesNotExistRef)
+ pol10, pol10Key := createTestPolicyAndKey(policyGVK, "pol10", svcRef)
pol1Conflict, pol1ConflictKey := createTestPolicyAndKey(policyGVK, "pol1-conflict", hrRef)
@@ -553,14 +661,16 @@ func TestProcessPolicies(t *testing.T) {
name: "mix of relevant and irrelevant policies",
validator: allValidValidator,
policies: map[PolicyKey]policies.Policy{
- pol1Key: pol1,
- pol2Key: pol2,
- pol3Key: pol3,
- pol4Key: pol4,
- pol5Key: pol5,
- pol6Key: pol6,
- pol7Key: pol7,
- pol8Key: pol8,
+ pol1Key: pol1,
+ pol2Key: pol2,
+ pol3Key: pol3,
+ pol4Key: pol4,
+ pol5Key: pol5,
+ pol6Key: pol6,
+ pol7Key: pol7,
+ pol8Key: pol8,
+ pol9Key: pol9,
+ pol10Key: pol10,
},
expProcessedPolicies: map[PolicyKey]*Policy{
pol1Key: {
@@ -611,6 +721,18 @@ func TestProcessPolicies(t *testing.T) {
Ancestors: []PolicyAncestor{},
Valid: true,
},
+ pol10Key: {
+ Source: pol10,
+ TargetRefs: []PolicyTargetRef{
+ {
+ Nsname: types.NamespacedName{Namespace: testNs, Name: "svc"},
+ Kind: kinds.Service,
+ Group: "core",
+ },
+ },
+ Ancestors: []PolicyAncestor{},
+ Valid: true,
+ },
},
},
{
@@ -740,12 +862,16 @@ func TestProcessPolicies(t *testing.T) {
},
}
+ services := map[types.NamespacedName]*ReferencedService{
+ {Namespace: testNs, Name: "svc"}: {},
+ }
+
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
t.Parallel()
g := NewWithT(t)
- processed := processPolicies(test.policies, test.validator, gateways, routes, nil)
+ processed := processPolicies(test.policies, test.validator, gateways, routes, services, nil)
g.Expect(processed).To(BeEquivalentTo(test.expProcessedPolicies))
})
}
@@ -885,7 +1011,7 @@ func TestProcessPolicies_RouteOverlap(t *testing.T) {
t.Parallel()
g := NewWithT(t)
- processed := processPolicies(test.policies, test.validator, gateways, test.routes, nil)
+ processed := processPolicies(test.policies, test.validator, gateways, test.routes, nil, nil)
g.Expect(processed).To(HaveLen(1))
for _, pol := range processed {
@@ -1051,6 +1177,45 @@ func TestMarkConflictedPolicies(t *testing.T) {
}
}
+func TestRefGroupKind(t *testing.T) {
+ t.Parallel()
+
+ tests := []struct {
+ name string
+ group v1.Group
+ kind v1.Kind
+ expString string
+ }{
+ {
+ name: "explicit group core",
+ group: "core",
+ kind: kinds.Service,
+ expString: "core/Service",
+ },
+ {
+ name: "implicit group core",
+ group: "",
+ kind: kinds.Service,
+ expString: "core/Service",
+ },
+ {
+ name: "gateway group",
+ group: v1.GroupName,
+ kind: kinds.HTTPRoute,
+ expString: "gateway.networking.k8s.io/HTTPRoute",
+ },
+ }
+
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ t.Parallel()
+ g := NewWithT(t)
+
+ g.Expect(refGroupKind(test.group, test.kind)).To(Equal(test.expString))
+ })
+ }
+}
+
func createTestPolicyWithAncestors(numAncestors int) policies.Policy {
policy := &policiesfakes.FakePolicy{}
@@ -1151,3 +1316,12 @@ func createTestRouteWithPaths(name string, paths ...string) *L7Route {
return route
}
+
+func getGatewayParentRef(gwNsName types.NamespacedName) v1.ParentReference {
+ return v1.ParentReference{
+ Group: helpers.GetPointer[v1.Group](v1.GroupName),
+ Kind: helpers.GetPointer[v1.Kind]("Gateway"),
+ Namespace: (*v1.Namespace)(&gwNsName.Namespace),
+ Name: v1.ObjectName(gwNsName.Name),
+ }
+}
diff --git a/internal/mode/static/state/graph/service.go b/internal/mode/static/state/graph/service.go
index ad33579f1e..ad6fb817ef 100644
--- a/internal/mode/static/state/graph/service.go
+++ b/internal/mode/static/state/graph/service.go
@@ -2,17 +2,31 @@ package graph
import (
"k8s.io/apimachinery/pkg/types"
+ "sigs.k8s.io/controller-runtime/pkg/client"
)
+// A ReferencedService represents a Kubernetes Service that is referenced by a Route and that belongs to the
+// winning Gateway. It does not contain the v1.Service object, because Services are resolved when building
+// the dataplane.Configuration.
+type ReferencedService struct {
+ // Policies is a list of NGF Policies that target this Service.
+ Policies []*Policy
+}
+
func buildReferencedServices(
l7routes map[RouteKey]*L7Route,
l4Routes map[L4RouteKey]*L4Route,
-) map[types.NamespacedName]struct{} {
- svcNames := make(map[types.NamespacedName]struct{})
+ gw *Gateway,
+) map[types.NamespacedName]*ReferencedService {
+ if gw == nil {
+ return nil
+ }
- attached := func(parentRefs []ParentRef) bool {
- for _, ref := range parentRefs {
- if ref.Attachment.Attached {
+ referencedServices := make(map[types.NamespacedName]*ReferencedService)
+
+ belongsToWinningGw := func(refs []ParentRef) bool {
+ for _, ref := range refs {
+ if ref.Gateway == client.ObjectKeyFromObject(gw.Source) {
return true
}
}
@@ -22,21 +36,24 @@ func buildReferencedServices(
// Processes both valid and invalid BackendRefs as invalid ones still have referenced services
// we may want to track.
-
- populateServiceNamesForL7Routes := func(routeRules []RouteRule) {
+ addServicesForL7Routes := func(routeRules []RouteRule) {
for _, rule := range routeRules {
for _, ref := range rule.BackendRefs {
if ref.SvcNsName != (types.NamespacedName{}) {
- svcNames[ref.SvcNsName] = struct{}{}
+ referencedServices[ref.SvcNsName] = &ReferencedService{
+ Policies: nil,
+ }
}
}
}
}
- populateServiceNamesForL4Routes := func(route *L4Route) {
+ addServicesForL4Routes := func(route *L4Route) {
nsname := route.Spec.BackendRef.SvcNsName
if nsname != (types.NamespacedName{}) {
- svcNames[nsname] = struct{}{}
+ referencedServices[nsname] = &ReferencedService{
+ Policies: nil,
+ }
}
}
@@ -48,12 +65,11 @@ func buildReferencedServices(
continue
}
- // If none of the ParentRefs are attached to the Gateway, we want to skip the route.
- if !attached(route.ParentRefs) {
+ if !belongsToWinningGw(route.ParentRefs) {
continue
}
- populateServiceNamesForL7Routes(route.Spec.Rules)
+ addServicesForL7Routes(route.Spec.Rules)
}
for _, route := range l4Routes {
@@ -61,16 +77,16 @@ func buildReferencedServices(
continue
}
- // If none of the ParentRefs are attached to the Gateway, we want to skip the route.
- if !attached(route.ParentRefs) {
+ if !belongsToWinningGw(route.ParentRefs) {
continue
}
- populateServiceNamesForL4Routes(route)
+ addServicesForL4Routes(route)
}
- if len(svcNames) == 0 {
+ if len(referencedServices) == 0 {
return nil
}
- return svcNames
+
+ return referencedServices
}
diff --git a/internal/mode/static/state/graph/service_test.go b/internal/mode/static/state/graph/service_test.go
index 5c60831a31..0fa316e73f 100644
--- a/internal/mode/static/state/graph/service_test.go
+++ b/internal/mode/static/state/graph/service_test.go
@@ -4,18 +4,30 @@ import (
"testing"
. "github.com/onsi/gomega"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
+ v1 "sigs.k8s.io/gateway-api/apis/v1"
)
func TestBuildReferencedServices(t *testing.T) {
t.Parallel()
+
+ gwNsname := types.NamespacedName{Namespace: "test", Name: "gwNsname"}
+ gw := &Gateway{
+ Source: &v1.Gateway{
+ ObjectMeta: metav1.ObjectMeta{
+ Namespace: gwNsname.Namespace,
+ Name: gwNsname.Name,
+ },
+ },
+ }
+ ignoredGw := types.NamespacedName{Namespace: "test", Name: "ignoredGw"}
+
getNormalL7Route := func() *L7Route {
return &L7Route{
ParentRefs: []ParentRef{
{
- Attachment: &ParentRefAttachmentStatus{
- Attached: true,
- },
+ Gateway: gwNsname,
},
},
Valid: true,
@@ -48,9 +60,7 @@ func TestBuildReferencedServices(t *testing.T) {
Valid: true,
ParentRefs: []ParentRef{
{
- Attachment: &ParentRefAttachmentStatus{
- Attached: true,
- },
+ Gateway: gwNsname,
},
},
}
@@ -117,111 +127,102 @@ func TestBuildReferencedServices(t *testing.T) {
return route
})
- unattachedRoute := getModifiedL7Route(func(route *L7Route) *L7Route {
- route.ParentRefs[0].Attachment.Attached = false
+ validRouteNoServiceNsName := getModifiedL7Route(func(route *L7Route) *L7Route {
+ route.Spec.Rules[0].BackendRefs[0].SvcNsName = types.NamespacedName{}
return route
})
- unattachedL4Route := getModifiedL4Route(func(route *L4Route) *L4Route {
- route.ParentRefs[0].Attachment.Attached = false
+ validL4RouteNoServiceNsName := getModifiedL4Route(func(route *L4Route) *L4Route {
+ route.Spec.BackendRef.SvcNsName = types.NamespacedName{}
return route
})
- attachedRouteWithManyParentRefs := getModifiedL7Route(func(route *L7Route) *L7Route {
+ normalL4RouteWinningAndIgnoredGws := getModifiedL4Route(func(route *L4Route) *L4Route {
route.ParentRefs = []ParentRef{
{
- Attachment: &ParentRefAttachmentStatus{
- Attached: false,
- },
+ Gateway: ignoredGw,
},
{
- Attachment: &ParentRefAttachmentStatus{
- Attached: false,
- },
+ Gateway: ignoredGw,
},
{
- Attachment: &ParentRefAttachmentStatus{
- Attached: true,
- },
+ Gateway: gwNsname,
},
}
-
return route
})
- attachedL4RoutesWithManyParentRefs := getModifiedL4Route(func(route *L4Route) *L4Route {
+ normalRouteWinningAndIgnoredGws := getModifiedL7Route(func(route *L7Route) *L7Route {
route.ParentRefs = []ParentRef{
{
- Attachment: &ParentRefAttachmentStatus{
- Attached: false,
- },
+ Gateway: ignoredGw,
},
{
- Attachment: &ParentRefAttachmentStatus{
- Attached: true,
- },
+ Gateway: gwNsname,
},
{
- Attachment: &ParentRefAttachmentStatus{
- Attached: false,
- },
+ Gateway: ignoredGw,
},
}
-
return route
})
- validRouteNoServiceNsName := getModifiedL7Route(func(route *L7Route) *L7Route {
- route.Spec.Rules[0].BackendRefs[0].SvcNsName = types.NamespacedName{}
+ normalL4RouteIgnoredGw := getModifiedL4Route(func(route *L4Route) *L4Route {
+ route.ParentRefs[0].Gateway = ignoredGw
return route
})
- validL4RouteNoServiceNsName := getModifiedL4Route(func(route *L4Route) *L4Route {
- route.Spec.BackendRef.SvcNsName = types.NamespacedName{}
+ normalL7RouteIgnoredGw := getModifiedL7Route(func(route *L7Route) *L7Route {
+ route.ParentRefs[0].Gateway = ignoredGw
return route
})
tests := []struct {
l7Routes map[RouteKey]*L7Route
l4Routes map[L4RouteKey]*L4Route
- exp map[types.NamespacedName]struct{}
+ exp map[types.NamespacedName]*ReferencedService
+ gw *Gateway
name string
}{
{
name: "normal routes",
+ gw: gw,
l7Routes: map[RouteKey]*L7Route{
{NamespacedName: types.NamespacedName{Name: "normal-route"}}: normalRoute,
},
l4Routes: map[L4RouteKey]*L4Route{
{NamespacedName: types.NamespacedName{Name: "normal-l4-route"}}: normalL4Route,
},
- exp: map[types.NamespacedName]struct{}{
+ exp: map[types.NamespacedName]*ReferencedService{
{Namespace: "banana-ns", Name: "service"}: {},
{Namespace: "tlsroute-ns", Name: "service"}: {},
},
},
{
name: "l7 route with two services in one Rule", // l4 routes don't support multiple services right now
+ gw: gw,
l7Routes: map[RouteKey]*L7Route{
{NamespacedName: types.NamespacedName{Name: "two-svc-one-rule"}}: validRouteTwoServicesOneRule,
},
- exp: map[types.NamespacedName]struct{}{
+ exp: map[types.NamespacedName]*ReferencedService{
{Namespace: "service-ns", Name: "service"}: {},
{Namespace: "service-ns2", Name: "service2"}: {},
},
},
{
name: "route with one service per rule", // l4 routes don't support multiple rules right now
+ gw: gw,
l7Routes: map[RouteKey]*L7Route{
{NamespacedName: types.NamespacedName{Name: "one-svc-per-rule"}}: validRouteTwoServicesTwoRules,
},
- exp: map[types.NamespacedName]struct{}{
+ exp: map[types.NamespacedName]*ReferencedService{
{Namespace: "service-ns", Name: "service"}: {},
{Namespace: "service-ns2", Name: "service2"}: {},
},
},
{
name: "multiple valid routes with same services",
+ gw: gw,
l7Routes: map[RouteKey]*L7Route{
{NamespacedName: types.NamespacedName{Name: "one-svc-per-rule"}}: validRouteTwoServicesTwoRules,
{NamespacedName: types.NamespacedName{Name: "two-svc-one-rule"}}: validRouteTwoServicesOneRule,
@@ -231,15 +232,41 @@ func TestBuildReferencedServices(t *testing.T) {
{NamespacedName: types.NamespacedName{Name: "l4-route-2"}}: normalL4Route2,
{NamespacedName: types.NamespacedName{Name: "l4-route-same-svc-as-l7-route"}}: normalL4RouteWithSameSvcAsL7Route,
},
- exp: map[types.NamespacedName]struct{}{
+ exp: map[types.NamespacedName]*ReferencedService{
{Namespace: "service-ns", Name: "service"}: {},
{Namespace: "service-ns2", Name: "service2"}: {},
{Namespace: "tlsroute-ns", Name: "service"}: {},
{Namespace: "tlsroute-ns", Name: "service2"}: {},
},
},
+ {
+ name: "valid routes that do not belong to winning gateway",
+ gw: gw,
+ l7Routes: map[RouteKey]*L7Route{
+ {NamespacedName: types.NamespacedName{Name: "belongs-to-ignored-gws"}}: normalL7RouteIgnoredGw,
+ },
+ l4Routes: map[L4RouteKey]*L4Route{
+ {NamespacedName: types.NamespacedName{Name: "belongs-to-ignored-gw"}}: normalL4RouteIgnoredGw,
+ },
+ exp: nil,
+ },
+ {
+ name: "valid routes that belong to both winning and ignored gateways",
+ gw: gw,
+ l7Routes: map[RouteKey]*L7Route{
+ {NamespacedName: types.NamespacedName{Name: "belongs-to-ignored-gws"}}: normalRouteWinningAndIgnoredGws,
+ },
+ l4Routes: map[L4RouteKey]*L4Route{
+ {NamespacedName: types.NamespacedName{Name: "ignored-gw"}}: normalL4RouteWinningAndIgnoredGws,
+ },
+ exp: map[types.NamespacedName]*ReferencedService{
+ {Namespace: "banana-ns", Name: "service"}: {},
+ {Namespace: "tlsroute-ns", Name: "service"}: {},
+ },
+ },
{
name: "valid routes with different services",
+ gw: gw,
l7Routes: map[RouteKey]*L7Route{
{NamespacedName: types.NamespacedName{Name: "one-svc-per-rule"}}: validRouteTwoServicesTwoRules,
{NamespacedName: types.NamespacedName{Name: "normal-route"}}: normalRoute,
@@ -247,7 +274,7 @@ func TestBuildReferencedServices(t *testing.T) {
l4Routes: map[L4RouteKey]*L4Route{
{NamespacedName: types.NamespacedName{Name: "normal-l4-route"}}: normalL4Route,
},
- exp: map[types.NamespacedName]struct{}{
+ exp: map[types.NamespacedName]*ReferencedService{
{Namespace: "service-ns", Name: "service"}: {},
{Namespace: "service-ns2", Name: "service2"}: {},
{Namespace: "banana-ns", Name: "service"}: {},
@@ -256,6 +283,7 @@ func TestBuildReferencedServices(t *testing.T) {
},
{
name: "invalid routes",
+ gw: gw,
l7Routes: map[RouteKey]*L7Route{
{NamespacedName: types.NamespacedName{Name: "invalid-route"}}: invalidRoute,
},
@@ -264,18 +292,9 @@ func TestBuildReferencedServices(t *testing.T) {
},
exp: nil,
},
- {
- name: "unattached route",
- l7Routes: map[RouteKey]*L7Route{
- {NamespacedName: types.NamespacedName{Name: "unattached-route"}}: unattachedRoute,
- },
- l4Routes: map[L4RouteKey]*L4Route{
- {NamespacedName: types.NamespacedName{Name: "unattached-l4-route"}}: unattachedL4Route,
- },
- exp: nil,
- },
{
name: "combination of valid and invalid routes",
+ gw: gw,
l7Routes: map[RouteKey]*L7Route{
{NamespacedName: types.NamespacedName{Name: "normal-route"}}: normalRoute,
{NamespacedName: types.NamespacedName{Name: "invalid-route"}}: invalidRoute,
@@ -284,26 +303,25 @@ func TestBuildReferencedServices(t *testing.T) {
{NamespacedName: types.NamespacedName{Name: "invalid-l4-route"}}: invalidL4Route,
{NamespacedName: types.NamespacedName{Name: "normal-l4-route"}}: normalL4Route,
},
- exp: map[types.NamespacedName]struct{}{
+ exp: map[types.NamespacedName]*ReferencedService{
{Namespace: "banana-ns", Name: "service"}: {},
{Namespace: "tlsroute-ns", Name: "service"}: {},
},
},
{
- name: "route with many parentRefs and one is attached",
+ name: "valid route no service nsname",
+ gw: gw,
l7Routes: map[RouteKey]*L7Route{
- {NamespacedName: types.NamespacedName{Name: "multiple-parent-ref-route"}}: attachedRouteWithManyParentRefs,
+ {NamespacedName: types.NamespacedName{Name: "no-service-nsname"}}: validRouteNoServiceNsName,
},
l4Routes: map[L4RouteKey]*L4Route{
- {NamespacedName: types.NamespacedName{Name: "multiple-parent-ref-l4-route"}}: attachedL4RoutesWithManyParentRefs,
- },
- exp: map[types.NamespacedName]struct{}{
- {Namespace: "banana-ns", Name: "service"}: {},
- {Namespace: "tlsroute-ns", Name: "service"}: {},
+ {NamespacedName: types.NamespacedName{Name: "no-service-nsname-l4"}}: validL4RouteNoServiceNsName,
},
+ exp: nil,
},
{
- name: "valid route no service nsname",
+ name: "nil gateway",
+ gw: nil,
l7Routes: map[RouteKey]*L7Route{
{NamespacedName: types.NamespacedName{Name: "no-service-nsname"}}: validRouteNoServiceNsName,
},
@@ -318,7 +336,8 @@ func TestBuildReferencedServices(t *testing.T) {
t.Run(test.name, func(t *testing.T) {
t.Parallel()
g := NewWithT(t)
- g.Expect(buildReferencedServices(test.l7Routes, test.l4Routes)).To(Equal(test.exp))
+
+ g.Expect(buildReferencedServices(test.l7Routes, test.l4Routes, test.gw)).To(Equal(test.exp))
})
}
}
diff --git a/internal/mode/static/telemetry/collector_test.go b/internal/mode/static/telemetry/collector_test.go
index 59bee05aae..3128531ebb 100644
--- a/internal/mode/static/telemetry/collector_test.go
+++ b/internal/mode/static/telemetry/collector_test.go
@@ -302,7 +302,7 @@ var _ = Describe("Collector", Ordered, func() {
},
client.ObjectKeyFromObject(nilsecret): nil,
},
- ReferencedServices: map[types.NamespacedName]struct{}{
+ ReferencedServices: map[types.NamespacedName]*graph.ReferencedService{
client.ObjectKeyFromObject(svc1): {},
client.ObjectKeyFromObject(svc2): {},
client.ObjectKeyFromObject(nilsvc): {},
@@ -583,7 +583,7 @@ var _ = Describe("Collector", Ordered, func() {
Source: secret,
},
},
- ReferencedServices: map[types.NamespacedName]struct{}{
+ ReferencedServices: map[types.NamespacedName]*graph.ReferencedService{
client.ObjectKeyFromObject(svc): {},
},
NGFPolicies: map[graph.PolicyKey]*graph.Policy{
From 6949ea3f00b51d7d7084c6ee26f23590b6f6be89 Mon Sep 17 00:00:00 2001
From: Kate Osborn <50597707+kate-osborn@users.noreply.github.com>
Date: Thu, 19 Dec 2024 12:01:33 -0700
Subject: [PATCH 4/7] Collect UpstreamSettingsPolicyCount in telemetry (#2922)
Problem: Want to collect number of UpstreamSettingsPolicies in cluster.
Solution: Collect the count of UpstreamSettingsPolicies in cluster.
---
internal/mode/static/telemetry/collector.go | 4 +++
.../mode/static/telemetry/collector_test.go | 27 ++++++++-----------
internal/mode/static/telemetry/data.avdl | 3 +++
internal/mode/static/telemetry/data_test.go | 3 +++
.../ngfresourcecounts_attributes_generated.go | 1 +
site/content/overview/product-telemetry.md | 2 +-
tests/suite/telemetry_test.go | 1 +
7 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/internal/mode/static/telemetry/collector.go b/internal/mode/static/telemetry/collector.go
index cf58cc3d20..d809ab342c 100644
--- a/internal/mode/static/telemetry/collector.go
+++ b/internal/mode/static/telemetry/collector.go
@@ -97,6 +97,8 @@ type NGFResourceCounts struct {
NginxProxyCount int64
// SnippetsFilterCount is the number of SnippetsFilters.
SnippetsFilterCount int64
+ // UpstreamSettingsPolicyCount is the number of UpstreamSettingsPolicies.
+ UpstreamSettingsPolicyCount int64
}
// DataCollectorConfig holds configuration parameters for DataCollectorImpl.
@@ -239,6 +241,8 @@ func collectGraphResourceCount(
}
case kinds.ObservabilityPolicy:
ngfResourceCounts.ObservabilityPolicyCount++
+ case kinds.UpstreamSettingsPolicy:
+ ngfResourceCounts.UpstreamSettingsPolicyCount++
}
}
diff --git a/internal/mode/static/telemetry/collector_test.go b/internal/mode/static/telemetry/collector_test.go
index 3128531ebb..5a7b11b582 100644
--- a/internal/mode/static/telemetry/collector_test.go
+++ b/internal/mode/static/telemetry/collector_test.go
@@ -329,6 +329,10 @@ var _ = Describe("Collector", Ordered, func() {
NsName: types.NamespacedName{Namespace: "test", Name: "ObservabilityPolicy-1"},
GVK: schema.GroupVersionKind{Kind: kinds.ObservabilityPolicy},
}: {},
+ {
+ NsName: types.NamespacedName{Namespace: "test", Name: "UpstreamSettingsPolicy-1"},
+ GVK: schema.GroupVersionKind{Kind: kinds.UpstreamSettingsPolicy},
+ }: {},
},
NginxProxy: &graph.NginxProxy{},
SnippetsFilters: map[types.NamespacedName]*graph.SnippetsFilter{
@@ -412,6 +416,7 @@ var _ = Describe("Collector", Ordered, func() {
ObservabilityPolicyCount: 1,
NginxProxyCount: 1,
SnippetsFilterCount: 3,
+ UpstreamSettingsPolicyCount: 1,
}
expData.ClusterVersion = "1.29.2"
expData.ClusterPlatform = "kind"
@@ -603,6 +608,10 @@ var _ = Describe("Collector", Ordered, func() {
NsName: types.NamespacedName{Namespace: "test", Name: "ObservabilityPolicy-1"},
GVK: schema.GroupVersionKind{Kind: kinds.ObservabilityPolicy},
}: {},
+ {
+ NsName: types.NamespacedName{Namespace: "test", Name: "UpstreamSettingsPolicy-1"},
+ GVK: schema.GroupVersionKind{Kind: kinds.UpstreamSettingsPolicy},
+ }: {},
},
NginxProxy: &graph.NginxProxy{},
SnippetsFilters: map[types.NamespacedName]*graph.SnippetsFilter{
@@ -682,6 +691,7 @@ var _ = Describe("Collector", Ordered, func() {
ObservabilityPolicyCount: 1,
NginxProxyCount: 1,
SnippetsFilterCount: 1,
+ UpstreamSettingsPolicyCount: 1,
}
data, err := dataCollector.Collect(ctx)
@@ -693,22 +703,7 @@ var _ = Describe("Collector", Ordered, func() {
It("ignores invalid and empty upstreams", func(ctx SpecContext) {
fakeGraphGetter.GetLatestGraphReturns(&graph.Graph{})
fakeConfigurationGetter.GetLatestConfigurationReturns(invalidUpstreamsConfig)
- expData.NGFResourceCounts = telemetry.NGFResourceCounts{
- GatewayCount: 0,
- GatewayClassCount: 0,
- HTTPRouteCount: 0,
- TLSRouteCount: 0,
- SecretCount: 0,
- ServiceCount: 0,
- EndpointCount: 0,
- GRPCRouteCount: 0,
- BackendTLSPolicyCount: 0,
- GatewayAttachedClientSettingsPolicyCount: 0,
- RouteAttachedClientSettingsPolicyCount: 0,
- ObservabilityPolicyCount: 0,
- NginxProxyCount: 0,
- SnippetsFilterCount: 0,
- }
+ expData.NGFResourceCounts = telemetry.NGFResourceCounts{}
data, err := dataCollector.Collect(ctx)
diff --git a/internal/mode/static/telemetry/data.avdl b/internal/mode/static/telemetry/data.avdl
index 71515e0e53..6909878866 100644
--- a/internal/mode/static/telemetry/data.avdl
+++ b/internal/mode/static/telemetry/data.avdl
@@ -99,6 +99,9 @@ attached at the Gateway level. */
/** SnippetsFilterCount is the number of SnippetsFilters. */
long? SnippetsFilterCount = null;
+ /** UpstreamSettingsPolicyCount is the number of UpstreamSettingsPolicies. */
+ long? UpstreamSettingsPolicyCount = null;
+
/** NGFReplicaCount is the number of replicas of the NGF Pod. */
long? NGFReplicaCount = null;
diff --git a/internal/mode/static/telemetry/data_test.go b/internal/mode/static/telemetry/data_test.go
index cb8f084b39..d0fe63820a 100644
--- a/internal/mode/static/telemetry/data_test.go
+++ b/internal/mode/static/telemetry/data_test.go
@@ -39,6 +39,7 @@ func TestDataAttributes(t *testing.T) {
ObservabilityPolicyCount: 11,
NginxProxyCount: 12,
SnippetsFilterCount: 13,
+ UpstreamSettingsPolicyCount: 14,
},
NGFReplicaCount: 3,
SnippetsFiltersDirectives: []string{"main-three-count", "http-two-count", "server-one-count"},
@@ -77,6 +78,7 @@ func TestDataAttributes(t *testing.T) {
attribute.Int64("ObservabilityPolicyCount", 11),
attribute.Int64("NginxProxyCount", 12),
attribute.Int64("SnippetsFilterCount", 13),
+ attribute.Int64("UpstreamSettingsPolicyCount", 14),
attribute.Int64("NGFReplicaCount", 3),
}
@@ -119,6 +121,7 @@ func TestDataAttributesWithEmptyData(t *testing.T) {
attribute.Int64("ObservabilityPolicyCount", 0),
attribute.Int64("NginxProxyCount", 0),
attribute.Int64("SnippetsFilterCount", 0),
+ attribute.Int64("UpstreamSettingsPolicyCount", 0),
attribute.Int64("NGFReplicaCount", 0),
}
diff --git a/internal/mode/static/telemetry/ngfresourcecounts_attributes_generated.go b/internal/mode/static/telemetry/ngfresourcecounts_attributes_generated.go
index 318cbd0b62..343c6fd9ed 100644
--- a/internal/mode/static/telemetry/ngfresourcecounts_attributes_generated.go
+++ b/internal/mode/static/telemetry/ngfresourcecounts_attributes_generated.go
@@ -26,6 +26,7 @@ func (d *NGFResourceCounts) Attributes() []attribute.KeyValue {
attrs = append(attrs, attribute.Int64("ObservabilityPolicyCount", d.ObservabilityPolicyCount))
attrs = append(attrs, attribute.Int64("NginxProxyCount", d.NginxProxyCount))
attrs = append(attrs, attribute.Int64("SnippetsFilterCount", d.SnippetsFilterCount))
+ attrs = append(attrs, attribute.Int64("UpstreamSettingsPolicyCount", d.UpstreamSettingsPolicyCount))
return attrs
}
diff --git a/site/content/overview/product-telemetry.md b/site/content/overview/product-telemetry.md
index 237abb1303..701774547d 100644
--- a/site/content/overview/product-telemetry.md
+++ b/site/content/overview/product-telemetry.md
@@ -27,7 +27,7 @@ Telemetry data is collected once every 24 hours and sent to a service managed by
- **Deployment Replica Count:** the count of NGINX Gateway Fabric Pods.
- **Image Build Source:** whether the image was built by GitHub or locally (values are `gha`, `local`, or `unknown`). The source repository of the images is **not** collected.
- **Deployment Flags:** a list of NGINX Gateway Fabric Deployment flags that are specified by a user. The actual values of non-boolean flags are **not** collected; we only record that they are either `true` or `false` for boolean flags and `default` or `user-defined` for the rest.
-- **Count of Resources:** the total count of resources related to NGINX Gateway Fabric. This includes `GatewayClasses`, `Gateways`, `HTTPRoutes`,`GRPCRoutes`, `TLSRoutes`, `Secrets`, `Services`, `BackendTLSPolicies`, `ClientSettingsPolicies`, `NginxProxies`, `ObservabilityPolicies`, `SnippetsFilters`, and `Endpoints`. The data within these resources is **not** collected.
+- **Count of Resources:** the total count of resources related to NGINX Gateway Fabric. This includes `GatewayClasses`, `Gateways`, `HTTPRoutes`,`GRPCRoutes`, `TLSRoutes`, `Secrets`, `Services`, `BackendTLSPolicies`, `ClientSettingsPolicies`, `NginxProxies`, `ObservabilityPolicies`, `UpstreamSettingsPolicies`, `SnippetsFilters`, and `Endpoints`. The data within these resources is **not** collected.
- **SnippetsFilters Info**a list of directive-context strings from applied SnippetFilters and a total count per strings. The actual value of any NGINX directive is **not** collected.
This data is used to identify the following information:
diff --git a/tests/suite/telemetry_test.go b/tests/suite/telemetry_test.go
index ac8f84267c..011b3d312e 100644
--- a/tests/suite/telemetry_test.go
+++ b/tests/suite/telemetry_test.go
@@ -88,6 +88,7 @@ var _ = Describe("Telemetry test with OTel collector", Label("telemetry"), func(
"ObservabilityPolicyCount: Int(0)",
"NginxProxyCount: Int(0)",
"SnippetsFilterCount: Int(0)",
+ "UpstreamSettingsPolicyCount: Int(0)",
"NGFReplicaCount: Int(1)",
},
)
From 939aee6a4825bd293af2e13cfbc70093ff955055 Mon Sep 17 00:00:00 2001
From: Kate Osborn <50597707+kate-osborn@users.noreply.github.com>
Date: Fri, 20 Dec 2024 08:48:40 -0700
Subject: [PATCH 5/7] Fix duplicate ancestors on UpstreamSettingsPolicy status
(#2937)
Problem: When an UpstreamSettingsPolicy targeted multiple Services,
NGF would write duplicate ancestors to its status.
Solution: Only add unique ancestors to policy ancestors list.
---
internal/mode/static/state/graph/policies.go | 9 +-
.../mode/static/state/graph/policies_test.go | 42 +++++++
.../static/state/graph/policy_ancestor.go | 34 ++++++
.../state/graph/policy_ancestor_test.go | 110 ++++++++++++++++++
4 files changed, 194 insertions(+), 1 deletion(-)
diff --git a/internal/mode/static/state/graph/policies.go b/internal/mode/static/state/graph/policies.go
index 9ce3e7eb23..7634c2d099 100644
--- a/internal/mode/static/state/graph/policies.go
+++ b/internal/mode/static/state/graph/policies.go
@@ -112,11 +112,18 @@ func attachPolicyToService(
if !gw.Valid {
ancestor.Conditions = []conditions.Condition{staticConds.NewPolicyTargetNotFound("Parent Gateway is invalid")}
+ if ancestorsContainsAncestorRef(policy.Ancestors, ancestor.Ancestor) {
+ return
+ }
+
policy.Ancestors = append(policy.Ancestors, ancestor)
return
}
- policy.Ancestors = append(policy.Ancestors, ancestor)
+ if !ancestorsContainsAncestorRef(policy.Ancestors, ancestor.Ancestor) {
+ policy.Ancestors = append(policy.Ancestors, ancestor)
+ }
+
svc.Policies = append(svc.Policies, policy)
}
diff --git a/internal/mode/static/state/graph/policies_test.go b/internal/mode/static/state/graph/policies_test.go
index 2c11ac2229..6a840230fb 100644
--- a/internal/mode/static/state/graph/policies_test.go
+++ b/internal/mode/static/state/graph/policies_test.go
@@ -539,6 +539,7 @@ func TestAttachPolicyToService(t *testing.T) {
t.Parallel()
gwNsname := types.NamespacedName{Namespace: testNs, Name: "gateway"}
+ gw2Nsname := types.NamespacedName{Namespace: testNs, Name: "gateway2"}
getGateway := func(valid bool) *Gateway {
return &Gateway{
@@ -572,6 +573,47 @@ func TestAttachPolicyToService(t *testing.T) {
},
},
},
+ {
+ name: "attachment; ancestor already exists so don't duplicate",
+ policy: &Policy{
+ Source: &policiesfakes.FakePolicy{},
+ Ancestors: []PolicyAncestor{
+ {
+ Ancestor: getGatewayParentRef(gwNsname),
+ },
+ },
+ },
+ svc: &ReferencedService{},
+ gw: getGateway(true /*valid*/),
+ expAttached: true,
+ expAncestors: []PolicyAncestor{
+ {
+ Ancestor: getGatewayParentRef(gwNsname), // only one ancestor per Gateway
+ },
+ },
+ },
+ {
+ name: "attachment; ancestor doesn't exists so add it",
+ policy: &Policy{
+ Source: &policiesfakes.FakePolicy{},
+ Ancestors: []PolicyAncestor{
+ {
+ Ancestor: getGatewayParentRef(gw2Nsname),
+ },
+ },
+ },
+ svc: &ReferencedService{},
+ gw: getGateway(true /*valid*/),
+ expAttached: true,
+ expAncestors: []PolicyAncestor{
+ {
+ Ancestor: getGatewayParentRef(gw2Nsname),
+ },
+ {
+ Ancestor: getGatewayParentRef(gwNsname),
+ },
+ },
+ },
{
name: "no attachment; gateway is invalid",
policy: &Policy{Source: &policiesfakes.FakePolicy{}},
diff --git a/internal/mode/static/state/graph/policy_ancestor.go b/internal/mode/static/state/graph/policy_ancestor.go
index 7120f94afa..b55990d6d3 100644
--- a/internal/mode/static/state/graph/policy_ancestor.go
+++ b/internal/mode/static/state/graph/policy_ancestor.go
@@ -4,6 +4,8 @@ import (
"k8s.io/apimachinery/pkg/types"
v1 "sigs.k8s.io/gateway-api/apis/v1"
"sigs.k8s.io/gateway-api/apis/v1alpha2"
+
+ "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
)
const maxAncestors = 16
@@ -61,3 +63,35 @@ func createParentReference(
Name: v1.ObjectName(nsname.Name),
}
}
+
+func ancestorsContainsAncestorRef(ancestors []PolicyAncestor, ref v1.ParentReference) bool {
+ for _, an := range ancestors {
+ if parentRefEqual(an.Ancestor, ref) {
+ return true
+ }
+ }
+
+ return false
+}
+
+func parentRefEqual(ref1, ref2 v1.ParentReference) bool {
+ if !helpers.EqualPointers(ref1.Kind, ref2.Kind) {
+ return false
+ }
+
+ if !helpers.EqualPointers(ref1.Group, ref2.Group) {
+ return false
+ }
+
+ if !helpers.EqualPointers(ref1.Namespace, ref2.Namespace) {
+ return false
+ }
+
+ // we don't check the other fields in ParentRef because we don't set them
+
+ if ref1.Name != ref2.Name {
+ return false
+ }
+
+ return true
+}
diff --git a/internal/mode/static/state/graph/policy_ancestor_test.go b/internal/mode/static/state/graph/policy_ancestor_test.go
index 794caf6c2f..0b34f8e1ef 100644
--- a/internal/mode/static/state/graph/policy_ancestor_test.go
+++ b/internal/mode/static/state/graph/policy_ancestor_test.go
@@ -4,10 +4,12 @@ import (
"testing"
. "github.com/onsi/gomega"
+ "k8s.io/apimachinery/pkg/types"
v1 "sigs.k8s.io/gateway-api/apis/v1"
"sigs.k8s.io/gateway-api/apis/v1alpha2"
ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
+ "github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds"
)
func TestBackendTLSPolicyAncestorsFull(t *testing.T) {
@@ -169,3 +171,111 @@ func TestNGFPolicyAncestorsFull(t *testing.T) {
})
}
}
+
+func TestAncestorContainsAncestorRef(t *testing.T) {
+ t.Parallel()
+
+ gw1 := types.NamespacedName{Namespace: testNs, Name: "gw1"}
+ gw2 := types.NamespacedName{Namespace: testNs, Name: "gw2"}
+ route := types.NamespacedName{Namespace: testNs, Name: "route"}
+ newRoute := types.NamespacedName{Namespace: testNs, Name: "new-route"}
+
+ ancestors := []PolicyAncestor{
+ {
+ Ancestor: createParentReference(v1.GroupName, kinds.Gateway, gw1),
+ },
+ {
+ Ancestor: createParentReference(v1.GroupName, kinds.Gateway, gw2),
+ },
+ {
+ Ancestor: createParentReference(v1.GroupName, kinds.HTTPRoute, route),
+ },
+ }
+
+ tests := []struct {
+ ref v1.ParentReference
+ name string
+ contains bool
+ }{
+ {
+ name: "contains Gateway ref",
+ ref: createParentReference(v1.GroupName, kinds.Gateway, gw1),
+ contains: true,
+ },
+ {
+ name: "contains Route ref",
+ ref: createParentReference(v1.GroupName, kinds.HTTPRoute, route),
+ contains: true,
+ },
+ {
+ name: "does not contain ref",
+ ref: createParentReference(v1.GroupName, kinds.HTTPRoute, newRoute),
+ contains: false,
+ },
+ }
+
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ t.Parallel()
+ g := NewWithT(t)
+
+ g.Expect(ancestorsContainsAncestorRef(ancestors, test.ref)).To(Equal(test.contains))
+ })
+ }
+}
+
+func TestParentRefEqual(t *testing.T) {
+ t.Parallel()
+ ref1NsName := types.NamespacedName{Namespace: testNs, Name: "ref1"}
+
+ ref1 := createParentReference(v1.GroupName, kinds.HTTPRoute, ref1NsName)
+
+ tests := []struct {
+ ref v1.ParentReference
+ name string
+ equal bool
+ }{
+ {
+ name: "kinds different",
+ ref: createParentReference(v1.GroupName, kinds.Gateway, ref1NsName),
+ equal: false,
+ },
+ {
+ name: "groups different",
+ ref: createParentReference("diff-group", kinds.HTTPRoute, ref1NsName),
+ equal: false,
+ },
+ {
+ name: "namespace different",
+ ref: createParentReference(
+ v1.GroupName,
+ kinds.HTTPRoute,
+ types.NamespacedName{Namespace: "diff-ns", Name: "ref1"},
+ ),
+ equal: false,
+ },
+ {
+ name: "name different",
+ ref: createParentReference(
+ v1.GroupName,
+ kinds.HTTPRoute,
+ types.NamespacedName{Namespace: testNs, Name: "diff-name"},
+ ),
+ equal: false,
+ },
+ {
+ name: "equal",
+ ref: createParentReference(v1.GroupName, kinds.HTTPRoute, ref1NsName),
+ equal: true,
+ },
+ }
+
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ t.Parallel()
+ g := NewWithT(t)
+
+ g.Expect(parentRefEqual(ref1, test.ref)).To(Equal(test.equal))
+ })
+ }
+}
From b0ed7e2154f202cfc0f3e0e478c46e6446eb2b14 Mon Sep 17 00:00:00 2001
From: salonichf5 <146118978+salonichf5@users.noreply.github.com>
Date: Fri, 20 Dec 2024 13:07:17 -0700
Subject: [PATCH 6/7] Functional tests for UpstreamSettings Policy (#2869)
Functional Tests for UpstreamSettings Policy.
---
tests/framework/crossplane.go | 64 ++-
tests/suite/client_settings_test.go | 16 +-
.../upstream-settings-policy/cafe.yaml | 66 +++
.../upstream-settings-policy/gateway.yaml | 11 +
.../grpc-backend.yaml | 39 ++
.../invalid-svc-usps.yaml | 15 +
.../invalid-target-usps.yaml | 81 +++
.../upstream-settings-policy/routes.yaml | 54 ++
.../valid-merge-usps.yaml | 60 ++
.../upstream-settings-policy/valid-usps.yaml | 33 ++
tests/suite/snippets_filter_test.go | 8 +-
tests/suite/upstream_settings_test.go | 523 ++++++++++++++++++
12 files changed, 943 insertions(+), 27 deletions(-)
create mode 100644 tests/suite/manifests/upstream-settings-policy/cafe.yaml
create mode 100644 tests/suite/manifests/upstream-settings-policy/gateway.yaml
create mode 100644 tests/suite/manifests/upstream-settings-policy/grpc-backend.yaml
create mode 100644 tests/suite/manifests/upstream-settings-policy/invalid-svc-usps.yaml
create mode 100644 tests/suite/manifests/upstream-settings-policy/invalid-target-usps.yaml
create mode 100644 tests/suite/manifests/upstream-settings-policy/routes.yaml
create mode 100644 tests/suite/manifests/upstream-settings-policy/valid-merge-usps.yaml
create mode 100644 tests/suite/manifests/upstream-settings-policy/valid-usps.yaml
create mode 100644 tests/suite/upstream_settings_test.go
diff --git a/tests/framework/crossplane.go b/tests/framework/crossplane.go
index b6925dc8e1..81f47e3567 100644
--- a/tests/framework/crossplane.go
+++ b/tests/framework/crossplane.go
@@ -28,8 +28,10 @@ type ExpectedNginxField struct {
File string
// Location is the location name that the directive should exist in.
Location string
- // Servers are the server names that the directive should exist in.
- Servers []string
+ // Server is the server name that the directive should exist in.
+ Server string
+ // Upstream is the upstream name that the directive should exist in.
+ Upstream string
// ValueSubstringAllowed allows the expected value to be a substring of the real value.
// This makes it easier for cases when real values are complex file names or contain things we
// don't care about, and we just want to check if a substring exists.
@@ -39,40 +41,66 @@ type ExpectedNginxField struct {
// ValidateNginxFieldExists accepts the nginx config and the configuration for the expected field,
// and returns whether or not that field exists where it should.
func ValidateNginxFieldExists(conf *Payload, expFieldCfg ExpectedNginxField) error {
+ b, err := json.Marshal(conf)
+ if err != nil {
+ return fmt.Errorf("error marshaling nginx config: %w", err)
+ }
+
for _, config := range conf.Config {
if !strings.Contains(config.File, expFieldCfg.File) {
continue
}
for _, directive := range config.Parsed {
- if len(expFieldCfg.Servers) == 0 {
+ if expFieldCfg.Server == "" && expFieldCfg.Upstream == "" {
if expFieldCfg.fieldFound(directive) {
return nil
}
continue
}
- for _, serverName := range expFieldCfg.Servers {
- if directive.Directive == "server" && getServerName(directive.Block) == serverName {
- for _, serverDirective := range directive.Block {
- if expFieldCfg.Location == "" && expFieldCfg.fieldFound(serverDirective) {
- return nil
- } else if serverDirective.Directive == "location" &&
- fieldExistsInLocation(serverDirective, expFieldCfg) {
- return nil
- }
- }
- }
+ if expFieldCfg.Server != "" && fieldExistsInServer(expFieldCfg, *directive) {
+ return nil
+ }
+
+ if expFieldCfg.Upstream != "" && fieldExistsInUpstream(expFieldCfg, *directive) {
+ return nil
}
}
}
- b, err := json.Marshal(conf)
- if err != nil {
- return fmt.Errorf("error marshaling nginx config: %w", err)
+ return fmt.Errorf("directive %s not found in: nginx config %s", expFieldCfg.Directive, string(b))
+}
+
+func fieldExistsInServer(
+ expFieldCfg ExpectedNginxField,
+ directive Directive,
+) bool {
+ if directive.Directive == "server" && getServerName(directive.Block) == expFieldCfg.Server {
+ for _, serverDirective := range directive.Block {
+ if expFieldCfg.Location == "" && expFieldCfg.fieldFound(serverDirective) {
+ return true
+ } else if serverDirective.Directive == "location" &&
+ fieldExistsInLocation(serverDirective, expFieldCfg) {
+ return true
+ }
+ }
}
+ return false
+}
- return fmt.Errorf("field not found; expected: %+v\nNGINX conf: %s", expFieldCfg, string(b))
+func fieldExistsInUpstream(
+ expFieldCfg ExpectedNginxField,
+ directive Directive,
+) bool {
+ if directive.Directive == "upstream" && directive.Args[0] == expFieldCfg.Upstream {
+ for _, directive := range directive.Block {
+ if expFieldCfg.fieldFound(directive) {
+ return true
+ }
+ }
+ }
+ return false
}
func getServerName(serverBlock Directives) string {
diff --git a/tests/suite/client_settings_test.go b/tests/suite/client_settings_test.go
index b5e5a8ec48..f7ab05c88f 100644
--- a/tests/suite/client_settings_test.go
+++ b/tests/suite/client_settings_test.go
@@ -117,7 +117,13 @@ var _ = Describe("ClientSettingsPolicy", Ordered, Label("functional", "cspolicy"
Directive: "include",
Value: fmt.Sprintf("%s_gw-csp.conf", filePrefix),
File: "http.conf",
- Servers: []string{"*.example.com", "cafe.example.com"},
+ Server: "*.example.com",
+ },
+ {
+ Directive: "include",
+ Value: fmt.Sprintf("%s_gw-csp.conf", filePrefix),
+ File: "http.conf",
+ Server: "cafe.example.com",
},
{
Directive: "client_max_body_size",
@@ -150,7 +156,7 @@ var _ = Describe("ClientSettingsPolicy", Ordered, Label("functional", "cspolicy"
Directive: "include",
Value: fmt.Sprintf("%s_coffee-route-csp.conf", filePrefix),
File: "http.conf",
- Servers: []string{"cafe.example.com"},
+ Server: "cafe.example.com",
Location: "/coffee",
},
{
@@ -164,7 +170,7 @@ var _ = Describe("ClientSettingsPolicy", Ordered, Label("functional", "cspolicy"
Directive: "include",
Value: fmt.Sprintf("%s_tea-route-csp.conf", filePrefix),
File: "http.conf",
- Servers: []string{"cafe.example.com"},
+ Server: "cafe.example.com",
Location: "/tea",
},
{
@@ -178,7 +184,7 @@ var _ = Describe("ClientSettingsPolicy", Ordered, Label("functional", "cspolicy"
Directive: "include",
Value: fmt.Sprintf("%s_soda-route-csp.conf", filePrefix),
File: "http.conf",
- Servers: []string{"cafe.example.com"},
+ Server: "cafe.example.com",
Location: "/soda",
},
{
@@ -192,7 +198,7 @@ var _ = Describe("ClientSettingsPolicy", Ordered, Label("functional", "cspolicy"
Directive: "include",
Value: fmt.Sprintf("%s_grpc-route-csp.conf", filePrefix),
File: "http.conf",
- Servers: []string{"*.example.com"},
+ Server: "*.example.com",
Location: "/helloworld.Greeter/SayHello",
},
{
diff --git a/tests/suite/manifests/upstream-settings-policy/cafe.yaml b/tests/suite/manifests/upstream-settings-policy/cafe.yaml
new file mode 100644
index 0000000000..c0466158f8
--- /dev/null
+++ b/tests/suite/manifests/upstream-settings-policy/cafe.yaml
@@ -0,0 +1,66 @@
+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: 8080
+---
+apiVersion: v1
+kind: Service
+metadata:
+ name: coffee
+spec:
+ ports:
+ - port: 80
+ targetPort: 8080
+ protocol: TCP
+ name: http
+ selector:
+ app: coffee
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+ name: tea
+spec:
+ replicas: 1
+ selector:
+ matchLabels:
+ app: tea
+ template:
+ metadata:
+ labels:
+ app: tea
+ spec:
+ containers:
+ - name: tea
+ image: nginxdemos/nginx-hello:plain-text
+ ports:
+ - containerPort: 8080
+---
+apiVersion: v1
+kind: Service
+metadata:
+ name: tea
+spec:
+ ports:
+ - port: 80
+ targetPort: 8080
+ protocol: TCP
+ name: http
+ selector:
+ app: tea
+---
diff --git a/tests/suite/manifests/upstream-settings-policy/gateway.yaml b/tests/suite/manifests/upstream-settings-policy/gateway.yaml
new file mode 100644
index 0000000000..e6507f613b
--- /dev/null
+++ b/tests/suite/manifests/upstream-settings-policy/gateway.yaml
@@ -0,0 +1,11 @@
+apiVersion: gateway.networking.k8s.io/v1
+kind: Gateway
+metadata:
+ name: gateway
+spec:
+ gatewayClassName: nginx
+ listeners:
+ - name: http
+ port: 80
+ protocol: HTTP
+ hostname: "*.example.com"
diff --git a/tests/suite/manifests/upstream-settings-policy/grpc-backend.yaml b/tests/suite/manifests/upstream-settings-policy/grpc-backend.yaml
new file mode 100644
index 0000000000..3ae352f573
--- /dev/null
+++ b/tests/suite/manifests/upstream-settings-policy/grpc-backend.yaml
@@ -0,0 +1,39 @@
+apiVersion: v1
+kind: Service
+metadata:
+ name: grpc-backend
+spec:
+ selector:
+ app: grpc-backend
+ ports:
+ - protocol: TCP
+ port: 8080
+ targetPort: 50051
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+ name: grpc-backend
+ labels:
+ app: grpc-backend
+spec:
+ replicas: 1
+ selector:
+ matchLabels:
+ app: grpc-backend
+ template:
+ metadata:
+ labels:
+ app: grpc-backend
+ spec:
+ containers:
+ - name: grpc-backend
+ image: ghcr.io/nginxinc/kic-test-grpc-server:0.2.3
+ env:
+ - name: POD_NAME
+ valueFrom:
+ fieldRef:
+ fieldPath: metadata.name
+ resources:
+ requests:
+ cpu: 10m
diff --git a/tests/suite/manifests/upstream-settings-policy/invalid-svc-usps.yaml b/tests/suite/manifests/upstream-settings-policy/invalid-svc-usps.yaml
new file mode 100644
index 0000000000..cc1ed6c55d
--- /dev/null
+++ b/tests/suite/manifests/upstream-settings-policy/invalid-svc-usps.yaml
@@ -0,0 +1,15 @@
+apiVersion: gateway.nginx.org/v1alpha1
+kind: UpstreamSettingsPolicy
+metadata:
+ name: usps-target-not-found
+spec:
+ zoneSize: 512k
+ targetRefs:
+ - group: core
+ kind: Service
+ name: targeted-svc-dne
+ keepAlive:
+ connections: 10
+ requests: 3
+ time: 10s
+ timeout: 50s
diff --git a/tests/suite/manifests/upstream-settings-policy/invalid-target-usps.yaml b/tests/suite/manifests/upstream-settings-policy/invalid-target-usps.yaml
new file mode 100644
index 0000000000..8bbd25366e
--- /dev/null
+++ b/tests/suite/manifests/upstream-settings-policy/invalid-target-usps.yaml
@@ -0,0 +1,81 @@
+apiVersion: gateway.networking.k8s.io/v1
+kind: Gateway
+metadata:
+ name: gateway-not-valid
+spec:
+ gatewayClassName: nginx
+ addresses:
+ - value: "10.0.0.1"
+ listeners:
+ - name: http
+ port: 80
+ protocol: HTTP
+ hostname: "soda.example.com"
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+ name: soda
+spec:
+ replicas: 1
+ selector:
+ matchLabels:
+ app: soda
+ template:
+ metadata:
+ labels:
+ app: soda
+ spec:
+ containers:
+ - name: soda
+ image: nginxdemos/nginx-hello:plain-text
+ ports:
+ - containerPort: 8080
+---
+apiVersion: v1
+kind: Service
+metadata:
+ name: soda
+spec:
+ ports:
+ - port: 80
+ targetPort: 8080
+ protocol: TCP
+ name: http
+ selector:
+ app: soda
+---
+apiVersion: gateway.networking.k8s.io/v1
+kind: HTTPRoute
+metadata:
+ name: soda
+spec:
+ parentRefs:
+ - name: gateway-not-valid
+ sectionName: http
+ hostnames:
+ - "soda.example.com"
+ rules:
+ - matches:
+ - path:
+ type: Exact
+ value: /soda
+ backendRefs:
+ - name: soda
+ port: 80
+---
+apiVersion: gateway.nginx.org/v1alpha1
+kind: UpstreamSettingsPolicy
+metadata:
+ name: soda-svc-usp
+spec:
+ zoneSize: 512k
+ targetRefs:
+ - group: core
+ kind: Service
+ name: soda
+ keepAlive:
+ connections: 10
+ requests: 3
+ time: 10s
+ timeout: 50s
diff --git a/tests/suite/manifests/upstream-settings-policy/routes.yaml b/tests/suite/manifests/upstream-settings-policy/routes.yaml
new file mode 100644
index 0000000000..f26e705a40
--- /dev/null
+++ b/tests/suite/manifests/upstream-settings-policy/routes.yaml
@@ -0,0 +1,54 @@
+apiVersion: gateway.networking.k8s.io/v1
+kind: HTTPRoute
+metadata:
+ name: coffee
+spec:
+ parentRefs:
+ - name: gateway
+ sectionName: http
+ hostnames:
+ - "cafe.example.com"
+ rules:
+ - matches:
+ - path:
+ type: PathPrefix
+ value: /coffee
+ backendRefs:
+ - name: coffee
+ port: 80
+---
+apiVersion: gateway.networking.k8s.io/v1
+kind: HTTPRoute
+metadata:
+ name: tea
+spec:
+ parentRefs:
+ - name: gateway
+ sectionName: http
+ hostnames:
+ - "cafe.example.com"
+ rules:
+ - matches:
+ - path:
+ type: Exact
+ value: /tea
+ backendRefs:
+ - name: tea
+ port: 80
+---
+apiVersion: gateway.networking.k8s.io/v1
+kind: GRPCRoute
+metadata:
+ name: grpc-route
+spec:
+ parentRefs:
+ - name: gateway
+ sectionName: http
+ rules:
+ - matches:
+ - method:
+ service: helloworld.Greeter
+ method: SayHello
+ backendRefs:
+ - name: grpc-backend
+ port: 8080
diff --git a/tests/suite/manifests/upstream-settings-policy/valid-merge-usps.yaml b/tests/suite/manifests/upstream-settings-policy/valid-merge-usps.yaml
new file mode 100644
index 0000000000..7c713b4752
--- /dev/null
+++ b/tests/suite/manifests/upstream-settings-policy/valid-merge-usps.yaml
@@ -0,0 +1,60 @@
+apiVersion: gateway.nginx.org/v1alpha1
+kind: UpstreamSettingsPolicy
+metadata:
+ name: merge-usp-1
+spec:
+ targetRefs:
+ - group: core
+ kind: Service
+ name: coffee
+ keepAlive:
+ time: 1m
+ timeout: 5h
+---
+apiVersion: gateway.nginx.org/v1alpha1
+kind: UpstreamSettingsPolicy
+metadata:
+ name: merge-usp-2
+spec:
+ targetRefs:
+ - group: core
+ kind: Service
+ name: coffee
+ keepAlive:
+ connections: 100
+ requests: 55
+---
+apiVersion: gateway.nginx.org/v1alpha1
+kind: UpstreamSettingsPolicy
+metadata:
+ name: z-merge-usp-3
+spec:
+ targetRefs:
+ - group: core
+ kind: Service
+ name: coffee
+ keepAlive:
+ connections: 11
+ requests: 15
+---
+apiVersion: gateway.nginx.org/v1alpha1
+kind: UpstreamSettingsPolicy
+metadata:
+ name: z-usp
+spec:
+ zoneSize: 64k
+ targetRefs:
+ - group: core
+ kind: Service
+ name: tea
+---
+apiVersion: gateway.nginx.org/v1alpha1
+kind: UpstreamSettingsPolicy
+metadata:
+ name: a-usp-wins
+spec:
+ zoneSize: 128k
+ targetRefs:
+ - group: core
+ kind: Service
+ name: tea
diff --git a/tests/suite/manifests/upstream-settings-policy/valid-usps.yaml b/tests/suite/manifests/upstream-settings-policy/valid-usps.yaml
new file mode 100644
index 0000000000..cb4e4bf058
--- /dev/null
+++ b/tests/suite/manifests/upstream-settings-policy/valid-usps.yaml
@@ -0,0 +1,33 @@
+apiVersion: gateway.nginx.org/v1alpha1
+kind: UpstreamSettingsPolicy
+metadata:
+ name: multiple-http-svc-usp
+spec:
+ targetRefs:
+ - group: core
+ kind: Service
+ name: coffee
+ - group: core
+ kind: Service
+ name: tea
+ keepAlive:
+ connections: 10
+ requests: 3
+ time: 10s
+ timeout: 50s
+---
+apiVersion: gateway.nginx.org/v1alpha1
+kind: UpstreamSettingsPolicy
+metadata:
+ name: grpc-svc-usp
+spec:
+ targetRefs:
+ - group: core
+ kind: Service
+ name: grpc-backend
+ zoneSize: 64k
+ keepAlive:
+ connections: 100
+ requests: 45
+ time: 1m
+ timeout: 5h
diff --git a/tests/suite/snippets_filter_test.go b/tests/suite/snippets_filter_test.go
index 632a8199e0..e92e44d0ad 100644
--- a/tests/suite/snippets_filter_test.go
+++ b/tests/suite/snippets_filter_test.go
@@ -149,7 +149,7 @@ var _ = Describe("SnippetsFilter", Ordered, Label("functional", "snippets-filter
{
Directive: "include",
Value: fmt.Sprintf("%s%s", httpServerContext, httpRouteSuffix),
- Servers: []string{"cafe.example.com"},
+ Server: "cafe.example.com",
File: "http.conf",
},
{
@@ -157,7 +157,7 @@ var _ = Describe("SnippetsFilter", Ordered, Label("functional", "snippets-filter
Value: fmt.Sprintf("%s%s", httpServerLocationContext, httpRouteSuffix),
File: "http.conf",
Location: "/coffee",
- Servers: []string{"cafe.example.com"},
+ Server: "cafe.example.com",
},
{
Directive: "keepalive_time",
@@ -194,7 +194,7 @@ var _ = Describe("SnippetsFilter", Ordered, Label("functional", "snippets-filter
{
Directive: "include",
Value: fmt.Sprintf("%s%s", httpServerContext, grpcRouteSuffix),
- Servers: []string{"*.example.com"},
+ Server: "*.example.com",
File: "http.conf",
},
{
@@ -207,7 +207,7 @@ var _ = Describe("SnippetsFilter", Ordered, Label("functional", "snippets-filter
Value: fmt.Sprintf("%s%s", httpServerLocationContext, grpcRouteSuffix),
File: "http.conf",
Location: "/helloworld.Greeter/SayHello",
- Servers: []string{"*.example.com"},
+ Server: "*.example.com",
},
}),
)
diff --git a/tests/suite/upstream_settings_test.go b/tests/suite/upstream_settings_test.go
new file mode 100644
index 0000000000..39159e4406
--- /dev/null
+++ b/tests/suite/upstream_settings_test.go
@@ -0,0 +1,523 @@
+package main
+
+import (
+ "context"
+ "errors"
+ "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"
+ "k8s.io/apimachinery/pkg/util/wait"
+ "sigs.k8s.io/controller-runtime/pkg/client"
+ gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
+ "sigs.k8s.io/gateway-api/apis/v1alpha2"
+
+ ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
+ "github.com/nginxinc/nginx-gateway-fabric/tests/framework"
+)
+
+var _ = Describe("UpstreamSettingsPolicy", Ordered, Label("functional", "uspolicy"), func() {
+ var (
+ files = []string{
+ "upstream-settings-policy/cafe.yaml",
+ "upstream-settings-policy/gateway.yaml",
+ "upstream-settings-policy/grpc-backend.yaml",
+ "upstream-settings-policy/routes.yaml",
+ }
+
+ namespace = "uspolicy"
+ gatewayName = "gateway"
+ )
+
+ 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())
+ })
+
+ AfterAll(func() {
+ Expect(resourceManager.DeleteNamespace(namespace)).To(Succeed())
+ })
+
+ When("UpstreamSettingsPolicies target distinct Services", func() {
+ usps := []string{
+ "upstream-settings-policy/valid-usps.yaml",
+ }
+
+ BeforeAll(func() {
+ Expect(resourceManager.ApplyFromFiles(usps, namespace)).To(Succeed())
+ })
+
+ AfterAll(func() {
+ Expect(resourceManager.DeleteFromFiles(usps, namespace)).To(Succeed())
+ })
+
+ Specify("they are accepted", func() {
+ usPolicies := []string{
+ "multiple-http-svc-usp",
+ "grpc-svc-usp",
+ }
+
+ for _, name := range usPolicies {
+ uspolicyNsName := types.NamespacedName{Name: name, Namespace: namespace}
+
+ err := waitForUSPolicyStatus(
+ uspolicyNsName,
+ gatewayName,
+ metav1.ConditionTrue,
+ v1alpha2.PolicyReasonAccepted,
+ )
+ Expect(err).ToNot(HaveOccurred(), fmt.Sprintf("%s was not accepted", name))
+ }
+ })
+
+ Context("verify working traffic", func() {
+ It("should return a 200 response for HTTPRoutes", func() {
+ port := 80
+ if portFwdPort != 0 {
+ port = portFwdPort
+ }
+ baseCoffeeURL := fmt.Sprintf("http://cafe.example.com:%d%s", port, "/coffee")
+ baseTeaURL := fmt.Sprintf("http://cafe.example.com:%d%s", port, "/tea")
+
+ Eventually(
+ func() error {
+ return expectRequestToSucceed(baseCoffeeURL, address, "URI: /coffee")
+ }).
+ WithTimeout(timeoutConfig.RequestTimeout).
+ WithPolling(500 * time.Millisecond).
+ Should(Succeed())
+
+ Eventually(
+ func() error {
+ return expectRequestToSucceed(baseTeaURL, address, "URI: /tea")
+ }).
+ WithTimeout(timeoutConfig.RequestTimeout).
+ WithPolling(500 * time.Millisecond).
+ Should(Succeed())
+ })
+ })
+
+ Context("nginx directives", func() {
+ var conf *framework.Payload
+
+ BeforeAll(func() {
+ podNames, err := framework.GetReadyNGFPodNames(k8sClient, ngfNamespace, releaseName, timeoutConfig.GetTimeout)
+ Expect(err).ToNot(HaveOccurred())
+ Expect(podNames).To(HaveLen(1))
+
+ ngfPodName := podNames[0]
+
+ conf, err = resourceManager.GetNginxConfig(ngfPodName, ngfNamespace)
+ 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("HTTP upstreams", []framework.ExpectedNginxField{
+ {
+ Directive: "upstream",
+ Value: "uspolicy_coffee_80",
+ File: "http.conf",
+ },
+ {
+ Directive: "upstream",
+ Value: "uspolicy_tea_80",
+ File: "http.conf",
+ },
+ {
+ Directive: "zone",
+ Value: "uspolicy_coffee_80 512k",
+ Upstream: "uspolicy_coffee_80",
+ File: "http.conf",
+ },
+ {
+ Directive: "zone",
+ Value: "uspolicy_tea_80 512k",
+ Upstream: "uspolicy_tea_80",
+ File: "http.conf",
+ },
+ {
+ Directive: "keepalive",
+ Value: "10",
+ Upstream: "uspolicy_coffee_80",
+ File: "http.conf",
+ },
+ {
+ Directive: "keepalive",
+ Value: "10",
+ Upstream: "uspolicy_tea_80",
+ File: "http.conf",
+ },
+ {
+ Directive: "keepalive_requests",
+ Value: "3",
+ Upstream: "uspolicy_coffee_80",
+ File: "http.conf",
+ },
+ {
+ Directive: "keepalive_requests",
+ Value: "3",
+ Upstream: "uspolicy_tea_80",
+ File: "http.conf",
+ },
+ {
+ Directive: "keepalive_time",
+ Value: "10s",
+ Upstream: "uspolicy_coffee_80",
+ File: "http.conf",
+ },
+ {
+ Directive: "keepalive_time",
+ Value: "10s",
+ Upstream: "uspolicy_tea_80",
+ File: "http.conf",
+ },
+ {
+ Directive: "keepalive_timeout",
+ Value: "50s",
+ Upstream: "uspolicy_coffee_80",
+ File: "http.conf",
+ },
+ {
+ Directive: "keepalive_timeout",
+ Value: "50s",
+ Upstream: "uspolicy_tea_80",
+ File: "http.conf",
+ },
+ }),
+ Entry("GRPC upstreams", []framework.ExpectedNginxField{
+ {
+ Directive: "upstream",
+ Value: "uspolicy_grpc-backend_8080",
+ File: "http.conf",
+ },
+ {
+ Directive: "zone",
+ Value: "uspolicy_grpc-backend_8080 64k",
+ Upstream: "uspolicy_grpc-backend_8080",
+ File: "http.conf",
+ },
+ {
+ Directive: "keepalive",
+ Value: "100",
+ Upstream: "uspolicy_grpc-backend_8080",
+ File: "http.conf",
+ },
+ {
+ Directive: "keepalive_requests",
+ Value: "45",
+ Upstream: "uspolicy_grpc-backend_8080",
+ File: "http.conf",
+ },
+ {
+ Directive: "keepalive_time",
+ Value: "1m",
+ Upstream: "uspolicy_grpc-backend_8080",
+ File: "http.conf",
+ },
+ {
+ Directive: "keepalive_timeout",
+ Value: "5h",
+ Upstream: "uspolicy_grpc-backend_8080",
+ File: "http.conf",
+ },
+ }),
+ )
+ })
+ })
+
+ When("multiple UpstreamSettingsPolicies with overlapping settings target the same Service", func() {
+ usps := []string{
+ "upstream-settings-policy/valid-merge-usps.yaml",
+ }
+
+ BeforeAll(func() {
+ Expect(resourceManager.ApplyFromFiles(usps, namespace)).To(Succeed())
+ })
+
+ AfterAll(func() {
+ Expect(resourceManager.DeleteFromFiles(usps, namespace)).To(Succeed())
+ })
+
+ DescribeTable("upstreamSettingsPolicy status is set as expected",
+ func(name string, status metav1.ConditionStatus, condReason v1alpha2.PolicyConditionReason) {
+ uspolicyNsName := types.NamespacedName{Name: name, Namespace: namespace}
+ Expect(waitForUSPolicyStatus(uspolicyNsName, gatewayName, status, condReason)).To(Succeed())
+ },
+ Entry("uspolicy merge-usp-1", "merge-usp-1", metav1.ConditionTrue, v1alpha2.PolicyReasonAccepted),
+ Entry("uspolicy merge-usp-2", "merge-usp-2", metav1.ConditionTrue, v1alpha2.PolicyReasonAccepted),
+ Entry("uspolicy merge-usp-3", "z-merge-usp-3", metav1.ConditionFalse, v1alpha2.PolicyReasonConflicted),
+ Entry("uspolicy a-usp-wins", "a-usp-wins", metav1.ConditionTrue, v1alpha2.PolicyReasonAccepted),
+ Entry("uspolicy z-usp", "z-usp", metav1.ConditionFalse, v1alpha2.PolicyReasonConflicted),
+ )
+
+ Context("verify working traffic", func() {
+ It("should return a 200 response for HTTPRoutes", func() {
+ port := 80
+ if portFwdPort != 0 {
+ port = portFwdPort
+ }
+ baseCoffeeURL := fmt.Sprintf("http://cafe.example.com:%d%s", port, "/coffee")
+ baseTeaURL := fmt.Sprintf("http://cafe.example.com:%d%s", port, "/tea")
+
+ Eventually(
+ func() error {
+ return expectRequestToSucceed(baseCoffeeURL, address, "URI: /coffee")
+ }).
+ WithTimeout(timeoutConfig.RequestTimeout).
+ WithPolling(1000 * time.Millisecond).
+ Should(Succeed())
+
+ Eventually(
+ func() error {
+ return expectRequestToSucceed(baseTeaURL, address, "URI: /tea")
+ }).
+ WithTimeout(timeoutConfig.RequestTimeout).
+ WithPolling(1000 * time.Millisecond).
+ Should(Succeed())
+ })
+ })
+
+ Context("nginx directives", func() {
+ var conf *framework.Payload
+
+ BeforeAll(func() {
+ podNames, err := framework.GetReadyNGFPodNames(k8sClient, ngfNamespace, releaseName, timeoutConfig.GetTimeout)
+ Expect(err).ToNot(HaveOccurred())
+ Expect(podNames).To(HaveLen(1))
+
+ ngfPodName := podNames[0]
+
+ conf, err = resourceManager.GetNginxConfig(ngfPodName, ngfNamespace)
+ 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("Coffee upstream", []framework.ExpectedNginxField{
+ {
+ Directive: "upstream",
+ Value: "uspolicy_coffee_80",
+ File: "http.conf",
+ },
+ {
+ Directive: "zone",
+ Value: "uspolicy_coffee_80 512k",
+ Upstream: "uspolicy_coffee_80",
+ File: "http.conf",
+ },
+ {
+ Directive: "keepalive",
+ Value: "100",
+ Upstream: "uspolicy_coffee_80",
+ File: "http.conf",
+ },
+ {
+ Directive: "keepalive_requests",
+ Value: "55",
+ Upstream: "uspolicy_coffee_80",
+ File: "http.conf",
+ },
+ {
+ Directive: "keepalive_time",
+ Value: "1m",
+ Upstream: "uspolicy_coffee_80",
+ File: "http.conf",
+ },
+ {
+ Directive: "keepalive_timeout",
+ Value: "5h",
+ Upstream: "uspolicy_coffee_80",
+ File: "http.conf",
+ },
+ }),
+ Entry("Tea upstream", []framework.ExpectedNginxField{
+ {
+ Directive: "zone",
+ Value: "uspolicy_tea_80 128k",
+ Upstream: "uspolicy_tea_80",
+ File: "http.conf",
+ },
+ {
+ Directive: "upstream",
+ Value: "uspolicy_tea_80",
+ File: "http.conf",
+ },
+ }),
+ )
+ })
+ })
+
+ When("UpstreamSettingsPolicy targets a Service that does not exist", func() {
+ Specify("upstreamSettingsPolicy sets no condition", func() {
+ files := []string{"upstream-settings-policy/invalid-svc-usps.yaml"}
+
+ Expect(resourceManager.ApplyFromFiles(files, namespace)).To(Succeed())
+
+ uspolicyNsName := types.NamespacedName{Name: "usps-target-not-found", Namespace: namespace}
+
+ Consistently(
+ func() bool {
+ return usPolicyHasNoAncestors(uspolicyNsName)
+ }).WithTimeout(timeoutConfig.GetTimeout).
+ WithPolling(500 * time.Millisecond).
+ Should(BeTrue())
+
+ Expect(resourceManager.DeleteFromFiles(files, namespace)).To(Succeed())
+ })
+ })
+
+ When("UpstreamSettingsPolicy targets a Service that is owned by an invalid Gateway", func() {
+ Specify("upstreamSettingsPolicy is not Accepted with the reason TargetNotFound", func() {
+ // delete existing gateway
+ gatewayFileName := "upstream-settings-policy/gateway.yaml"
+ Expect(resourceManager.DeleteFromFiles([]string{gatewayFileName}, namespace)).To(Succeed())
+
+ files := []string{"upstream-settings-policy/invalid-target-usps.yaml"}
+ Expect(resourceManager.ApplyFromFiles(files, namespace)).To(Succeed())
+
+ uspolicyNsName := types.NamespacedName{Name: "soda-svc-usp", Namespace: namespace}
+ gatewayName = "gateway-not-valid"
+ Expect(waitForUSPolicyStatus(
+ uspolicyNsName,
+ gatewayName,
+ metav1.ConditionFalse,
+ v1alpha2.PolicyReasonTargetNotFound,
+ )).To(Succeed())
+
+ Expect(resourceManager.DeleteFromFiles(files, namespace)).To(Succeed())
+ })
+ })
+})
+
+func usPolicyHasNoAncestors(usPolicyNsName types.NamespacedName) bool {
+ GinkgoWriter.Printf("Checking that UpstreamSettingsPolicy %q has no ancestors in status", usPolicyNsName)
+
+ ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.GetStatusTimeout)
+ defer cancel()
+
+ var usPolicy ngfAPI.UpstreamSettingsPolicy
+ if err := k8sClient.Get(ctx, usPolicyNsName, &usPolicy); err != nil {
+ GinkgoWriter.Printf("Failed to get UpstreamSettingsPolicy %q: %s", usPolicyNsName, err.Error())
+ return false
+ }
+
+ return len(usPolicy.Status.Ancestors) == 0
+}
+
+func waitForUSPolicyStatus(
+ usPolicyNsName types.NamespacedName,
+ gatewayName string,
+ condStatus metav1.ConditionStatus,
+ condReason v1alpha2.PolicyConditionReason,
+) error {
+ ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.GetStatusTimeout*2)
+ defer cancel()
+
+ GinkgoWriter.Printf(
+ "Waiting for UpstreamSettings Policy %q to have the condition %q/%q\n",
+ usPolicyNsName,
+ condStatus,
+ condReason,
+ )
+
+ return wait.PollUntilContextCancel(
+ ctx,
+ 2000*time.Millisecond,
+ true, /* poll immediately */
+ func(ctx context.Context) (bool, error) {
+ var usPolicy ngfAPI.UpstreamSettingsPolicy
+ var err error
+
+ if err := k8sClient.Get(ctx, usPolicyNsName, &usPolicy); err != nil {
+ return false, err
+ }
+
+ if len(usPolicy.Status.Ancestors) == 0 {
+ GinkgoWriter.Printf("UpstreamSettingsPolicy %q does not have an ancestor status yet\n", usPolicy)
+
+ return false, nil
+ }
+
+ if len(usPolicy.Status.Ancestors) != 1 {
+ return false, fmt.Errorf("policy has %d ancestors, expected 1", len(usPolicy.Status.Ancestors))
+ }
+
+ ancestors := usPolicy.Status.Ancestors
+
+ for _, ancestor := range ancestors {
+ if err := ancestorMustEqualGatewayRef(ancestor, gatewayName, usPolicy.Namespace); err != nil {
+ return false, err
+ }
+
+ err = ancestorStatusMustHaveAcceptedCondition(ancestor, condStatus, condReason)
+ }
+ return err == nil, err
+ },
+ )
+}
+
+func ancestorMustEqualGatewayRef(
+ ancestor v1alpha2.PolicyAncestorStatus,
+ gatewayName string,
+ namespace string,
+) error {
+ if ancestor.ControllerName != ngfControllerName {
+ return fmt.Errorf(
+ "expected ancestor controller name to be %s, got %s",
+ ngfControllerName,
+ ancestor.ControllerName,
+ )
+ }
+
+ if ancestor.AncestorRef.Namespace == nil {
+ return fmt.Errorf("expected ancestor namespace to be %s, got nil", namespace)
+ }
+
+ if string(*ancestor.AncestorRef.Namespace) != namespace {
+ return fmt.Errorf(
+ "expected ancestor namespace to be %s, got %s",
+ namespace,
+ string(*ancestor.AncestorRef.Namespace),
+ )
+ }
+
+ ancestorRef := ancestor.AncestorRef
+
+ if string(ancestorRef.Name) != gatewayName {
+ return fmt.Errorf("expected ancestorRef to have name %s, got %s", gatewayName, ancestorRef.Name)
+ }
+
+ if ancestorRef.Kind == nil {
+ return errors.New("expected ancestorRef to have kind Gateway, got nil")
+ }
+
+ if *ancestorRef.Kind != gatewayv1.Kind("Gateway") {
+ return fmt.Errorf(
+ "expected ancestorRef to have kind %s, got %s",
+ "Gateway",
+ string(*ancestorRef.Kind),
+ )
+ }
+
+ return nil
+}
From be4d34904de66a82914ca6d78b7db2954b1e3b9b Mon Sep 17 00:00:00 2001
From: salonichf5 <146118978+salonichf5@users.noreply.github.com>
Date: Fri, 20 Dec 2024 15:04:08 -0700
Subject: [PATCH 7/7] modify zone size for oss and plus when testing
---
tests/suite/upstream_settings_test.go | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/tests/suite/upstream_settings_test.go b/tests/suite/upstream_settings_test.go
index 39159e4406..fcc4e6ef42 100644
--- a/tests/suite/upstream_settings_test.go
+++ b/tests/suite/upstream_settings_test.go
@@ -33,6 +33,11 @@ var _ = Describe("UpstreamSettingsPolicy", Ordered, Label("functional", "uspolic
gatewayName = "gateway"
)
+ zoneSize := "512k"
+ if *plusEnabled {
+ zoneSize = "1m"
+ }
+
BeforeAll(func() {
ns := &core.Namespace{
ObjectMeta: metav1.ObjectMeta{
@@ -141,13 +146,13 @@ var _ = Describe("UpstreamSettingsPolicy", Ordered, Label("functional", "uspolic
},
{
Directive: "zone",
- Value: "uspolicy_coffee_80 512k",
+ Value: fmt.Sprintf("uspolicy_coffee_80 %s", zoneSize),
Upstream: "uspolicy_coffee_80",
File: "http.conf",
},
{
Directive: "zone",
- Value: "uspolicy_tea_80 512k",
+ Value: fmt.Sprintf("uspolicy_tea_80 %s", zoneSize),
Upstream: "uspolicy_tea_80",
File: "http.conf",
},
@@ -321,7 +326,7 @@ var _ = Describe("UpstreamSettingsPolicy", Ordered, Label("functional", "uspolic
},
{
Directive: "zone",
- Value: "uspolicy_coffee_80 512k",
+ Value: fmt.Sprintf("uspolicy_coffee_80 %s", zoneSize),
Upstream: "uspolicy_coffee_80",
File: "http.conf",
},
@@ -410,7 +415,7 @@ var _ = Describe("UpstreamSettingsPolicy", Ordered, Label("functional", "uspolic
})
func usPolicyHasNoAncestors(usPolicyNsName types.NamespacedName) bool {
- GinkgoWriter.Printf("Checking that UpstreamSettingsPolicy %q has no ancestors in status", usPolicyNsName)
+ GinkgoWriter.Printf("Checking that UpstreamSettingsPolicy %q has no ancestors in status\n", usPolicyNsName)
ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.GetStatusTimeout)
defer cancel()