Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: skip autoplan if a pull request matches skip keywords for GitHub #1799

Closed

Conversation

minamijoyo
Copy link
Contributor

@minamijoyo minamijoyo commented Sep 8, 2021

This is an attempt to partially support for #932.

If an author of the pull request has a confidence of "no changes for real resources", it would be great if atlantis could skip autoplan.

The initial implementation of this feature as follows:

If a title of pull request contains the following keywords, skip autoplan.

  • [skip atlantis]
  • [skip ci]
  • [atlantis skip]
  • [ci skip]

We should always force to invoke plan with explicit comment command.
This feature is currently implemented only for GitHub just because I'm a user of GitHub, but I expect it's possible to support other VCS providers.

Note:

Most of general purpose CI/CD platforms support a concept for skip build. For examples:
https://circleci.com/docs/2.0/skip-build/
https://docs.github.com/en/actions/guides/about-continuous-integration#skipping-workflow-runs

As far as I know, they check all commits included in the pull request, not a title of the pull request because they need to support triggered on push event. On the other hand, the current implementation of atlantis doesn't triggered on push event and doesn't have all commits on open event. To simplify the implementation, I think checking the title is reasonable. Of course it's possible to get all commits included in the pull request dynamically via additional API calls, please let me know if we should check commit messages instead of the title.

The original feature request said that the 'keyword' could be configurable, but I don't think most of users including me need such a flexibility. So the initial implementation embeds keywords in source. If someone need to be configurable, feel free to open another feature request.

@minamijoyo minamijoyo requested a review from a team as a code owner September 8, 2021 06:58
@minamijoyo minamijoyo force-pushed the skip-pr-based-on-title branch from 8898f59 to f1b2637 Compare September 8, 2021 08:47
@nishkrishnan
Copy link
Contributor

Commented on #932. But I'm not inclined to support this. Autoplan should be a default and likely will be in a future release (when we tackle some of the cleanup work of the 20 million flags). This opens up security risk for anyone relying on Autoplan to ensure that PRs with terraform changes are not just shipped without being applied.

@chenrui333 chenrui333 changed the title Skip autoplan if a pull request matches skip keywords for GitHub feat: skip autoplan if a pull request matches skip keywords for GitHub Oct 21, 2021
@raynigon
Copy link

raynigon commented Nov 3, 2021

@nishkrishnan What dou you think of a solution where the keywords or labels are configured in the atlantis.yaml file?

@minamijoyo Personally i would prefer a solution where the keywords and labels are configurable. I think this solution is a good start for the evaluation logic, but the configuration part should also be implemented. This would allow to create a PR in an open source project where label selection is disabled without any security issue. (At least for GitHub)

@minamijoyo
Copy link
Contributor Author

Hi @raynigon, thank you for your suggestions. @nishkrishnan hasn't agreed to add this feature, so I'd like to confirm it first before moving this forward.

@jamengual jamengual added feature New functionality/enhancement waiting-on-review Waiting for a review from a maintainer labels May 13, 2022
@jamengual
Copy link
Contributor

@minamijoyo do you still have time to work on this or I should close this PR?

@minamijoyo
Copy link
Contributor Author

Hi @jamengual, Thanks for the reminder.

When I created this PR, I was using Atlantis in combination with CodeBuild, as I commented in #932 (comment), but now I have thrown CodeBuild away and integrated a drift detection implementation for tfupdate and daily regression test into Atlantis with custom workflows. Thus, I actually do not currently need this feature for me.

Having said that, I understand that the ability to skip CI is a generic requirement, as many have upvoted. I would be happy to help implement this feature if it's acceptable to the current maintainer. Is it mergeable if I rebase this branch to the current master?

@jamengual
Copy link
Contributor

ok, we will look into this PR soon

@minamijoyo minamijoyo force-pushed the skip-pr-based-on-title branch from f1b2637 to 7f53c38 Compare September 22, 2022 01:44
@minamijoyo
Copy link
Contributor Author

