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

Added option to select sslmode when connecting to DB in Grafana #2846

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LeeHanYeong
Copy link

In teslamate, you can configure an SSL connection with the DB using the DATABASE_SSL environment variable.
https://docs.teslamate.org/docs/configuration/environment_variables

But grafana doesn't have that option.

I found the sslmode option of DB connection in Grafana's documentation. This is in the jsonData entry in datasources.yml.
https://grafana.com/docs/grafana/latest/administration/provisioning/#data-sources

Guide documentation
I configured it to receive this as an environment variable. I also added information to the traefik and apache guides documentation.

  grafana:
    image: teslamate/grafana:latest
    restart: always
    environment:
      - DATABASE_USER=${TM_DB_USER}
      - DATABASE_PASS=${TM_DB_PASS}
      - DATABASE_NAME=${TM_DB_NAME}
      - DATABASE_HOST=database
      - DATABASE_SSLMODE=disable  # this line

Dockerfile
By adding the ENV to Dockerfile, users who don't have this option in docker-compose can use it without any problems.

FROM grafana/grafana:8.5.9

ENV GF_ANALYTICS_REPORTING_ENABLED=FALSE \
    ...
    DATABASE_SSLMODE=disable

When I ran grafana using the Dockerfile with this option added, it was confirmed that it was connected to PostgreSQL using SSL.

This is also related to #447 from about 2 years ago. It seems that the user who opened the issue had the same problem.

Thank you for your review. have a good day :)

@LeeHanYeong
Copy link
Author

This problem has not been resolved for several years. I didn't make it an environment variable option like {TM_DB_...} in docker-compose.yml because I thought it wasn't a frequently used option.
I expect users who need this option to modify docker-compose.yml appropriately.

@mikesmitty
Copy link

Seconded, this would be a nice and simple quality of life change. Manually updating the datasource every time the grafana container restarts is a little less than ideal.

@LeeHanYeong
Copy link
Author

@adriankumpf
It would be great if you could review this PR. I need to create a custom Grafana image when updating the TeslaMate version to apply SSL to the DB connection.
Thank you :)

@JakobLichterfeld JakobLichterfeld added area:grafana Related to Grafana docker Pull requests that update Docker code labels Nov 16, 2023
Copy link
Collaborator

@JakobLichterfeld JakobLichterfeld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your suggestion!

@@ -69,6 +69,7 @@ services:
- DATABASE_PASS=${TM_DB_PASS}
- DATABASE_NAME=${TM_DB_NAME}
- DATABASE_HOST=database
- DATABASE_SSLMODE=disable
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is the default value, I vote not to include it in the documentation at this location.

@@ -79,6 +79,7 @@ services:
- DATABASE_PASS=${TM_DB_PASS}
- DATABASE_NAME=${TM_DB_NAME}
- DATABASE_HOST=database
- DATABASE_SSLMODE=disable
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is the default value, I vote not to include it in the documentation at this location.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good practise is to include the default action commented out
# - DATABASE_SSLMODE=disable # defaults to disable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:grafana Related to Grafana docker Pull requests that update Docker code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants