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

add filter_command (e.g. for gpg signature validation) (git only) #823

Merged
merged 3 commits into from
Jul 22, 2020

Conversation

mhumpula
Copy link
Contributor

@mhumpula mhumpula commented Jul 7, 2018

please consider for merging.

motivation

In my current puppet setup, the git repository is hosted on the shared git manager with wide user base. Though the repository has correct permissions configured I would like to be able to add some extra checks. Specifically stop the deployment of the code, that is not signed with proper gpg key by using built-in git gpg funkcionality.

The check script is easy, but I need to intercept the r10k deployment between fetching the latest repository state to cache and the actual deployment to puppet environments. So I came up with the branch filtering hooks, which IMHO can be used by larger user base for filtering branches and environments based on their requirements.

implementation

I've tried to model this in the same fashion as ignore_branch_prefixes. I've only done git implementation, because I know very little about the svn.

@binford2k binford2k changed the title add filter_command directive (git only) add filter_command (e.g. for gpg signature validation) (git only) Mar 26, 2019
@anarcat
Copy link

anarcat commented Jun 30, 2020

any reason why this has not been merged yet? interest?

does this work with forge modules?

@mhumpula
Copy link
Contributor Author

mhumpula commented Jul 1, 2020

@anarcat I'm using this for two years now with simple script check https://gist.github.com/mhumpula/10b7a333d2a9a470f5e0e44beb5c91f0 and it works ok for my requirements. The checkout of puppet modules is done in different part of the code I believe, so it's not affected by this change.

Copy link

@dhollinger dhollinger left a comment

Choose a reason for hiding this comment

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

Just one little change in the documentation for the change. Otherwise looks good.

doc/dynamic-environments/configuration.mkd Outdated Show resolved Hide resolved
Copy link
Contributor

@binford2k binford2k left a comment

Choose a reason for hiding this comment

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

I like it. Small typo fixes.

@mwaggett
Copy link
Contributor

hey @mhumpula would you mind adding a changelog entry to the 'Unreleased' section describing this change?

also @dhollinger @binford2k would love if y'all could take another look at this.

@binford2k
Copy link
Contributor

@mhumpula Can you also add some docs to clarify that this only works when using the git source? We'll file a ticket for the missing svn counterpart when it's merged. Otherwise, 👍

@mhumpula
Copy link
Contributor Author

@binford2k note about git added, @mwaggett added entry to changelog, but I'm not sure if it is in format you like.

@mwaggett
Copy link
Contributor

hey @mhumpula! Would you mind rebasing against the master branch? There have been many updates to the changelog since this PR was first opened. 😅 And then, usually, we add the PR as a link at the end of the note if there wasn't a ticket the work was based on, for example: https://github.com/puppetlabs/r10k/blob/master/CHANGELOG.mkd#350

Let us know if you need help with any of that! And thanks again for your contribution!

@binford2k
Copy link
Contributor

I went ahead and resolved the conflicts for you. One final review from @dhollinger and we'll get this merged! Thanks so much for the contribution @mhumpula 🎉

@binford2k binford2k merged commit 2eb088a into puppetlabs:master Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants