Split repository secrets to variables in GH Actions #137
Conversation
tnevrlka
force-pushed
the
github-unmask-variables
branch
from
c4e6504 to
0decbdb
Compare
tnevrlka
changed the title
tnevrlka
changed the title
| if: '!isGitHub' | ||
|
|
||
| - name: QUAY_IO_CREDS_USR | ||
| if: '!isJenkins' |
There was a problem hiding this comment.
I'm a bit confused about the various conditionals in this data file.
Do we need a better mechanism than isJenkins, isGitHub, isGitLab? Maybe a boolean that flags each as sensitive information?
There was a problem hiding this comment.
Yeah, I'm not the biggest fan of that too.
Took me a lot of time to understand why/how it works and it's pretty fragile:
When trying to render it locally, the state of the variables changes based on targetFile=<value> key-value pair where the value has to match this...Jenkins has to start with a capital J for some reason for example...
To be honest, I took all of it from the original data.yaml. I wasn't sure if I could come up with something different quick enough and not spend time that could be spent on the Azure DevOps pipelines effort instead
There was a problem hiding this comment.
We don't need to address this as part of this PR. We can come back to it.
There was a problem hiding this comment.
I've re-read this thread and I believe that the isXXX mechanism is used as a workaround for various quirks of the different CI mechanisms - mainly for commenting out the variables.
The masked/unmasked logic should be handled by the split to xxx_secrets and xxx_variables where secrets should be masked and variables unmasked in all CIs.
- GitHub should be handled in this PR by using the repository variables functionality
- GitLab should decide on masked/unmasked during the secret creation
- Jenkins will have to be addressed, my current idea is to use
credentials()for secrets (like it already is anyway) and global variables like I'm testing
|
Make sure you rebase. I just merged some updates to the konflux build pipeline that should fix the ec failure reported in the CI checks here. |
tnevrlka
force-pushed
the
github-unmask-variables
branch
from
0decbdb to
10dae33
Compare
|
By the way, merging this would break rhtap-e2e. If there will be no further comments, I will create a PR to change the test behavior |
Sounds good. Converted to draft to avoid an accidental merge. Post back here when things are ready. |
tnevrlka
force-pushed
the
github-unmask-variables
branch
from
10dae33 to
db72ce8
Compare
ghub-set-vars currently sets every provided environment variable as a repository secret. GitHub automatically masks values of secrets with asterisks. Some of the environment variables are not confidential and should actually be shown to users. This can be done by setting the environment variables as 'repository variables' instead of 'repository secrets' in GitHub Signed-off-by: Tomáš Nevrlka <[email protected]> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
All environment variables used for builds are currently stored under the key 'build-secrets' Some of the environment variables are not actually secret and should be visible. Differentiate the environment variables that are not secret by adding them under a new key 'build_variables' Do the same with 'gitops_secrets' and 'gitops_variables' Signed-off-by: Tomáš Nevrlka <[email protected]> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
Previously, all environment variables were handled as secrets and handled as such in CI workflow definitions. Handle both variables and secrets in Jenkinsfile and GitHub Actions workflow generation templates. GitLab should not be affected due to a different way of handling variables. Signed-off-by: Tomáš Nevrlka <[email protected]>
tnevrlka
force-pushed
the
github-unmask-variables
branch
from
db72ce8 to
b3520c5
Compare
See individual commits for more details.
This PR is motivated by the need to expose some non-sensitive environment variables which were treated as secrets and thus masked with asterisks by GitHub in the GitHub Actions logs.
This is a problem in Jenkins too and will be addressed in another PR