-
Notifications
You must be signed in to change notification settings - Fork 582
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
refactor: 'create_initial_user.sh' helper script #59
Conversation
Codecov Report
@@ Coverage Diff @@
## main #59 +/- ##
=======================================
Coverage 71.91% 71.91%
=======================================
Files 79 79
Lines 2902 2902
Branches 49 49
=======================================
Hits 2087 2087
Misses 641 641
Partials 174 174
Continue to review full report at Codecov.
|
scripts/create_initial_user.sh
Outdated
PASSWORD="${PASSWORD:-p@ssword1}" | ||
|
||
curl -X POST \ | ||
-d "{\"email\": \"$EMAIL\", \"username\": \"$USERNAME\", \"organization\": \"$ORGANIZATION\", \"password\": \"$PASSWORD\"}" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can probably make this look more readable if you quote with ''
on the outside, you can see coderd-setup for examples: https://github.com/coder/m/blob/b397beb481dfc03192b1b7af53f681330bccfb0f/devbin/coderd-setup#L97-L104 (this example needs to double escape due to run_trace, but you probably don't need to do that)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I actually tried this first! But when I ran lint
, I got this shellcheck
error:
In scripts/create_initial_user.sh line 11:
-d '{"email": "$EMAIL", "username": "$USERNAME", "organization": "$ORGANIZATION", "password": "$PASSWORD"}' \
^-- SC2016: Expressions don't expand in single quotes, use double quotes for that.
I'm open to ideas to make this cleaner though, the current approach is not so readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bash will only interpolate $-variables inside ""s, but double quotes don't do what they normally do when inside a single-quoted expression. So the solution is to exit the single-quote context, add the quoted variable, and then re-enter. Something like this:
-d '{"email": "'"$EMAIL"'", "username": "'"$USERNAME"'", "organization": "'"$ORGANIZATION"'", "password": "'"$PASSWORD"'"}' \
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another option would be making this happen in the develop.sh
script. I'd be game for that!
scripts/create_initial_user.sh
Outdated
|
||
EMAIL="${EMAIL:-admin@coder.com}" | ||
USERNAME="${USERNAME:-admin}" | ||
ORGANIZATION="${ORGANIZATION:-default}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make this something like ACME-Corp
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, made the change in a5427bb
scripts/create_initial_user.sh
Outdated
EMAIL="${EMAIL:-admin@coder.com}" | ||
USERNAME="${USERNAME:-admin}" | ||
ORGANIZATION="${ORGANIZATION:-default}" | ||
PASSWORD="${PASSWORD:-p@ssword1}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we use password
instead? Nobody should be using this in a secure way anyhow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing 👍 Fixed in a5427bb
I've been running this
curl
command a bunch of times every day (whenever I restart my dev server) - so I thought it might nice to have as a utility. Let me know if you'd prefer a different place to put this, @kylecarbs !