Skip to content

Commit 2f2edb5

Browse files
laveryalaverya
authored
Merge pull request #302 from replicatedhq/laverya/dont-rbac-check-excluded-collectors
check whether a collector is excluded before checking RBAC for it
2 parents c81576f + 0ec1c43 commit 2f2edb5

File tree

3 files changed

+117
-73
lines changed

3 files changed

+117
-73
lines changed

‎.github/workflows/build-test-deploy.yaml‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ jobs:
1818

1919
- name: setup env
2020
run: |
21-
echo "::set-env name=GOPATH::$(go env GOPATH)"
22-
echo "::add-path::$(go env GOPATH)/bin"
21+
echo "GOPATH=$(go env GOPATH)" >> $GITHUB_ENV
22+
echo "$(go env GOPATH)/bin" >> $GITHUB_PATH
2323
shell: bash
2424

2525
- uses: actions/checkout@v2
@@ -35,8 +35,8 @@ jobs:
3535
go-version: '1.14'
3636
- name: setup env
3737
run: |
38-
echo "::set-env name=GOPATH::$(go env GOPATH)"
39-
echo "::add-path::$(go env GOPATH)/bin"
38+
echo "GOPATH=$(go env GOPATH)" >> $GITHUB_ENV
39+
echo "$(go env GOPATH)/bin" >> $GITHUB_PATH
4040
shell: bash
4141
- uses: actions/checkout@master
4242
- run: make preflight
@@ -54,7 +54,7 @@ jobs:
5454
with:
5555
name: preflight
5656
path: bin/
57-
- uses: engineerd/setup-kind@v0.2.0
57+
- uses: engineerd/setup-kind@v0.5.0
5858
- run: chmod +x bin/preflight
5959
- run: ./bin/preflight --interactive=false --format=json https://preflight.replicated.com
6060

@@ -67,8 +67,8 @@ jobs:
6767
go-version: '1.14'
6868
- name: setup env
6969
run: |
70-
echo "::set-env name=GOPATH::$(go env GOPATH)"
71-
echo "::add-path::$(go env GOPATH)/bin"
70+
echo "GOPATH=$(go env GOPATH)" >> $GITHUB_ENV
71+
echo "$(go env GOPATH)/bin" >> $GITHUB_PATH
7272
shell: bash
7373
- uses: actions/checkout@master
7474
- run: make support-bundle
@@ -87,7 +87,7 @@ jobs:
8787
with:
8888
name: support-bundle
8989
path: bin/
90-
- uses: engineerd/setup-kind@v0.2.0
90+
- uses: engineerd/setup-kind@v0.5.0
9191
- run: chmod +x bin/support-bundle
9292
- run: ./bin/support-bundle ./examples/support-bundle/sample-collectors.yaml
9393
- run: ./bin/support-bundle ./examples/support-bundle/sample-supportbundle.yaml
@@ -130,4 +130,4 @@ jobs:
130130
- name: Update new support-bundle version in krew-index
131131
uses: rajatjindal/[email protected]
132132
with:
133-
krew_template_file: deploy/krew/support-bundle.yaml
133+
krew_template_file: deploy/krew/support-bundle.yaml

‎pkg/collect/collector.go‎

Lines changed: 91 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -42,140 +42,164 @@ func isExcluded(excludeVal multitype.BoolOrString) (bool, error) {
4242
return parsed, nil
4343
}
4444

