Draft: Fine-grained excludes

[moredhel]

Introduction

This PR is aiming to include an exclusions keyword which would allow more fine-grained control over which policies to deny. The current exception blocks will apply to an entire file, rather than a single entity within that file.

This PR aims to expand the functionality of conftest to allow for excluding a single deny instance of a policy within a file. This should bring assumed the functionality of conftest (exceptions) and the actual functionality in-line (via excludes).

Outstanding

---

• Write Tests
  • bats tests
  • go tests (standard_test.go)
• Documentation
  • How to use
  • Examples for the website
  • Clarity on exceptions & their scope

This is an initial PoC/thought process on how to do fine-grained Exclusions of policies.

There are currently some limitations that come with it:

• such as having to recreate the msg from both the deny/warn rule & the exclusion rule.
• The exclusion_<name> section must match with the corresponding deny_<name> that you would like to have match the exceptions against. (More a feature than a limitation)
• The response object from both the rule & the exclusion is more complicated (but optional, both methods still work)

Examples

I've included a sample in examples/exceptions2 which details some policies, a working exception & a working exclusion.
see for examples:

• examples/excludes
• examples/excludes2

You can run it similarly to in the tests:

$ ./conftest test -p examples/excludes/policy/ examples/excludes
FAIL - examples/exceptions2/main.tf - main - Resource Name 'invalid-name' contains dashes
EXCP - examples/exceptions2/main.tf - main - data.main.exception[_][_] == "resource_type"
EXCL - examples/exceptions2/main.tf - main - data.main.exclude_name[_][_] = "Resource Name 'exception-name' contains dashes"

3 tests, 0 passed, 0 warnings, 1 failure, 1 exception, 1 exclusion

The aim of this PR is to open discussion on a potential avenue towards having more fine-grained exclusions within conftest.

Have a play with it & let me know what your thoughts are.

Decisions / points of discussion

• modelling the exclusions similarly to denies & warns by naming them.
• Return value of an exclusion must match the error message from the matching deny/warn.
• Potential performance hit due to evaluating the excludes for each failure (1 run per failure)

[mykter]

Is there any mileage in having exclusions actually modify the rule itself?

say you write something like:

exclude_name[vars] {
  # we can't fix wid-get, its name is everywhere, just ignore it.
  vars.name == "wid-get"
}

and then deny_name is implicitly modified to look like this?

deny_name[msg] {
# unchanged:
	input.resource[_][name]
	contains(name, "-")
	msg := func_name_msg(name)
# magic happens here:
        not exclude_name({"name": name})
}

my thinking is you get access to the internal state of the rule, which means you can add arbitrary conditions

It's fragile, in that any changes to how the original rule works (including renaming variables) could break your exclusion, but then matching on messages is also fragile.

[rdsubhas]

Contextual exclusions will greatly improve quality of policies. Thank you for kickstarting this and for the PoC PR! I would like to offer one additional viewpoint, if I may...

A lot of the cumbersomeness with contextual exclusions – is passing the context. The actual deny or warn rule has the full localized context, and somehow bits & pieces of this have to be passed on to evaluate exclusions. This adds an additional layer of connection/indirection, which is hard to enforce.

What if we never have to pass on the context, and the exclusion can be done inline – right where the context is? i.e. What if exclusion is not a separate construct, but a function. For example:

deny_root[result] {
	input.kind == "Deployment"

	exclude(input.name == "mydep")
	# ^^^^^^^^^^^^ let's use context right here

	c = input.spec.template.spec.containers[_]
	not c.securityContext.runAsNonRoot

	exclude(c.name == "host-agent")
	# ^^^^^^^^^^^^ let's use context right here

	result := sprintf("container %s in deployment %s doesn't set runAsNonRoot", [c.name, input.metadata.name])
	# ^^^^^^^^^^^^ normal sprintf result, no need to pass around context
}

Would be awesome if the exclude could be caught & the context made a note of without breaking the flow – i.e. the exclude function doesn't do anything, but the rule is allowed to continue executing so that the reason is captured, and then at the end of evaluation the rule is marked as excluded with it's reason intact. Or maybe the excludes have to be moved to the bottom to have the reason already present in the context like:

deny_root[result] {
	input.kind == "Deployment"
	c = input.spec.template.spec.containers[_]
	not c.securityContext.runAsNonRoot
	result := sprintf("container %s in deployment %s doesn't set runAsNonRoot", [c.name, input.metadata.name])

	exclude(c.name == "host-agent")
	exclude(input.name == "mydep")
}

[jalseth]

Hi @moredhel. Friendly ping, please provide an update on the status of this PR.

[moredhel]

Hi @jalseth,

Apologies for the late reply, have been on holiday the last few weeks.

I have the following issue where I am asking for more general feedback on if this is a reasonable way to proceed #591

I'm still wanting this to go ahead, but would like to get some feedback for a member on how valuable this is (for myself I feel the current solution isn't flexible enough, so would appreciate the extra flexibility of what I'm proposing).

Essentially lost of the work is done, outstanding tasks:

• Documentation
• Unit tests
• a once-over to verify everything looks good.

Let me know your thoughts and if you're happy I can push this through to be mergeable.