-
Notifications
You must be signed in to change notification settings - Fork 136
[AINFRA-1533] Adopt git-crypt in this repo #14979
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
8c092f4
d57d039
764ba71
8683c1c
f95e392
0ce40a7
2921a67
989cef3
a1db855
b71c497
04681db
cd7e854
6ed308b
51542b1
3c35f48
fc3ab33
67f9404
e5bb8b6
6816a2a
b6ecd9c
66fa753
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| #!/bin/bash | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| echo "$GIT_CRYPT_ENCRYPTION_KEY" | base64 -d | git-crypt unlock - |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
| # This file is `source`'d before calling `buildkite-agent pipeline upload`, and can be used | ||
| # to set up some variables that will be interpolated in the `.yml` pipeline before uploading it. | ||
|
|
||
| export CI_TOOLKIT="automattic/a8c-ci-toolkit#5.4.0" | ||
| # "git-crypt-unlock" branch / https://github.com/Automattic/a8c-ci-toolkit-buildkite-plugin/pull/195 | ||
| export CI_TOOLKIT="automattic/a8c-ci-toolkit#0a3f10921096cee57c18ac5667fc64c1aaad4a7d" | ||
|
Comment on lines
-6
to
+7
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎗️ TODO: Revert back to pointing to a tag version once Automattic/a8c-ci-toolkit-buildkite-plugin#195 is merged and we have an official new version of the |
||
| export TEST_COLLECTOR="test-collector#v1.10.1" | ||
| export CLAUDE_PLUGIN="claude-summarize#v1.1.0" | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,13 @@ | ||
| RELEASE-NOTES.txt merge=union | ||
|
|
||
| .configure-files/*.enc binary | ||
| ######################################### | ||
| # Secrets files encrypted with git-crypt | ||
| ######################################### | ||
|
|
||
| secrets.properties filter=git-crypt diff=git-crypt | ||
| sentry.properties filter=git-crypt diff=git-crypt | ||
| google-services.json filter=git-crypt diff=git-crypt | ||
| firebase.secrets.json filter=git-crypt diff=git-crypt | ||
| google-upload-credentials.json filter=git-crypt diff=git-crypt | ||
| *.keystore filter=git-crypt diff=git-crypt | ||
| *.jks filter=git-crypt diff=git-crypt |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💭 Again thinking out loud, not sure if it's worth the trouble: what do you think of grouping encrypted / secret files in the same folder and making a convention out of this for all projects (well, kinda similar to what we had before but making it more obvious)?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I thought about that. I think that for some particular secret files, they do need to be at specific locations in the project structure for the compilation to work (e.g. For others ( The only trick is that if they were already existing in the repo a particular location on disk before that PR, and we move them elsewhere as part of this PR, we'd then need to add their old location back to I still see the appeal of having secret files grouped nicely in a dedicated folder rather than keeping them at the root of the repo still. But I'm not sure moving them is worth the potential risk (or having to keep legacy entries in
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
👍 I'm also leaning towards that. It's also annoying that they can't be in the same place, which would be the ideal scenario and perhaps would make the change worth it. |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💭 I was wondering if there could be a way to be sure, in reviews, that this has in fact been encrypted or not specially given files like this are binary files. Then I've noticed all
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean like I do here? 😛 Indeed maybe we can write a Dangermattic plugin to detect which file in the PR are encrypted and add an inline comment on the file if so as an extra information? Is that what you meant? As for manually testing locally if a file is properly encrypted before pushing a commit to the remote, one can use
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ha, yes, I realized that function was doing that after I posted the comment 😂
Yeah, though at the same time I find it a bit difficult to do that in a systemic way that will be useful (as we don't add secrets that often)...
👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having the repo
git-crypt-unlocked during the branch dance that is done internally bycomment_with_manifest_diffcausedYour local changes to the following files would be overwritten by checkoutissues, especially if thegit-crypt'd files listed in.gitattributeson the HEAD branch are not the same as the ones in the BASE branch1Since we don't need any secret in practice to generate the manifest and call
process{variant}Manifest, the solution is simple: just don't bother unlocking the repo's secrets for that task.A better long-term solution to make
comment_with_manifest_diffmore resilient to situations like this would be to make it usegit worktreeinstead of switching branches in-place:git worktree add $TMP_DIR_FOR_BASE $BASE_BRANCH && cd $TMP_DIR_FOR_BASEthen run./gradlew process{variant}Manifesttherecd $CHECKOUT_DIR && rm $TMP_DIR_FOR_BASE && git worktree prunethen run./gradlew process{variant}ManifestthereThat way each checkout is done in independent folders, eliminating the risk of conflicts during the branch dance.
Footnotes
like will be the case during that transition to
git-crypt, or when we'll add a new secret file, especially if that secret file previously existed unencrypted in the BASE branch as an example file for external contributors I think? ↩