From b8cc75fa3d97bdbbb60e61423aaf19e93ac56efe Mon Sep 17 00:00:00 2001 From: zirain Date: Sun, 31 Mar 2024 11:27:09 +0800 Subject: [PATCH] api: Model OpenTelelemetry Sinks as a BackendRef Signed-off-by: zirain --- api/v1alpha1/accesslogging_types.go | 14 +- api/v1alpha1/tracing_types.go | 13 ++ .../gateway.envoyproxy.io_envoyproxies.yaml | 185 +++++++++++++++++- test/cel-validation/envoyproxy_test.go | 101 ++++++++++ tools/make/golang.mk | 7 + 5 files changed, 312 insertions(+), 8 deletions(-) diff --git a/api/v1alpha1/accesslogging_types.go b/api/v1alpha1/accesslogging_types.go index 483a224fab4c..9a13bccf3ee0 100644 --- a/api/v1alpha1/accesslogging_types.go +++ b/api/v1alpha1/accesslogging_types.go @@ -5,6 +5,8 @@ package v1alpha1 +import gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" + type ProxyAccessLog struct { // Disable disables access logging for managed proxies if set to true. Disable bool `json:"disable,omitempty"` @@ -92,16 +94,26 @@ type FileEnvoyProxyAccessLog struct { Path string `json:"path,omitempty"` } -// TODO: consider reuse ExtensionService? +// OpenTelemetryEnvoyProxyAccessLog defines the OpenTelemetry access log sink. +// +// +kubebuilder:validation:XValidation:message="BackendRef only support Service Kind.",rule="has(self.backendRef) && has(self.backendRef.kind) && self.backendRef.kind == 'Service'" type OpenTelemetryEnvoyProxyAccessLog struct { // Host define the extension service hostname. + // Deprecated: Use BackendRef instead. Host string `json:"host"` // Port defines the port the extension service is exposed on. + // Deprecated: Use BackendRef instead. // // +optional // +kubebuilder:validation:Minimum=0 // +kubebuilder:default=4317 Port int32 `json:"port,omitempty"` + // BackendRef references a Kubernetes object that represents the + // backend server to which the accesslog will be sent. + // Only service Kind is supported for now. + // + // +optional + BackendRef *gwapiv1.BackendObjectReference `json:"backendRef,omitempty"` // Resources is a set of labels that describe the source of a log entry, including envoy node info. // It's recommended to follow [semantic conventions](https://opentelemetry.io/docs/reference/specification/resource/semantic_conventions/). // +optional diff --git a/api/v1alpha1/tracing_types.go b/api/v1alpha1/tracing_types.go index 63529df54f2d..100229f712a0 100644 --- a/api/v1alpha1/tracing_types.go +++ b/api/v1alpha1/tracing_types.go @@ -5,6 +5,8 @@ package v1alpha1 +import gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" + type ProxyTracing struct { // SamplingRate controls the rate at which traffic will be // selected for tracing if no prior sampling decision has been made. @@ -28,6 +30,9 @@ const ( TracingProviderTypeOpenTelemetry TracingProviderType = "OpenTelemetry" ) +// TracingProvider defines the tracing provider configuration. +// +// +kubebuilder:validation:XValidation:message="BackendRef only support Service Kind.",rule="has(self.backendRef) && has(self.backendRef.kind) && self.backendRef.kind == 'Service'" type TracingProvider struct { // Type defines the tracing provider type. // EG currently only supports OpenTelemetry. @@ -35,13 +40,21 @@ type TracingProvider struct { // +kubebuilder:default=OpenTelemetry Type TracingProviderType `json:"type"` // Host define the provider service hostname. + // Deprecated: Use BackendRef instead. Host string `json:"host"` // Port defines the port the provider service is exposed on. + // Deprecated: Use BackendRef instead. // // +optional // +kubebuilder:validation:Minimum=0 // +kubebuilder:default=4317 Port int32 `json:"port,omitempty"` + // BackendRef references a Kubernetes object that represents the + // backend server to which the accesslog will be sent. + // Only service Kind is supported for now. + // + // +optional + BackendRef *gwapiv1.BackendObjectReference `json:"backendRef"` } type CustomTagType string diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml index 4e092a8e739b..eeea3fd9ea89 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml @@ -5899,14 +5899,95 @@ spec: description: OpenTelemetry defines the OpenTelemetry accesslog sink. properties: + backendRef: + description: |- + BackendRef references a Kubernetes object that represents the + backend server to which the accesslog will be sent. + Only service Kind is supported for now. + properties: + group: + default: "" + description: |- + Group is the group of the referent. For example, "gateway.networking.k8s.io". + When unspecified or empty string, core API group is inferred. + 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: Service + description: |- + Kind is the Kubernetes resource kind of the referent. For example + "Service". + + + Defaults to "Service" when not specified. + + + ExternalName services can refer to CNAME DNS records that may live + outside of the cluster and as such are difficult to reason about in + terms of conformance. They also may not be safe to forward to (see + CVE-2021-25740 for more information). Implementations SHOULD NOT + support ExternalName Services. + + + Support: Core (Services with a type other than ExternalName) + + + Support: Implementation-specific (Services with type ExternalName) + 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. + maxLength: 253 + minLength: 1 + type: string + namespace: + description: |- + Namespace is the namespace of the backend. When unspecified, the local + namespace is inferred. + + + Note that when a namespace different than the local namespace is specified, + a ReferenceGrant object is required in the referent namespace to allow that + namespace's owner to accept the reference. See the ReferenceGrant + documentation for details. + + + Support: Core + maxLength: 63 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ + type: string + port: + description: |- + Port specifies the destination port number to use for this resource. + Port is required when the referent is a Kubernetes Service. In this + case, the port number is the service port number, not the target port. + For other resources, destination port might be derived from the referent + resource or this field. + format: int32 + maximum: 65535 + minimum: 1 + type: integer + required: + - name + type: object + x-kubernetes-validations: + - message: Must have port for Service reference + rule: '(size(self.group) == 0 && self.kind + == ''Service'') ? has(self.port) : true' host: - description: Host define the extension service - hostname. + description: |- + Host define the extension service hostname. + Deprecated: Use BackendRef instead. type: string port: default: 4317 - description: Port defines the port the extension - service is exposed on. + description: |- + Port defines the port the extension service is exposed on. + Deprecated: Use BackendRef instead. format: int32 minimum: 0 type: integer @@ -5920,6 +6001,10 @@ spec: required: - host type: object + x-kubernetes-validations: + - message: BackendRef only support Service Kind. + rule: has(self.backendRef) && has(self.backendRef.kind) + && self.backendRef.kind == 'Service' type: description: Type defines the type of accesslog sink. @@ -6111,13 +6196,95 @@ spec: Provider defines the tracing provider. Only OpenTelemetry is supported currently. properties: + backendRef: + description: |- + BackendRef references a Kubernetes object that represents the + backend server to which the accesslog will be sent. + Only service Kind is supported for now. + properties: + group: + default: "" + description: |- + Group is the group of the referent. For example, "gateway.networking.k8s.io". + When unspecified or empty string, core API group is inferred. + 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: Service + description: |- + Kind is the Kubernetes resource kind of the referent. For example + "Service". + + + Defaults to "Service" when not specified. + + + ExternalName services can refer to CNAME DNS records that may live + outside of the cluster and as such are difficult to reason about in + terms of conformance. They also may not be safe to forward to (see + CVE-2021-25740 for more information). Implementations SHOULD NOT + support ExternalName Services. + + + Support: Core (Services with a type other than ExternalName) + + + Support: Implementation-specific (Services with type ExternalName) + 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. + maxLength: 253 + minLength: 1 + type: string + namespace: + description: |- + Namespace is the namespace of the backend. When unspecified, the local + namespace is inferred. + + + Note that when a namespace different than the local namespace is specified, + a ReferenceGrant object is required in the referent namespace to allow that + namespace's owner to accept the reference. See the ReferenceGrant + documentation for details. + + + Support: Core + maxLength: 63 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ + type: string + port: + description: |- + Port specifies the destination port number to use for this resource. + Port is required when the referent is a Kubernetes Service. In this + case, the port number is the service port number, not the target port. + For other resources, destination port might be derived from the referent + resource or this field. + format: int32 + maximum: 65535 + minimum: 1 + type: integer + required: + - name + type: object + x-kubernetes-validations: + - message: Must have port for Service reference + rule: '(size(self.group) == 0 && self.kind == ''Service'') + ? has(self.port) : true' host: - description: Host define the provider service hostname. + description: |- + Host define the provider service hostname. + Deprecated: Use BackendRef instead. type: string port: default: 4317 - description: Port defines the port the provider service - is exposed on. + description: |- + Port defines the port the provider service is exposed on. + Deprecated: Use BackendRef instead. format: int32 minimum: 0 type: integer @@ -6133,6 +6300,10 @@ spec: - host - type type: object + x-kubernetes-validations: + - message: BackendRef only support Service Kind. + rule: has(self.backendRef) && has(self.backendRef.kind) + && self.backendRef.kind == 'Service' samplingRate: default: 100 description: |- diff --git a/test/cel-validation/envoyproxy_test.go b/test/cel-validation/envoyproxy_test.go index cc414e314b66..4976c8bf086c 100644 --- a/test/cel-validation/envoyproxy_test.go +++ b/test/cel-validation/envoyproxy_test.go @@ -17,6 +17,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" + gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" ) @@ -533,6 +534,106 @@ func TestEnvoyProxyProvider(t *testing.T) { } }, }, + { + desc: "invalid-accesslog-backendref", + mutate: func(envoy *egv1a1.EnvoyProxy) { + envoy.Spec = egv1a1.EnvoyProxySpec{ + Telemetry: &egv1a1.ProxyTelemetry{ + AccessLog: &egv1a1.ProxyAccessLog{ + Settings: []egv1a1.ProxyAccessLogSetting{ + { + Format: egv1a1.ProxyAccessLogFormat{ + Type: "Text", + Text: ptr.To("[%START_TIME%]"), + }, + Sinks: []egv1a1.ProxyAccessLogSink{ + { + Type: egv1a1.ProxyAccessLogSinkTypeOpenTelemetry, + OpenTelemetry: &egv1a1.OpenTelemetryEnvoyProxyAccessLog{ + BackendRef: &gwapiv1.BackendObjectReference{ + Name: "fake-service", + Kind: ptr.To(gwapiv1.Kind("foo")), + }, + }, + }, + }, + }, + }, + }, + }, + } + }, + wantErrors: []string{"BackendRef only support Service Kind."}, + }, + { + desc: "accesslog-backendref", + mutate: func(envoy *egv1a1.EnvoyProxy) { + envoy.Spec = egv1a1.EnvoyProxySpec{ + Telemetry: &egv1a1.ProxyTelemetry{ + AccessLog: &egv1a1.ProxyAccessLog{ + Settings: []egv1a1.ProxyAccessLogSetting{ + { + Format: egv1a1.ProxyAccessLogFormat{ + Type: "Text", + Text: ptr.To("[%START_TIME%]"), + }, + Sinks: []egv1a1.ProxyAccessLogSink{ + { + Type: egv1a1.ProxyAccessLogSinkTypeOpenTelemetry, + OpenTelemetry: &egv1a1.OpenTelemetryEnvoyProxyAccessLog{ + BackendRef: &gwapiv1.BackendObjectReference{ + Name: "fake-service", + Kind: ptr.To(gwapiv1.Kind("Service")), + Port: ptr.To(gwapiv1.PortNumber(8080)), + }, + }, + }, + }, + }, + }, + }, + }, + } + }, + }, + { + desc: "invalid-tracing-backendref", + mutate: func(envoy *egv1a1.EnvoyProxy) { + envoy.Spec = egv1a1.EnvoyProxySpec{ + Telemetry: &egv1a1.ProxyTelemetry{ + Tracing: &egv1a1.ProxyTracing{ + Provider: egv1a1.TracingProvider{ + Type: egv1a1.TracingProviderTypeOpenTelemetry, + BackendRef: &gwapiv1.BackendObjectReference{ + Name: "fake-service", + Kind: ptr.To(gwapiv1.Kind("foo")), + }, + }, + }, + }, + } + }, + wantErrors: []string{"BackendRef only support Service Kind."}, + }, + { + desc: "tracing-backendref", + mutate: func(envoy *egv1a1.EnvoyProxy) { + envoy.Spec = egv1a1.EnvoyProxySpec{ + Telemetry: &egv1a1.ProxyTelemetry{ + Tracing: &egv1a1.ProxyTracing{ + Provider: egv1a1.TracingProvider{ + Type: egv1a1.TracingProviderTypeOpenTelemetry, + BackendRef: &gwapiv1.BackendObjectReference{ + Name: "fake-service", + Kind: ptr.To(gwapiv1.Kind("Service")), + Port: ptr.To(gwapiv1.PortNumber(8080)), + }, + }, + }, + }, + } + }, + }, } for _, tc := range cases { diff --git a/tools/make/golang.mk b/tools/make/golang.mk index 8f30d4a667f5..658a2074565e 100644 --- a/tools/make/golang.mk +++ b/tools/make/golang.mk @@ -61,6 +61,13 @@ go.test.coverage: $(tools/setup-envtest) ## Run go unit and integration tests in KUBEBUILDER_ASSETS="$(shell $(tools/setup-envtest) use $(ENVTEST_K8S_VERSION) -p path)" \ go test ./... --tags=integration,celvalidation -race -coverprofile=coverage.xml -covermode=atomic +.PHONY: go.test.validate +go.test.validate: manifests $(tools/setup-envtest) + @$(LOG_TARGET) + go clean -testcache # Ensure we're not using cached test results + KUBEBUILDER_ASSETS="$(shell $(tools/setup-envtest) use $(ENVTEST_K8S_VERSION) -p path)" \ + go test ./test/cel-validation --tags=celvalidation -race + .PHONY: go.clean go.clean: ## Clean the building output files @$(LOG_TARGET)