AzureAD/microsoft-authentication-library-for-go

Improve pull request CI

chlowell opened this issue · 3 comments

As configured, CI runs only when a PR is labeled or merged to dev or main (see the linting action, for example). This prevents non-maintainers from executing malicious code in CI, which is great. However, it also stops CI from running automatically when a PR is updated with new commits--a maintainer must add or remove a label to trigger CI. There may be ways to improve this with Action configuration. We could also separate the "safe" tests with no access to live resources or secrets and run those on PR updates.

As configured, CI runs only when a PR is labeled or merged to dev or main (see the linting action, for example). This prevents non-maintainers from executing malicious code in CI, which is great. However, it also stops CI from running automatically when a PR is updated with new commits--a maintainer must add or remove a label to trigger CI. There may be ways to improve this with Action configuration. We could also separate the "safe" tests with no access to live resources or secrets and run those on PR updates.

It should be sufficient to disable PR builds for PRs originating from forks.

I want to run CI on PRs by default though, and run it again on each push, to prevent merging broken code. I assume the motivation for the current setup was to avoid exposing secrets to everyone who opens a PR, however it's unnecessary because GitHub doesn't pass secrets to workflows triggered by PRs from a fork anyway.

There's more work here than reconfiguring the Action triggers though--we also need to skip the integration tests for external PRs because those tests need the secrets and will just fail without them (for example). The simple solution is to skip these tests when the secrets are absent, however that would enable a misconfigured run to succeed without trying all tests; it would be best to make the misconfiguration obvious by failing the run.

In my experience, the number of contributors slowly drops as the complexity of the library increases and rarely do contributors write tests anyway. So might as well optimize for your daily flow.