Skip to content

Commit 12ee67e

Browse files
pepovtarokkk
pepov
authored and
tarokkk
committed
add errors for invalid configurations and cover it with tests
1 parent 74186e3 commit 12ee67e

File tree

3 files changed

+259
-38
lines changed

3 files changed

+259
-38
lines changed

‎controllers/logging_controller_match_test.go‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
// Copyright © 2020 Banzai Cloud
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 controllers_test
16+
17+
import (
18+
"fmt"
19+
"testing"
20+
21+
"github.com/banzaicloud/logging-operator/pkg/sdk/api/v1beta1"
22+
"github.com/banzaicloud/operator-tools/pkg/utils"
23+
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
24+
)
25+
26+
func TestInvalidFlowIfMatchAndSelectorBothSet(t *testing.T) {
27+
defer beforeEach(t)()
28+
29+
logging := testLogging()
30+
output := testOutput()
31+
32+
flow := &v1beta1.Flow{
33+
ObjectMeta: v1.ObjectMeta{
34+
Name: "test-flow",
35+
Namespace: output.Namespace,
36+
},
37+
Spec: v1beta1.FlowSpec{
38+
Selectors: map[string]string{
39+
"a": "b",
40+
},
41+
Match: []v1beta1.Match{
42+
{
43+
Select: &v1beta1.Select{
44+
Labels: map[string]string{
45+
"c": "d",
46+
},
47+
},
48+
},
49+
},
50+
OutputRefs: []string{output.Name},
51+
},
52+
}
53+
54+
defer ensureCreated(t, logging)()
55+
defer ensureCreated(t, output)()
56+
defer ensureCreated(t, flow)()
57+
58+
expected := fmt.Sprintf("failed to create model: match and selectors cannot be defined simultaneously for flow %s",
59+
utils.ObjectKeyFromObjectMeta(flow).String(),
60+
)
61+
62+
expectError(t, expected)
63+
}
64+
65+
func TestInvalidFlowIfSelectorAndExcludeBothSet(t *testing.T) {
66+
defer beforeEach(t)()
67+
68+
logging := testLogging()
69+
output := testOutput()
70+
71+
flow := &v1beta1.Flow{
72+
ObjectMeta: v1.ObjectMeta{
73+
Name: "test-flow",
74+
Namespace: output.Namespace,
75+
},
76+
Spec: v1beta1.FlowSpec{
77+
Match: []v1beta1.Match{
78+
{
79+
Select: &v1beta1.Select{
80+
Labels: map[string]string{
81+
"c": "d",
82+
},
83+
},
84+
Exclude: &v1beta1.Exclude{
85+
Labels: map[string]string{
86+
"c": "d",
87+
},
88+
},
89+
},
90+
},
91+
OutputRefs: []string{output.Name},
92+
},
93+
}
94+
95+
defer ensureCreated(t, logging)()
96+
defer ensureCreated(t, output)()
97+
defer ensureCreated(t, flow)()
98+
99+
expected := fmt.Sprintf("failed to create model: failed to process match for %s: select and exclude cannot be set simultaneously",
100+
utils.ObjectKeyFromObjectMeta(flow).String(),
101+
)
102+
103+
expectError(t, expected)
104+
}
105+
106+
func TestInvalidClusterFlowIfSelectorAndExcludeBothSet(t *testing.T) {
107+
defer beforeEach(t)()
108+
109+
logging := testLogging()
110+
output := testOutput()
111+
112+
flow := &v1beta1.ClusterFlow{
113+
ObjectMeta: v1.ObjectMeta{
114+
Name: "test-flow",
115+
Namespace: logging.Spec.ControlNamespace,
116+
},
117+
Spec: v1beta1.ClusterFlowSpec{
118+
Match: []v1beta1.ClusterMatch{
119+
{
120+
ClusterSelect: &v1beta1.ClusterSelect{
121+
Labels: map[string]string{
122+
"c": "d",
123+
},
124+
},
125+
ClusterExclude: &v1beta1.ClusterExclude{
126+
Labels: map[string]string{
127+
"c": "d",
128+
},
129+
},
130+
},
131+
},
132+
OutputRefs: []string{output.Name},
133+
},
134+
}
135+
136+
defer ensureCreated(t, logging)()
137+
defer ensureCreated(t, output)()
138+
defer ensureCreated(t, flow)()
139+
140+
expected := fmt.Sprintf("failed to create model: failed to process match for %s: select and exclude cannot be set simultaneously",
141+
utils.ObjectKeyFromObjectMeta(flow).String(),
142+
)
143+
144+
expectError(t, expected)
145+
}
146+
147+
func TestInvalidClusterFlowIfMatchAndSelectorBothSet(t *testing.T) {
148+
defer beforeEach(t)()
149+
150+
logging := testLogging()
151+
output := testOutput()
152+
153+
flow := &v1beta1.ClusterFlow{
154+
ObjectMeta: v1.ObjectMeta{
155+
Name: "test-flow",
156+
Namespace: logging.Spec.ControlNamespace,
157+
},
158+
Spec: v1beta1.ClusterFlowSpec{
159+
Selectors: map[string]string{
160+
"a": "b",
161+
},
162+
Match: []v1beta1.ClusterMatch{
163+
{
164+
ClusterSelect: &v1beta1.ClusterSelect{
165+
Labels: map[string]string{
166+
"c": "d",
167+
},
168+
},
169+
},
170+
},
171+
OutputRefs: []string{output.Name},
172+
},
173+
}
174+
175+
defer ensureCreated(t, logging)()
176+
defer ensureCreated(t, output)()
177+
defer ensureCreated(t, flow)()
178+
179+
expected := fmt.Sprintf("failed to create model: match and selectors cannot be defined simultaneously for clusterflow %s",
180+
utils.ObjectKeyFromObjectMeta(flow).String(),
181+
)
182+
183+
expectError(t, expected)
184+
}

