ECT-1803 Security Updates #7
Conversation
docker_push.sh
Outdated
| VERSION="$(git describe --tags --match 'v*')" # Get version tag from git | ||
| VERSION=${VERSION#v} # Remove leading 'v' | ||
|
|
||
| read -p "This action will build and publish docker image version $VERSION (this will automatically proceed in 15 seconds)" -t 15 |
There was a problem hiding this comment.
If the 15 second timeout occurs, read will return a non-zero exit status that will cause the script to terminate before the push due to set -e. Also, this will error (and terminate the script due to set -e) if stdin isn't present or has been redirected from /dev/null. I'm not sure exactly what sort of environment our automated systems might provide, but it might be safer to just echo the warning, and sleep 15.
There was a problem hiding this comment.
Obviously echo + sleep 15 doesn't allow the user to quickly bypass the pause. (But hey, maybe we want people to have time to think long and hard about the push.) But if we want the bypass capability, we could do the read followed by || true so it is immune to the set -e.
There was a problem hiding this comment.
TBH, I'm not sure if I'm correct, but I guess waiting doesn't make sense in an automated setup. So, maybe provide an optional argument for the script (e.g. -b or --batch), where you just echo and run the command without waiting.
And when running without that option, you might wait for 15 seconds or you might even wait forever for user's reply.
There was a problem hiding this comment.
We're not likely to use this script in automated scenarios, so it's a bit moot.
| VERSION="1.2.5" | ||
| set -e | ||
|
|
||
| VERSION="$(git describe --tags --match 'v*')" # Get version tag from git |
There was a problem hiding this comment.
I was thinking maybe its better to have a more specific match pattern; at least making sure that 'v' is followed by a number. If we think there might be non-version tags, it's likely that a tag can be a word starting with 'v'.
| VERSION="$(git describe --tags --match 'v*')" # Get version tag from git | |
| VERSION="$(git describe --tags --match 'v[[:digit:]]*')" # Get version tag from git |
There was a problem hiding this comment.
Sorry, didn't see this until now. The match flag takes a glob, not a regex.
--match <pattern>
Only consider tags matching the given glob(7) pattern, excluding the "refs/tags/" prefix.There was a problem hiding this comment.
Sorry for the late comment. Yeah, if it was a regex we could actually match the complete version pattern. But, that's also a glob pattern to just match the first character of the tag after 'v' to be a number. According to 'man 7 glob', the [[:digit:]] pattern is also recognized, so I used it instead of [0-9] range.
Pull docker tag from git