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

ci: use env variable to store branch name #26779

Merged
merged 1 commit into from
Jul 18, 2023
Merged

Conversation

ferozsalam
Copy link
Contributor

Instead of using the branch name directly in the run command.

Instead of using the branch name directly in the run command.

Signed-off-by: Feroz Salam <feroz.salam@isovalent.com>
@ferozsalam ferozsalam added the release-note/ci This PR makes changes to the CI. label Jul 12, 2023
@ferozsalam ferozsalam requested review from a team as code owners July 12, 2023 08:58
if: ${{ steps.cilium-runtime-tag-in-repositories.outputs.exists == 'false' || steps.cilium-builder-tag-in-repositories.outputs.exists == 'false' }}
run: |
git diff HEAD^
git push https://x-access-token:${{ steps.get_token.outputs.app_token }}@github.com/${{ env.QUAY_ORGANIZATION }}/cilium.git HEAD:${{ github.event.pull_request.head.ref }}
git push https://x-access-token:${{ steps.get_token.outputs.app_token }}@github.com/${{ env.QUAY_ORGANIZATION }}/cilium.git HEAD:"$REF"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need quotes here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The quotes will prevent splitting of the variable value, which prevents command injection.

Copy link
Contributor

@viktor-kurchenko viktor-kurchenko left a comment

Choose a reason for hiding this comment

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

LGTM.

@ferozsalam
Copy link
Contributor Author

As this is a CI change I don't believe this would benefit from running the extra /test suite. Marking as ready-to-merge.

@ferozsalam ferozsalam added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 13, 2023
@aditighag aditighag merged commit 22e066d into main Jul 18, 2023
56 checks passed
@aditighag aditighag deleted the pr/github-action-env-var branch July 18, 2023 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants