This message was deleted.

Hi Dylan, I’m from the same company as Brett, the author of this PR. Let me just clarify a couple of things - we are using our custom policy tooling with atlantis on v0.21.0 and it works just fine. However, recent changes to how the rendering of output from conftest is done breaks for us as the format for our tool does not work with that. Currently, we are unable to upgrade Atlantis due to that issue. I want to ask you to consider approving this PR as a means to unblock users wanting to use custom tooling but don’t expect the full support conftest has. Then if you’d like to consider further improvements and others are motivated to contribute those improvements that can be a follow up.

Hi Amina! No worries, I definitely am considering it. My concerns is that the work is just a patch that will become permanent in the long run. I want us to be considerate of how we best support custom tooling in a way that moves Atlantis towards support all tooling as first class instead of a single tool. The current implementation kind of shoehorns custom policy checks into the existing Conftest model. It can be a bit confusing for future contributors to understand. While im great til for the contribution, I have a need to balance functionality vs code sprawl. My compromise here is this, if I don’t hear any more feedback by next week, I’ll approve and merge.

Does anyone else have thoughts/feedback on this implementation for custom policy tools?

would it make sense to determine fail/pass based on exist code instead of string “`fail`” in the output?

That seems reasonable to me, I can change that

@Brett Galkowski Given the issues with supporting multiple tools, would it be possible to use a different struct/model/field for custom policySetResults instead of trying to shove them into the current

ConfTestOutput

field?

I can rename the existing

PolicySetResult

field to be more generic, i.e.

ConfTestOutput

->

PolicyOutput

. However I'm wary of making a duplicate struct that would coexist next to

PolicySetResult

. If the end goal is supporting multiple policy tools, I feel that one model which is treated differently depending on the tool used is a clearer design than maintaining two similar models (one Conftest-only object, one bucket for everything else). I also don't understand the Conftest integration / policy sets implementation that well, and I wouldn't want to introduce bugs in the places expecting exactly this struct

That's understandable. I think having an additional field

ConfTestOutput

and

CustomPolicyOutput

would be okay if you don't want to have to adjust the Conftest policy set implementations unless you are comfortable renaming

@Dylan Page Pushed a change that at least shifts

ConfTestOutput

field ->

PolicyOutput

for clarity

Merged and a new release is out

Awesome! Of course now I notice a typo in the added docs facepalm

Thank you Dylan - appreciate your partnership on this!

You're very welcome. Thank you both for understanding my concerns