Use the LocalStack pro image by default#63
Conversation
|
The ephemeral instance for the application preview has been shut down |
alexrashed
left a comment
There was a problem hiding this comment.
Lookin' great! Thanks! This makes it very explicit that the auth token is expected in the future in just a minimal change, while listing and leaving the next steps for future PRs. 💯
| description: 'Deprecated. Whether to use LocalStack Pro (requires a valid CI Auth Token)' | ||
| required: false | ||
| default: 'false' | ||
| default: 'true' | ||
| deprecationMessage: 'use-pro is deprecated and will be removed in a future release. The pro image is now the default.' |
There was a problem hiding this comment.
praise: TIL GitHub actions have an explicit declaration of deprecation for variables 🤩
| run: | | ||
| if [ "$USE_PRO" = true ]; then | ||
| if [ "x$LOCALSTACK_AUTH_TOKEN" = "x" -o "x$LOCALSTACK_API_KEY" = "x" ]; then | ||
| if [ "x$LOCALSTACK_API_KEY" != "x" ]; then |
There was a problem hiding this comment.
thought (non-blocking): This is an interesting way of testing if a variable is empty or not (but I se this was already the case before, so totally out of scope). 😅
I guess I would rather use -z (check if a string is empty) or -n (check if a string is non-empty) as outlined in the Bash Reference Manual.
There was a problem hiding this comment.
As you said, I mostly just kept it to stay close to the current version, especially since I expect a more fundamental rewrite here anyway. 👍
Motivation
With our image consolidation the previous PRO image will become the default and both
localstack/localstack&localstack/localstack-prowill start requiring authentication. This PR is a first step to default to authenticated use by defaulting to the (legacy)localstack/localstack-proimage.Fixes DRG-527
Changes
Changes are kept minimal while having clear (deprecation) messaging to support the image consolidation.
Follow-ups
Following up to this we need to:
use_procompletely and explicitly warn when no auth is configured.localstack/localstack, but require authentication.These will arrive fairly soon in a v0.4.0 release after the image consolidation.