Skip to content

Commit

Permalink
Make sure OTLP export can report data to OTLP ingress/route without a…
Browse files Browse the repository at this point in the history
…dditional configuration.

Signed-off-by: Pavol Loffay <[email protected]>
  • Loading branch information
pavolloffay committed Aug 7, 2023
1 parent 1e776ce commit b907755
Show file tree
Hide file tree
Showing 28 changed files with 462 additions and 41 deletions.
19 changes: 19 additions & 0 deletions .chloggen/1967-ingress-path.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

# The name of the component, or a single word describing the area of concern, (e.g. operator, target allocator, github action)
component: operator

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Make sure OTLP export can report data to OTLP ingress/route without additional configuration

# One or more tracking issues related to the change
issues: [1967]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: |
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.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -484,4 +484,4 @@ catalog-build: opm bundle-build bundle-push ## Build a catalog image.
# Push the catalog image.
.PHONY: catalog-push
catalog-push: ## Push a catalog image.
docker push $(CATALOG_IMG)
docker push $(CATALOG_IMG)
15 changes: 15 additions & 0 deletions apis/v1alpha1/ingress_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,18 @@ const (
// and re-encrypt using a new certificate.
TLSRouteTerminationTypeReencrypt TLSRouteTerminationType = "reencrypt"
)

// 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"
)
8 changes: 7 additions & 1 deletion apis/v1alpha1/opentelemetrycollector_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,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"`
Expand Down
3 changes: 3 additions & 0 deletions apis/v1alpha1/opentelemetrycollector_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ metadata:
categories: Logging & Tracing
certified: "false"
containerImage: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator
createdAt: "2023-07-05T18:15:51Z"
createdAt: "2023-08-03T08:51:07Z"
description: Provides the OpenTelemetry components, including the Collector
operators.operatorframework.io/builder: operator-sdk-v1.29.0
operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
Expand Down
11 changes: 10 additions & 1 deletion bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
11 changes: 10 additions & 1 deletion config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions config/manager/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
resources:
- manager.yaml
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
11 changes: 10 additions & 1 deletion docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.<br/>
</td>
<td>false</td>
</tr><tr>
<td><b>ruleType</b></td>
<td>enum</td>
<td>
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.<br/>
<br/>
<i>Enum</i>: path, subdomain<br/>
</td>
<td>false</td>
</tr><tr>
<td><b><a href="#opentelemetrycollectorspecingresstlsindex">tls</a></b></td>
<td>[]object</td>
Expand All @@ -6094,7 +6103,7 @@ Ingress is used to specify how OpenTelemetry Collector is exposed. This function
<td><b>type</b></td>
<td>enum</td>
<td>
Type default value is: "" Supported types are: ingress<br/>
Type default value is: "" Supported types are: ingress, route<br/>
<br/>
<i>Enum</i>: ingress, route<br/>
</td>
Expand Down
95 changes: 70 additions & 25 deletions internal/manifests/collector/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,24 +45,15 @@ func Ingress(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemet
return nil
}

pathType := networkingv1.PathTypePrefix
paths := make([]networkingv1.HTTPIngressPath, len(ports))
for i, p := range ports {
paths[i] = networkingv1.HTTPIngressPath{
Path: "/" + p.Name,
PathType: &pathType,
Backend: networkingv1.IngressBackend{
Service: &networkingv1.IngressServiceBackend{
Name: naming.Service(otelcol.Name),
Port: networkingv1.ServiceBackendPort{
// Valid names must be non-empty and no more than 15 characters long.
Name: naming.Truncate(p.Name, 15),
},
},
},
}
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),
Expand All @@ -75,22 +66,76 @@ func Ingress(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemet
},
},
Spec: networkingv1.IngressSpec{
TLS: otelcol.Spec.Ingress.TLS,
Rules: []networkingv1.IngressRule{
{
Host: otelcol.Spec.Ingress.Hostname,
IngressRuleValue: networkingv1.IngressRuleValue{
HTTP: &networkingv1.HTTPIngressRuleValue{
Paths: paths,
},
TLS: otelcol.Spec.Ingress.TLS,
Rules: rules,
IngressClassName: otelcol.Spec.Ingress.IngressClassName,
},
}
}

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,
},
},
},
IngressClassName: otelcol.Spec.Ingress.IngressClassName,
}
}
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)

