Skip to content

Commit c8c5d47

Browse files
jaronoff97jaronoff97
committed
Change to use the builder pattern instead of the option pattern
1 parent 5c498fc commit c8c5d47

14 files changed

+495
-295
lines changed

Diff for: ‎internal/components/builder.go

+44-1
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,55 @@ import (
1919

2020
corev1 "k8s.io/api/core/v1"
2121
"k8s.io/apimachinery/pkg/util/intstr"
22+
23+
"github.com/open-telemetry/opentelemetry-operator/internal/naming"
2224
)
2325

26+
type ParserOption[T any] func(*Option[T])
27+
28+
type Option[T any] struct {
29+
protocol corev1.Protocol
30+
appProtocol *string
31+
targetPort intstr.IntOrString
32+
nodePort int32
33+
name string
34+
port int32
35+
portParser PortParser[T]
36+
rbacGen RBACRuleGenerator[T]
37+
}
38+
39+
func NewEmptyOption[T any]() *Option[T] {
40+
return &Option[T]{}
41+
}
42+
43+
func NewOption[T any](name string, port int32) *Option[T] {
44+
return &Option[T]{
45+
name: name,
46+
port: port,
47+
}
48+
}
49+
50+
func (o *Option[T]) Apply(opts ...ParserOption[T]) {
51+
for _, opt := range opts {
52+
opt(o)
53+
}
54+
}
55+
56+
func (o *Option[T]) GetServicePort() *corev1.ServicePort {
57+
return &corev1.ServicePort{
58+
Name: naming.PortName(o.name, o.port),
59+
Port: o.port,
60+
Protocol: o.protocol,
61+
AppProtocol: o.appProtocol,
62+
TargetPort: o.targetPort,
63+
NodePort: o.nodePort,
64+
}
65+
}
66+
2467
type Builder[T any] []ParserOption[T]
2568

2669
func NewBuilder[T any]() Builder[T] {
27-
return make([]ParserOption[T], 8)
70+
return []ParserOption[T]{}
2871
}
2972

