From d907f7fe49ef8817fa4533b2fd698f8f5b6dac24 Mon Sep 17 00:00:00 2001 From: Ben B Date: Tue, 11 Oct 2022 11:36:29 +0200 Subject: [PATCH] Add ingress options (#1128) * add ingress name generation function Signed-off-by: Benedikt Bongartz * extend otelcol crd with minimalistic ingress options Signed-off-by: Benedikt Bongartz * support collector ingress reconciling Signed-off-by: Benedikt Bongartz * register ingress reconciler Signed-off-by: Benedikt Bongartz * grant permission to create, modify and delete ingress entries Signed-off-by: Benedikt Bongartz * add ingress integration tests Signed-off-by: Benedikt Bongartz * verify if collector mode is compatible with ingress settings Signed-off-by: Benedikt Bongartz * create dedicated ingress type Signed-off-by: Benedikt Bongartz * follow recommendations Signed-off-by: Benedikt Bongartz * regenerate Signed-off-by: Benedikt Bongartz Signed-off-by: Benedikt Bongartz --- apis/v1alpha1/ingress_type.go | 26 ++ apis/v1alpha1/opentelemetrycollector_types.go | 28 ++ .../opentelemetrycollector_webhook.go | 6 + .../opentelemetrycollector_webhook_test.go | 15 + apis/v1alpha1/zz_generated.deepcopy.go | 31 ++ ...emetry-operator.clusterserviceversion.yaml | 12 + ...ntelemetry.io_opentelemetrycollectors.yaml | 46 +++ ...ntelemetry.io_opentelemetrycollectors.yaml | 46 +++ config/rbac/role.yaml | 12 + .../opentelemetrycollector_controller.go | 6 + docs/api.md | 91 ++++++ pkg/collector/reconcile/ingress.go | 242 ++++++++++++++++ pkg/collector/reconcile/ingress_test.go | 268 ++++++++++++++++++ pkg/collector/testdata/ingress_testdata.yaml | 16 ++ pkg/naming/main.go | 5 + tests/e2e/ingress/00-assert.yaml | 47 +++ tests/e2e/ingress/00-install.yaml | 28 ++ 17 files changed, 925 insertions(+) create mode 100644 apis/v1alpha1/ingress_type.go create mode 100644 pkg/collector/reconcile/ingress.go create mode 100644 pkg/collector/reconcile/ingress_test.go create mode 100644 pkg/collector/testdata/ingress_testdata.yaml create mode 100644 tests/e2e/ingress/00-assert.yaml create mode 100644 tests/e2e/ingress/00-install.yaml diff --git a/apis/v1alpha1/ingress_type.go b/apis/v1alpha1/ingress_type.go new file mode 100644 index 0000000000..5ef8528b04 --- /dev/null +++ b/apis/v1alpha1/ingress_type.go @@ -0,0 +1,26 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package v1alpha1 + +type ( + // IngressType represents how a collector should be exposed (ingress vs route). + // +kubebuilder:validation:Enum=ingress + IngressType string +) + +const ( + // IngressTypeNginx specifies that an ingress entry should be created. + IngressTypeNginx IngressType = "ingress" +) diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index 8688ca69be..5b2ee5f0b9 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -17,9 +17,32 @@ package v1alpha1 import ( autoscalingv2 "k8s.io/api/autoscaling/v2" v1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +// 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. +type Ingress struct { + // Type default value is: "" + // Supported types are: ingress + Type IngressType `json:"type,omitempty"` + + // Hostname by which the ingress proxy can be reached. + // +optional + Hostname string `json:"hostname,omitempty"` + + // Annotations to add to ingress. + // e.g. 'cert-manager.io/cluster-issuer: "letsencrypt"' + // +optional + Annotations map[string]string `json:"annotations,omitempty"` + + // TLS configuration. + // +optional + TLS []networkingv1.IngressTLS `json:"tls,omitempty"` +} + // OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. type OpenTelemetryCollectorSpec struct { // Resources to set on the OpenTelemetry Collector pods. @@ -107,6 +130,11 @@ type OpenTelemetryCollectorSpec struct { // +optional // +listType=atomic Volumes []v1.Volume `json:"volumes,omitempty"` + // 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. + // +optional + Ingress Ingress `json:"ingress,omitempty"` // HostNetwork indicates if the pod should run in the host networking namespace. // +optional HostNetwork bool `json:"hostNetwork,omitempty"` diff --git a/apis/v1alpha1/opentelemetrycollector_webhook.go b/apis/v1alpha1/opentelemetrycollector_webhook.go index 0eb4c624b0..6c8c22ccdc 100644 --- a/apis/v1alpha1/opentelemetrycollector_webhook.go +++ b/apis/v1alpha1/opentelemetrycollector_webhook.go @@ -166,5 +166,11 @@ func (r *OpenTelemetryCollector) validateCRDSpec() error { } + if r.Spec.Ingress.Type == IngressTypeNginx && r.Spec.Mode == ModeSidecar { + return fmt.Errorf("the OptenTelemetry Spec Ingress configuiration is incorrect. Ingress can only be used in combination with the modes: %s, %s, %s", + ModeDeployment, ModeDaemonSet, ModeStatefulSet, + ) + } + return nil } diff --git a/apis/v1alpha1/opentelemetrycollector_webhook_test.go b/apis/v1alpha1/opentelemetrycollector_webhook_test.go index 5e1856c21b..008067f224 100644 --- a/apis/v1alpha1/opentelemetrycollector_webhook_test.go +++ b/apis/v1alpha1/opentelemetrycollector_webhook_test.go @@ -15,6 +15,7 @@ package v1alpha1 import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -317,6 +318,20 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, expectedErr: "targetCPUUtilization should be greater than 0 and less than 100", }, + { + name: "invalid deployment mode incompabible with ingress settings", + otelcol: OpenTelemetryCollector{ + Spec: OpenTelemetryCollectorSpec{ + Mode: ModeSidecar, + Ingress: Ingress{ + Type: IngressTypeNginx, + }, + }, + }, + expectedErr: fmt.Sprintf("Ingress can only be used in combination with the modes: %s, %s, %s", + ModeDeployment, ModeDaemonSet, ModeStatefulSet, + ), + }, } for _, test := range tests { diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index 655948e0ea..f03c0e2ce7 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -22,6 +22,7 @@ package v1alpha1 import ( "k8s.io/api/autoscaling/v2" "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -87,6 +88,35 @@ func (in *Exporter) DeepCopy() *Exporter { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Ingress) DeepCopyInto(out *Ingress) { + *out = *in + if in.Annotations != nil { + in, out := &in.Annotations, &out.Annotations + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.TLS != nil { + in, out := &in.TLS, &out.TLS + *out = make([]networkingv1.IngressTLS, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Ingress. +func (in *Ingress) DeepCopy() *Ingress { + if in == nil { + return nil + } + out := new(Ingress) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Instrumentation) DeepCopyInto(out *Instrumentation) { *out = *in @@ -403,6 +433,7 @@ func (in *OpenTelemetryCollectorSpec) DeepCopyInto(out *OpenTelemetryCollectorSp (*in)[i].DeepCopyInto(&(*out)[i]) } } + in.Ingress.DeepCopyInto(&out.Ingress) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OpenTelemetryCollectorSpec. diff --git a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml index fb2be6e9b1..c8debce019 100644 --- a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -209,6 +209,18 @@ spec: - get - list - update + - apiGroups: + - networking.k8s.io + resources: + - ingresses + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - opentelemetry.io resources: diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index d9efcacdde..ca5d28a00c 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -354,6 +354,52 @@ spec: description: ImagePullPolicy indicates the pull policy to be used for retrieving the container image (Always, Never, IfNotPresent) type: string + ingress: + description: '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.' + properties: + annotations: + additionalProperties: + type: string + description: 'Annotations to add to ingress. e.g. ''cert-manager.io/cluster-issuer: + "letsencrypt"''' + type: object + hostname: + description: Hostname by which the ingress proxy can be reached. + type: string + tls: + description: TLS configuration. + items: + description: IngressTLS describes the transport layer security + associated with an Ingress. + properties: + hosts: + description: Hosts are a list of hosts included in the TLS + certificate. The values in this list must match the name/s + used in the tlsSecret. Defaults to the wildcard host setting + for the loadbalancer controller fulfilling this Ingress, + if left unspecified. + items: + type: string + type: array + x-kubernetes-list-type: atomic + secretName: + description: SecretName is the name of the secret used to + terminate TLS traffic on port 443. Field is left optional + to allow TLS routing based on SNI hostname alone. If the + SNI host in a listener conflicts with the "Host" header + field used by an IngressRule, the SNI host is used for + termination and value of the Host header is used for routing. + type: string + type: object + type: array + type: + description: 'Type default value is: "" Supported types are: ingress' + enum: + - ingress + type: string + type: object maxReplicas: description: MaxReplicas sets an upper bound to the autoscaling feature. If MaxReplicas is set autoscaling is enabled. diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml index 16a0124882..57681b4950 100644 --- a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml +++ b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml @@ -352,6 +352,52 @@ spec: description: ImagePullPolicy indicates the pull policy to be used for retrieving the container image (Always, Never, IfNotPresent) type: string + ingress: + description: '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.' + properties: + annotations: + additionalProperties: + type: string + description: 'Annotations to add to ingress. e.g. ''cert-manager.io/cluster-issuer: + "letsencrypt"''' + type: object + hostname: + description: Hostname by which the ingress proxy can be reached. + type: string + tls: + description: TLS configuration. + items: + description: IngressTLS describes the transport layer security + associated with an Ingress. + properties: + hosts: + description: Hosts are a list of hosts included in the TLS + certificate. The values in this list must match the name/s + used in the tlsSecret. Defaults to the wildcard host setting + for the loadbalancer controller fulfilling this Ingress, + if left unspecified. + items: + type: string + type: array + x-kubernetes-list-type: atomic + secretName: + description: SecretName is the name of the secret used to + terminate TLS traffic on port 443. Field is left optional + to allow TLS routing based on SNI hostname alone. If the + SNI host in a listener conflicts with the "Host" header + field used by an IngressRule, the SNI host is used for + termination and value of the Host header is used for routing. + type: string + type: object + type: array + type: + description: 'Type default value is: "" Supported types are: ingress' + enum: + - ingress + type: string + type: object maxReplicas: description: MaxReplicas sets an upper bound to the autoscaling feature. If MaxReplicas is set autoscaling is enabled. diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index a321ff7891..5655552356 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -120,6 +120,18 @@ rules: - get - list - update +- apiGroups: + - networking.k8s.io + resources: + - ingresses + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - opentelemetry.io resources: diff --git a/controllers/opentelemetrycollector_controller.go b/controllers/opentelemetrycollector_controller.go index ab1e2c2ac7..11483b6fb9 100644 --- a/controllers/opentelemetrycollector_controller.go +++ b/controllers/opentelemetrycollector_controller.go @@ -102,6 +102,11 @@ func NewReconciler(p Params) *OpenTelemetryCollectorReconciler { "stateful sets", true, }, + { + reconcile.Ingresses, + "ingresses", + true, + }, { reconcile.Self, "opentelemetry", @@ -123,6 +128,7 @@ func NewReconciler(p Params) *OpenTelemetryCollectorReconciler { // +kubebuilder:rbac:groups=opentelemetry.io,resources=opentelemetrycollectors,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=opentelemetry.io,resources=opentelemetrycollectors/status,verbs=get;update;patch // +kubebuilder:rbac:groups=opentelemetry.io,resources=opentelemetrycollectors/finalizers,verbs=get;update;patch +// +kubebuilder:rbac:groups=networking.k8s.io,resources=ingresses,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=get;list;create;update // +kubebuilder:rbac:groups="",resources=events,verbs=create;patch diff --git a/docs/api.md b/docs/api.md index aa9e100583..de084d0efd 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1740,6 +1740,13 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. ImagePullPolicy indicates the pull policy to be used for retrieving the container image (Always, Never, IfNotPresent)
false + + ingress + object + + 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.
+ + false maxReplicas integer @@ -2474,6 +2481,90 @@ The Secret to select from +### OpenTelemetryCollector.spec.ingress +[↩ Parent](#opentelemetrycollectorspec) + + + +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. + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
NameTypeDescriptionRequired
annotationsmap[string]string + Annotations to add to ingress. e.g. 'cert-manager.io/cluster-issuer: "letsencrypt"'
+ 		false
hostnamestring + Hostname by which the ingress proxy can be reached.
+ 		false
tls[]object + TLS configuration.
+ 		false
typeenum + Type default value is: "" Supported types are: ingress
+
+ Enum: ingress
+ 		false
+ + +### OpenTelemetryCollector.spec.ingress.tls[index] +[↩ Parent](#opentelemetrycollectorspecingress) + + + +IngressTLS describes the transport layer security associated with an Ingress. + + + + + + + + + + + + + + + + + + + + + +
NameTypeDescriptionRequired
hosts[]string + Hosts are a list of hosts included in the TLS certificate. The values in this list must match the name/s used in the tlsSecret. Defaults to the wildcard host setting for the loadbalancer controller fulfilling this Ingress, if left unspecified.
+ 		false
secretNamestring + SecretName is the name of the secret used to terminate TLS traffic on port 443. Field is left optional to allow TLS routing based on SNI hostname alone. If the SNI host in a listener conflicts with the "Host" header field used by an IngressRule, the SNI host is used for termination and value of the Host header is used for routing.
+ 		false
+ + ### OpenTelemetryCollector.spec.podSecurityContext [↩ Parent](#opentelemetrycollectorspec) diff --git a/pkg/collector/reconcile/ingress.go b/pkg/collector/reconcile/ingress.go new file mode 100644 index 0000000000..79576ebf74 --- /dev/null +++ b/pkg/collector/reconcile/ingress.go @@ -0,0 +1,242 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package reconcile + +import ( + "context" + "fmt" + + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + "github.com/open-telemetry/opentelemetry-operator/pkg/naming" +) + +func desiredIngresses(_ context.Context, params Params) *networkingv1.Ingress { + if params.Instance.Spec.Ingress.Type != v1alpha1.IngressTypeNginx { + return nil + } + + config, err := adapters.ConfigFromString(params.Instance.Spec.Config) + if err != nil { + params.Log.Error(err, "couldn't extract the configuration from the context") + return nil + } + + ports, err := adapters.ConfigToReceiverPorts(params.Log, config) + if err != nil { + params.Log.Error(err, "couldn't build the ingress for this instance") + return nil + } + + if len(params.Instance.Spec.Ports) > 0 { + // we should add all the ports from the CR + // there are two cases where problems might occur: + // 1) when the port number is already being used by a receiver + // 2) same, but for the port name + // + // in the first case, we remove the port we inferred from the list + // in the second case, we rename our inferred port to something like "port-%d" + portNumbers, portNames := extractPortNumbersAndNames(params.Instance.Spec.Ports) + resultingInferredPorts := []corev1.ServicePort{} + for _, inferred := range ports { + if filtered := filterPort(params.Log, inferred, portNumbers, portNames); filtered != nil { + resultingInferredPorts = append(resultingInferredPorts, *filtered) + } + } + + ports = append(params.Instance.Spec.Ports, resultingInferredPorts...) + } + + // if we have no ports, we don't need a ingress entry + if len(ports) == 0 { + params.Log.V(1).Info( + "the instance's configuration didn't yield any ports to open, skipping ingress", + "instance.name", params.Instance.Name, + "instance.namespace", params.Instance.Namespace, + ) + 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(params.Instance), + Port: networkingv1.ServiceBackendPort{ + // Valid names must be non-empty and no more than 15 characters long. + Name: naming.Truncate(p.Name, 15), + }, + }, + }, + } + } + + return &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: naming.Ingress(params.Instance), + Namespace: params.Instance.Namespace, + Annotations: params.Instance.Spec.Ingress.Annotations, + Labels: map[string]string{ + "app.kubernetes.io/name": naming.Ingress(params.Instance), + "app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.Instance.Namespace, params.Instance.Name), + "app.kubernetes.io/managed-by": "opentelemetry-operator", + }, + }, + Spec: networkingv1.IngressSpec{ + TLS: params.Instance.Spec.Ingress.TLS, + Rules: []networkingv1.IngressRule{ + { + Host: params.Instance.Spec.Ingress.Hostname, + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: paths, + }, + }, + }, + }, + }, + } +} + +// Ingresses reconciles the ingress(s) required for the instance in the current context. +func Ingresses(ctx context.Context, params Params) error { + isSupportedMode := true + if params.Instance.Spec.Mode == v1alpha1.ModeSidecar { + params.Log.V(3).Info("ingress settings are not supported in sidecar mode") + isSupportedMode = false + } + + nns := types.NamespacedName{Namespace: params.Instance.Namespace, Name: params.Instance.Name} + err := params.Client.Get(ctx, nns, &corev1.Service{}) // NOTE: check if service exists. + serviceExists := err != nil + + var desired []networkingv1.Ingress + if isSupportedMode && serviceExists { + if d := desiredIngresses(ctx, params); d != nil { + desired = append(desired, *d) + } + } + + // first, handle the create/update parts + if err := expectedIngresses(ctx, params, desired); err != nil { + return fmt.Errorf("failed to reconcile the expected ingresses: %w", err) + } + + // then, delete the extra objects + if err := deleteIngresses(ctx, params, desired); err != nil { + return fmt.Errorf("failed to reconcile the ingresses to be deleted: %w", err) + } + + return nil +} + +func expectedIngresses(ctx context.Context, params Params, expected []networkingv1.Ingress) error { + for _, obj := range expected { + desired := obj + + if err := controllerutil.SetControllerReference(¶ms.Instance, &desired, params.Scheme); err != nil { + return fmt.Errorf("failed to set controller reference: %w", err) + } + + existing := &networkingv1.Ingress{} + nns := types.NamespacedName{Namespace: desired.Namespace, Name: desired.Name} + err := params.Client.Get(ctx, nns, existing) + if err != nil && k8serrors.IsNotFound(err) { + if err := params.Client.Create(ctx, &desired); err != nil { + return fmt.Errorf("failed to create: %w", err) + } + params.Log.V(2).Info("created", "ingress.name", desired.Name, "ingress.namespace", desired.Namespace) + return nil + } else if err != nil { + return fmt.Errorf("failed to get: %w", err) + } + + // it exists already, merge the two if the end result isn't identical to the existing one + updated := existing.DeepCopy() + if updated.Annotations == nil { + updated.Annotations = map[string]string{} + } + if updated.Labels == nil { + updated.Labels = map[string]string{} + } + updated.ObjectMeta.OwnerReferences = desired.ObjectMeta.OwnerReferences + updated.Spec.Rules = desired.Spec.Rules + updated.Spec.TLS = desired.Spec.TLS + updated.Spec.DefaultBackend = desired.Spec.DefaultBackend + updated.Spec.IngressClassName = desired.Spec.IngressClassName + + for k, v := range desired.ObjectMeta.Annotations { + updated.ObjectMeta.Annotations[k] = v + } + for k, v := range desired.ObjectMeta.Labels { + updated.ObjectMeta.Labels[k] = v + } + + patch := client.MergeFrom(existing) + + if err := params.Client.Patch(ctx, updated, patch); err != nil { + return fmt.Errorf("failed to apply changes: %w", err) + } + + params.Log.V(2).Info("applied", "ingress.name", desired.Name, "ingress.namespace", desired.Namespace) + } + return nil +} + +func deleteIngresses(ctx context.Context, params Params, expected []networkingv1.Ingress) error { + opts := []client.ListOption{ + client.InNamespace(params.Instance.Namespace), + client.MatchingLabels(map[string]string{ + "app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.Instance.Namespace, params.Instance.Name), + "app.kubernetes.io/managed-by": "opentelemetry-operator", + }), + } + list := &networkingv1.IngressList{} + if err := params.Client.List(ctx, list, opts...); err != nil { + return fmt.Errorf("failed to list: %w", err) + } + + for i := range list.Items { + existing := list.Items[i] + del := true + for _, keep := range expected { + if keep.Name == existing.Name && keep.Namespace == existing.Namespace { + del = false + break + } + } + + if del { + if err := params.Client.Delete(ctx, &existing); err != nil { + return fmt.Errorf("failed to delete: %w", err) + } + params.Log.V(2).Info("deleted", "ingress.name", existing.Name, "ingress.namespace", existing.Namespace) + } + } + + return nil +} diff --git a/pkg/collector/reconcile/ingress_test.go b/pkg/collector/reconcile/ingress_test.go new file mode 100644 index 0000000000..2c3447c7f4 --- /dev/null +++ b/pkg/collector/reconcile/ingress_test.go @@ -0,0 +1,268 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package reconcile + +import ( + "context" + _ "embed" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + v1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "github.com/open-telemetry/opentelemetry-operator/internal/config" + "github.com/open-telemetry/opentelemetry-operator/pkg/naming" +) + +const test_file_ingress = "../testdata/ingress_testdata.yaml" + +func TestDesiredIngresses(t *testing.T) { + t.Run("should return nil invalid ingress type", func(t *testing.T) { + params := Params{ + Config: config.Config{}, + Client: k8sClient, + Log: logger, + Instance: v1alpha1.OpenTelemetryCollector{ + Spec: v1alpha1.OpenTelemetryCollectorSpec{ + Ingress: v1alpha1.Ingress{ + Type: v1alpha1.IngressType("unknown"), + }, + }, + }, + } + + actual := desiredIngresses(context.Background(), params) + assert.Nil(t, actual) + }) + + t.Run("should return nil unable to parse config", func(t *testing.T) { + params := Params{ + Config: config.Config{}, + Client: k8sClient, + Log: logger, + Instance: v1alpha1.OpenTelemetryCollector{ + Spec: v1alpha1.OpenTelemetryCollectorSpec{ + Config: "!!!", + Ingress: v1alpha1.Ingress{ + Type: v1alpha1.IngressTypeNginx, + }, + }, + }, + } + + actual := desiredIngresses(context.Background(), params) + assert.Nil(t, actual) + }) + + t.Run("should return nil unable to parse receiver ports", func(t *testing.T) { + params := Params{ + Config: config.Config{}, + Client: k8sClient, + Log: logger, + Instance: v1alpha1.OpenTelemetryCollector{ + Spec: v1alpha1.OpenTelemetryCollectorSpec{ + Config: "---", + Ingress: v1alpha1.Ingress{ + Type: v1alpha1.IngressTypeNginx, + }, + }, + }, + } + + actual := desiredIngresses(context.Background(), params) + assert.Nil(t, actual) + }) + + t.Run("should return nil unable to do something else", func(t *testing.T) { + var ( + ns = "test" + hostname = "example.com" + ) + + params, err := newParams("something:tag", test_file_ingress) + if err != nil { + t.Fatal(err) + } + + params.Instance.Namespace = ns + params.Instance.Spec.Ingress = v1alpha1.Ingress{ + Type: v1alpha1.IngressTypeNginx, + Hostname: hostname, + Annotations: map[string]string{"some.key": "some.value"}, + } + + got := desiredIngresses(context.Background(), params) + pathType := networkingv1.PathTypePrefix + + assert.NotEqual(t, &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: naming.Ingress(params.Instance), + Namespace: ns, + Annotations: params.Instance.Spec.Ingress.Annotations, + Labels: map[string]string{ + "app.kubernetes.io/name": naming.Ingress(params.Instance), + "app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.Instance.Namespace, params.Instance.Name), + "app.kubernetes.io/managed-by": "opentelemetry-operator", + }, + }, + Spec: networkingv1.IngressSpec{ + Rules: []networkingv1.IngressRule{ + { + Host: hostname, + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{ + { + Path: "/another-port", + PathType: &pathType, + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "test-collector", + Port: networkingv1.ServiceBackendPort{ + Name: "another-port", + }, + }, + }, + }, + { + Path: "/otlp-grpc", + PathType: &pathType, + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "test-collector", + Port: networkingv1.ServiceBackendPort{ + Name: "otlp-grpc", + }, + }, + }, + }, + { + Path: "/otlp-test-grpc", + PathType: &pathType, + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "test-collector", + Port: networkingv1.ServiceBackendPort{ + Name: "otlp-test-grpc", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, got) + }) + +} + +func TestExpectedIngresses(t *testing.T) { + t.Run("should create and update ingress entry", func(t *testing.T) { + ctx := context.Background() + + params, err := newParams("something:tag", test_file_ingress) + if err != nil { + t.Fatal(err) + } + params.Instance.Spec.Ingress.Type = "ingress" + + err = expectedIngresses(ctx, params, []networkingv1.Ingress{*desiredIngresses(ctx, params)}) + assert.NoError(t, err) + + nns := types.NamespacedName{Namespace: "default", Name: "test-ingress"} + exists, err := populateObjectIfExists(t, &networkingv1.Ingress{}, nns) + assert.NoError(t, err) + assert.True(t, exists) + + // update fields + const expectHostname = "something-else.com" + params.Instance.Spec.Ingress.Annotations = map[string]string{"blub": "blob"} + params.Instance.Spec.Ingress.Hostname = expectHostname + + err = expectedIngresses(ctx, params, []networkingv1.Ingress{*desiredIngresses(ctx, params)}) + assert.NoError(t, err) + + got := &networkingv1.Ingress{} + err = params.Client.Get(ctx, nns, got) + assert.NoError(t, err) + + gotHostname := got.Spec.Rules[0].Host + if gotHostname != expectHostname { + t.Errorf("host name is not up-to-date. expect: %s, got: %s", expectHostname, gotHostname) + } + + if v, ok := got.Annotations["blub"]; !ok || v != "blob" { + t.Error("annotations are not up-to-date. Missing entry or value is invalid.") + } + }) +} + +func TestDeleteIngresses(t *testing.T) { + t.Run("should delete excess ingress", func(t *testing.T) { + // create + ctx := context.Background() + + myParams, err := newParams("something:tag", test_file_ingress) + if err != nil { + t.Fatal(err) + } + myParams.Instance.Spec.Ingress.Type = "ingress" + + err = expectedIngresses(ctx, myParams, []networkingv1.Ingress{*desiredIngresses(ctx, myParams)}) + assert.NoError(t, err) + + nns := types.NamespacedName{Namespace: "default", Name: "test-ingress"} + exists, err := populateObjectIfExists(t, &networkingv1.Ingress{}, nns) + assert.NoError(t, err) + assert.True(t, exists) + + // delete + if err := deleteIngresses(ctx, params(), []networkingv1.Ingress{}); err != nil { + t.Error(err) + } + + // check + exists, err = populateObjectIfExists(t, &v1.Ingress{}, nns) + assert.NoError(t, err) + assert.False(t, exists) + }) +} + +func TestIngresses(t *testing.T) { + t.Run("wrong mode", func(t *testing.T) { + ctx := context.Background() + err := Ingresses(ctx, params()) + assert.Nil(t, err) + }) + + t.Run("supported mode and service exists", func(t *testing.T) { + ctx := context.Background() + myParams := params() + err := expectedServices(context.Background(), myParams, []corev1.Service{service("test-collector", params().Instance.Spec.Ports)}) + assert.NoError(t, err) + + assert.Nil(t, Ingresses(ctx, myParams)) + }) + +} diff --git a/pkg/collector/testdata/ingress_testdata.yaml b/pkg/collector/testdata/ingress_testdata.yaml new file mode 100644 index 0000000000..54f9342a76 --- /dev/null +++ b/pkg/collector/testdata/ingress_testdata.yaml @@ -0,0 +1,16 @@ +--- +receivers: + otlp: + protocols: + grpc: + endpoint: 0.0.0.0:12345 + otlp/test: + protocols: + grpc: + endpoint: 0.0.0.0:98765 + +service: + pipelines: + traces: + receivers: [otlp, otlp/test] + exporters: [nop] diff --git a/pkg/naming/main.go b/pkg/naming/main.go index f6b896212f..59b8029a8e 100644 --- a/pkg/naming/main.go +++ b/pkg/naming/main.go @@ -89,6 +89,11 @@ func Service(otelcol v1alpha1.OpenTelemetryCollector) string { return DNSName(Truncate("%s-collector", 63, otelcol.Name)) } +// Ingress builds the ingress name based on the instance. +func Ingress(otelcol v1alpha1.OpenTelemetryCollector) string { + return DNSName(Truncate("%s-ingress", 63, otelcol.Name)) +} + // TAService returns the name to use for the TargetAllocator service. func TAService(otelcol v1alpha1.OpenTelemetryCollector) string { return DNSName(Truncate("%s-targetallocator", 63, otelcol.Name)) diff --git a/tests/e2e/ingress/00-assert.yaml b/tests/e2e/ingress/00-assert.yaml new file mode 100644 index 0000000000..7d98dc4c40 --- /dev/null +++ b/tests/e2e/ingress/00-assert.yaml @@ -0,0 +1,47 @@ +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: simplest-collector +--- +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + annotations: + something.com: "true" + labels: + app.kubernetes.io/managed-by: opentelemetry-operator + app.kubernetes.io/name: simplest-ingress + name: simplest-ingress + ownerReferences: + - apiVersion: opentelemetry.io/v1alpha1 + blockOwnerDeletion: true + controller: true + kind: OpenTelemetryCollector + name: simplest +spec: + rules: + - host: example.com + http: + paths: + - backend: + service: + name: simplest-collector + port: + name: otlp-grpc + path: /otlp-grpc + pathType: Prefix + - backend: + service: + name: simplest-collector + port: + name: otlp-http + path: /otlp-http + pathType: Prefix + - backend: + service: + name: simplest-collector + port: + name: otlp-http-legac + path: /otlp-http-legacy + pathType: Prefix diff --git a/tests/e2e/ingress/00-install.yaml b/tests/e2e/ingress/00-install.yaml new file mode 100644 index 0000000000..9da41295dc --- /dev/null +++ b/tests/e2e/ingress/00-install.yaml @@ -0,0 +1,28 @@ +--- +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: simplest +spec: + mode: "deployment" + ingress: + type: ingress + hostname: "example.com" + annotations: + something.com: "true" + config: | + receivers: + otlp: + protocols: + grpc: + http: + + exporters: + logging: + + service: + pipelines: + traces: + receivers: [otlp] + processors: [] + exporters: [logging]