From 858d00ce41baf0a68f67d7d502f76e7356dac654 Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Thu, 19 Nov 2020 18:31:56 -0500 Subject: [PATCH 1/3] check whether a collector is excluded before checking RBAC for it --- pkg/collect/collector.go | 227 +++++++++++++++++++--------------- pkg/collect/collector_test.go | 17 +++ 2 files changed, 144 insertions(+), 100 deletions(-) diff --git a/pkg/collect/collector.go b/pkg/collect/collector.go index df985d4b..4b1cb729 100644 --- a/pkg/collect/collector.go +++ b/pkg/collect/collector.go @@ -42,6 +42,125 @@ func isExcluded(excludeVal multitype.BoolOrString) (bool, error) { return parsed, nil } +// checks if a given collector has a spec with 'exclude' that evaluates to true. +func (c *Collector) IsExcluded() bool { + if c.Collect.ClusterInfo != nil { + isExcludedResult, err := isExcluded(c.Collect.ClusterInfo.Exclude) + if err != nil { + return true + } + if isExcludedResult { + return true + } + } else if c.Collect.ClusterResources != nil { + isExcludedResult, err := isExcluded(c.Collect.ClusterResources.Exclude) + if err != nil { + return true + } + if isExcludedResult { + return true + } + } else if c.Collect.Secret != nil { + isExcludedResult, err := isExcluded(c.Collect.Secret.Exclude) + if err != nil { + return true + } + if isExcludedResult { + return true + } + } else if c.Collect.Logs != nil { + isExcludedResult, err := isExcluded(c.Collect.Logs.Exclude) + if err != nil { + return true + } + if isExcludedResult { + return true + } + } else if c.Collect.Run != nil { + isExcludedResult, err := isExcluded(c.Collect.Run.Exclude) + if err != nil { + return true + } + if isExcludedResult { + return true + } + } else if c.Collect.Exec != nil { + isExcludedResult, err := isExcluded(c.Collect.Exec.Exclude) + if err != nil { + return true + } + if isExcludedResult { + return true + } + } else if c.Collect.Data != nil { + isExcludedResult, err := isExcluded(c.Collect.Data.Exclude) + if err != nil { + return true + } + if isExcludedResult { + return true + } + } else if c.Collect.Copy != nil { + isExcludedResult, err := isExcluded(c.Collect.Copy.Exclude) + if err != nil { + return true + } + if isExcludedResult { + return true + } + } else if c.Collect.HTTP != nil { + isExcludedResult, err := isExcluded(c.Collect.HTTP.Exclude) + if err != nil { + return true + } + if isExcludedResult { + return true + } + } else if c.Collect.Postgres != nil { + isExcludedResult, err := isExcluded(c.Collect.Postgres.Exclude) + if err != nil { + return true + } + if isExcludedResult { + return true + } + } else if c.Collect.Mysql != nil { + isExcludedResult, err := isExcluded(c.Collect.Mysql.Exclude) + if err != nil { + return true + } + if isExcludedResult { + return true + } + } else if c.Collect.Redis != nil { + isExcludedResult, err := isExcluded(c.Collect.Redis.Exclude) + if err != nil { + return true + } + if isExcludedResult { + return true + } + } else if c.Collect.Collectd != nil { + // TODO: see if redaction breaks these + isExcludedResult, err := isExcluded(c.Collect.Collectd.Exclude) + if err != nil { + return true + } + if isExcludedResult { + return true + } + } else if c.Collect.Ceph != nil { + isExcludedResult, err := isExcluded(c.Collect.Ceph.Exclude) + if err != nil { + return true + } + if isExcludedResult { + return true + } + } + return false +} + func (c *Collector) RunCollectorSync(globalRedactors []*troubleshootv1beta2.Redact) (result map[string][]byte, err error) { defer func() { if r := recover(); r != nil { @@ -49,133 +168,38 @@ func (c *Collector) RunCollectorSync(globalRedactors []*troubleshootv1beta2.Reda } }() - var isExcludedResult bool + if c.IsExcluded() { + return + } + if c.Collect.ClusterInfo != nil { - isExcludedResult, err = isExcluded(c.Collect.ClusterInfo.Exclude) - if err != nil { - return - } - if isExcludedResult { - return - } result, err = ClusterInfo(c) } else if c.Collect.ClusterResources != nil { - isExcludedResult, err = isExcluded(c.Collect.ClusterResources.Exclude) - if err != nil { - return - } - if isExcludedResult { - return - } result, err = ClusterResources(c) } else if c.Collect.Secret != nil { - isExcludedResult, err = isExcluded(c.Collect.Secret.Exclude) - if err != nil { - return - } - if isExcludedResult { - return - } result, err = Secret(c, c.Collect.Secret) } else if c.Collect.Logs != nil { - isExcludedResult, err = isExcluded(c.Collect.Logs.Exclude) - if err != nil { - return - } - if isExcludedResult { - return - } result, err = Logs(c, c.Collect.Logs) } else if c.Collect.Run != nil { - isExcludedResult, err = isExcluded(c.Collect.Run.Exclude) - if err != nil { - return - } - if isExcludedResult { - return - } result, err = Run(c, c.Collect.Run) } else if c.Collect.Exec != nil { - isExcludedResult, err = isExcluded(c.Collect.Exec.Exclude) - if err != nil { - return - } - if isExcludedResult { - return - } result, err = Exec(c, c.Collect.Exec) } else if c.Collect.Data != nil { - isExcludedResult, err = isExcluded(c.Collect.Data.Exclude) - if err != nil { - return - } - if isExcludedResult { - return - } result, err = Data(c, c.Collect.Data) } else if c.Collect.Copy != nil { - isExcludedResult, err = isExcluded(c.Collect.Copy.Exclude) - if err != nil { - return - } - if isExcludedResult { - return - } result, err = Copy(c, c.Collect.Copy) } else if c.Collect.HTTP != nil { - isExcludedResult, err = isExcluded(c.Collect.HTTP.Exclude) - if err != nil { - return - } - if isExcludedResult { - return - } result, err = HTTP(c, c.Collect.HTTP) } else if c.Collect.Postgres != nil { - isExcludedResult, err = isExcluded(c.Collect.Postgres.Exclude) - if err != nil { - return - } - if isExcludedResult { - return - } result, err = Postgres(c, c.Collect.Postgres) } else if c.Collect.Mysql != nil { - isExcludedResult, err = isExcluded(c.Collect.Mysql.Exclude) - if err != nil { - return - } - if isExcludedResult { - return - } result, err = Mysql(c, c.Collect.Mysql) } else if c.Collect.Redis != nil { - isExcludedResult, err = isExcluded(c.Collect.Redis.Exclude) - if err != nil { - return - } - if isExcludedResult { - return - } result, err = Redis(c, c.Collect.Redis) } else if c.Collect.Collectd != nil { // TODO: see if redaction breaks these - isExcludedResult, err = isExcluded(c.Collect.Collectd.Exclude) - if err != nil { - return - } - if isExcludedResult { - return - } result, err = Collectd(c, c.Collect.Collectd) } else if c.Collect.Ceph != nil { - isExcludedResult, err = isExcluded(c.Collect.Ceph.Exclude) - if err != nil { - return nil, err - } - if isExcludedResult { - return nil, nil - } result, err = Ceph(c, c.Collect.Ceph) } else { err = errors.New("no spec found to run") @@ -206,6 +230,10 @@ func (c *Collector) GetDisplayName() string { } func (c *Collector) CheckRBAC(ctx context.Context) error { + if c.IsExcluded() { + return nil // excluded collectors require no permissions + } + client, err := kubernetes.NewForConfig(c.ClientConfig) if err != nil { return errors.Wrap(err, "failed to create client from config") @@ -215,7 +243,6 @@ func (c *Collector) CheckRBAC(ctx context.Context) error { specs := c.Collect.AccessReviewSpecs(c.Namespace) for _, spec := range specs { - sar := &authorizationv1.SelfSubjectAccessReview{ Spec: spec, } diff --git a/pkg/collect/collector_test.go b/pkg/collect/collector_test.go index 9a771c94..282fe5fd 100644 --- a/pkg/collect/collector_test.go +++ b/pkg/collect/collector_test.go @@ -260,6 +260,23 @@ abc `, }, }, + { + name: "excluded data", + Collect: &troubleshootv1beta2.Collect{ + Data: &troubleshootv1beta2.Data{ + CollectorMeta: troubleshootv1beta2.CollectorMeta{ + CollectorName: "datacollectorname", + Exclude: multitype.BoolOrString{Type: multitype.String, StrVal: "true"}, + }, + Name: "data", + Data: `abc 123 +another line here +pwd=somethinggoeshere;`, + }, + }, + Redactors: []*troubleshootv1beta2.Redact{}, + want: map[string]string{}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From b43ef17b8fbe75a0dc7fd93498ebaa1b3294af23 Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Thu, 19 Nov 2020 18:37:57 -0500 Subject: [PATCH 2/3] remve set-env and add-path from CI --- .github/workflows/build-test-deploy.yaml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/build-test-deploy.yaml b/.github/workflows/build-test-deploy.yaml index dc875f0b..82f90786 100644 --- a/.github/workflows/build-test-deploy.yaml +++ b/.github/workflows/build-test-deploy.yaml @@ -18,8 +18,8 @@ jobs: - name: setup env run: | - echo "::set-env name=GOPATH::$(go env GOPATH)" - echo "::add-path::$(go env GOPATH)/bin" + echo "GOPATH=$(go env GOPATH)" >> $GITHUB_ENV + echo "$(go env GOPATH)/bin" >> $GITHUB_PATH shell: bash - uses: actions/checkout@v2 @@ -35,8 +35,8 @@ jobs: go-version: '1.14' - name: setup env run: | - echo "::set-env name=GOPATH::$(go env GOPATH)" - echo "::add-path::$(go env GOPATH)/bin" + echo "GOPATH=$(go env GOPATH)" >> $GITHUB_ENV + echo "$(go env GOPATH)/bin" >> $GITHUB_PATH shell: bash - uses: actions/checkout@master - run: make preflight @@ -67,8 +67,8 @@ jobs: go-version: '1.14' - name: setup env run: | - echo "::set-env name=GOPATH::$(go env GOPATH)" - echo "::add-path::$(go env GOPATH)/bin" + echo "GOPATH=$(go env GOPATH)" >> $GITHUB_ENV + echo "$(go env GOPATH)/bin" >> $GITHUB_PATH shell: bash - uses: actions/checkout@master - run: make support-bundle @@ -130,4 +130,4 @@ jobs: - name: Update new support-bundle version in krew-index uses: rajatjindal/krew-release-bot@v0.0.38 with: - krew_template_file: deploy/krew/support-bundle.yaml \ No newline at end of file + krew_template_file: deploy/krew/support-bundle.yaml From 0ec1c43afd6ef8b79c4d6e9290811191739aae08 Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Thu, 19 Nov 2020 18:57:52 -0500 Subject: [PATCH 3/3] update setup-kind action --- .github/workflows/build-test-deploy.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build-test-deploy.yaml b/.github/workflows/build-test-deploy.yaml index 82f90786..0b681e7e 100644 --- a/.github/workflows/build-test-deploy.yaml +++ b/.github/workflows/build-test-deploy.yaml @@ -54,7 +54,7 @@ jobs: with: name: preflight path: bin/ - - uses: engineerd/setup-kind@v0.2.0 + - uses: engineerd/setup-kind@v0.5.0 - run: chmod +x bin/preflight - run: ./bin/preflight --interactive=false --format=json https://preflight.replicated.com @@ -87,7 +87,7 @@ jobs: with: name: support-bundle path: bin/ - - uses: engineerd/setup-kind@v0.2.0 + - uses: engineerd/setup-kind@v0.5.0 - run: chmod +x bin/support-bundle - run: ./bin/support-bundle ./examples/support-bundle/sample-collectors.yaml - run: ./bin/support-bundle ./examples/support-bundle/sample-supportbundle.yaml