From 3bb5fdffabfb82ae1bce8c0ae37397f01ba97b2d Mon Sep 17 00:00:00 2001 From: Alessio Trivisonno Date: Wed, 21 Feb 2024 08:37:11 +0100 Subject: [PATCH 1/6] fix: exporter panic on malformed endpoint configuration --- .../collector/parser/exporter/exporter_prometheus.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/manifests/collector/parser/exporter/exporter_prometheus.go b/internal/manifests/collector/parser/exporter/exporter_prometheus.go index 61d58588da..30047de70e 100644 --- a/internal/manifests/collector/parser/exporter/exporter_prometheus.go +++ b/internal/manifests/collector/parser/exporter/exporter_prometheus.go @@ -59,9 +59,9 @@ func (o *PrometheusExporterParser) Ports() ([]corev1.ServicePort, error) { }, ) } else { - ports = append( - ports, *singlePortFromConfigEndpoint(o.logger, o.name, o.config), - ) + if port := singlePortFromConfigEndpoint(o.logger, o.name, o.config); port != nil { + ports = append(ports, *port) + } } return ports, nil From ab899d4890ed34263e4398ff7c1a55ad4a8c47ef Mon Sep 17 00:00:00 2001 From: Alessio Trivisonno Date: Wed, 21 Feb 2024 10:08:18 +0100 Subject: [PATCH 2/6] feat: adds tests for Ports in exporter.go --- .../parser/exporter/exporter_test.go | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 internal/manifests/collector/parser/exporter/exporter_test.go diff --git a/internal/manifests/collector/parser/exporter/exporter_test.go b/internal/manifests/collector/parser/exporter/exporter_test.go new file mode 100644 index 0000000000..8267c6de82 --- /dev/null +++ b/internal/manifests/collector/parser/exporter/exporter_test.go @@ -0,0 +1,66 @@ +package exporter + +import ( + "reflect" + "testing" + + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +func TestPorts(t *testing.T) { + tests := []struct { + testName string + parser *PrometheusExporterParser + want []v1.ServicePort + }{ + { + testName: "Valid Configuration", + parser: &PrometheusExporterParser{ + name: "test-exporter", + config: map[interface{}]interface{}{ + "endpoint": "http://myprometheus.io:9090", + }, + }, + want: []v1.ServicePort{ + { + Name: "test-exporter", + Port: 9091, + }, + }, + }, + { + testName: "Empty Configuration", + parser: &PrometheusExporterParser{ + name: "test-exporter", + config: nil, // Simulate no configuration provided + }, + want: []v1.ServicePort{ + { + Name: "test-exporter", + Port: defaultPrometheusPort, + TargetPort: intstr.FromInt(int(defaultPrometheusPort)), + Protocol: v1.ProtocolTCP, + }, + }, + }, + { + testName: "Invalid Endpoint No Port", + parser: &PrometheusExporterParser{ + name: "test-exporter", + config: map[interface{}]interface{}{ + "endpoint": "invalidendpoint", + }, + }, + want: []v1.ServicePort{}, + }, + } + + for _, tt := range tests { + t.Run(tt.testName, func(t *testing.T) { + if got, _ := tt.parser.Ports(); !reflect.DeepEqual(got, tt.want) { + t.Errorf("Ports(%v, = %v, want %v", tt.parser, got, tt.want) + } + }) + } +} From 2da532bb44feaedcbbde94c2e03e22f04daa4d86 Mon Sep 17 00:00:00 2001 From: Alessio Trivisonno Date: Wed, 21 Feb 2024 16:16:16 +0100 Subject: [PATCH 3/6] add chloggen file --- .chloggen/fix-exporter-conf-panic.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 .chloggen/fix-exporter-conf-panic.yaml diff --git a/.chloggen/fix-exporter-conf-panic.yaml b/.chloggen/fix-exporter-conf-panic.yaml new file mode 100644 index 0000000000..55dd24ee3e --- /dev/null +++ b/.chloggen/fix-exporter-conf-panic.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: "bug_fix" + +# 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: "Fixes a panic on exporter prometheus endpoint not valid" + +# One or more tracking issues related to the change +issues: [2628] + +# (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: From 2feb5bae2a1271877ce27706b50aceb69158206a Mon Sep 17 00:00:00 2001 From: Alessio Trivisonno Date: Wed, 21 Feb 2024 16:16:33 +0100 Subject: [PATCH 4/6] adds header + fix test --- .../collector/parser/exporter/exporter_test.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/internal/manifests/collector/parser/exporter/exporter_test.go b/internal/manifests/collector/parser/exporter/exporter_test.go index 8267c6de82..a000540586 100644 --- a/internal/manifests/collector/parser/exporter/exporter_test.go +++ b/internal/manifests/collector/parser/exporter/exporter_test.go @@ -1,3 +1,17 @@ +// 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 exporter import ( @@ -25,7 +39,7 @@ func TestPorts(t *testing.T) { want: []v1.ServicePort{ { Name: "test-exporter", - Port: 9091, + Port: 9090, }, }, }, From 1cda5fd7766c38e9bbedfad1ab9fd87836431994 Mon Sep 17 00:00:00 2001 From: Alessio Trivisonno Date: Wed, 21 Feb 2024 20:02:49 +0100 Subject: [PATCH 5/6] Update internal/manifests/collector/parser/exporter/exporter_test.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mikołaj Świątek --- internal/manifests/collector/parser/exporter/exporter_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/manifests/collector/parser/exporter/exporter_test.go b/internal/manifests/collector/parser/exporter/exporter_test.go index a000540586..e27ead4c31 100644 --- a/internal/manifests/collector/parser/exporter/exporter_test.go +++ b/internal/manifests/collector/parser/exporter/exporter_test.go @@ -72,9 +72,7 @@ func TestPorts(t *testing.T) { for _, tt := range tests { t.Run(tt.testName, func(t *testing.T) { - if got, _ := tt.parser.Ports(); !reflect.DeepEqual(got, tt.want) { - t.Errorf("Ports(%v, = %v, want %v", tt.parser, got, tt.want) - } + assert.Equal(t, tt.want, tt.parser.Ports()) }) } } From c8fe544b587a5c12bb9db5de6ce757f3247f7132 Mon Sep 17 00:00:00 2001 From: Alessio Trivisonno Date: Thu, 22 Feb 2024 07:24:02 +0100 Subject: [PATCH 6/6] fix: test exporter_test.go --- .../manifests/collector/parser/exporter/exporter_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/manifests/collector/parser/exporter/exporter_test.go b/internal/manifests/collector/parser/exporter/exporter_test.go index e27ead4c31..e022160784 100644 --- a/internal/manifests/collector/parser/exporter/exporter_test.go +++ b/internal/manifests/collector/parser/exporter/exporter_test.go @@ -15,9 +15,10 @@ package exporter import ( - "reflect" "testing" + "github.com/stretchr/testify/assert" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/intstr" ) @@ -72,7 +73,8 @@ func TestPorts(t *testing.T) { for _, tt := range tests { t.Run(tt.testName, func(t *testing.T) { - assert.Equal(t, tt.want, tt.parser.Ports()) + ports, _ := tt.parser.Ports() + assert.Equal(t, tt.want, ports) }) } }