Skip to content

Commit

Permalink
build: Add bundle inspection (#1010)
Browse files Browse the repository at this point in the history 
* build: Add bundle inspection
  • Loading branch information
@liamg
liamg authored Oct 13, 2022
1 parent e855694 commit d128e52
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 3 deletions.
23 changes: 23 additions & 0 deletions .github/workflows/test-bundle.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: test rego bundle
on:
pull_request:
env:
GO_VERSION: "1.18"
jobs:
opa-tests:
name: OPA tests
runs-on: ubuntu-20.04
steps:
- name: Checkout code
uses: actions/checkout@v3
- name: Build bundle
run: make bundle
- name: Setup OPA
run: |
curl -L -o opa_linux_amd64 https://openpolicyagent.org/downloads/latest/opa_linux_amd64
curl -L -o checksum https://openpolicyagent.org/downloads/latest/opa_linux_amd64.sha256
sha256sum -c checksum
chmod 755 ./opa_linux_amd64
sudo mv ./opa_linux_amd64 /usr/local/bin/opa
- name: Check bundle
run: opa inspect bundle.tar.gz
2 changes: 1 addition & 1 deletion internal/rules/policies/.manifest
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"revision" : "[GITHUB_SHA]",
"roots": ["policies"]
"roots": [""]

This comment has been minimized.

Sign in to view

Copy link
@simar7

simar7 Oct 14, 2022

Member

So while this is a correct change, it has exposed a new challenge with downloading policies. It's a little convoluted so bear with me:

  1. Without this change, opa inspect rightly throws an error:
error: bundle bundle.tar.gz: manifest roots [policies] do not permit 'package lib.utils' in module './policies/kubernetes/lib/utils.rego'
  1. Before this change, loading up a bundle would "just work" because the downloaded libraries were actually not getting loaded. As there were no downloaded libraries being loaded, the embedded libraries were being used, resulting in no errors.
  2. When this bundle (rooted at "") gets loaded, the embedded libraries get defined twice, resulting in OPA throwing up:
kubernetes/lib/kubernetes.rego:10: rego_type_error: multiple default rules named is_gatekeeper found
kubernetes/lib/kubernetes.rego:36: rego_type_error: multiple default rules named namespace found
kubernetes/lib/kubernetes.rego:52: rego_type_error: multiple default rules named is_controller found

Notice the issues are only in kubernetes/lib/kubernetes dirs, which are the libraries.

AFAIK, there's a flag to disable embedded policies, https://github.com/aquasecurity/trivy/blob/5f0bf1445aad6e8107a9f3977ba7e1969c09724a/pkg/fanal/analyzer/config/config.go#L13 – but this still lets embedded libraries to be loaded.

I can see two ways:

  1. If we want to fall back to using embedded policies and libraries (in case airgap or otherwise), we'd still need to embed them. In this case we'll probably need to add support to defsec for disabling embedded libraries?
  2. We completely give up on embedding policies/libs in Trivy and fully embrace fetching policies (just like Trivy DB). This used to be the old behaviour.
  3. Somehow namespace the policies/libs so they don't conflict - not sure how.

My thoughts:
For point 2. since the embedded policies are loaded via an init() https://github.com/aquasecurity/defsec/blob/master/pkg/rego/embed.go#L19-L27 – how can we completely stop embedding policies and libraries altogether when using defsec?

Any other thoughts? I'm open to ideas.

We can discuss this over a call (8AM pacific, oct 14 if you prefer).

This comment has been minimized.

Sign in to view

Copy link
@liamg

liamg Oct 14, 2022

Author Contributor

Good catch - it looks like this commit forced embedded libraries to always be loaded whether we want them or not - 01c35b4

I think perhaps this should be reverted to solve the problem...

This comment has been minimized.

Sign in to view

Copy link
@simar7

simar7 Oct 14, 2022 •
edited
Loading

Member

thanks, actually this is great that you found this – just reverting it back resolves the issue with library conflicts. So our shortest path to land this policy fetching logic might just be to revert this commit (assuming Chen is fine with it), rather than the other two approaches we discussed. Less code changes the better IMO.
}
4 changes: 2 additions & 2 deletions test/bundle_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func Test_ManifestValidity(t *testing.T) {

require.Equal(t, "[GITHUB_SHA]", m.Revision)
require.Len(t, m.Roots, 1)
require.Equal(t, "policies", m.Roots[0])
require.Equal(t, "", m.Roots[0])

cmd := exec.Command("scripts/bundle.sh")
cmd.Env = append(os.Environ(), "GITHUB_REF=refs/tags/v1.2.3")
Expand Down Expand Up @@ -84,7 +84,7 @@ func Test_ManifestValidity(t *testing.T) {
require.NoError(t, json.NewDecoder(mf).Decode(&m2))
assert.Equal(t, "1.2.3", m2.Revision)
assert.Len(t, m2.Roots, 1)
assert.Equal(t, "policies", m2.Roots[0])
assert.Equal(t, "", m2.Roots[0])

policies, err := mfs.ReadDir("./policies")
require.NoError(t, err)
Expand Down

0 comments on commit d128e52

Please sign in to comment.