3073
func (b Builder[T]) WithProtocol(protocol corev1.Protocol) Builder[T] {

Diff for: ‎internal/components/builder_test.go

+156
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
// Copyright The OpenTelemetry Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package components_test
16+
17+
import (
18+
"fmt"
19+
"testing"
20+
21+
"github.com/go-logr/logr"
22+
"github.com/stretchr/testify/assert"
23+
corev1 "k8s.io/api/core/v1"
24+
rbacv1 "k8s.io/api/rbac/v1"
25+
26+
"github.com/open-telemetry/opentelemetry-operator/internal/components"
27+
)
28+
29+
func TestBuilder_Build(t *testing.T) {
30+
type sampleConfig struct {
31+
example string
32+
number int
33+
m map[string]interface{}
34+
}
35+
type want struct {
36+
name string
37+
ports []corev1.ServicePort
38+
rules []rbacv1.PolicyRule
39+
}
40+
type fields[T any] struct {
41+
b components.Builder[T]
42+
}
43+
type params struct {
44+
conf interface{}
45+
}
46+
type testCase[T any] struct {
47+
name string
48+
fields fields[T]
49+
params params
50+
want want
51+
wantErr assert.ErrorAssertionFunc
52+
}
53+
examplePortParser := func(logger logr.Logger, name string, defaultPort *corev1.ServicePort, config sampleConfig) ([]corev1.ServicePort, error) {
54+
if defaultPort != nil {
55+
return []corev1.ServicePort{*defaultPort}, nil
56+
}
57+
return nil, nil
58+
}
59+
tests := []testCase[sampleConfig]{
60+
{
61+
name: "basic valid configuration",
62+
fields: fields[sampleConfig]{
63+
b: components.NewBuilder[sampleConfig]().
64+
WithPortParser(examplePortParser).
65+
WithName("test-service").
66+
WithPort(80).
67+
WithNodePort(80).
68+
WithProtocol(corev1.ProtocolTCP),
69+
},
70+
params: params{
71+
conf: sampleConfig{},
72+
},
73+
want: want{
74+
name: "__test-service",
75+
ports: []corev1.ServicePort{
76+
{
77+
Name: "test-service",
78+
Port: 80,
79+
NodePort: 80,
80+
Protocol: corev1.ProtocolTCP,
81+
},
82+
},
83+
rules: nil,
84+
},
85+
wantErr: assert.NoError,
86+
},
87+
{
88+
name: "missing name",
89+
fields: fields[sampleConfig]{
90+
b: components.NewBuilder[sampleConfig]().
91+
WithPort(8080).
92+
WithProtocol(corev1.ProtocolUDP),
93+
},
94+
params: params{
95+
conf: sampleConfig{},
96+
},
97+
want: want{},
98+
wantErr: assert.Error,
99+
},
100+
{
101+
name: "complete configuration with RBAC rules",
102+
fields: fields[sampleConfig]{
103+
b: components.NewBuilder[sampleConfig]().
104+
WithName("secure-service").
105+
WithPort(443).
106+
WithProtocol(corev1.ProtocolTCP).
107+
WithRbacGen(func(logger logr.Logger, config sampleConfig) ([]rbacv1.PolicyRule, error) {
108+
return []rbacv1.PolicyRule{
109+
{
110+
APIGroups: []string{""},
111+
Resources: []string{"pods"},
112+
Verbs: []string{"get", "list"},
113+
},
114+
}, nil
115+
}),
116+
},
117+
params: params{
118+
conf: sampleConfig{},
119+
},
120+
want: want{
121+
name: "__secure-service",
122+
ports: nil,
123+
rules: []rbacv1.PolicyRule{
124+
{
125+
APIGroups: []string{""},
126+
Resources: []string{"pods"},
127+
Verbs: []string{"get", "list"},
128+
},
129+
},
130+
},
131+
wantErr: assert.NoError,
132+
},
133+
}
134+
for _, tt := range tests {
135+
t.Run(tt.name, func(t *testing.T) {
136+
got, err := tt.fields.b.Build()
137+
if tt.wantErr(t, err, fmt.Sprintf("WantErr()")) && err != nil {
138+
return
139+
}
140+
assert.Equalf(t, tt.want.name, got.ParserName(), "ParserName()")
141+
ports, err := got.Ports(logr.Discard(), got.ParserType(), tt.params.conf)
142+
assert.NoError(t, err)
143+
assert.Equalf(t, tt.want.ports, ports, "Ports()")
144+
rules, err := got.GetRBACRules(logr.Discard(), tt.params.conf)
145+
assert.NoError(t, err)
146+
assert.Equalf(t, tt.want.rules, rules, "GetRBACRules()")
147+
})
148+
}
149+
}
150+
151+
func TestMustBuildPanics(t *testing.T) {
152+
b := components.Builder[*components.SingleEndpointConfig]{}
153+
assert.Panics(t, func() {
154+
b.MustBuild()
155+
})
156+
}

Diff for: ‎internal/components/component.go

-72
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@ import (
2424
corev1 "k8s.io/api/core/v1"
2525
rbacv1 "k8s.io/api/rbac/v1"
2626
"k8s.io/apimachinery/pkg/util/intstr"
27-
28-
"github.com/open-telemetry/opentelemetry-operator/internal/naming"
2927
)
3028

3129
var (
@@ -46,76 +44,6 @@ type PortParser[T any] func(logger logr.Logger, name string, defaultPort *corev1
4644
// RBACRuleGenerator is a function that generates a list of RBAC Rules given a configuration of type T
4745
// It's expected that type T is the configuration used by a parser.
4846
type RBACRuleGenerator[T any] func(logger logr.Logger, config T) ([]rbacv1.PolicyRule, error)
49-
type ParserOption[T any] func(*Option[T])
50-
51-
type Option[T any] struct {
52-
protocol corev1.Protocol
53-
appProtocol *string
54-
targetPort intstr.IntOrString
55-
nodePort int32
56-
name string
57-
port int32
58-
portParser PortParser[T]
59-
rbacGen RBACRuleGenerator[T]
60-
}
61-
62-
func NewEmptyOption[T any]() *Option[T] {
63-
return &Option[T]{}
64-
}
65-
66-
func NewOption[T any](name string, port int32) *Option[T] {
67-
return &Option[T]{
68-
name: name,
69-
port: port,
70-
}
71-
}
72-
73-
func (o *Option[T]) Apply(opts ...ParserOption[T]) {
74-
for _, opt := range opts {
75-
opt(o)
76-
}
77-
}
78-
79-
func (o *Option[T]) GetServicePort() *corev1.ServicePort {
80-
return &corev1.ServicePort{
81-
Name: naming.PortName(o.name, o.port),
82-
Port: o.port,
83-
Protocol: o.protocol,
84-
AppProtocol: o.appProtocol,
85-
TargetPort: o.targetPort,
86-
NodePort: o.nodePort,
87-
}
88-
}
89-
90-
func WithRBACRuleGenerator[T any](r RBACRuleGenerator[T]) ParserOption[T] {
91-
return func(opt *Option[T]) {
92-
opt.rbacGen = r
93-
}
94-
}
95-
96-
func WithPortParser[T any](p PortParser[T]) ParserOption[T] {
97-
return func(opt *Option[T]) {
98-
opt.portParser = p
99-
}
100-
}
101-
102-
func WithTargetPort[T any](targetPort int32) ParserOption[T] {
103-
return func(opt *Option[T]) {
104-
opt.targetPort = intstr.FromInt32(targetPort)
105-
}
106-
}
107-
108-
func WithAppProtocol[T any](proto *string) ParserOption[T] {
109-
return func(opt *Option[T]) {
110-
opt.appProtocol = proto
111-
}
112-
}
113-
114-
func WithProtocol[T any](proto corev1.Protocol) ParserOption[T] {
115-
return func(opt *Option[T]) {
116-
opt.protocol = proto
117-
}
118-
}
11947

12048
// ComponentType returns the type for a given component name.
12149
// components have a name like:

Diff for: ‎internal/components/exporters/helpers.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func ParserFor(name string) components.Parser {
4343

4444
var (
4545
componentParsers = []components.Parser{
46-
components.NewSinglePortParser("prometheus", 8888),
46+
components.NewSinglePortParserBuilder("prometheus", 8888).MustBuild(),
4747
}
4848
)
4949

Diff for: ‎internal/components/exporters/helpers_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func TestParserForReturns(t *testing.T) {
3939

4040
func TestCanRegister(t *testing.T) {
4141
const testComponentName = "test"
42-
exporters.Register(testComponentName, components.NewSinglePortParser(testComponentName, 9000))
42+
exporters.Register(testComponentName, components.NewSinglePortParserBuilder(testComponentName, 9000).MustBuild())
4343
assert.True(t, exporters.IsRegistered(testComponentName))
4444
parser := exporters.ParserFor(testComponentName)
4545
assert.Equal(t, "test", parser.ParserType())

Diff for: ‎internal/components/generic_parser.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,7 @@ func (g *GenericParser[T]) ParserName() string {
6666
return fmt.Sprintf("__%s", g.name)
6767
}
6868

69-
func NewGenericParser[T any](name string, port int32, opts ...ParserOption[T]) *GenericParser[T] {
69+
func NewGenericParser[T any](name string, port int32) *GenericParser[T] {
7070
o := NewOption[T](name, port)
71-
o.Apply(opts...)
7271
return &GenericParser[T]{name: name, portParser: o.portParser, rbacGen: o.rbacGen, option: o}
7372
}

0 commit comments

Comments
 (0)