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

fix: docker compose properties #682

Closed
wants to merge 1 commit into from

Conversation

eschrewe
Copy link
Contributor

Description

  • recent versions of docker compose no longer support variable names containing hyphens in .properties or .env files
  • this fix removes the affected variables from those files and applies them via "environment" sections of services

Pre-review checks

Please ensure to do as many of the following checks as possible, before asking for committer review:

  • DEPENDENCIES are up-to-date. Dash license tool. Committers can open IP issues for restricted libs.
  • Copyright and license header are present on all affected files
  • If helm chart has been changed, the chart version has been bumped to either next major, minor or patch level (compared to released chart).

Copy link
Contributor

@tom-rm-meyer-ISST tom-rm-meyer-ISST left a comment

Choose a reason for hiding this comment

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

Thanks a lot for providing this! Please checkout the small review comments.

Please rerequest review afterwards. I will then add a commit to fix trufflehog. I think we can exclude the local folder from trufflehog checks.

Just to let you know (or if you want to do it), we should be able to do it by changing this line as follows:

extra_args: --filter-entropy=4 --results=verified,unknown --debug --exlude-paths=.thignore

Then add a file .thignore (not sure if root or in workflow folder) with regexes to ignore (currently only one line): `local/.*

Test run should be testable using act.

@@ -73,6 +73,11 @@ services:
SPRING_DATASOURCE_URL: jdbc:postgresql://postgres-all:5432/puris_customer
SPRING_DATASOURCE_USERNAME: ${PG_USER}
SPRING_DATASOURCE_PASSWORD: ${PG_PW}
server.error.include-message: always
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this line. Same for other supplier-backend

@@ -5,5 +5,5 @@ web.http.directory.port=8582
web.http.directory.path=/api/directory
# looking up DIDs should not use https
edc.iam.did.web.use.https=false
edc.iam.trusted-issuer.issuer.id=did:web:mock-util-service/trusted-issuer
#edc.iam.trusted-issuer.issuer.id=did:web:mock-util-service/trusted-issuer
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to document this somewhere. Else future us (or others) might think: hey let's move the things again to the application.properties. They see it fails but will need to do an analysis.

How about removing the outcommented lines in the .properties and add a comment in the compose files stating something like:

"Within docker and docker compose dashs sometimes may not be used in environment-variables (see e.g. docker/compose#12123 (comment)). To be independent of their future change, one can put these variables into the compose files environment variable section)."

Still not ideal but putting it everywhere would be too much.

@eschrewe eschrewe closed this Dec 11, 2024
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

Successfully merging this pull request may close these issues.

2 participants