‎controllers/logging_controller_test.go‎

Lines changed: 49 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -306,23 +306,7 @@ func TestClusterFlowWithNamespacedOutput(t *testing.T) {
306306
defer ensureCreated(t, output)()
307307
defer ensureCreated(t, flow)()
308308

309-
err := wait.Poll(time.Second, time.Second*3, func() (bool, error) {
310-
select {
311-
case err := <-reconcilerErrors:
312-
expected := "referenced output not found: test-output"
313-
if !strings.Contains(err.Error(), expected) {
314-
return false, errors.Errorf("expected `%s` but received `%s`", expected, err.Error())
315-
} else {
316-
return true, nil
317-
}
318-
case <-time.After(100 * time.Millisecond):
319-
return false, nil
320-
}
321-
})
322-
323-
if err != nil {
324-
t.Fatalf("%+v", err)
325-
}
309+
expectError(t, "referenced output not found: test-output")
326310
}
327311

328312
func TestSingleFlowWithOutputRef(t *testing.T) {
@@ -408,23 +392,8 @@ func TestSingleFlowDefaultLoggingRefInvalidOutputRef(t *testing.T) {
408392
defer ensureCreated(t, logging)()
409393
defer ensureCreated(t, flow)()
410394

411-
err := wait.Poll(time.Second, time.Second*3, func() (bool, error) {
412-
select {
413-
case err := <-reconcilerErrors:
414-
expected := "referenced output not found: test-output-nonexistent"
415-
if !strings.Contains(err.Error(), expected) {
416-
return false, errors.Errorf("expected `%s` but received `%s`", expected, err.Error())
417-
} else {
418-
return true, nil
419-
}
420-
case <-time.After(100 * time.Millisecond):
421-
return false, nil
422-
}
423-
})
424-
425-
if err != nil {
426-
t.Fatalf("%+v", err)
427-
}
395+
expected := "referenced output not found: test-output-nonexistent"
396+
expectError(t, expected)
428397
}
429398

430399
func TestSingleFlowWithSecretInOutput(t *testing.T) {
@@ -581,3 +550,49 @@ func ensureCreatedEventually(t *testing.T, ns, name string, object runtime.Objec
581550
}
582551
}
583552
}
553+
554+
555+
func expectError(t *testing.T, expected string) {
556+
err := wait.Poll(time.Second, time.Second*3, func() (bool, error) {
557+
select {
558+
case err := <-reconcilerErrors:
559+
560+
if !strings.Contains(err.Error(), expected) {
561+
return false, errors.Errorf("expected `%s` but received `%s`", expected, err.Error())
562+
} else {
563+
return true, nil
564+
}
565+
case <-time.After(100 * time.Millisecond):
566+
return false, nil
567+
}
568+
})
569+
if err != nil {
570+
t.Fatalf("%+v", err)
571+
}
572+
}
573+
574+
func testOutput() *v1beta1.Output {
575+
return &v1beta1.Output{
576+
ObjectMeta: v1.ObjectMeta{
577+
Name: "test-output",
578+
Namespace: testNamespace,
579+
},
580+
Spec: v1beta1.OutputSpec{
581+
NullOutputConfig: output.NewNullOutputConfig(),
582+
},
583+
}
584+
}
585+
586+
func testLogging() *v1beta1.Logging {
587+
return &v1beta1.Logging{
588+
ObjectMeta: v1.ObjectMeta{
589+
Name: "test-" + uuid.New()[:8],
590+
},
591+
Spec: v1beta1.LoggingSpec{
592+
WatchNamespaces: []string{testNamespace},
593+
FluentdSpec: &v1beta1.FluentdSpec{},
594+
FlowConfigCheckDisabled: true,
595+
ControlNamespace: controlNamespace,
596+
},
597+
}
598+
}

