integrations / terraform-provider-github

Terraform GitHub provider
https://www.terraform.io/docs/providers/github/
MIT License
905 stars 746 forks source link

feat: support signed commits for resource 'github_repository_file' #2102

Open wparr-circle opened 9 months ago

wparr-circle commented 9 months ago

Resolves #879


Before the change?

After the change?

image

(note unverified due to github not having public key of the pgp key used in test and author/committer being mismatched).

Pull request checklist

Does this introduce a breaking change?

Please see our docs on breaking changes to help!


nickfloyd commented 9 months ago

Hey @wparr-circle Thanks for the contributions here. Please run lint when you get the chance! It looks like CI is getting hung up on that. Thanks.

wparr-circle commented 9 months ago

Ran against linters now @nickfloyd! Thanks :)

kfcampbell commented 9 months ago

@wparr-circle do you mind explaining more about the below part of your writeup? I'm not sure I understand, sorry.

where we manipulate a commit and push it to the reference rather than using the contents API to provide a higher level interface.

wparr-circle commented 9 months ago

@kfcampbell Sure no problem! Sorry if I wasn't clear. Current implementation of this resource is utilising the GitHub Contents API. We get some verified signature support using this like auto sign for bots/github actions. However, for the use case of GPG based signing - we can't leverage the contents API. Rather we need to manipulate the git tree directly.

Does that help explain?

I left the old contents API way of working as the default behaviour, because of the size of change creeping up.

marek-karwacki-rdx commented 7 months ago

Hi, is there a timeline on this feature? Thanks

M0NsTeRRR commented 4 months ago

Hello, is something missing @kfcampbell to get this merged ?

ahanafy commented 3 months ago

Landed at this PR after realizing the resource doesn't support signing. @kfcampbell do you have any direction or feedback on this PR to get it completed? Trying to get an idea on whether this feature is planned for this resource or if its not achievable?

kfcampbell commented 3 months ago

@wparr-circle we just merged #2100 and we're ready to go with this before we cut a release with your new features in it. I didn't anticipate that there might be a merge conflict between your two PRs though, and I'm wondering if you'd feel comfortable resolving the conflict. If you'd prefer that Nick and I do it, please let me know and we'll get to it!

Thanks for the contributions, and I'm looking forward to seeing commit signing in the wild.

wparr-circle commented 2 months ago

Hey @kfcampbell @nickfloyd thanks for getting around to looking at these 👀 Let me quickly fix the conflicts

ahanafy commented 1 month ago

@wparr-circle / @nickfloyd / @kfcampbell I think this PR will need to have conflicts resolved and review occur pretty tightly together in order to get it past the goal. Thoughts?

rwblokzijl commented 2 weeks ago

@nickfloyd / @kfcampbell Thank you for all the work on this provider! At the risk of sounding too pushy, we would love to see this PR merged. Would it be possible to review again?