ci: add script to open pr to update bazel builder version #92348
Conversation
|
This change is |
healthy-pod
force-pushed
the
open-pr
branch
24 times, most recently
from
d5d08c5 to
c7a7417
Compare
|
Corresponding build config: Open New Bazel Builder Image PR |
| configure_git_ssh_key | ||
| mv .cockroach-teamcity-key /tmp/.cockroach-teamcity-key | ||
|
|
||
| git_ssh clone "ssh://[email protected]/cockroachdb/cockroach.git" "/tmp/cockroach" && cd "/tmp/cockroach" |
There was a problem hiding this comment.
Can you use a temp directory (mktemp -d) instead of hardcoded /tmp/cocroach for a few reasons:
- harder to predict (and attack)
- no need to rely on absence of the directory - you would need to make sure it doesn't exist or empty before cloning - otherwise git fails.
Also, can you rm -rf that directory, so we clean after it's done. Maybe add all needed cleanup command to a function and trap it? But we should be careful using trap - IIRC there is one already sourced, and only one can be used.
There was a problem hiding this comment.
Moved the clone and key to a temp dir which gets cleaned at the end
build/teamcity/internal/cockroach/build/ci/open-new-bazel-builder-image-pr.sh
Show resolved
Hide resolved
| wget -O /tmp/gh.tar.gz https://github.com/cli/cli/releases/download/v2.13.0/gh_2.13.0_linux_amd64.tar.gz | ||
| echo "9e833e02428cd49e0af73bc7dc4cafa329fe3ecba1bfe92f0859bf5b11916401 /tmp/gh.tar.gz" | sha256sum -c - | ||
| tar --strip-components 1 -xf /tmp/gh.tar.gz | ||
| export PATH=$PWD/bin:$PATH |
There was a problem hiding this comment.
Or you use the absolute path when you call gh.
There was a problem hiding this comment.
One more nit. Can you move the gh setup part up, so we fail early if we cannot install it for some reason.
| git config --global user.email "[email protected]" | ||
| git config --global user.name "cockroach-teamcity" | ||
| configure_git_ssh_key | ||
| mv .cockroach-teamcity-key /tmp/.cockroach-teamcity-key |
There was a problem hiding this comment.
Why do you need to move the key under /tmp? I don't think it'll be cleaned up in this case.
There was a problem hiding this comment.
Moved it to the temp dir along with the clone
build/teamcity/internal/cockroach/build/ci/open-new-bazel-builder-image-pr.sh
Show resolved
Hide resolved
| wget -O /tmp/gh.tar.gz https://github.com/cli/cli/releases/download/v2.13.0/gh_2.13.0_linux_amd64.tar.gz | ||
| echo "9e833e02428cd49e0af73bc7dc4cafa329fe3ecba1bfe92f0859bf5b11916401 /tmp/gh.tar.gz" | sha256sum -c - | ||
| tar --strip-components 1 -xf /tmp/gh.tar.gz | ||
| export PATH=$PWD/bin:$PATH |
There was a problem hiding this comment.
One more nit. Can you move the gh setup part up, so we fail early if we cannot install it for some reason.
build/teamcity/internal/cockroach/build/ci/open-new-bazel-builder-image-pr.sh
Show resolved
Hide resolved
|
|
||
| git_ssh() { | ||
| # $@ passes all arguments to this function to the command | ||
| GIT_SSH_COMMAND="ssh -i /tmp/.cockroach-teamcity-key" git "$@" |
There was a problem hiding this comment.
I don't think this command is going to work, because the key lives in a different spot now.
I have a suggestion. How about using GIT_SSH_COMMAND="ssh -i $dir/.cockroach-teamcity-key" git "$@" instead. This way you don't need to move the key anywhere.
| git config --global user.name "cockroach-teamcity" | ||
| configure_git_ssh_key | ||
| WORKDIR="$(mktemp -d ./workdir.XXXXXX)" | ||
| mv .cockroach-teamcity-key $WORKDIR/.cockroach-teamcity-key |
There was a problem hiding this comment.
If the suggestions above make sense, you can get rid of the mv line.
Release note: None Part of: CRDB-11061
|
TFTR! bors r=rail |
|
Build succeeded: |
Release note: None
Part of: CRDB-11061