Skip to content

Commit 2be5b0c

Browse files
bjee19bjee19
authored
Add validation to ensure no duplicate TargetRefs in policies (#2998)
Validate that there are no duplicate TargetRefs in the UpstreamSettingsPolicy and ObservabilityPolicy. Additionally, since this change strengthens the validation rules on the ObservabilityPolicy API, we are bumping the ObservabilityPolicy API version from v1alpha1 to v1alpha2. Problem: It's possible to create an ObservabilityPolicy or UpstreamSettingsPolicy with duplicate targetRefs. This is not a valid configuration and should be prevented. Solution: Add CEL validation to prevent this. Testing: Manually deployed both the UpstreamSettingsPolicy and ObservabilityPolicy and verified that duplicate targetRefs are invalid.
1 parent 58e8b07 commit 2be5b0c

31 files changed

+2108
-489
lines changed

Diff for: ‎apis/v1alpha1/observabilitypolicy_types.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,12 @@ import (
77

88
// +genclient
99
// +kubebuilder:object:root=true
10-
// +kubebuilder:storageversion
10+
// +kubebuilder:deprecatedversion:warning="The 'v1alpha1' version of ObservabilityPolicy API is deprecated, please migrate to 'v1alpha2'."
1111
// +kubebuilder:subresource:status
1212
// +kubebuilder:resource:categories=nginx-gateway-fabric,scope=Namespaced
1313
// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
1414
// +kubebuilder:metadata:labels="gateway.networking.k8s.io/policy=direct"
15+
//nolint:lll
1516

1617
// ObservabilityPolicy is a Direct Attached Policy. It provides a way to configure observability settings for
1718
// the NGINX Gateway Fabric data plane. Used in conjunction with the NginxProxy CRD that is attached to the
@@ -50,7 +51,7 @@ type ObservabilityPolicySpec struct {
5051
// +kubebuilder:validation:MinItems=1
5152
// +kubebuilder:validation:MaxItems=16
5253
// +kubebuilder:validation:XValidation:message="TargetRef Kind must be: HTTPRoute or GRPCRoute",rule="(self.exists(t, t.kind=='HTTPRoute') || self.exists(t, t.kind=='GRPCRoute'))"
53-
// +kubebuilder:validation:XValidation:message="TargetRef Group must be gateway.networking.k8s.io.",rule="self.all(t, t.group=='gateway.networking.k8s.io')"
54+
// +kubebuilder:validation:XValidation:message="TargetRef Group must be gateway.networking.k8s.io",rule="self.all(t, t.group=='gateway.networking.k8s.io')"
5455
//nolint:lll
5556
TargetRefs []gatewayv1alpha2.LocalPolicyTargetReference `json:"targetRefs"`
5657
}

Diff for: ‎apis/v1alpha1/upstreamsettingspolicy_types.go

+3
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,13 @@ type UpstreamSettingsPolicySpec struct {
5555
// Objects must be in the same namespace as the policy.
5656
// Support: Service
5757
//
58+
// TargetRefs must be _distinct_. The `name` field must be unique for all targetRef entries in the UpstreamSettingsPolicy.
59+
//
5860
// +kubebuilder:validation:MinItems=1
5961
// +kubebuilder:validation:MaxItems=16
6062
// +kubebuilder:validation:XValidation:message="TargetRefs Kind must be: Service",rule="self.all(t, t.kind=='Service')"
6163
// +kubebuilder:validation:XValidation:message="TargetRefs Group must be core",rule="self.exists(t, t.group=='') || self.exists(t, t.group=='core')"
64+
// +kubebuilder:validation:XValidation:message="TargetRef Name must be unique",rule="self.all(p1, self.exists_one(p2, p1.name == p2.name))"
6265
//nolint:lll
6366
TargetRefs []gatewayv1alpha2.LocalPolicyTargetReference `json:"targetRefs"`
6467
}

Diff for: ‎apis/v1alpha2/doc.go

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// Package v1alpha2 contains API Schema definitions for the
2+
// gateway.nginx.org API group.
3+
//
4+
// +kubebuilder:object:generate=true
5+
// +groupName=gateway.nginx.org
6+
package v1alpha2

Diff for: ‎apis/v1alpha2/observabilitypolicy_types.go

+139
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
package v1alpha2
2+
3+
import (
4+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
5+
gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
6+
7+
ngfAPIv1alpha1 "github.com/nginx/nginx-gateway-fabric/apis/v1alpha1"
8+
)
9+
10+
// +genclient
11+
// +kubebuilder:object:root=true
12+
// +kubebuilder:storageversion
13+
// +kubebuilder:subresource:status
14+
// +kubebuilder:resource:categories=nginx-gateway-fabric,scope=Namespaced
15+
// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
16+
// +kubebuilder:metadata:labels="gateway.networking.k8s.io/policy=direct"
17+
18+
// ObservabilityPolicy is a Direct Attached Policy. It provides a way to configure observability settings for
19+
// the NGINX Gateway Fabric data plane. Used in conjunction with the NginxProxy CRD that is attached to the
20+
// GatewayClass parametersRef.
21+
type ObservabilityPolicy struct {
22+
metav1.TypeMeta `json:",inline"`
23+
metav1.ObjectMeta `json:"metadata,omitempty"`
24+
25+
// Spec defines the desired state of the ObservabilityPolicy.
26+
Spec ObservabilityPolicySpec `json:"spec"`
27+
28+
// Status defines the state of the ObservabilityPolicy.
29+
Status gatewayv1alpha2.PolicyStatus `json:"status,omitempty"`
30+
}
31+
32+
// +kubebuilder:object:root=true
33+
34+
// ObservabilityPolicyList contains a list of ObservabilityPolicies.
35+
type ObservabilityPolicyList struct {
36+
metav1.TypeMeta `json:",inline"`
37+
metav1.ListMeta `json:"metadata,omitempty"`
38+
Items []ObservabilityPolicy `json:"items"`
39+
}
40+
41+
// ObservabilityPolicySpec defines the desired state of the ObservabilityPolicy.
42+
type ObservabilityPolicySpec struct {
43+
// Tracing allows for enabling and configuring tracing.
44+
//
45+
// +optional
46+
Tracing *Tracing `json:"tracing,omitempty"`
47+
48+
// TargetRefs identifies the API object(s) to apply the policy to.
49+
// Objects must be in the same namespace as the policy.
50+
// Support: HTTPRoute, GRPCRoute.
51+
//
52+
// TargetRefs must be _distinct_. This means that the multi-part key defined by `kind` and `name` must
53+
// be unique across all targetRef entries in the ObservabilityPolicy.
54+
//
55+
// +kubebuilder:validation:MinItems=1
56+
// +kubebuilder:validation:MaxItems=16
57+
// +kubebuilder:validation:XValidation:message="TargetRef Kind must be: HTTPRoute or GRPCRoute",rule="(self.exists(t, t.kind=='HTTPRoute') || self.exists(t, t.kind=='GRPCRoute'))"
58+
// +kubebuilder:validation:XValidation:message="TargetRef Group must be gateway.networking.k8s.io",rule="self.all(t, t.group=='gateway.networking.k8s.io')"
59+
// +kubebuilder:validation:XValidation:message="TargetRef Kind and Name combination must be unique",rule="self.all(p1, self.exists_one(p2, (p1.name == p2.name) && (p1.kind == p2.kind)))"
60+
//nolint:lll
61+
TargetRefs []gatewayv1alpha2.LocalPolicyTargetReference `json:"targetRefs"`
62+
}
63+
64+
// Tracing allows for enabling and configuring OpenTelemetry tracing.
65+
//
66+
// +kubebuilder:validation:XValidation:message="ratio can only be specified if strategy is of type ratio",rule="!(has(self.ratio) && self.strategy != 'ratio')"
67+
//
68+
//nolint:lll
69+
type Tracing struct {
70+
// Strategy defines if tracing is ratio-based or parent-based.
71+
Strategy TraceStrategy `json:"strategy"`
72+
73+
// Ratio is the percentage of traffic that should be sampled. Integer from 0 to 100.
74+
// By default, 100% of http requests are traced. Not applicable for parent-based tracing.
75+
// If ratio is set to 0, tracing is disabled.
76+
//
77+
// +optional
78+
// +kubebuilder:validation:Minimum=0
79+
// +kubebuilder:validation:Maximum=100
80+
Ratio *int32 `json:"ratio,omitempty"`
81+
82+
// Context specifies how to propagate traceparent/tracestate headers.
83+
// Default: https://nginx.org/en/docs/ngx_otel_module.html#otel_trace_context
84+
//
85+
// +optional
86+
Context *TraceContext `json:"context,omitempty"`
87+
88+
// SpanName defines the name of the Otel span. By default is the name of the location for a request.
89+
// If specified, applies to all locations that are created for a route.
90+
// Format: must have all '"' escaped and must not contain any '$' or end with an unescaped '\'
91+
// Examples of invalid names: some-$value, quoted-"value"-name, unescaped\
92+
//
93+
// +optional
94+
// +kubebuilder:validation:MinLength=1
95+
// +kubebuilder:validation:MaxLength=255
96+
// +kubebuilder:validation:Pattern=`^([^"$\\]|\\[^$])*$`
97+
SpanName *string `json:"spanName,omitempty"`
98+
99+
// SpanAttributes are custom key/value attributes that are added to each span.
100+
//
101+
// +optional
102+
// +listType=map
103+
// +listMapKey=key
104+
// +kubebuilder:validation:MaxItems=64
105+
SpanAttributes []ngfAPIv1alpha1.SpanAttribute `json:"spanAttributes,omitempty"`
106+
}
107+
108+
// TraceStrategy defines the tracing strategy.
109+
//
110+
// +kubebuilder:validation:Enum=ratio;parent
111+
type TraceStrategy string
112+
113+
const (
114+
// TraceStrategyRatio enables ratio-based tracing, defaulting to 100% sampling rate.
115+
TraceStrategyRatio TraceStrategy = "ratio"
116+
117+
// TraceStrategyParent enables tracing and only records spans if the parent span was sampled.
118+
TraceStrategyParent TraceStrategy = "parent"
119+
)
120+
121+
// TraceContext specifies how to propagate traceparent/tracestate headers.
122+
//
123+
// +kubebuilder:validation:Enum=extract;inject;propagate;ignore
124+
type TraceContext string
125+
126+
const (
127+
// TraceContextExtract uses an existing trace context from the request, so that the identifiers
128+
// of a trace and the parent span are inherited from the incoming request.
129+
TraceContextExtract TraceContext = "extract"
130+
131+
// TraceContextInject adds a new context to the request, overwriting existing headers, if any.
132+
TraceContextInject TraceContext = "inject"
133+
134+
// TraceContextPropagate updates the existing context (combines extract and inject).
135+
TraceContextPropagate TraceContext = "propagate"
136+
137+
// TraceContextIgnore skips context headers processing.
138+
TraceContextIgnore TraceContext = "ignore"
139+
)

Diff for: ‎apis/v1alpha2/policy_methods.go

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package v1alpha2
2+
3+
import (
4+
"sigs.k8s.io/gateway-api/apis/v1alpha2"
5+
)
6+
7+
// FIXME(kate-osborn): https://github.com/nginxinc/nginx-gateway-fabric/issues/1939.
8+
// Figure out a way to generate these methods for all our policies.
9+
// These methods implement the policies.Policy interface which extends client.Object to add the following methods.
10+
11+
func (p *ObservabilityPolicy) GetTargetRefs() []v1alpha2.LocalPolicyTargetReference {
12+
return p.Spec.TargetRefs
13+
}
14+
15+
func (p *ObservabilityPolicy) GetPolicyStatus() v1alpha2.PolicyStatus {
16+
return p.Status
17+
}
18+
19+
func (p *ObservabilityPolicy) SetPolicyStatus(status v1alpha2.PolicyStatus) {
20+
p.Status = status
21+
}

Diff for: ‎apis/v1alpha2/register.go

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
package v1alpha2
2+
3+
import (
4+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
5+
"k8s.io/apimachinery/pkg/runtime"
6+
"k8s.io/apimachinery/pkg/runtime/schema"
7+
)
8+
9+
// GroupName specifies the group name used to register the objects.
10+
const GroupName = "gateway.nginx.org"
11+
12+
// SchemeGroupVersion is group version used to register these objects.
13+
var SchemeGroupVersion = schema.GroupVersion{Group: GroupName, Version: "v1alpha2"}
14+
15+
// Resource takes an unqualified resource and returns a Group qualified GroupResource.
16+
func Resource(resource string) schema.GroupResource {
17+
return SchemeGroupVersion.WithResource(resource).GroupResource()
18+
}
19+
20+
var (
21+
// SchemeBuilder collects functions that add things to a scheme. It's to allow
22+
// code to compile without explicitly referencing generated types. You should
23+
// declare one in each package that will have generated deep copy or conversion
24+
// functions.
25+
SchemeBuilder = runtime.NewSchemeBuilder(addKnownTypes)
26+
27+
// AddToScheme applies all the stored functions to the scheme. A non-nil error
28+
// indicates that one function failed and the attempt was abandoned.
29+
AddToScheme = SchemeBuilder.AddToScheme
30+
)
31+
32+
// Adds the list of known types to Scheme.
33+
func addKnownTypes(scheme *runtime.Scheme) error {
34+
scheme.AddKnownTypes(SchemeGroupVersion,
35+
&ObservabilityPolicy{},
36+
&ObservabilityPolicyList{},
37+
)
38+
// AddToGroupVersion allows the serialization of client types like ListOptions.
39+
metav1.AddToGroupVersion(scheme, SchemeGroupVersion)
40+
41+
return nil
42+
}

Diff for: ‎apis/v1alpha2/zz_generated.deepcopy.go

+130
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)