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

Support ENV based database credentials #2

Closed
pirog opened this issue Apr 1, 2016 · 8 comments
Closed

Support ENV based database credentials #2

pirog opened this issue Apr 1, 2016 · 8 comments

Comments

@pirog
Copy link
Contributor

pirog commented Apr 1, 2016

Similar to Wordpress, Backdrop is a CMS firmly rooted in the 2010's. As a result you can set your database creds in the environment instead of in a flat file. Docker tooling makes this pretty easy and awesome and a good working example is over at https://hub.docker.com/_/wordpress/

Here is a first pass on what ENV might be worth adding

-e BACKDROP_DB_HOST=... (defaults to the IP and port of the linked mysql container)
-e BACKDROP_DB_USER=... (defaults to "root")
-e BACKDROP_DB_PASSWORD=... (defaults to the value of the MYSQL_ROOT_PASSWORD environment variable from the linked mysql container)
-e BACKDROP_DB_NAME=... (defaults to "wordpress")

@jenlampton @quicksketch i would totally be down to add support for this so in kalabox spin ups that are not tied to pantheon the user doesnt get annoyed entering their DB credentials like this is drupal 8 or something.

Presumably we could take the user defined ENV responses and merge them with defaults we set and then construct the BACKDROP_SETTINGS json and inject it into $_SERVER. That would provide some limitations to the user re: using BACKDROP_SETTINGS for other purposes.

Anyway, LMK!

@jenlampton
Copy link
Member

Is what you need already in https://github.com/backdrop-ops/backdrop-pantheon ? If so, maybe the answer is to start with that repo instead?

@pirog pirog mentioned this issue Apr 1, 2016
@pirog
Copy link
Contributor Author

pirog commented Apr 1, 2016

I think we would need to do something like what @populist did on the pantheon-side in backdrop-ops/backdrop-pantheon#1 just on the docker-image side here.

Let me do a quick PoC and on what this would look like!

@quicksketch
Copy link
Member

For background on the current $_SERVER['BACKDROP_SETTINGS'] functionality, see backdrop/backdrop-issues#281. (Note necessarily for @pirog, but for my own reference. 😉)

Presumably we could take the user defined ENV responses and merge them with defaults we set and then construct the BACKDROP_SETTINGS json and inject it into $_SERVER.

I think that having both $_SERVER['BACKDROP_SETTINGS'] and $_ENV options sounds fine, though I don't think we should be merging them into each other. I'd prefer just checking each of them separately as different options. I'm not sure which should win if both were specified, I'd lean towards $_ENV taking precedence. Of course, such a situation would be rather unlikely.

I'm also not sure about the granularity of such options. I think I might prefer a single string for database as we have in settings.php:

-e BACKDROP_DB_STRING="mysql://user:[email protected]/backdrop"

Though I am ignorant of whether that is considered atypical compared to other systems.

@pirog
Copy link
Contributor Author

pirog commented Apr 1, 2016

sorry sorry, by ENV i meant the environmental variables defined by the user in docker via the -e flag. These would override the defaults WE define in the image. The result of these things is then set as $_SERVER['BACKDROP_SETTINGS'] and $_ENV['BACKDROP_SETTINGS'] as appropriate. This approach is modeled heavily after what docker does with their wordpress image.

I'm with you on the granularity but i think its probably less brittle to take the granular pieces and build up BACKDROP_SETTINGS vs parsing a connection string in BASH and then doing the same.

Going to push up what i had in mind in a sec for you to scope out.

@pirog
Copy link
Contributor Author

pirog commented Apr 1, 2016

@quicksketch ^ check that out.

@quicksketch
Copy link
Member

sorry sorry, by ENV i meant the environmental variables defined by the user in docker via the -e flag.

Ah! The PR makes it all much more clear. Thanks! Seems like a no-brainer to me. If this ready, feel free to merge it!

@pirog
Copy link
Contributor Author

pirog commented Apr 1, 2016

bash > my english i guess ? :)

i def think this is certainly ready enough so ill merge and we can go from there

pirog added a commit that referenced this issue Apr 1, 2016
#2: Add support for DB creds in the ENV because its 2016
@serundeputy
Copy link
Member

This is merged. closing. reopen if necessary.

sthomen pushed a commit to sthomen/backdrop-docker that referenced this issue Dec 20, 2019
…ts 2016" because it didn't make sense even in 2016

This reverts commit 2b27e44.
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

4 participants