@jamengual I rebased the patch onto the latest master and confirmed that it is still working as expected.

@nitrocode
Copy link
Member

@minamijoyo thank you for your patience here. Could you please fix the conflicts so we can re-run tests?

Also, wouldn't a PR label make more sense than parsing the PR title?

This is an attempt to partially support for runatlantis#932.

If an author of the pull request has a confidence of "no changes for
real resources", it would be great if atlantis could skip autoplan.

The initial implementation of this feature as follows:

If a title of pull request contains the following keywords, skip autoplan.

* [skip atlantis]
* [skip ci]
* [atlantis skip]
* [ci skip]

We should always force to invoke plan with explicit comment command.

This feature is currently implemented only for GitHub just because
I'm a user of GitHub, but I expect it's possible to support other VCS
providers.

Note:

Most of general purpose CI/CD platforms support a concept for skip
build. For examples:
https://circleci.com/docs/2.0/skip-build/
https://docs.github.com/en/actions/guides/about-continuous-integration#skipping-workflow-runs

As far as I know, they check all commits included in the pull request,
not a title of the pull request because they need to support triggered
on push event. On the other hand, the current implementation of atlantis
doesn't triggered on push event and doesn't have all commits on open
event. To simplify the implementation, I think checking the title is
reasonable. Of course it's possible to get all commits included in the
pull request dynamically via additional API calls, please let me know if
we should check commit messages instead of the title.

The original feature request said that the 'keyword' could be
configurable, but I don't think most of users including me need such a
flexibility. So the initial implementation embeds keywords in source. If
someone need to be configurable, feel free to open another feature
request.
@minamijoyo minamijoyo force-pushed the skip-pr-based-on-title branch from 7f53c38 to 2b3842a Compare November 14, 2022 02:06
@minamijoyo
Copy link
Contributor Author

Hi @nitrocode, I've resolved the conflict.

As already mentioned, I no longer need this feature myself after waiting a year for a review. If label support is needed as a requirement for merging, I would like to take it over to someone who needs it.

Copy link
Member

@nitrocode nitrocode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@minamijoyo thank you for fixing the conflict. Approved.

cc: @jamengual please re-review.

@jamengual
Copy link
Contributor

to be honest I'm not so keen to add this feature if a label makes more sense or a combination of both.

@nitrocode
Copy link
Member

Ok fair enough. Thank you everyone for contributing and thank you minamijoyo for your patience. Let's close this for now

@nitrocode nitrocode closed this Nov 14, 2022
@dbarranco
Copy link

to be honest I'm not so keen to add this feature if a label makes more sense or a combination of both.

Is there any alternative nowadays to skip the auto-plan?
I'm also interested in this feature

@jamengual
Copy link
Contributor

jamengual commented Nov 14, 2022 via email

@danielfm
Copy link
Contributor

danielfm commented Nov 14, 2022

Atlantis doesn't run the autoplan if the PR is opened as a draft, so you can trigger plans on demand via the atlantis plan comment, fix any issues, and when the PR is ready, add the reviewers and mark the PR as ready for review.

This workflow might not work for everyone, but it's working okay for us.

@minamijoyo
Copy link
Contributor Author

Hi all,

I remembered one thing I forgot to mention, in my original use case, a pull request was created from the GitHub API, and as far as I know, there is no way to set a label when creating a pull request. Adding labels needs an additional issue API call, which could be too late to skip CI.

https://docs.github.com/en/rest/pulls/pulls#create-a-pull-request
https://docs.github.com/en/graphql/reference/input-objects#createpullrequestinput
https://docs.github.com/en/rest/issues/labels#add-labels-to-an-issue

I'm just sharing what I have noticed about the technical limitations. What makes sense depends on your use cases, so I'll leave it to people who need this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality/enhancement waiting-on-review Waiting for a review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants