From 609f7dc3eebfbac3d077da1ce33d70dfe5b8def3 Mon Sep 17 00:00:00 2001 From: John Howard Date: Thu, 12 Sep 2024 09:54:34 -0700 Subject: [PATCH] Improve validation for targetRefs Per https://gateway-api.sigs.k8s.io/geps/gep-2648/?h=targetrefs#multiple, only 16 max allowed -- which is quite reasonable. Additionally, consistently allow only workloadSelector OR targetRef; we had this only on some types --- extensions/v1alpha1/wasm.pb.go | 2 ++ extensions/v1alpha1/wasm.proto | 2 ++ kubernetes/customresourcedefinitions.gen.yaml | 30 +++++++++++++++++-- networking/v1alpha3/envoy_filter.pb.go | 2 ++ networking/v1alpha3/envoy_filter.proto | 2 ++ security/v1/authorization_policy_alias.gen.go | 1 + .../v1/request_authentication_alias.gen.go | 2 +- security/v1beta1/authorization_policy.pb.go | 2 ++ security/v1beta1/authorization_policy.proto | 2 ++ security/v1beta1/request_authentication.pb.go | 3 +- security/v1beta1/request_authentication.proto | 3 +- telemetry/v1/telemetry_alias.gen.go | 1 + telemetry/v1alpha1/telemetry.pb.go | 2 ++ telemetry/v1alpha1/telemetry.proto | 2 ++ 14 files changed, 51 insertions(+), 5 deletions(-) diff --git a/extensions/v1alpha1/wasm.pb.go b/extensions/v1alpha1/wasm.pb.go index 2352c3041e3..3bbbbfdcc2b 100644 --- a/extensions/v1alpha1/wasm.pb.go +++ b/extensions/v1alpha1/wasm.pb.go @@ -539,6 +539,7 @@ func (FailStrategy) EnumDescriptor() ([]byte, []int) { // +genclient // +k8s:deepcopy-gen=true // --> +// +kubebuilder:validation:XValidation:message="only one of targetRefs or selector can be set",rule="(has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1" type WasmPlugin struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -572,6 +573,7 @@ type WasmPlugin struct { // from misinterpreting the policy as namespace-wide during the upgrade process. // // NOTE: Waypoint proxies are required to use this field for policies to apply; `selector` policies will be ignored. + // +kubebuilder:validation:MaxItems=16 TargetRefs []*v1beta1.PolicyTargetReference `protobuf:"bytes,16,rep,name=targetRefs,proto3" json:"targetRefs,omitempty"` // URL of a Wasm module or OCI container. If no scheme is present, // defaults to `oci://`, referencing an OCI image. Other valid schemes diff --git a/extensions/v1alpha1/wasm.proto b/extensions/v1alpha1/wasm.proto index afc1ec4fb84..eb7d8fd2a47 100644 --- a/extensions/v1alpha1/wasm.proto +++ b/extensions/v1alpha1/wasm.proto @@ -236,6 +236,7 @@ option go_package="istio.io/api/extensions/v1alpha1"; // +genclient // +k8s:deepcopy-gen=true // --> +// +kubebuilder:validation:XValidation:message="only one of targetRefs or selector can be set",rule="(has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1" message WasmPlugin { // Criteria used to select the specific set of pods/VMs on which // this plugin configuration should be applied. If omitted, this @@ -267,6 +268,7 @@ message WasmPlugin { // from misinterpreting the policy as namespace-wide during the upgrade process. // // NOTE: Waypoint proxies are required to use this field for policies to apply; `selector` policies will be ignored. + // +kubebuilder:validation:MaxItems=16 repeated istio.type.v1beta1.PolicyTargetReference targetRefs = 16; // URL of a Wasm module or OCI container. If no scheme is present, diff --git a/kubernetes/customresourcedefinitions.gen.yaml b/kubernetes/customresourcedefinitions.gen.yaml index 45c7e2b61ae..3bbe1653ad2 100644 --- a/kubernetes/customresourcedefinitions.gen.yaml +++ b/kubernetes/customresourcedefinitions.gen.yaml @@ -217,6 +217,7 @@ spec: gateway.networking.k8s.io/Gateway rule: '[self.group, self.kind] in [[''core'',''Service''], ['''',''Service''], [''gateway.networking.k8s.io'',''Gateway''], [''networking.istio.io'',''ServiceEntry'']]' + maxItems: 16 type: array type: description: |- @@ -282,6 +283,9 @@ spec: required: - url type: object + x-kubernetes-validations: + - message: only one of targetRefs or selector can be set + rule: (has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1 status: properties: conditions: @@ -6140,6 +6144,7 @@ spec: gateway.networking.k8s.io/Gateway rule: '[self.group, self.kind] in [[''core'',''Service''], ['''',''Service''], [''gateway.networking.k8s.io'',''Gateway''], [''networking.istio.io'',''ServiceEntry'']]' + maxItems: 16 type: array workloadSelector: description: Criteria used to select the specific set of pods/VMs @@ -6153,6 +6158,9 @@ spec: type: object type: object type: object + x-kubernetes-validations: + - message: only one of targetRefs or workloadSelector can be set + rule: (has(self.workloadSelector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1 status: properties: conditions: @@ -14237,8 +14245,12 @@ spec: gateway.networking.k8s.io/Gateway rule: '[self.group, self.kind] in [[''core'',''Service''], ['''',''Service''], [''gateway.networking.k8s.io'',''Gateway''], [''networking.istio.io'',''ServiceEntry'']]' + maxItems: 16 type: array type: object + x-kubernetes-validations: + - message: only one of targetRefs or selector can be set + rule: (has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1 status: properties: conditions: @@ -14586,8 +14598,12 @@ spec: gateway.networking.k8s.io/Gateway rule: '[self.group, self.kind] in [[''core'',''Service''], ['''',''Service''], [''gateway.networking.k8s.io'',''Gateway''], [''networking.istio.io'',''ServiceEntry'']]' + maxItems: 16 type: array type: object + x-kubernetes-validations: + - message: only one of targetRefs or selector can be set + rule: (has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1 status: properties: conditions: @@ -15241,10 +15257,11 @@ spec: gateway.networking.k8s.io/Gateway rule: '[self.group, self.kind] in [[''core'',''Service''], ['''',''Service''], [''gateway.networking.k8s.io'',''Gateway''], [''networking.istio.io'',''ServiceEntry'']]' + maxItems: 16 type: array type: object x-kubernetes-validations: - - message: only one of targetRefs or workloadSelector can be set + - message: only one of targetRefs or selector can be set rule: (has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1 status: properties: @@ -15527,10 +15544,11 @@ spec: gateway.networking.k8s.io/Gateway rule: '[self.group, self.kind] in [[''core'',''Service''], ['''',''Service''], [''gateway.networking.k8s.io'',''Gateway''], [''networking.istio.io'',''ServiceEntry'']]' + maxItems: 16 type: array type: object x-kubernetes-validations: - - message: only one of targetRefs or workloadSelector can be set + - message: only one of targetRefs or selector can be set rule: (has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1 status: properties: @@ -15892,6 +15910,7 @@ spec: gateway.networking.k8s.io/Gateway rule: '[self.group, self.kind] in [[''core'',''Service''], ['''',''Service''], [''gateway.networking.k8s.io'',''Gateway''], [''networking.istio.io'',''ServiceEntry'']]' + maxItems: 16 type: array tracing: description: Optional. @@ -16003,6 +16022,9 @@ spec: type: object type: array type: object + x-kubernetes-validations: + - message: only one of targetRefs or selector can be set + rule: (has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1 status: properties: conditions: @@ -16336,6 +16358,7 @@ spec: gateway.networking.k8s.io/Gateway rule: '[self.group, self.kind] in [[''core'',''Service''], ['''',''Service''], [''gateway.networking.k8s.io'',''Gateway''], [''networking.istio.io'',''ServiceEntry'']]' + maxItems: 16 type: array tracing: description: Optional. @@ -16447,6 +16470,9 @@ spec: type: object type: array type: object + x-kubernetes-validations: + - message: only one of targetRefs or selector can be set + rule: (has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1 status: properties: conditions: diff --git a/networking/v1alpha3/envoy_filter.pb.go b/networking/v1alpha3/envoy_filter.pb.go index 7e5588a9bce..90e8b653233 100644 --- a/networking/v1alpha3/envoy_filter.pb.go +++ b/networking/v1alpha3/envoy_filter.pb.go @@ -822,6 +822,7 @@ func (EnvoyFilter_Patch_FilterClass) EnumDescriptor() ([]byte, []int) { // +genclient // +k8s:deepcopy-gen=true // --> +// +kubebuilder:validation:XValidation:message="only one of targetRefs or workloadSelector can be set",rule="(has(self.workloadSelector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1" type EnvoyFilter struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -851,6 +852,7 @@ type EnvoyFilter struct { // from misinterpreting the policy as namespace-wide during the upgrade process. // // NOTE: Waypoint proxies are required to use this field for policies to apply; `selector` policies will be ignored. + // +kubebuilder:validation:MaxItems=16 TargetRefs []*v1beta1.PolicyTargetReference `protobuf:"bytes,6,rep,name=targetRefs,proto3" json:"targetRefs,omitempty"` // One or more patches with match conditions. ConfigPatches []*EnvoyFilter_EnvoyConfigObjectPatch `protobuf:"bytes,4,rep,name=config_patches,json=configPatches,proto3" json:"config_patches,omitempty"` diff --git a/networking/v1alpha3/envoy_filter.proto b/networking/v1alpha3/envoy_filter.proto index ded85cf98aa..699ab3d0f4b 100644 --- a/networking/v1alpha3/envoy_filter.proto +++ b/networking/v1alpha3/envoy_filter.proto @@ -421,6 +421,7 @@ option go_package = "istio.io/api/networking/v1alpha3"; // +genclient // +k8s:deepcopy-gen=true // --> +// +kubebuilder:validation:XValidation:message="only one of targetRefs or workloadSelector can be set",rule="(has(self.workloadSelector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1" message EnvoyFilter { // `ApplyTo` specifies where in the Envoy configuration, the given patch should be applied. enum ApplyTo { @@ -866,6 +867,7 @@ message EnvoyFilter { // from misinterpreting the policy as namespace-wide during the upgrade process. // // NOTE: Waypoint proxies are required to use this field for policies to apply; `selector` policies will be ignored. + // +kubebuilder:validation:MaxItems=16 repeated istio.type.v1beta1.PolicyTargetReference targetRefs = 6; // One or more patches with match conditions. diff --git a/security/v1/authorization_policy_alias.gen.go b/security/v1/authorization_policy_alias.gen.go index 61936b9149d..d018f0fba96 100644 --- a/security/v1/authorization_policy_alias.gen.go +++ b/security/v1/authorization_policy_alias.gen.go @@ -28,6 +28,7 @@ import "istio.io/api/security/v1beta1" // +genclient // +k8s:deepcopy-gen=true // --> +// +kubebuilder:validation:XValidation:message="only one of targetRefs or selector can be set",rule="(has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1" type AuthorizationPolicy = v1beta1.AuthorizationPolicy type AuthorizationPolicy_ExtensionProvider = v1beta1.AuthorizationPolicy_ExtensionProvider diff --git a/security/v1/request_authentication_alias.gen.go b/security/v1/request_authentication_alias.gen.go index 87f17b23a1c..bde0f9c9618 100644 --- a/security/v1/request_authentication_alias.gen.go +++ b/security/v1/request_authentication_alias.gen.go @@ -260,7 +260,7 @@ import "istio.io/api/security/v1beta1" // +genclient // +k8s:deepcopy-gen=true // --> -// +kubebuilder:validation:XValidation:message="only one of targetRefs or workloadSelector can be set",rule="(has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1" +// +kubebuilder:validation:XValidation:message="only one of targetRefs or selector can be set",rule="(has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1" type RequestAuthentication = v1beta1.RequestAuthentication // JSON Web Token (JWT) token format for authentication as defined by diff --git a/security/v1beta1/authorization_policy.pb.go b/security/v1beta1/authorization_policy.pb.go index 2f291bcb4e8..2991e63f4ae 100644 --- a/security/v1beta1/authorization_policy.pb.go +++ b/security/v1beta1/authorization_policy.pb.go @@ -377,6 +377,7 @@ func (AuthorizationPolicy_Action) EnumDescriptor() ([]byte, []int) { // +genclient // +k8s:deepcopy-gen=true // --> +// +kubebuilder:validation:XValidation:message="only one of targetRefs or selector can be set",rule="(has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1" type AuthorizationPolicy struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -409,6 +410,7 @@ type AuthorizationPolicy struct { // from misinterpreting the policy as namespace-wide during the upgrade process. // // NOTE: Waypoint proxies are required to use this field for policies to apply; `selector` policies will be ignored. + // +kubebuilder:validation:MaxItems=16 TargetRefs []*v1beta1.PolicyTargetReference `protobuf:"bytes,6,rep,name=targetRefs,proto3" json:"targetRefs,omitempty"` // Optional. A list of rules to match the request. A match occurs when at least one rule matches the request. // diff --git a/security/v1beta1/authorization_policy.proto b/security/v1beta1/authorization_policy.proto index 3a085988079..3bb1049b6ea 100644 --- a/security/v1beta1/authorization_policy.proto +++ b/security/v1beta1/authorization_policy.proto @@ -270,6 +270,7 @@ option go_package="istio.io/api/security/v1beta1"; // +genclient // +k8s:deepcopy-gen=true // --> +// +kubebuilder:validation:XValidation:message="only one of targetRefs or selector can be set",rule="(has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1" message AuthorizationPolicy { // Optional. The selector decides where to apply the authorization policy. The selector will match with workloads // in the same namespace as the authorization policy. If the authorization policy is in the root namespace, the selector @@ -300,6 +301,7 @@ message AuthorizationPolicy { // from misinterpreting the policy as namespace-wide during the upgrade process. // // NOTE: Waypoint proxies are required to use this field for policies to apply; `selector` policies will be ignored. + // +kubebuilder:validation:MaxItems=16 repeated istio.type.v1beta1.PolicyTargetReference targetRefs = 6; // Optional. A list of rules to match the request. A match occurs when at least one rule matches the request. diff --git a/security/v1beta1/request_authentication.pb.go b/security/v1beta1/request_authentication.pb.go index f854a9ddd7d..fab20b6f6dc 100644 --- a/security/v1beta1/request_authentication.pb.go +++ b/security/v1beta1/request_authentication.pb.go @@ -300,7 +300,7 @@ const ( // +genclient // +k8s:deepcopy-gen=true // --> -// +kubebuilder:validation:XValidation:message="only one of targetRefs or workloadSelector can be set",rule="(has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1" +// +kubebuilder:validation:XValidation:message="only one of targetRefs or selector can be set",rule="(has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1" type RequestAuthentication struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -333,6 +333,7 @@ type RequestAuthentication struct { // from misinterpreting the policy as namespace-wide during the upgrade process. // // NOTE: Waypoint proxies are required to use this field for policies to apply; `selector` policies will be ignored. + // +kubebuilder:validation:MaxItems=16 TargetRefs []*v1beta1.PolicyTargetReference `protobuf:"bytes,4,rep,name=targetRefs,proto3" json:"targetRefs,omitempty"` // Define the list of JWTs that can be validated at the selected workloads' proxy. A valid token // will be used to extract the authenticated identity. diff --git a/security/v1beta1/request_authentication.proto b/security/v1beta1/request_authentication.proto index 89377012ccf..4ba3dfe2c28 100644 --- a/security/v1beta1/request_authentication.proto +++ b/security/v1beta1/request_authentication.proto @@ -244,7 +244,7 @@ option go_package="istio.io/api/security/v1beta1"; // +genclient // +k8s:deepcopy-gen=true // --> -// +kubebuilder:validation:XValidation:message="only one of targetRefs or workloadSelector can be set",rule="(has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1" +// +kubebuilder:validation:XValidation:message="only one of targetRefs or selector can be set",rule="(has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1" message RequestAuthentication { // Optional. The selector decides where to apply the request authentication policy. The selector will match with workloads // in the same namespace as the request authentication policy. If the request authentication policy is in the root namespace, @@ -275,6 +275,7 @@ message RequestAuthentication { // from misinterpreting the policy as namespace-wide during the upgrade process. // // NOTE: Waypoint proxies are required to use this field for policies to apply; `selector` policies will be ignored. + // +kubebuilder:validation:MaxItems=16 repeated istio.type.v1beta1.PolicyTargetReference targetRefs = 4; // Define the list of JWTs that can be validated at the selected workloads' proxy. A valid token diff --git a/telemetry/v1/telemetry_alias.gen.go b/telemetry/v1/telemetry_alias.gen.go index 1d45e270c6e..93c066c6d5b 100644 --- a/telemetry/v1/telemetry_alias.gen.go +++ b/telemetry/v1/telemetry_alias.gen.go @@ -27,6 +27,7 @@ import "istio.io/api/telemetry/v1alpha1" // +genclient // +k8s:deepcopy-gen=true // --> +// +kubebuilder:validation:XValidation:message="only one of targetRefs or selector can be set",rule="(has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1" type Telemetry = v1alpha1.Telemetry // Tracing configures tracing behavior for workloads within a mesh. diff --git a/telemetry/v1alpha1/telemetry.pb.go b/telemetry/v1alpha1/telemetry.pb.go index e2b67456052..72c13245ccc 100644 --- a/telemetry/v1alpha1/telemetry.pb.go +++ b/telemetry/v1alpha1/telemetry.pb.go @@ -544,6 +544,7 @@ func (MetricsOverrides_TagOverride_Operation) EnumDescriptor() ([]byte, []int) { // +genclient // +k8s:deepcopy-gen=true // --> +// +kubebuilder:validation:XValidation:message="only one of targetRefs or selector can be set",rule="(has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1" type Telemetry struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -574,6 +575,7 @@ type Telemetry struct { // from misinterpreting the policy as namespace-wide during the upgrade process. // // NOTE: Waypoint proxies are required to use this field for policies to apply; `selector` policies will be ignored. + // +kubebuilder:validation:MaxItems=16 TargetRefs []*v1beta1.PolicyTargetReference `protobuf:"bytes,6,rep,name=targetRefs,proto3" json:"targetRefs,omitempty"` // Optional. Tracing configures the tracing behavior for all // selected workloads. diff --git a/telemetry/v1alpha1/telemetry.proto b/telemetry/v1alpha1/telemetry.proto index 9f03ebdb35c..98e277b9515 100644 --- a/telemetry/v1alpha1/telemetry.proto +++ b/telemetry/v1alpha1/telemetry.proto @@ -258,6 +258,7 @@ option go_package = "istio.io/api/telemetry/v1alpha1"; // +genclient // +k8s:deepcopy-gen=true // --> +// +kubebuilder:validation:XValidation:message="only one of targetRefs or selector can be set",rule="(has(self.selector)?1:0)+(has(self.targetRef)?1:0)+(has(self.targetRefs)?1:0)<=1" message Telemetry { // Optional. The selector decides where to apply the policy. // If not set, the policy will be applied to all workloads in the @@ -286,6 +287,7 @@ message Telemetry { // from misinterpreting the policy as namespace-wide during the upgrade process. // // NOTE: Waypoint proxies are required to use this field for policies to apply; `selector` policies will be ignored. + // +kubebuilder:validation:MaxItems=16 repeated istio.type.v1beta1.PolicyTargetReference targetRefs = 6; // Optional. Tracing configures the tracing behavior for all