linuxserver / docker-bookstack

A Docker container for the BookStack documentation wiki
GNU General Public License v3.0
747 stars 108 forks source link

Please update the documentation DB_PASS -> DB_PASSWORD, etc #129

Closed BloodyIron closed 1 year ago

BloodyIron commented 2 years ago

So I'm not building the image from a docker-compose, I'm using a k8s yaml manifest deployment, and pulling the image from here : https://hub.docker.com/r/linuxserver/bookstack

The thing is the documentation says the environment variables to use are:

  1. DB_USER
  2. DB_PASS

Which are factually incorrect.

My pod would never successfully connect to the database until I changed the declared environment variables to:

  1. DB_USERNAME
  2. DB_PASSWORD

Considering that I am nowhere near the first person to have this issue, it stands to reason the documentation is leading users astray, and should be updated to the actually correct information. Can we please have this corrected?

github-actions[bot] commented 2 years ago

Thanks for opening your first issue here! Be sure to follow the bug or feature issue templates!

ssddanbrown commented 2 years ago

Which are factually incorrect.

It's not factually incorrect, It's a little more nuanced than that. I commonly use those variables fine in docker-compose set-ups. See my comment here for an explanation into this.

BloodyIron commented 2 years ago

But I'm not using docker-compose at all, and the docker image does not successfully connect with the variables used as documented. It is factually incorrectly declared. Where exactly did you get the impression I'm using docker-compose? And even still, I had to go digging through a lot of other threads that this issue say they experienced to even find that changing the variables (as I described) was the solution.

This is a shortcoming of documentation, and I can't fathom why you think asking this change to happen is unacceptable. The variables documented don't work when pulling the image directly. This is the documented on the page for the docker images. Why would I bother doing a docker-compose if I can just pull the images directly that are already made, and follow the documentation on the page for that same image?

j0nnymoe commented 2 years ago

For what it's worth, those variables are converted to the correct ones during startup of the container. https://github.com/linuxserver/docker-bookstack/blob/master/root/etc/cont-init.d/50-config#L61

BloodyIron commented 2 years ago

Well if that's the expectation, well it's not happening. I passed Envs of DB_USER and DB_PASS and they weren't converted, and the app would not connect to the database until I set the passed Envs to DB_USERNAME and DB_PASSWORD. So either the documentation is inaccurate, or the actual automation of that isn't working as intended.

It's not like I didn't actually try here folks, and I'm nowhere near the first person to experience this issue. Not only do I see this issue come up in the "closed" section of issues on this repo, but I found multiple forum discussions elsewhere where people said change the Envs to the longer form and it worked for them.

So... what's more important, telling me I'm wrong, or investigating that there's something not quite right here? I'd appreciate the "invalid" label being removed, instead of assuming by default I'm just wrong (when, I'm not).

Roxedus commented 2 years ago

This is marked as invalid as it does not use the issue template.

BloodyIron commented 2 years ago

@Roxedus okay well I hear that. But I don't think this necessarily classifies as either a bug, nor a feature request. So I don't really see those templates being applicable here. Which would you prefer I use?

ssddanbrown commented 2 years ago

@BloodyIron

Where exactly did you get the impression I'm using docker-compose?

I didn't, I was just giving context to the environment I use those variables within, though their functionality should remain the same for whatever method you use to run the container.

... and I can't fathom why you think asking this change to happen is unacceptable.

No one has stated that.

The variables documented don't work when pulling the image directly.

They do, I just gave this a test to double check it.

As described in my comment linked previously, the scenario is a little nuanced. There can be scenarios where the variables won't take affect (Where a .env file exists with non-default DB values, often after first run). Using the full DB_PASSWORD variable works since it overrides those in the .env file. I'm not stating the current method of things is ideal, or that it could not be improved, but just trying to provide insight here.

BloodyIron commented 2 years ago

Okay so I think I'm getting unreasonably frustrated on this situation, and I want to apologise to all in this thread. My frustration comes from a simple "fix" where the documentation seems to not meet "reality"/"expectation". I'm sure I'm coming across as abrasive, and I'm sorry, I should "be better".

Now, to add some detail, and let's get a few things out of the way:

  1. I'm not using docker-compose (so that this is now explicit, and not implied)
  2. I'm using YAML manifest with "kind: deployment" into kubernetes, pulling the latest image from the linuxserver docker image repo (thanks for running that btw!)
  3. I am not passing an .env file, what I am doing is passing Environment Variables under "env:" as a subset of "containers:": in the YAML manifest.
  4. When I passed DB_USER and DB_PASS, it never worked, but the moment I changed it to DB_USERNAME and DB_PASSWORD, it "worked". "not working" as in "access denied" when trying to connect to the database, "working" as in the application connects to the database, actually provisions the database, and works "as intended" (so it seems).

