Random password generation occasionally suffers from incompatible characters #24
Comments
|
Hi @werenall do you get the error in the "shell phase" (while running the It's to be able to narrow down which component is "holding it wrong". With proper quoting / handling of the value (which we should be doing, but apparently are not), almost any character in the ASCII range (maybe U+0000 and U+001F would be the exceptions here) should be usable. Granted, using characters in the control characters range would be a UX disaster (you won't see them easily, and typing them is a pain in the ass), but we don't use such characters. Thanks a lot in advance! |
|
Local development starts just fine (because it doesn't really need to assume that test env role?) It's the EB that crashes. I was getting Bad Gateway errors and upon logs inspection I realised that Anyway, changing to a very long but no special characters password worked. So happy and excited to move forward now 🦜 |
|
@werenall Thanks a lot for the feedback! So it's actually AWS Elastic Beanstalk the one choking. We'll have a look into that. We are probably missing some quoting in the code that deals with setting up the environment variables from the values stored in AWS Parameter Store. Nice finding! Again, thanks a lot! |
|
@werenall After analyzing this, it seems to be a change in the semantics in the "environment files" in VAR=$OTHERVARwould make It seems that our code pre-dates those changes in the documentation. In addition to that, we seem to be using older versions of We'll see what we can do, as we think it will not be possible to support both older versions and newer versions of |
Some background for the changes introduced in this commit. The unexpected behavior detected by @werenall in #24 seems to be related with a change in the semantics of the processing of "environment files" in docker-compose (and `docker compose`). In older versions, variable definitions didn't have any kind of expansion / interpolation. E.g.: VAR=$OTHERVAR would make VAR have exactly the string `$OTHERVAR` (verbatim) as its value. At some point, this behavior was changed (but not documented!) in docker-compose. But it was changed in an inconsistent way (different versions having slightly different behaviors). And in Sept 2022, the documentation was updated to reflect the changes, and make them the official behavior. It seems that our code for the creation of "environment files" pre-dates those changes in the documentation. In addition to that, we seem to be using older versions of docker-compose (i.e., not the "latest and greatest"), so we didn't catch some edge cases. Given that: - the minimum required docker-compose version for HOP is 1.27.0, - version 1.27.0 is the first version that fully implements the "compose specification"[1], - and that the "compose specification" is where the change in the expansion / interpolation is standardized, we have decided to quote the values of the variables in the "environment file" we produce. For that, we need to make sure we quote any single quotes that are part of the original value, which in turn means we need to quote any quotes that were part of the original value. [Re: #24] [1] https://compose-spec.io/
|
Closing as fixed in f4b8b88 |
When we fixed issue #24, we focused on the AWS ElasticBeanstalk use case (where we populate the .env file from SSM Parameter Store). But it turns out we missed the other big use case where the same problem may happen: when creating the local development .env file (or the test / production .env files for on-premises deployments). This commit addresses that second use case. There is a bit of code churn in this commit because we wanted to handle both use cases using the same utility functions (so we apply the quoting policy in a single place). That in turn meant that we had to refactor a bunch of SSM Parameter Store related code, because in that case we directly used the data in the shape that Parameter Store returned. And that was incompatible with the data shape used in the rest of the code dealing with docker-compose environment variables.
|
Reopening the issue, because we've hit another different use case that breaks with the current implementationm |
hop-cli/src/hop_cli/util/random.clj
Line 22 in 605f0e3
If you're unlucky enough (like I was) you'll get APP_DB_PASSWORD that looks like this:
ABCD=EFG)#)HIJ!(GRJ3P$oX'. I sanitized it mostly but the unchanged part are the last 3 characters:$oX. Apparently that doesn't go well with env variables readers because I got errors like:What I ended up doing was changing the password in AWS Parameter Store and altering the role:
The build is in progress now. Will post here with updates.
The text was updated successfully, but these errors were encountered: