Verify the validity of policy.yml files before PRs merge

[palantir]

Verify the validity of policy.yml files before PRs merge

[CRogers]

Also hit an important internal repo again!

Additionally, combined with the remote loading of config, merging a broken file can result in breaking hundreds of repos in one go, which is terrifying (in the past I have blocked pretty much all merges into an org that used a centrally defined policy.yml).

[iamdanfox]

Can we try the validation endpoint first? I think this would provide 80% of the value internally as we put a lot of our remote policies in one repo, so if we could just protect this I think we'd be in business.

[jeremy-asher]

I recently broke PCloud's puppet-modules repo with an invalid policy. It would be pretty nice if we could do validations. We could put that on our central policies repo and call it a day.

[ferozco]

I just ran into this again today, +1 from me for providing some feedback. Perhaps you could use the check run API to call out the exact issues?

[NargiT]

Maybe a POST on a url is enough and can be used by the CI when a changes is detected on the policy.yaml file. It would work also with remote policy as it is decoupled, but requires more effort from the development point of view.

[iamdanfox]

I've just hit this again and merged a broken policy onto develop of conjure-typescript :( palantir/conjure-typescript#118

[swanysimon]

This has now happened multiple times recently in one of our monorepos and it's been pretty rough finding and fixing up what usually amounts to a whitespace error.

Another reason I'd like to be able to validate the policy yaml is that I tried to use a perl regex for one of the policies and broke said monorepo briefly.

[jrfitzsimmons]

+1 this blocked a number of PRs on a repo. The people who broke the policy file were offline and we had to chase repo admins to fix it.

[bluekeyes]

I appreciate the annoyance that merging invalid policy.yml files causes, but I'm not sure of the best way to fix this. I'm concerned about some aspects of the original proposal to add an additional status check for PRs:

• It requires examining the modified files of every pull request opened or updated in an organization where the bot is enabled, even in repositories that currently have no policy.yml file. This is a potential performance or API rate issue, but perhaps wouldn't be that bad in practice or could be mitigated by only validating policies in repositories that already have a valid policy file.
• In the case of a remote reference, it can only verify that the referenced policy exists and is valid at the time the reference is added. Since remote policies can be located anywhere in a repository, I can't think of a way to validate changes to these policies as part of an automatic status check.

I have some alternate ides that avoid these issues, but they shift the burden of validating policy changes to users or project-level CI. Would people find any of these valuable?

Validation Endpoint

As proposed above by @NargiT, we could add an API endpoint that accepts a policy.yml and validates it, returning any validation errors. Users would have to add a step to their CI builds that posts the content of the policy file on the current branch to the endpoint and fails if there are any errors.

This has the advantage of being on-demand and working well for centralized policy repositories, but requires more work for repositories that use unique, local policy files.

Simulation UI

Add a new UI route similar to the details page that allows users to upload a policy file (or maybe reference one on a repository branch) and simulate how it would behave on any pull request. As part of this, it would report syntax errors in addition to evaluation-time errors, like referencing a team that doesn't exist.

This had the advantage of being on-demand and allowing exploration of policies without committing them to a repository, but requires users to manually validate policy files using the simulator when making changes.

[swanysimon]

Of the options you've listed, the former is the most useful to me because it can actually prevent people from merging a broken configuration, though I do understand that there may be an issue for more complex setups.

Before going down that right, I would really like to try your idea of only validating a policy if there is an existing policy file. To me, policy-bot has the best understanding of what is and isn't a valid configuration, and also has the best idea of which files it will read from. Making everyone bake their own test will likely lead to everyone inventing the same testing wheel and copy-pasting it across organizations.

With regard to remote policies, why does this cause problems? If we're computing the policy on every commit, shouldn't they get pulled in to the validation, as well?