Skip to content

Commit 6c0b5e9

Browse files
jaronoff97jaronoff97
committed
working tests
1 parent 5a4e4e0 commit 6c0b5e9

18 files changed

+108
-75
lines changed

‎apis/v1alpha1/collector_webhook.go

+23-22
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ import (
3030
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
3131

3232
"github.com/open-telemetry/opentelemetry-operator/internal/config"
33+
ta "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters"
3334
"github.com/open-telemetry/opentelemetry-operator/internal/rbac"
35+
"github.com/open-telemetry/opentelemetry-operator/pkg/featuregate"
3436
)
3537

3638
var (
@@ -357,28 +359,27 @@ func (c CollectorWebhook) validateTargetAllocatorConfig(ctx context.Context, r *
357359
if r.Spec.TargetAllocator.AllocationStrategy == OpenTelemetryTargetAllocatorAllocationStrategyPerNode && r.Spec.Mode != ModeDaemonSet {
358360
return nil, fmt.Errorf("target allocation strategy %s is only supported in OpenTelemetry Collector mode %s", OpenTelemetryTargetAllocatorAllocationStrategyPerNode, ModeDaemonSet)
359361
}
360-
// TODO: Fix cycle
361-
//// validate Prometheus config for target allocation
362-
//promCfg, err := ta.ConfigToPromConfig(r.Spec.Config)
363-
//if err != nil {
364-
// return nil, fmt.Errorf("the OpenTelemetry Spec Prometheus configuration is incorrect, %w", err)
365-
//}
366-
//err = ta.ValidatePromConfig(promCfg, r.Spec.TargetAllocator.Enabled, featuregate.EnableTargetAllocatorRewrite.IsEnabled())
367-
//if err != nil {
368-
// return nil, fmt.Errorf("the OpenTelemetry Spec Prometheus configuration is incorrect, %w", err)
369-
//}
370-
//err = ta.ValidateTargetAllocatorConfig(r.Spec.TargetAllocator.PrometheusCR.Enabled, promCfg)
371-
//if err != nil {
372-
// return nil, fmt.Errorf("the OpenTelemetry Spec Prometheus configuration is incorrect, %w", err)
373-
//}
374-
//// if the prometheusCR is enabled, it needs a suite of permissions to function
375-
//if r.Spec.TargetAllocator.PrometheusCR.Enabled {
376-
// if subjectAccessReviews, err := c.reviewer.CheckPolicyRules(ctx, r.GetNamespace(), r.Spec.TargetAllocator.ServiceAccount, targetAllocatorCRPolicyRules...); err != nil {
377-
// return nil, fmt.Errorf("unable to check rbac rules %w", err)
378-
// } else if allowed, deniedReviews := rbac.AllSubjectAccessReviewsAllowed(subjectAccessReviews); !allowed {
379-
// return warningsGroupedByResource(deniedReviews), nil
380-
// }
381-
//}
362+
// validate Prometheus config for target allocation
363+
promCfg, err := ta.ConfigToPromConfig(r.Spec.Config)
364+
if err != nil {
365+
return nil, fmt.Errorf("the OpenTelemetry Spec Prometheus configuration is incorrect, %w", err)
366+
}
367+
err = ta.ValidatePromConfig(promCfg, r.Spec.TargetAllocator.Enabled, featuregate.EnableTargetAllocatorRewrite.IsEnabled())
368+
if err != nil {
369+
return nil, fmt.Errorf("the OpenTelemetry Spec Prometheus configuration is incorrect, %w", err)
370+
}
371+
err = ta.ValidateTargetAllocatorConfig(r.Spec.TargetAllocator.PrometheusCR.Enabled, promCfg)
372+
if err != nil {
373+
return nil, fmt.Errorf("the OpenTelemetry Spec Prometheus configuration is incorrect, %w", err)
374+
}
375+
// if the prometheusCR is enabled, it needs a suite of permissions to function
376+
if r.Spec.TargetAllocator.PrometheusCR.Enabled {
377+
if subjectAccessReviews, err := c.reviewer.CheckPolicyRules(ctx, r.GetNamespace(), r.Spec.TargetAllocator.ServiceAccount, targetAllocatorCRPolicyRules...); err != nil {
378+
return nil, fmt.Errorf("unable to check rbac rules %w", err)
379+
} else if allowed, deniedReviews := rbac.AllSubjectAccessReviewsAllowed(subjectAccessReviews); !allowed {
380+
return warningsGroupedByResource(deniedReviews), nil
381+
}
382+
}
382383

383384
return nil, nil
384385
}

‎apis/v1beta1/config.go

+20-1
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,30 @@ type Telemetry struct {
124124
type Service struct {
125125
Extensions []string `json:"extensions,omitempty" yaml:"extensions,omitempty"`
126126
// +kubebuilder:pruning:PreserveUnknownFields
127-
Telemetry *Telemetry `json:"telemetry,omitempty" yaml:"telemetry,omitempty"`
127+
Telemetry *AnyConfig `json:"telemetry,omitempty" yaml:"telemetry,omitempty"`
128128
// +kubebuilder:pruning:PreserveUnknownFields
129129
Pipelines AnyConfig `json:"pipelines" yaml:"pipelines"`
130130
}
131131

132+
// GetTelemetry serves as a helper function to access the fields we care about in the underlying telemetry struct.
133+
// This exists to avoid needing to worry extra fields in the telemetry struct.
134+
func (s *Service) GetTelemetry() *Telemetry {
135+
if s.Telemetry == nil {
136+
return nil
137+
}
138+
// Convert map to JSON bytes
139+
jsonData, err := json.Marshal(s.Telemetry)
140+
if err != nil {
141+
return nil
142+
}
143+
t := &Telemetry{}
144+
// Unmarshal JSON into the provided struct
145+
if err := json.Unmarshal(jsonData, t); err != nil {
146+
return nil
147+
}
148+
return t
149+
}
150+
132151
// Returns null objects in the config.
133152
func (c Config) nullObjects() []string {
134153
var nullKeys []string

‎apis/v1beta1/config_test.go

+5-3
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,11 @@ func TestConfigYaml(t *testing.T) {
140140
},
141141
Service: Service{
142142
Extensions: []string{"addon"},
143-
Telemetry: &Telemetry{
144-
Metrics: MetricsConfig{
145-
Address: "0.0.0.0:9000",
143+
Telemetry: &AnyConfig{
144+
Object: map[string]interface{}{
145+
"metrics": map[string]interface{}{
146+
"address": "0.0.0.0:9000",
147+
},
146148
},
147149
},
148150
Pipelines: AnyConfig{

‎apis/v1beta1/zz_generated.deepcopy.go

+1-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎internal/manifests/collector/adapters/config_to_ports.go

+10-7
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,15 @@ func (c ComponentType) String() string {
4343

4444
func PortsForExporters(l logr.Logger, c v1beta1.Config) ([]corev1.ServicePort, error) {
4545
compEnabled := getEnabledComponents(c.Service, ComponentTypeExporter)
46-
return componentPorts(l, c.Exporters, exporterParser.BuilderFor, compEnabled)
46+
return componentPorts(l, c.Exporters, exporterParser.For, compEnabled)
4747
}
4848

4949
func PortsForReceivers(l logr.Logger, c v1beta1.Config) ([]corev1.ServicePort, error) {
5050
compEnabled := getEnabledComponents(c.Service, ComponentTypeReceiver)
51-
return componentPorts(l, c.Receivers, receiverParser.BuilderFor, compEnabled)
51+
return componentPorts(l, c.Receivers, receiverParser.For, compEnabled)
5252
}
5353

54-
func componentPorts(l logr.Logger, c v1beta1.AnyConfig, p parser.BuilderFor, enabledComponents map[string]bool) ([]corev1.ServicePort, error) {
54+
func componentPorts(l logr.Logger, c v1beta1.AnyConfig, p parser.For, enabledComponents map[string]bool) ([]corev1.ServicePort, error) {
5555
var ports []corev1.ServicePort
5656
for cmptName, val := range c.Object {
5757
if !enabledComponents[cmptName] {
@@ -61,8 +61,11 @@ func componentPorts(l logr.Logger, c v1beta1.AnyConfig, p parser.BuilderFor, ena
6161
if !ok {
6262
component = map[string]interface{}{}
6363
}
64-
builder := p(cmptName)
65-
componentParser := builder(l, cmptName, component)
64+
componentParser, err := p(l, cmptName, component)
65+
if err != nil {
66+
l.V(2).Info("no parser found for '%s'", cmptName)
67+
continue
68+
}
6669
componentPorts, err := componentParser.Ports()
6770
if err != nil {
6871
l.Error(err, "parser for '%s' has returned an error: %w", cmptName, err)
@@ -101,11 +104,11 @@ func ConfigToPorts(logger logr.Logger, config v1beta1.Config) ([]corev1.ServiceP
101104

102105
// ConfigToMetricsPort gets the port number for the metrics endpoint from the collector config if it has been set.
103106
func ConfigToMetricsPort(config v1beta1.Service) (int32, error) {
104-
if config.Telemetry == nil {
107+
if config.GetTelemetry() == nil {
105108
// telemetry isn't set, use the default
106109
return 8888, nil
107110
}
108-
_, port, netErr := net.SplitHostPort(config.Telemetry.Metrics.Address)
111+
_, port, netErr := net.SplitHostPort(config.GetTelemetry().Metrics.Address)
109112
if netErr != nil && strings.Contains(netErr.Error(), "missing port in address") {
110113
return 8888, nil
111114
} else if netErr != nil {

‎internal/manifests/collector/adapters/config_to_ports_test.go

+16-10
Original file line numberDiff line numberDiff line change
@@ -185,9 +185,11 @@ func TestParserFailed(t *testing.T) {
185185
func TestConfigToMetricsPort(t *testing.T) {
186186
t.Run("custom port specified", func(t *testing.T) {
187187
config := v1beta1.Service{
188-
Telemetry: &v1beta1.Telemetry{
189-
Metrics: v1beta1.MetricsConfig{
190-
Address: "0.0.0.0:9090",
188+
Telemetry: &v1beta1.AnyConfig{
189+
Object: map[string]interface{}{
190+
"metrics": map[string]interface{}{
191+
"address": "0.0.0.0:9090",
192+
},
191193
},
192194
},
193195
}
@@ -204,27 +206,31 @@ func TestConfigToMetricsPort(t *testing.T) {
204206
{
205207
"bad address",
206208
v1beta1.Service{
207-
Telemetry: &v1beta1.Telemetry{
208-
Metrics: v1beta1.MetricsConfig{
209-
Address: "0.0.0.0",
209+
Telemetry: &v1beta1.AnyConfig{
210+
Object: map[string]interface{}{
211+
"metrics": map[string]interface{}{
212+
"address": "0.0.0.0",
213+
},
210214
},
211215
},
212216
},
213217
},
214218
{
215219
"missing address",
216220
v1beta1.Service{
217-
Telemetry: &v1beta1.Telemetry{
218-
Metrics: v1beta1.MetricsConfig{
219-
Level: "detailed",
221+
Telemetry: &v1beta1.AnyConfig{
222+
Object: map[string]interface{}{
223+
"metrics": map[string]interface{}{
224+
"level": "detailed",
225+
},
220226
},
221227
},
222228
},
223229
},
224230
{
225231
"missing metrics",
226232
v1beta1.Service{
227-
Telemetry: &v1beta1.Telemetry{},
233+
Telemetry: &v1beta1.AnyConfig{},
228234
},
229235
},
230236
{

‎internal/manifests/collector/ingress.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424

2525
"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
2626
"github.com/open-telemetry/opentelemetry-operator/internal/manifests"
27+
"github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters"
2728
"github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils"
2829
"github.com/open-telemetry/opentelemetry-operator/internal/naming"
2930
)
@@ -135,7 +136,7 @@ func createSubdomainIngressRules(otelcol string, hostname string, ports []corev1
135136
}
136137

137138
func servicePortsFromCfg(logger logr.Logger, otelcol v1beta1.OpenTelemetryCollector) ([]corev1.ServicePort, error) {
138-
ports, err := otelcol.Spec.Config.Receivers.Ports(logger)
139+
ports, err := adapters.PortsForReceivers(logger, otelcol.Spec.Config)
139140
if err != nil {
140141
logger.Error(err, "couldn't build the ingress for this instance")
141142
return nil, err

‎internal/manifests/collector/parser/exporter/exporter_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func TestPorts(t *testing.T) {
3333
testName: "Valid Configuration",
3434
parser: &PrometheusExporterParser{
3535
name: "test-exporter",
36-
config: map[interface{}]interface{}{
36+
config: map[string]interface{}{
3737
"endpoint": "http://myprometheus.io:9090",
3838
},
3939
},
@@ -63,7 +63,7 @@ func TestPorts(t *testing.T) {
6363
testName: "Invalid Endpoint No Port",
6464
parser: &PrometheusExporterParser{
6565
name: "test-exporter",
66-
config: map[interface{}]interface{}{
66+
config: map[string]interface{}{
6767
"endpoint": "invalidendpoint",
6868
},
6969
},

‎internal/manifests/collector/parser/parser.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -30,5 +30,5 @@ type ComponentPortParser interface {
3030
// Builder specifies the signature required for parser builders.
3131
type Builder func(logr.Logger, string, map[string]interface{}) ComponentPortParser
3232

33-
// BuilderFor returns a builder for a given component
34-
type BuilderFor func(component string) Builder
33+
// For returns a builder for a given component if found, otherwise it returns an error
34+
type For func(logger logr.Logger, name string, config map[string]interface{}) (ComponentPortParser, error)

‎internal/manifests/collector/parser/receiver/receiver_syslog.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,13 @@ const parserNameSyslog = "__syslog"
3030

3131
// SyslogReceiverParser parses the configuration for TCP log receivers.
3232
type SyslogReceiverParser struct {
33-
config map[interface{}]interface{}
33+
config map[string]interface{}
3434
logger logr.Logger
3535
name string
3636
}
3737

3838
// NewSyslogReceiverParser builds a new parser for TCP log receivers.
39-
func NewSyslogReceiverParser(logger logr.Logger, name string, config map[interface{}]interface{}) parser.ComponentPortParser {
39+
func NewSyslogReceiverParser(logger logr.Logger, name string, config map[string]interface{}) parser.ComponentPortParser {
4040
return &SyslogReceiverParser{
4141
logger: logger,
4242
name: name,
@@ -48,19 +48,19 @@ func (o *SyslogReceiverParser) Ports() ([]corev1.ServicePort, error) {
4848
var endpoint interface{}
4949
var endpointName string
5050
var protocol corev1.Protocol
51-
var c map[interface{}]interface{}
51+
var c map[string]interface{}
5252

5353
// syslog receiver contains the endpoint
5454
// that needs to be exposed one level down inside config
5555
// i.e. either in tcp or udp section with field key
5656
// as `listen_address`
5757
if tcp, isTCP := o.config["tcp"]; isTCP && tcp != nil {
58-
c = tcp.(map[interface{}]interface{})
58+
c = tcp.(map[string]interface{})
5959
endpointName = "tcp"
6060
endpoint = getAddressFromConfig(o.logger, o.name, listenAddressKey, c)
6161
protocol = corev1.ProtocolTCP
6262
} else if udp, isUDP := o.config["udp"]; isUDP && udp != nil {
63-
c = udp.(map[interface{}]interface{})
63+
c = udp.(map[string]interface{})
6464
endpointName = "udp"
6565
endpoint = getAddressFromConfig(o.logger, o.name, listenAddressKey, c)
6666
protocol = corev1.ProtocolUDP

‎internal/manifests/collector/parser/receiver/receiver_syslog_test.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func TestSyslogSelfRegisters(t *testing.T) {
2828

2929
func TestSyslogIsFoundByName(t *testing.T) {
3030
// test
31-
p, err := For(logger, "syslog", map[interface{}]interface{}{})
31+
p, err := For(logger, "syslog", map[string]interface{}{})
3232
assert.NoError(t, err)
3333

3434
// verify
@@ -38,15 +38,15 @@ func TestSyslogIsFoundByName(t *testing.T) {
3838
func TestSyslogConfiguration(t *testing.T) {
3939
for _, tt := range []struct {
4040
desc string
41-
config map[interface{}]interface{}
41+
config map[string]interface{}
4242
expected []corev1.ServicePort
4343
}{
44-
{"Empty configuration", map[interface{}]interface{}{}, []corev1.ServicePort{}},
44+
{"Empty configuration", map[string]interface{}{}, []corev1.ServicePort{}},
4545
{"UDP port configuration",
46-
map[interface{}]interface{}{"udp": map[interface{}]interface{}{"listen_address": "0.0.0.0:1234"}},
46+
map[string]interface{}{"udp": map[string]interface{}{"listen_address": "0.0.0.0:1234"}},
4747
[]corev1.ServicePort{{Name: "syslog", Port: 1234, Protocol: corev1.ProtocolUDP}}},
4848
{"TCP port configuration",
49-
map[interface{}]interface{}{"tcp": map[interface{}]interface{}{"listen_address": "0.0.0.0:1234"}},
49+
map[string]interface{}{"tcp": map[string]interface{}{"listen_address": "0.0.0.0:1234"}},
5050
[]corev1.ServicePort{{Name: "syslog", Port: 1234, Protocol: corev1.ProtocolTCP}}},
5151
} {
5252
t.Run(tt.desc, func(t *testing.T) {

‎internal/manifests/collector/parser/receiver/receiver_tcplog.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,13 @@ const parserNameTcpLog = "__tcplog"
3030

3131
// TcpLogReceiverParser parses the configuration for TCP log receivers.
3232
type TcpLogReceiverParser struct {
33-
config map[interface{}]interface{}
33+
config map[string]interface{}
3434
logger logr.Logger
3535
name string
3636
}
3737

3838
// NewTcpLogReceiverParser builds a new parser for TCP log receivers.
39-
func NewTcpLogReceiverParser(logger logr.Logger, name string, config map[interface{}]interface{}) parser.ComponentPortParser {
39+
func NewTcpLogReceiverParser(logger logr.Logger, name string, config map[string]interface{}) parser.ComponentPortParser {
4040
return &TcpLogReceiverParser{
4141
logger: logger,
4242
name: name,

‎internal/manifests/collector/parser/receiver/receiver_tcplog_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func TestTcpLogSelfRegisters(t *testing.T) {
2828

2929
func TestTcpLogIsFoundByName(t *testing.T) {
3030
// test
31-
p, err := For(logger, "tcplog", map[interface{}]interface{}{})
31+
p, err := For(logger, "tcplog", map[string]interface{}{})
3232
assert.NoError(t, err)
3333

3434
// verify
@@ -38,12 +38,12 @@ func TestTcpLogIsFoundByName(t *testing.T) {
3838
func TestTcpLogConfiguration(t *testing.T) {
3939
for _, tt := range []struct {
4040
desc string
41-
config map[interface{}]interface{}
41+
config map[string]interface{}
4242
expected []corev1.ServicePort
4343
}{
44-
{"Empty configuration", map[interface{}]interface{}{}, []corev1.ServicePort{}},
44+
{"Empty configuration", map[string]interface{}{}, []corev1.ServicePort{}},
4545
{"TCP port configuration",
46-
map[interface{}]interface{}{"listen_address": "0.0.0.0:1234"},
46+
map[string]interface{}{"listen_address": "0.0.0.0:1234"},
4747
[]corev1.ServicePort{{Name: "tcplog", Port: 1234, Protocol: corev1.ProtocolTCP}}},
4848
} {
4949
t.Run(tt.desc, func(t *testing.T) {

‎internal/manifests/collector/parser/receiver/receiver_udplog.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,13 @@ const parserNameUdpLog = "__udplog"
3030

3131
// UdpLogReceiverParser parses the configuration for UDP log receivers.
3232
type UdpLogReceiverParser struct {
33-
config map[interface{}]interface{}
33+
config map[string]interface{}
3434
logger logr.Logger
3535
name string
3636
}
3737

3838
// NewUdpLogReceiverParser builds a new parser for UDP log receivers.
39-
func NewUdpLogReceiverParser(logger logr.Logger, name string, config map[interface{}]interface{}) parser.ComponentPortParser {
39+
func NewUdpLogReceiverParser(logger logr.Logger, name string, config map[string]interface{}) parser.ComponentPortParser {
4040
return &UdpLogReceiverParser{
4141
logger: logger,
4242
name: name,

0 commit comments

Comments
 (0)