host := fmt.Sprintf("%s.%s", portName, hostname)
if hostname == "" || hostname == "*" {
host = portName
}
rules = append(rules, networkingv1.IngressRule{
Host: host,
IngressRuleValue: networkingv1.IngressRuleValue{
HTTP: &networkingv1.HTTPIngressRuleValue{
Paths: []networkingv1.HTTPIngressPath{
{
Path: "/",
PathType: &pathType,
Backend: networkingv1.IngressBackend{
Service: &networkingv1.IngressServiceBackend{
Name: naming.Service(otelcol),
Port: networkingv1.ServiceBackendPort{
Name: portName,
},
},
},
},
},
},
},
})
}
return rules
}

// TODO: Update this to properly return an error https://github.com/open-telemetry/opentelemetry-operator/issues/1972
func servicePortsFromCfg(logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) []corev1.ServicePort {
configFromString, err := adapters.ConfigFromString(otelcol.Spec.Config)
Expand Down
106 changes: 105 additions & 1 deletion internal/manifests/collector/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func TestDesiredIngresses(t *testing.T) {
assert.Nil(t, actual)
})

t.Run("should return nil unable to do something else", func(t *testing.T) {
t.Run("path per port", func(t *testing.T) {
var (
ns = "test"
hostname = "example.com"
Expand Down Expand Up @@ -172,5 +172,109 @@ func TestDesiredIngresses(t *testing.T) {
},
}, got)
})
t.Run("subdomain per port", func(t *testing.T) {
var (
ns = "test"
hostname = "example.com"
ingressClassName = "nginx"
)

params, err := newParams("something:tag", testFileIngress)
if err != nil {
t.Fatal(err)
}

params.Instance.Namespace = ns
params.Instance.Spec.Ingress = v1alpha1.Ingress{
Type: v1alpha1.IngressTypeNginx,
RuleType: v1alpha1.IngressRuleTypeSubdomain,
Hostname: hostname,
Annotations: map[string]string{"some.key": "some.value"},
IngressClassName: &ingressClassName,
}

got := Ingress(params.Config, params.Log, params.Instance)
pathType := networkingv1.PathTypeExact

assert.NotEqual(t, &networkingv1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: naming.Ingress(params.Instance.Name),
Namespace: ns,
Annotations: params.Instance.Spec.Ingress.Annotations,
Labels: map[string]string{
"app.kubernetes.io/name": naming.Ingress(params.Instance.Name),
"app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.Instance.Namespace, params.Instance.Name),
"app.kubernetes.io/managed-by": "opentelemetry-operator",
},
},
Spec: networkingv1.IngressSpec{
IngressClassName: &ingressClassName,
Rules: []networkingv1.IngressRule{
{
Host: "another-port." + hostname,
IngressRuleValue: networkingv1.IngressRuleValue{
HTTP: &networkingv1.HTTPIngressRuleValue{
Paths: []networkingv1.HTTPIngressPath{
{
Path: "/",
PathType: &pathType,
Backend: networkingv1.IngressBackend{
Service: &networkingv1.IngressServiceBackend{
Name: "test-collector",
Port: networkingv1.ServiceBackendPort{
Name: "another-port",
},
},
},
},
},
},
},
},
{
Host: "otlp-grpc." + hostname,
IngressRuleValue: networkingv1.IngressRuleValue{
HTTP: &networkingv1.HTTPIngressRuleValue{
Paths: []networkingv1.HTTPIngressPath{
{
Path: "/",
PathType: &pathType,
Backend: networkingv1.IngressBackend{
Service: &networkingv1.IngressServiceBackend{
Name: "test-collector",
Port: networkingv1.ServiceBackendPort{
Name: "otlp-grpc",
},
},
},
},
},
},
},
},
{
Host: "otlp-test-grpc." + hostname,
IngressRuleValue: networkingv1.IngressRuleValue{
HTTP: &networkingv1.HTTPIngressRuleValue{
Paths: []networkingv1.HTTPIngressPath{
{
Path: "/",
PathType: &pathType,
Backend: networkingv1.IngressBackend{
Service: &networkingv1.IngressServiceBackend{
Name: "test-collector",
Port: networkingv1.ServiceBackendPort{
Name: "otlp-test-grpc",
},
},
},
},
},
},
},
},
},
},
}, got)
})
}
Loading

0 comments on commit b907755

Please sign in to comment.