From bf08205f7088101d17cfef51838df4d795a4c245 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Thu, 3 Aug 2023 23:20:08 +0200 Subject: [PATCH] Fix Signed-off-by: Pavol Loffay --- .chloggen/1967-ingress-path.yaml | 5 ++- apis/v1alpha1/opentelemetrycollector_types.go | 23 +++++++++- .../opentelemetrycollector_webhook.go | 3 ++ ...ntelemetry.io_opentelemetrycollectors.yaml | 11 ++++- ...ntelemetry.io_opentelemetrycollectors.yaml | 11 ++++- docs/api.md | 11 ++++- internal/manifests/collector/ingress.go | 43 +++++++++++++++++-- internal/manifests/collector/route.go | 3 +- pkg/collector/reconcile/ingress_test.go | 2 +- tests/e2e/ingress/00-assert.yaml | 4 +- tests/e2e/ingress/00-install.yaml | 1 + 11 files changed, 103 insertions(+), 14 deletions(-) diff --git a/.chloggen/1967-ingress-path.yaml b/.chloggen/1967-ingress-path.yaml index 21c4ba9bd4..9bb3456296 100755 --- a/.chloggen/1967-ingress-path.yaml +++ b/.chloggen/1967-ingress-path.yaml @@ -14,5 +14,6 @@ issues: [1967] # These lines will be padded with 2 spaces and then inserted directly into the document. # Use pipe (|) for multiline entries. subtext: | - The ingress and route now uses only root path `/` for all exposed collector receivers. - Each receiver is exposed on a subdomain that matches the port name of the receiver. + The ingress can be configured to create a single host with multiple paths or + multiple hosts with subdomains (one per receiver port). + The path from OpenShift route was removed. diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index 2d599eb675..d4cdc85158 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -36,6 +36,21 @@ const ( ManagementStateUnmanaged ManagementStateType = "Unmanaged" ) +// IngressRuleType defines how the collector receivers will be exposed in the Ingress. +// +// +kubebuilder:validation:Enum=path;subdomain +type IngressRuleType string + +const ( + // IngressRuleTypePath configures Ingress to use single host with multiple paths. + // This configuration might require additional ingress setting to rewrite paths. + IngressRuleTypePath IngressRuleType = "path" + + // IngressRuleTypeSubdomain configures Ingress to use multiple hosts - one for each exposed + // receiver port. The port name is used as a subdomain for the host defined in the Ingress e.g. otlp-http.example.com. + IngressRuleTypeSubdomain IngressRuleType = "subdomain" +) + // Ingress is used to specify how OpenTelemetry Collector is exposed. This // functionality is only available if one of the valid modes is set. // Valid modes are: deployment, daemonset and statefulset. @@ -48,9 +63,15 @@ const ( // SEE: OpenTelemetryCollector.spec.ports[index]. type Ingress struct { // Type default value is: "" - // Supported types are: ingress + // Supported types are: ingress, route Type IngressType `json:"type,omitempty"` + // RuleType defines how Ingress exposes collector receivers. + // Either multiple hosts with subdomain per receiver port are created or + // a single host with multiple paths. + // Default is path. + RuleType IngressRuleType `json:"ruleType,omitempty"` + // Hostname by which the ingress proxy can be reached. // +optional Hostname string `json:"hostname,omitempty"` diff --git a/apis/v1alpha1/opentelemetrycollector_webhook.go b/apis/v1alpha1/opentelemetrycollector_webhook.go index bff4099c7a..8cbca98cd6 100644 --- a/apis/v1alpha1/opentelemetrycollector_webhook.go +++ b/apis/v1alpha1/opentelemetrycollector_webhook.go @@ -109,6 +109,9 @@ func (r *OpenTelemetryCollector) Default() { if r.Spec.Ingress.Type == IngressTypeRoute && r.Spec.Ingress.Route.Termination == "" { r.Spec.Ingress.Route.Termination = TLSRouteTerminationTypeEdge } + if r.Spec.Ingress.Type == IngressTypeNginx && r.Spec.Ingress.RuleType == "" { + r.Spec.Ingress.RuleType = IngressRuleTypePath + } } // +kubebuilder:webhook:verbs=create;update,path=/validate-opentelemetry-io-v1alpha1-opentelemetrycollector,mutating=false,failurePolicy=fail,groups=opentelemetry.io,resources=opentelemetrycollectors,versions=v1alpha1,name=vopentelemetrycollectorcreateupdate.kb.io,sideEffects=none,admissionReviewVersions=v1 diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index 6357ffd4dc..a2396954ee 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -1373,6 +1373,14 @@ spec: - reencrypt type: string type: object + ruleType: + description: RuleType defines how Ingress exposes collector receivers. + Either multiple hosts with subdomain per receiver port are created + or a single host with multiple paths. Default is path. + enum: + - path + - subdomain + type: string tls: description: TLS configuration. items: @@ -1401,7 +1409,8 @@ spec: type: object type: array type: - description: 'Type default value is: "" Supported types are: ingress' + description: 'Type default value is: "" Supported types are: ingress, + route' enum: - ingress - route diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml index f1a732b9e9..3c6c780994 100644 --- a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml +++ b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml @@ -1370,6 +1370,14 @@ spec: - reencrypt type: string type: object + ruleType: + description: RuleType defines how Ingress exposes collector receivers. + Either multiple hosts with subdomain per receiver port are created + or a single host with multiple paths. Default is path. + enum: + - path + - subdomain + type: string tls: description: TLS configuration. items: @@ -1398,7 +1406,8 @@ spec: type: object type: array type: - description: 'Type default value is: "" Supported types are: ingress' + description: 'Type default value is: "" Supported types are: ingress, + route' enum: - ingress - route diff --git a/docs/api.md b/docs/api.md index 9a0fe3bbfa..670452cd07 100644 --- a/docs/api.md +++ b/docs/api.md @@ -6083,6 +6083,15 @@ Ingress is used to specify how OpenTelemetry Collector is exposed. This function Route is an OpenShift specific section that is only considered when type "route" is used.
false + + ruleType + enum + + RuleType defines how Ingress exposes collector receivers. Either multiple hosts with subdomain per receiver port are created or a single host with multiple paths. Default is path.
+
+ Enum: path, subdomain
+ + false tls []object @@ -6094,7 +6103,7 @@ Ingress is used to specify how OpenTelemetry Collector is exposed. This function type enum - Type default value is: "" Supported types are: ingress
+ Type default value is: "" Supported types are: ingress, route