‎pkg/resources/model/system.go‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"github.com/banzaicloud/logging-operator/pkg/sdk/model/types"
2626
"github.com/banzaicloud/logging-operator/pkg/sdk/plugins"
2727
"github.com/banzaicloud/operator-tools/pkg/secret"
28+
"github.com/banzaicloud/operator-tools/pkg/utils"
2829
"github.com/go-logr/logr"
2930
"sigs.k8s.io/controller-runtime/pkg/client"
3031
)
@@ -111,11 +112,14 @@ type CommonFlow struct {
111112
}
112113

113114
// Create []FlowMatch from v1beta1.Match and v1beta1.ClusterMatch
114-
func GetFlowMatchFromSpec(namespace string, matches interface{}) []types.FlowMatch {
115+
func GetFlowMatchFromSpec(namespace string, matches interface{}) ([]types.FlowMatch, error) {
115116
var flowMatches []types.FlowMatch
116117
switch matchList := matches.(type) {
117118
case []v1beta1.Match:
118119
for _, match := range matchList {
120+
if match.Select != nil && match.Exclude != nil {
121+
return nil, errors.Errorf("select and exclude cannot be set simultaneously")
122+
}
119123
if match.Select != nil {
120124
flowMatches = append(flowMatches, types.FlowMatch{
121125
Labels: match.Select.Labels,
@@ -133,6 +137,9 @@ func GetFlowMatchFromSpec(namespace string, matches interface{}) []types.FlowMat
133137
}
134138
case []v1beta1.ClusterMatch:
135139
for _, match := range matchList {
140+
if match.ClusterSelect != nil && match.ClusterExclude != nil {
141+
return nil, errors.Errorf("select and exclude cannot be set simultaneously")
142+
}
136143
if match.ClusterSelect != nil {
137144
flowMatches = append(flowMatches, types.FlowMatch{
138145
Labels: match.ClusterSelect.Labels,
@@ -149,11 +156,12 @@ func GetFlowMatchFromSpec(namespace string, matches interface{}) []types.FlowMat
149156
}
150157
}
151158
}
152-
return flowMatches
159+
return flowMatches, nil
153160
}
154161

155162
func FlowDispatcher(flowCr interface{}) (*CommonFlow, error) {
156163
var commonFlow *CommonFlow
164+
var err error
157165
switch f := flowCr.(type) {
158166
case v1beta1.ClusterFlow:
159167
var matches []types.FlowMatch
@@ -164,8 +172,15 @@ func FlowDispatcher(flowCr interface{}) (*CommonFlow, error) {
164172
OutputRefs: f.Spec.OutputRefs,
165173
Filters: f.Spec.Filters,
166174
}
175+
if f.Spec.Match != nil && f.Spec.Selectors != nil {
176+
return nil, errors.Errorf("match and selectors cannot be defined simultaneously for clusterflow %s",
177+
utils.ObjectKeyFromObjectMeta(&f).String())
178+
}
167179
if f.Spec.Match != nil {
168-
matches = GetFlowMatchFromSpec(f.Namespace, f.Spec.Match)
180+
matches, err = GetFlowMatchFromSpec(f.Namespace, f.Spec.Match)
181+
if err != nil {
182+
return nil, errors.WrapIff(err, "failed to process match for %s", utils.ObjectKeyFromObjectMeta(&f).String())
183+
}
169184
} else {
170185
matches = []types.FlowMatch{
171186
{
@@ -190,8 +205,15 @@ func FlowDispatcher(flowCr interface{}) (*CommonFlow, error) {
190205
OutputRefs: f.Spec.OutputRefs,
191206
Filters: f.Spec.Filters,
192207
}
208+
if f.Spec.Match != nil && f.Spec.Selectors != nil {
209+
return nil, errors.Errorf("match and selectors cannot be defined simultaneously for flow %s",
210+
utils.ObjectKeyFromObjectMeta(&f).String())
211+
}
193212
if f.Spec.Match != nil {
194-
matches = GetFlowMatchFromSpec(f.Namespace, f.Spec.Match)
213+
matches, err = GetFlowMatchFromSpec(f.Namespace, f.Spec.Match)
214+
if err != nil {
215+
return nil, errors.WrapIff(err, "failed to process match for %s", utils.ObjectKeyFromObjectMeta(&f).String())
216+
}
195217
} else {
196218
matches = []types.FlowMatch{
197219
{

0 commit comments

Comments
 (0)