refactor

Signed-off-by: zirain <zirain2009@gmail.com>
envoyproxy · May 17, 2024 · bc2883e · bc2883e
1 parent 3c1d844
commit bc2883e
Show file tree

Hide file tree

Showing 26 changed files with 181 additions and 184 deletions.
diff --git a/internal/gatewayapi/listener.go b/internal/gatewayapi/listener.go
@@ -11,6 +11,7 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	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"
@@ -215,7 +216,7 @@ func (t *Translator) processAccessLog(envoyproxy *egv1a1.EnvoyProxy, resources *
 
 	irAccessLog := &ir.AccessLog{}
 	// translate the access log configuration to the IR
-	for _, accessLog := range envoyproxy.Spec.Telemetry.AccessLog.Settings {
+	for idx, accessLog := range envoyproxy.Spec.Telemetry.AccessLog.Settings {
 		for _, sink := range accessLog.Sinks {
 			switch sink.Type {
 			case egv1a1.ProxyAccessLogSinkTypeFile:
@@ -249,20 +250,27 @@ func (t *Translator) processAccessLog(envoyproxy *egv1a1.EnvoyProxy, resources *
 
 				// TODO: remove support for Host/Port in v1.2
 				al := &ir.OpenTelemetryAccessLog{
-					Port:      uint32(sink.OpenTelemetry.Port),
 					Resources: sink.OpenTelemetry.Resources,
 				}
 
-				if sink.OpenTelemetry.Host != nil {
-					al.Host = *sink.OpenTelemetry.Host
+				ds, err := t.processBackendRefs(sink.OpenTelemetry.BackendRefs, envoyproxy.Namespace, resources)
+				if err != nil {
+					return nil, err
+				}
+				al.Destination = ir.RouteDestination{
+					Name:     fmt.Sprintf("accesslog-%d", idx), // TODO: rename this, so that we can share backend with tracing?
+					Settings: ds,
 				}
 
-				if len(sink.OpenTelemetry.BackendRefs) > 0 {
-					ds, err := t.processBackendRefs(sink.OpenTelemetry.BackendRefs, envoyproxy.Namespace, resources)
-					if err != nil {
-						return nil, err
+				if len(ds) == 0 {
+					// fallback to host and port
+					var host string
+					var port uint32
+					if sink.OpenTelemetry.Host != nil {
+						host, port = *sink.OpenTelemetry.Host, uint32(sink.OpenTelemetry.Port)
 					}
-					al.Destinations = ds
+					al.Destination.Settings = destinationSettingFromHostAndPort(host, port)
+					al.Authority = host
 				}
 
 				switch accessLog.Format.Type {
@@ -288,18 +296,25 @@ func (t *Translator) processTracing(gw *gwapiv1.Gateway, envoyproxy *egv1a1.Envo
 	}
 	tracing := envoyproxy.Spec.Telemetry.Tracing
 
-	// TODO: remove support for Host/Port in v1.2
-	var host string
-	var port uint32
-	if tracing.Provider.Host != nil {
-		host, port = *tracing.Provider.Host, uint32(tracing.Provider.Port)
-	}
-
 	ds, err := t.processBackendRefs(tracing.Provider.BackendRefs, envoyproxy.Namespace, resources)
 	if err != nil {
 		return nil, err
 	}
 
+	var authority string
+
+	// fallback to host and port
+	// TODO: remove support for Host/Port in v1.2
+	if len(ds) == 0 {
+		var host string
+		var port uint32
+		if tracing.Provider.Host != nil {
+			host, port = *tracing.Provider.Host, uint32(tracing.Provider.Port)
+		}
+		ds = destinationSettingFromHostAndPort(host, port)
+		authority = host
+	}
+
 	samplingRate := 100.0
 	if tracing.SamplingRate != nil {
 		samplingRate = float64(*tracing.SamplingRate)
@@ -311,12 +326,14 @@ func (t *Translator) processTracing(gw *gwapiv1.Gateway, envoyproxy *egv1a1.Envo
 	}
 
 	return &ir.Tracing{
+		Authority:    authority,
 		ServiceName:  serviceName,
-		Host:         host,
-		Port:         port,
 		SamplingRate: samplingRate,
 		CustomTags:   tracing.CustomTags,
-		Destinations: ds,
+		Destination: ir.RouteDestination{
+			Name:     "tracing", // TODO: rename this, so that we can share backend with accesslog?
+			Settings: ds,
+		},
 	}, nil
 }
 
@@ -364,3 +381,13 @@ func (t *Translator) processBackendRefs(backendRefs []egv1a1.BackendRef, namespa
 	}
 	return result, nil
 }
+
+func destinationSettingFromHostAndPort(host string, port uint32) []*ir.DestinationSetting {
+	return []*ir.DestinationSetting{
+		{
+			Weight:    ptr.To[uint32](1),
+			Protocol:  ir.GRPC,
+			Endpoints: []*ir.DestinationEndpoint{ir.NewDestEndpoint(host, port)},
+		},
+	}
+}
diff --git a/internal/gatewayapi/testdata/envoyproxy-accesslog-backend.out.yaml b/internal/gatewayapi/testdata/envoyproxy-accesslog-backend.out.yaml
@@ -136,14 +136,14 @@ xdsIR:
   envoy-gateway/gateway-1:
     accessLog:
       openTelemetry:
-      - destinations:
-        - addressType: IP
-          endpoints:
-          - host: 8.7.6.5
-            port: 4317
-          protocol: GRPC
-        host: ""
-        port: 0
+      - destination:
+          name: accesslog-0
+          settings:
+          - addressType: IP
+            endpoints:
+            - host: 8.7.6.5
+              port: 4317
+            protocol: GRPC
         resources:
           k8s.cluster.name: cluster-1
         text: |

diff --git a/internal/gatewayapi/testdata/envoyproxy-accesslog.out.yaml b/internal/gatewayapi/testdata/envoyproxy-accesslog.out.yaml
@@ -134,8 +134,15 @@ xdsIR:
   envoy-gateway/gateway-1:
     accessLog:
       openTelemetry:
-      - host: otel-collector.monitoring.svc.cluster.local
-        port: 4317
+      - authority: otel-collector.monitoring.svc.cluster.local
+        destination:
+          name: accesslog-0
+          settings:
+          - endpoints:
+            - host: otel-collector.monitoring.svc.cluster.local
+              port: 4317
+            protocol: GRPC
+            weight: 1
         resources:
           k8s.cluster.name: cluster-1
         text: |

diff --git a/internal/gatewayapi/testdata/envoyproxy-tracing-backend.out.yaml b/internal/gatewayapi/testdata/envoyproxy-tracing-backend.out.yaml
@@ -138,13 +138,13 @@ xdsIR:
         mergeSlashes: true
       port: 10080
     tracing:
-      destinations:
-      - addressType: IP
-        endpoints:
-        - host: 8.7.6.5
-          port: 4317
-        protocol: GRPC
-      host: ""
-      port: 0
+      destination:
+        name: tracing
+        settings:
+        - addressType: IP
+          endpoints:
+          - host: 8.7.6.5
+            port: 4317
+          protocol: GRPC
       samplingRate: 100
       serviceName: gateway-1.envoy-gateway
diff --git a/internal/gatewayapi/testdata/tracing-merged-multiple-routes.out.yaml b/internal/gatewayapi/testdata/tracing-merged-multiple-routes.out.yaml
@@ -292,7 +292,14 @@ xdsIR:
           name: ""
           prefix: /
     tracing:
-      host: otel-collector.monitoring.svc.cluster.local
-      port: 4317
+      authority: otel-collector.monitoring.svc.cluster.local
+      destination:
+        name: tracing
+        settings:
+        - endpoints:
+          - host: otel-collector.monitoring.svc.cluster.local
+            port: 4317
+          protocol: GRPC
+          weight: 1
       samplingRate: 100
       serviceName: envoy-gateway-class
diff --git a/internal/gatewayapi/testdata/tracing-multiple-routes.out.yaml b/internal/gatewayapi/testdata/tracing-multiple-routes.out.yaml
@@ -282,8 +282,15 @@ xdsIR:
           name: ""
           prefix: /
     tracing:
-      host: otel-collector.monitoring.svc.cluster.local
-      port: 4317
+      authority: otel-collector.monitoring.svc.cluster.local
+      destination:
+        name: tracing
+        settings:
+        - endpoints:
+          - host: otel-collector.monitoring.svc.cluster.local
+            port: 4317
+          protocol: GRPC
+          weight: 1
       samplingRate: 100
       serviceName: gateway-1.envoy-gateway
   envoy-gateway/gateway-2:
@@ -327,7 +334,14 @@ xdsIR:
           name: ""
           prefix: /
     tracing:
-      host: otel-collector.monitoring.svc.cluster.local
-      port: 4317
+      authority: otel-collector.monitoring.svc.cluster.local
+      destination:
+        name: tracing
+        settings:
+        - endpoints:
+          - host: otel-collector.monitoring.svc.cluster.local
+            port: 4317
+          protocol: GRPC
+          weight: 1
       samplingRate: 100
       serviceName: gateway-2.envoy-gateway
diff --git a/internal/ir/xds.go b/internal/ir/xds.go
@@ -1452,12 +1452,11 @@ type JSONAccessLog struct {
 // OpenTelemetryAccessLog holds the configuration for OpenTelemetry access logging.
 // +k8s:deepcopy-gen=true
 type OpenTelemetryAccessLog struct {
-	Text         *string               `json:"text,omitempty" yaml:"text,omitempty"`
-	Attributes   map[string]string     `json:"attributes,omitempty" yaml:"attributes,omitempty"`
-	Host         string                `json:"host" yaml:"host"`
-	Port         uint32                `json:"port" yaml:"port"`
-	Resources    map[string]string     `json:"resources,omitempty" yaml:"resources,omitempty"`
-	Destinations []*DestinationSetting `json:"destinations,omitempty" yaml:"destinations,omitempty"`
+	Authority   string            `json:"authority,omitempty" yaml:"authority,omitempty"`
+	Text        *string           `json:"text,omitempty" yaml:"text,omitempty"`
+	Attributes  map[string]string `json:"attributes,omitempty" yaml:"attributes,omitempty"`
+	Resources   map[string]string `json:"resources,omitempty" yaml:"resources,omitempty"`
+	Destination RouteDestination  `json:"destination,omitempty" yaml:"destination,omitempty"`
 }
 
 // EnvoyPatchPolicy defines the intermediate representation of the EnvoyPatchPolicy resource.
@@ -1512,11 +1511,10 @@ type JSONPatchOperation struct {
 // +k8s:deepcopy-gen=true
 type Tracing struct {
 	ServiceName  string                      `json:"serviceName"`
-	Host         string                      `json:"host"`
-	Port         uint32                      `json:"port"`
+	Authority    string                      `json:"authority,omitempty"`
 	SamplingRate float64                     `json:"samplingRate,omitempty"`
 	CustomTags   map[string]egv1a1.CustomTag `json:"customTags,omitempty"`
-	Destinations []*DestinationSetting       `json:"destinations,omitempty"`
+	Destination  RouteDestination            `json:"destination,omitempty"`
 }
 
 // Metrics defines the configuration for metrics generated by Envoy

diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go
diff --git a/internal/xds/translator/accesslog.go b/internal/xds/translator/accesslog.go
@@ -23,7 +23,6 @@ import (
 	"golang.org/x/exp/maps"
 	"google.golang.org/protobuf/types/known/anypb"
 	"google.golang.org/protobuf/types/known/structpb"
-	"k8s.io/utils/ptr"
 
 	"github.com/envoyproxy/gateway/internal/ir"
 	"github.com/envoyproxy/gateway/internal/utils/protocov"
@@ -179,8 +178,8 @@ func buildXdsAccessLog(al *ir.AccessLog, forListener bool) []*accesslog.AccessLo
 				GrpcService: &cfgcore.GrpcService{
 					TargetSpecifier: &cfgcore.GrpcService_EnvoyGrpc_{
 						EnvoyGrpc: &cfgcore.GrpcService_EnvoyGrpc{
-							ClusterName: buildClusterName("accesslog", otel.Host, otel.Port),
-							Authority:   otel.Host,
+							ClusterName: otel.Destination.Name,
+							Authority:   otel.Authority,
 						},
 					},
 				},
@@ -334,24 +333,9 @@ func processClusterForAccessLog(tCtx *types.ResourceVersionTable, al *ir.AccessL
 	}
 
 	for _, otel := range al.OpenTelemetry {
-		clusterName := buildClusterName("accesslog", otel.Host, otel.Port)
-
-		var dests []*ir.DestinationSetting
-		if len(otel.Destinations) > 0 {
-			dests = otel.Destinations
-		} else {
-			dests = []*ir.DestinationSetting{
-				{
-					Weight:    ptr.To[uint32](1),
-					Protocol:  ir.GRPC,
-					Endpoints: []*ir.DestinationEndpoint{ir.NewDestEndpoint(otel.Host, otel.Port)},
-				},
-			}
-		}
-
 		if err := addXdsCluster(tCtx, &xdsClusterArgs{
-			name:         clusterName,
-			settings:     dests,
+			name:         otel.Destination.Name,
+			settings:     otel.Destination.Settings,
 			tSocket:      nil,
 			endpointType: EndpointTypeDNS,
 			metrics:      metrics,

diff --git a/internal/xds/translator/testdata/in/xds-ir/accesslog-endpoint-stats.yaml b/internal/xds/translator/testdata/in/xds-ir/accesslog-endpoint-stats.yaml
@@ -21,8 +21,13 @@ accesslog:
         "response_code": "%RESPONSE_CODE%"
       resources:
         "cluster_name": "cluster1"
-      host: otel-collector.default.svc.cluster.local
-      port: 4317
+      destination:
+        name: "accesslog-0"
+        authority: "otel-collector.default.svc.cluster.local"
+        settings:
+          - endpoints:
+              - host: "otel-collector.default.svc.cluster.local"
+                port: 4317
 http:
   - name: "first-listener"
     address: "0.0.0.0"

diff --git a/internal/xds/translator/testdata/in/xds-ir/accesslog-formatters.yaml b/internal/xds/translator/testdata/in/xds-ir/accesslog-formatters.yaml
@@ -27,8 +27,13 @@ accesslog:
       "response_code": "%RESPONSE_CODE%"
     resources:
       "cluster_name": "cluster1"
-    host: otel-collector.default.svc.cluster.local
-    port: 4317
+    destination:
+      name: "accesslog-0"
+      authority: "otel-collector.default.svc.cluster.local"
+      settings:
+        - endpoints:
+            - host: "otel-collector.default.svc.cluster.local"
+              port: 4317
 http:
 - name: "first-listener"
   address: "0.0.0.0"

diff --git a/internal/xds/translator/testdata/in/xds-ir/accesslog-invalid.yaml b/internal/xds/translator/testdata/in/xds-ir/accesslog-invalid.yaml
@@ -19,8 +19,12 @@ accesslog:
       "response_code": "%RESPONSE_CODE%"
     resources:
       "cluster_name": "cluster1"
-    host: ""
-    port: 4317
+    destination:
+      name: "accesslog-0"
+      settings:
+        - endpoints:
+            - host: ""
+              port: 4317
 http:
 - name: "first-listener"
   address: "0.0.0.0"