Enum: ingress, route
diff --git a/internal/manifests/collector/ingress.go b/internal/manifests/collector/ingress.go index ee36808926..e9a2f5506f 100644 --- a/internal/manifests/collector/ingress.go +++ b/internal/manifests/collector/ingress.go @@ -45,6 +45,15 @@ func Ingress(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemet return nil } + var rules []networkingv1.IngressRule + switch otelcol.Spec.Ingress.RuleType { + case v1alpha1.IngressRuleTypePath, "": + rules = []networkingv1.IngressRule{createPathIngressRules(otelcol.Name, otelcol.Spec.Ingress.Hostname, ports)} + case v1alpha1.IngressRuleTypeSubdomain: + rules = createSubdomainIngressRules(otelcol.Name, otelcol.Spec.Ingress.Hostname, ports) + } + + createSubdomainIngressRules(otelcol.Name, otelcol.Spec.Ingress.Hostname, ports) return &networkingv1.Ingress{ ObjectMeta: metav1.ObjectMeta{ Name: naming.Ingress(otelcol.Name), @@ -58,15 +67,43 @@ func Ingress(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemet }, Spec: networkingv1.IngressSpec{ TLS: otelcol.Spec.Ingress.TLS, - Rules: createIngressRules(otelcol.Name, otelcol.Spec.Ingress.Hostname, ports), + Rules: rules, IngressClassName: otelcol.Spec.Ingress.IngressClassName, }, } } -func createIngressRules(otelcol string, hostname string, ports []corev1.ServicePort) []networkingv1.IngressRule { - var rules []networkingv1.IngressRule +func createPathIngressRules(otelcol string, hostname string, ports []corev1.ServicePort) networkingv1.IngressRule { pathType := networkingv1.PathTypePrefix + paths := make([]networkingv1.HTTPIngressPath, len(ports)) + for i, port := range ports { + portName := naming.PortName(port.Name, port.Port) + paths[i] = networkingv1.HTTPIngressPath{ + Path: "/" + port.Name, + PathType: &pathType, + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: naming.Service(otelcol), + Port: networkingv1.ServiceBackendPort{ + Name: portName, + }, + }, + }, + } + } + return networkingv1.IngressRule{ + Host: hostname, + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: paths, + }, + }, + } +} + +func createSubdomainIngressRules(otelcol string, hostname string, ports []corev1.ServicePort) []networkingv1.IngressRule { + var rules []networkingv1.IngressRule + pathType := networkingv1.PathTypeExact for _, port := range ports { portName := naming.PortName(port.Name, port.Port) diff --git a/internal/manifests/collector/route.go b/internal/manifests/collector/route.go index cfe8868e7c..ed283a77db 100644 --- a/internal/manifests/collector/route.go +++ b/internal/manifests/collector/route.go @@ -85,8 +85,7 @@ func Routes(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetr Name: naming.Service(otelcol.Name), }, Port: &routev1.RoutePort{ - // Valid names must be non-empty and no more than 15 characters long. - TargetPort: intstr.FromString(naming.Truncate(p.Name, 15)), + TargetPort: intstr.FromString(portName), }, WildcardPolicy: routev1.WildcardPolicyNone, TLS: tlsCfg, diff --git a/pkg/collector/reconcile/ingress_test.go b/pkg/collector/reconcile/ingress_test.go index 5da5e1f77e..67dfe0d8de 100644 --- a/pkg/collector/reconcile/ingress_test.go +++ b/pkg/collector/reconcile/ingress_test.go @@ -59,7 +59,7 @@ func TestExpectedIngresses(t *testing.T) { err = params.Client.Get(ctx, nns, got) assert.NoError(t, err) - assert.Equal(t, "web.something-else.com", got.Spec.Rules[0].Host) + assert.Equal(t, "something-else.com", got.Spec.Rules[0].Host) if v, ok := got.Annotations["blub"]; !ok || v != "blob" { t.Error("annotations are not up-to-date. Missing entry or value is invalid.") diff --git a/tests/e2e/ingress/00-assert.yaml b/tests/e2e/ingress/00-assert.yaml index d92353b3d6..7f337f1fcc 100644 --- a/tests/e2e/ingress/00-assert.yaml +++ b/tests/e2e/ingress/00-assert.yaml @@ -32,7 +32,7 @@ spec: port: name: otlp-grpc path: / - pathType: Prefix + pathType: Exact - host: otlp-http.test.otel http: paths: @@ -42,4 +42,4 @@ spec: port: name: otlp-http path: / - pathType: Prefix + pathType: Exact diff --git a/tests/e2e/ingress/00-install.yaml b/tests/e2e/ingress/00-install.yaml index cf54fba957..a49320fa99 100644 --- a/tests/e2e/ingress/00-install.yaml +++ b/tests/e2e/ingress/00-install.yaml @@ -7,6 +7,7 @@ spec: mode: "deployment" ingress: type: ingress + ruleType: subdomain hostname: "test.otel" annotations: something.com: "true"