45-
func (c *Collector) RunCollectorSync(globalRedactors []*troubleshootv1beta2.Redact) (result map[string][]byte, err error) {
46-
defer func() {
47-
if r := recover(); r != nil {
48-
err = errors.Errorf("recovered rom panic: %v", r)
49-
}
50-
}()
51-
52-
var isExcludedResult bool
45+
// checks if a given collector has a spec with 'exclude' that evaluates to true.
46+
func (c *Collector) IsExcluded() bool {
5347
if c.Collect.ClusterInfo != nil {
54-
isExcludedResult, err = isExcluded(c.Collect.ClusterInfo.Exclude)
48+
isExcludedResult, err := isExcluded(c.Collect.ClusterInfo.Exclude)
5549
if err != nil {
56-
return
50+
return true
5751
}
5852
if isExcludedResult {
59-
return
53+
return true
6054
}
61-
result, err = ClusterInfo(c)
6255
} else if c.Collect.ClusterResources != nil {
63-
isExcludedResult, err = isExcluded(c.Collect.ClusterResources.Exclude)
56+
isExcludedResult, err := isExcluded(c.Collect.ClusterResources.Exclude)
6457
if err != nil {
65-
return
58+
return true
6659
}
6760
if isExcludedResult {
68-
return
61+
return true
6962
}
70-
result, err = ClusterResources(c)
7163
} else if c.Collect.Secret != nil {
72-
isExcludedResult, err = isExcluded(c.Collect.Secret.Exclude)
64+
isExcludedResult, err := isExcluded(c.Collect.Secret.Exclude)
7365
if err != nil {
74-
return
66+
return true
7567
}
7668
if isExcludedResult {
77-
return
69+
return true
7870
}
79-
result, err = Secret(c, c.Collect.Secret)
8071
} else if c.Collect.Logs != nil {
81-
isExcludedResult, err = isExcluded(c.Collect.Logs.Exclude)
72+
isExcludedResult, err := isExcluded(c.Collect.Logs.Exclude)
8273
if err != nil {
83-
return
74+
return true
8475
}
8576
if isExcludedResult {
86-
return
77+
return true
8778
}
88-
result, err = Logs(c, c.Collect.Logs)
8979
} else if c.Collect.Run != nil {
90-
isExcludedResult, err = isExcluded(c.Collect.Run.Exclude)
80+
isExcludedResult, err := isExcluded(c.Collect.Run.Exclude)
9181
if err != nil {
92-
return
82+
return true
9383
}
9484
if isExcludedResult {
95-
return
85+
return true
9686
}
97-
result, err = Run(c, c.Collect.Run)
9887
} else if c.Collect.Exec != nil {
99-
isExcludedResult, err = isExcluded(c.Collect.Exec.Exclude)
88+
isExcludedResult, err := isExcluded(c.Collect.Exec.Exclude)
10089
if err != nil {
101-
return
90+
return true
10291
}
10392
if isExcludedResult {
104-
return
93+
return true
10594
}
106-
result, err = Exec(c, c.Collect.Exec)
10795
} else if c.Collect.Data != nil {
108-
isExcludedResult, err = isExcluded(c.Collect.Data.Exclude)
96+
isExcludedResult, err := isExcluded(c.Collect.Data.Exclude)
10997
if err != nil {
110-
return
98+
return true
11199
}
112100
if isExcludedResult {
113-
return
101+
return true
114102
}
115-
result, err = Data(c, c.Collect.Data)
116103
} else if c.Collect.Copy != nil {
117-
isExcludedResult, err = isExcluded(c.Collect.Copy.Exclude)
104+
isExcludedResult, err := isExcluded(c.Collect.Copy.Exclude)
118105
if err != nil {
119-
return
106+
return true
120107
}
121108
if isExcludedResult {
122-
return
109+
return true
123110
}
124-
result, err = Copy(c, c.Collect.Copy)
125111
} else if c.Collect.HTTP != nil {
126-
isExcludedResult, err = isExcluded(c.Collect.HTTP.Exclude)
112+
isExcludedResult, err := isExcluded(c.Collect.HTTP.Exclude)
127113
if err != nil {
128-
return
114+
return true
129115
}
130116
if isExcludedResult {
131-
return
117+
return true
132118
}
133-
result, err = HTTP(c, c.Collect.HTTP)
134119
} else if c.Collect.Postgres != nil {
135-
isExcludedResult, err = isExcluded(c.Collect.Postgres.Exclude)
120+
isExcludedResult, err := isExcluded(c.Collect.Postgres.Exclude)
136121
if err != nil {
137-
return
122+
return true
138123
}
139124
if isExcludedResult {
140-
return
125+
return true
141126
}
142-
result, err = Postgres(c, c.Collect.Postgres)
143127
} else if c.Collect.Mysql != nil {
144-
isExcludedResult, err = isExcluded(c.Collect.Mysql.Exclude)
128+
isExcludedResult, err := isExcluded(c.Collect.Mysql.Exclude)
145129
if err != nil {
146-
return
130+
return true
147131
}
148132
if isExcludedResult {
149-
return
133+
return true
150134
}
151-
result, err = Mysql(c, c.Collect.Mysql)
152135
} else if c.Collect.Redis != nil {
153-
isExcludedResult, err = isExcluded(c.Collect.Redis.Exclude)
136+
isExcludedResult, err := isExcluded(c.Collect.Redis.Exclude)
154137
if err != nil {
155-
return
138+
return true
156139
}
157140
if isExcludedResult {
158-
return
141+
return true
159142
}
160-
result, err = Redis(c, c.Collect.Redis)
161143
} else if c.Collect.Collectd != nil {
162144
// TODO: see if redaction breaks these
163-
isExcludedResult, err = isExcluded(c.Collect.Collectd.Exclude)
145+
isExcludedResult, err := isExcluded(c.Collect.Collectd.Exclude)
164146
if err != nil {
165-
return
147+
return true
166148
}
167149
if isExcludedResult {
168-
return
150+
return true
169151
}
170-
result, err = Collectd(c, c.Collect.Collectd)
171152
} else if c.Collect.Ceph != nil {
172-
isExcludedResult, err = isExcluded(c.Collect.Ceph.Exclude)
153+
isExcludedResult, err := isExcluded(c.Collect.Ceph.Exclude)
173154
if err != nil {
174-
return nil, err
155+
return true
175156
}
176157
if isExcludedResult {
177-
return nil, nil
158+
return true
159+
}
160+
}
161+
return false
162+
}
163+
164+
func (c *Collector) RunCollectorSync(globalRedactors []*troubleshootv1beta2.Redact) (result map[string][]byte, err error) {
165+
defer func() {
166+
if r := recover(); r != nil {
167+
err = errors.Errorf("recovered rom panic: %v", r)
178168
}
169+
}()
170+
171+
if c.IsExcluded() {
172+
return
173+
}
174+
175+
if c.Collect.ClusterInfo != nil {
176+
result, err = ClusterInfo(c)
177+
} else if c.Collect.ClusterResources != nil {
178+
result, err = ClusterResources(c)
179+
} else if c.Collect.Secret != nil {
180+
result, err = Secret(c, c.Collect.Secret)
181+
} else if c.Collect.Logs != nil {
182+
result, err = Logs(c, c.Collect.Logs)
183+
} else if c.Collect.Run != nil {
184+
result, err = Run(c, c.Collect.Run)
185+
} else if c.Collect.Exec != nil {
186+
result, err = Exec(c, c.Collect.Exec)
187+
} else if c.Collect.Data != nil {
188+
result, err = Data(c, c.Collect.Data)
189+
} else if c.Collect.Copy != nil {
190+
result, err = Copy(c, c.Collect.Copy)
191+
} else if c.Collect.HTTP != nil {
192+
result, err = HTTP(c, c.Collect.HTTP)
193+
} else if c.Collect.Postgres != nil {
194+
result, err = Postgres(c, c.Collect.Postgres)
195+
} else if c.Collect.Mysql != nil {
196+
result, err = Mysql(c, c.Collect.Mysql)
197+
} else if c.Collect.Redis != nil {
198+
result, err = Redis(c, c.Collect.Redis)
199+
} else if c.Collect.Collectd != nil {
200+
// TODO: see if redaction breaks these
201+
result, err = Collectd(c, c.Collect.Collectd)
202+
} else if c.Collect.Ceph != nil {
179203
result, err = Ceph(c, c.Collect.Ceph)
180204
} else {
181205
err = errors.New("no spec found to run")
@@ -206,6 +230,10 @@ func (c *Collector) GetDisplayName() string {
206230
}
207231

208232
func (c *Collector) CheckRBAC(ctx context.Context) error {
233+
if c.IsExcluded() {
234+
return nil // excluded collectors require no permissions
235+
}
236+
209237
client, err := kubernetes.NewForConfig(c.ClientConfig)
210238
if err != nil {
211239
return errors.Wrap(err, "failed to create client from config")
@@ -215,7 +243,6 @@ func (c *Collector) CheckRBAC(ctx context.Context) error {
215243

216244
specs := c.Collect.AccessReviewSpecs(c.Namespace)
217245
for _, spec := range specs {
218-
219246
sar := &authorizationv1.SelfSubjectAccessReview{
220247
Spec: spec,
221248
}

‎pkg/collect/collector_test.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,23 @@ abc
260260
`,
261261
},
262262
},
263+
{
264+
name: "excluded data",
265+
Collect: &troubleshootv1beta2.Collect{
266+
Data: &troubleshootv1beta2.Data{
267+
CollectorMeta: troubleshootv1beta2.CollectorMeta{
268+
CollectorName: "datacollectorname",
269+
Exclude: multitype.BoolOrString{Type: multitype.String, StrVal: "true"},
270+
},
271+
Name: "data",
272+
Data: `abc 123
273+
another line here
274+
pwd=somethinggoeshere;`,
275+
},
276+
},
277+
Redactors: []*troubleshootv1beta2.Redact{},
278+
want: map[string]string{},
279+
},
263280
}
264281
for _, tt := range tests {
265282
t.Run(tt.name, func(t *testing.T) {

0 commit comments

Comments
 (0)