Now, I don't know the justification for the use of DB_USER and DB_PASS for the docker-compose, or the documentation page on the docker image repo, but for me, those look to be the only variables that are communicated as what "should be used" so to say when using this docker image. And I think that updating the documentation to improve this communication would be beneficial to reduce future mistakes/tickets/issues/errors from whomever wants to just use the image in similar way to what I have been doing.

Perhaps that could involve a section talking about "YAML manifest for k8s" in addition to the "docker-compose" and then clarifies the DB_USERNAME and DB_PASSWORD distinction. But perhaps that is up to the decision of the linuxserver folks.

My original intent was to try and promote change so that the documentation, in whatever way, would be improved to avoid future confusion. I'd like to think that would be helpful to many, and reasonable. So at this point, are there any aspects for me to clarify? Are there disagreements about whether this is reasonable or not? Or...?

daufinsyd commented 1 year ago

I have the same error (tho using docker file).

Renaming to DB_USERNAME and DB_PASSWORD solved the issue.

Too bad to lose so much time figuring out what the problem is, whereas renaming the var in the doc would have made it just work.

Edit: I'm not using k8s

leozulfiu commented 1 year ago

Just had the same issue and changed to DB_USERNAME and DB_PASSWORD in my docker-compose.yml file and it worked. With the old env variables I just got a white page i.e. a 500 error. Below is the error I got in log/nginx/error.log:

2022/09/11 10:48:09 [error] 265#265: *1 FastCGI sent in stderr: "PHP message: PHP Fatal error:  Uncaught PDOException: SQLSTATE[HY000] [1045] Access denied for user 'bookstack'@'bookstack.bookstack_default' (using password: YES) in /app/www/vendor/laravel/framework/src/Illuminate/Database/Connectors/Connector.php:70
Stack trace:
#0 /app/www/vendor/laravel/framework/src/Illuminate/Database/Connectors/Connector.php(70): PDO->__construct()
#1 /app/www/vendor/laravel/framework/src/Illuminate/Database/Connectors/Connector.php(46): Illuminate\Database\Connectors\Connector->createPdoConnection()
#2 /app/www/vendor/laravel/framework/src/Illuminate/Database/Connectors/MySqlConnector.php(24): Illuminate\Database\Connectors\Connector->createConnection()
#3 /app/www/vendor/laravel/framework/src/Illuminate/Database/Connectors/ConnectionFactory.php(184): Illuminate\Database\Connectors\MySqlConnector->connect()
#4 [internal function]: Illuminate\Database\Connectors\ConnectionFactory->Illuminate\Database\Connectors\{closure}()
#5 /app/www/vendor/laravel/framework/src/Illuminat...PHP message: PHP Fatal error:  Uncaught PDOException: SQLSTATE[HY000] [1045] Access denied for user 'bookstack'@'bookstack.bookstack_default' (using password: YES) in /app/www/vendor/laravel/framework/src/Illuminate/Database/Connectors/Connector.php:70
Stack trace:
#0 /app/www/vendor/laravel/framework/src/Illuminate/Database/Connectors/Connector.php(70): PDO->__construct()
#1 /app/www/vendor/laravel/framework/src/Illuminate/Database/Connectors/Connector.php(46): Illuminate\Database\Connectors\Connector->createPdoConnection()
#2 /app/www/vendor/laravel/framework/src/Illuminate/Database/Connectors/MySqlConnector.php(24): Illuminate\Database\Connectors\Connector->createConnection()
#3 /app/www/vendor/laravel/framework/src/Illuminate/Database/Connectors/ConnectionFactory.php(184): Illuminate\Database\Connectors\MySqlConnector->connect()
#4 [internal function]: Illuminate\Database\Connectors\ConnectionFactory->Illuminate\Database\Connectors\{closure}()
#5 /a

And this is my (fixed) docker-compose.yml file:

version: "2"
services:
  bookstack:
    image: lscr.io/linuxserver/bookstack
    container_name: bookstack
    environment:
      - PUID=1000
      - PGID=1000
      - APP_URL=http://192.168.1.19:6875
      - DB_HOST=bookstack_db
      - DB_USERNAME=bookstack
      - DB_PASSWORD=asdfasdf
      - DB_DATABASE=bookstackapp
    volumes:
      - /home/asdf/bookstack/config:/config
    ports:
      - 6875:80
    restart: unless-stopped
    depends_on:
      - bookstack_db
  bookstack_db:
    image: lscr.io/linuxserver/mariadb
    container_name: bookstack_db
    environment:
      - PUID=1000
      - PGID=1000
      - MYSQL_ROOT_PASSWORD=asdfasdf
      - TZ=Europe/London
      - MYSQL_DATABASE=bookstackapp
      - MYSQL_USER=bookstack
      - MYSQL_PASSWORD=asdfasdf
    volumes:
      - /home/asdf/bookstack/config:/config
    restart: unless-stopped

