Skip to content
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

envsubst strips content from SQL #379

Open
digitalicon opened this issue Jan 8, 2022 · 5 comments
Open

envsubst strips content from SQL #379

digitalicon opened this issue Jan 8, 2022 · 5 comments

Comments

@digitalicon
Copy link

Chased this around for a while.

*.sql) echo "$0: running $f"; cat $f| envsubst | tee | mysql -u root -p${MARIADB_ROOT_PASSWORD}; echo ;;

When importing SQL dump via a volume (eg in CI), any content with a $ is stripped if it doesn't have a corresponding envar.

Example of issue:

echo 'query SampleThingy($path: String!)' > sample.sql
cat sample.sql | envsubst 

Result:

query SampleThingy(: String!)

This wreaks havoc with automated persisted GraphQL queries, and probably any other kind of wysiwyg content.

What is the purpose of requiring envsubst in a db import? Could be be more specific with the use and only replace defined vars?

@tobybellwood
Copy link
Member

hmm - looking at Lagoon itself reveals why we do it - the api-db image inherits the mariadb one
https://github.com/uselagoon/lagoon/tree/main/services/api-db/docker-entrypoint-initdb.d

I wonder if there's a less greedy way than envsubst?

@digitalicon
Copy link
Author

Seems if you define what you want done, it can be less greedy
https://stackoverflow.com/a/24964184

@digitalicon
Copy link
Author

Or possibly you could introduce .sql.gz importing (similar to mariadb on dockerhub) that didnt have envar replacement

*.sql.gz)    echo "$0: running $f"; gunzip -c $f| mysql -u root -p${MARIADB_ROOT_PASSWORD}; echo ;;

@rocketeerbkw
Copy link
Member

I think importing data (like persisted queries) is a bit out of scope for that mechanism. In lagoon we use it to bootstrap the database structure and run migrations only (see link in previous comment from @tobybellwood). Since the queries are always hard coded, we'd never run into this kind of issue (I imagine it'd be a problem for user generated content too if you were importing that?).

You can get around this by moving the .sql out of this folder and adding a sql-import.sh script instead which can read the file from another directory. This feels like the more "correct" approach and potentially more secure since you aren't dynamically adding .sql/.sh scripts that run on boot to your image anymore.

On the Lagoon side, we added this to facilitate non-hardcoded passwords in the lagoon images, and we use the substitutions in a migration. It would be possible for us to remove the envsubst from .sql entirely and do that migration in a .sh in api-db which would already have access to the variables we need. This would be a BC break in case anyone else was dependent on envsubst. Same BC break if we hardcode a list of environment variables because it's possible that users added their own custom one to the image and wouldn't be included in our hardcoded list.

@digitalicon
Copy link
Author

digitalicon commented Jan 8, 2022

Edge cases aside, stripping content is the core issue here. The script actively removes content, which it shouldn't? envsubst might not be the way. Possibly you need a script that does the substitution without removing content?

Edit: I'll work around it how you have suggested, but the issue possibly still stands

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants