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.

Sign up for GitHub

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 default db params to dev file (fix) #467

Merged
merged 7 commits into from
Dec 10, 2024
Merged

added default db params to dev file (fix) #467

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
25799b5
added default paramaters to dev file
arinkulshi-skylight Dec 6, 2024
4a9fd8d
added default values for dev-env file
arinkulshi-skylight Dec 6, 2024
c1e301b
formatting
arinkulshi-skylight Dec 6, 2024
f4c8b32
added bash exp synthax
arinkulshi-skylight Dec 6, 2024
0475e30
Merge branch 'main' into add_default_dev_values
arinkulshi-skylight Dec 9, 2024
e03daea
edited url for spring db
arinkulshi-skylight Dec 10, 2024
173a49c
added entry point to dev docker file
arinkulshi-skylight Dec 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions backend/src/main/resources/application.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
spring:
datasource:
url: ${POSTGRES_HOST:postgres_fqdn}?sslmode=require
username: ${DB_USERNAME:postgres_user}
password: ${POSTGRES_USER:postgres_password}
name: ${POSTGRES_DB:postgres_db_name}
url: jdbc:postgresql://${POSTGRES_HOST:localhost}:${DB_PORT:5432}/${POSTGRES_DB:reportvision}?sslmode=${SSL_MODE:disable}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to enable ssl_mode to enforce SSL encryption for this connection?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was thinking we can pass SSL_MODE as a argument(enabled,disabled) but kept it disabled by default for connecting to local host. Would this work?

Copy link
Collaborator

@marycrawford marycrawford Dec 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It appears @jonchang and @derekadombek have tested this in the dev-env.yaml and application.yaml as disabled. SSL_MODE enabled adds additional security. However, I will defer to the majority decision of the team on whether it should be enabled at this time or before we hand it off.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok we can set it as enabled by default then.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@derekadombek and i tested this in a pairing session today and we found that this was one of the changes needed to get the middleware container image to be able to talk to the database container image when running locally in docker compose. (There may be other changes needed as well, I need to see if I can replicate it locally)

Suggested change
url: jdbc:postgresql://${POSTGRES_HOST:localhost}:${DB_PORT:5432}/${POSTGRES_DB:reportvision}?sslmode=${SSL_MODE:disable}
url: jdbc:postgresql://${POSTGRES_HOST:db}:${DB_PORT:5432}/${POSTGRES_DB:reportvision}?sslmode=${SSL_MODE:disable}

username: ${POSTGRES_USER:postgres}
password: ${POSTGRES_PASSWORD:super_secret_password}
devtools:
restart:
enabled: true
Expand Down
11 changes: 6 additions & 5 deletions dev-env.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,12 @@ services:
ports:
- "5432:5432"
environment:
POSTGRES_DB: ${postgres_db_name}
POSTGRES_HOST: ${postgres_fqdn}
POSTGRES_USER: ${postgres_user}
POSTGRES_PASSWORD: ${postgres_password}
sslmode: require
POSTGRES_DB: ${postgres_db_name:-reportvision}
Copy link
Collaborator

@marycrawford marycrawford Dec 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Does it matter if the dev-env.yaml is using :-name and the application.yaml file is not? For example, POSTGRES_DB: ${postgres_db_name:-reportvision} in the dev-env.yaml and name: ${POSTGRES_DB:postgres_db_name} in the name: ${POSTGRES_DB:postgres_db_name}.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It seems one is springboot synthax and the other is bash synthax. app.yaml is being used locally so it should work as is.

POSTGRES_HOST: ${postgres_fqdn:-localhost}
POSTGRES_USER: ${postgres_user:-postgres}
POSTGRES_PASSWORD: ${postgres_password:-super_secret_password}
DB_PORT: ${postgres_port:-5432}
SSL_MODE: ${postgres_sslmode:-disable}
api:
build:
context: ./backend
Expand Down
Loading