I'm using this exact image:

REPOSITORY                      TAG                  IMAGE ID       CREATED         SIZE
lscr.io/linuxserver/bookstack   latest               27e1bce57a31   2 days ago      255MB
riccardospeggiorin-centropaghe commented 1 year ago

Same problems here. The container cannot connect to the database until I change the variables name! thanks!

oulmanc commented 1 year ago

As others are reporting, I had the same issue using docker compose with the database not connecting until I changed the variables to DB_USERNAME and DB_PASSWORD. As @BloodyIron suggested, the documentation should be updated to reflect this inaccuracy. I was getting a blank 500 error page until I updated the variables.

aptalca commented 1 year ago

The documentation is accurate

oulmanc commented 1 year ago

@aptalca please explain how the documentation is correct.

image

The included screengrab shows the environment variables for the Docker compose file. The values for the database username and password need to be corrected or the application does not load and there is a database connection error in the Docker logs. As @BloodyIron also commented, why is it such as big deal to change this?

aptalca commented 1 year ago

https://github.com/linuxserver/docker-bookstack/issues/129#issuecomment-1218469325

oulmanc commented 1 year ago

@aptalca that was not the case for me. When using Docker compose and copying the included template from the documentation, I was presented with a 500 error and an error connecting to the database in my logs. If the variables were converted, why would others be reporting that they needed to change the variables as well. If that's the expected behavior, it seems to not be working. I'm somewhat new to Docker and was frustrated when the container was not working since there is a provided template to get this running. Others that are looking for a quick and easy way to get this up and running would from my experience, run into the same thing. I was only able to solve the issue after looking at the issues for this repo and seeing that others were having success after changing these variables. If using those variables works and from what it seems, make no difference since the expected behavior is to convert them anyway, then what's the problem with changing them in the documentation if they work?

aptalca commented 1 year ago

If there is a bug, provide the info necessary to identify it. You and many others come in here and make false statements such as documentation is inaccurate and I'm just correcting it.

I tested it some time ago and it was working fine.

ssddanbrown commented 1 year ago

@oulmanc I'm not a maintainer of this repo, but just want to re-iterate my previous comment above that the current documented system does work (In my testing via a docker compose setup).

why would others be reporting that they needed to change the variables as well

Because there are scenarios, as i've mentioned above, where you can get into this scenario. Specifically, when changing variables after first run.

The current system used for database environment variables somewhat aligns with the behavior of the database container, where the (certain) values are only used on first run.

If someone would like this behavior to be addressed, altered, or documented, I'd advise they first spend a little time understanding the nuances of what's going on so a productive suggestion can be made that respects current/historical usage, and possible intentions of the current behavior.

oulmanc commented 1 year ago

@ssddanbrown thanks for your comment. As stated by @BloodyIron I may also be coming off a little abrasive on this. I'm somewhat new to Docker and still learning. When I see a template to get a container up and running, I'm expecting to be able to copy and paste it, fill in the variables with valid data and spin up the container and have it work. That was not the experience I had and lead to some frustration. It just seems like others are reporting that changing the database variables was the fix for them to get this running. I'll admit I don't understand the nuances you are talking about. I just know that changing the variables is what made the container work for me and it looks like that fixed it for others as well.

drizuid commented 1 year ago

Closing this as the issue template was not filled out and the title is factually incorrect. If users are experiencing an issue, we will need relevant data on the failure, not unfounded accusations. We are here to help when users make that possible.

BloodyIron commented 1 year ago

@drizuid the title is not factually incorrect. How exactly can you read this thread and come to that conclusion? The image is not-operational without this change, which is not documented. This is 100% reproducible. Did you even try to follow the steps that have been followed by multiple people in this thread? Because it sounds like you didn't.

I originally reported this back in August, and several others have reported this same issue, plus there are discussions elsewhere on the internet of the same problem before myself. And this is the first time we've seen you involved in this discussion, and you immediately close it without actually considering the merit.

chrisonline commented 1 year ago

Same to me, after change from DB_USER to DB_USERNAME and DB_PASS to DB_PASSWORD it is working.

drizuid commented 1 year ago

I'm going to go ahead and lock this. not a one of you is providing any detail that is useful to us.

making broad assumptions is not helpful, especially when incorrect saying me too is not ever helpful

saying I used a password with these characters and it caused this issue is helpful. hell, showing the password you used is even more helpful, especially when we consider the readme tells you to properly escape them and about 2% of you people actually do so.