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

Add CORS_ENABLED env var for docker-compose #620

Merged
merged 8 commits into from
Oct 10, 2023

Conversation

pasquy73
Copy link
Contributor

Added the CORS_ENABLED environment variable as described in the issue 608.
Please, check where it is necessary to update the doc (i.e. https://github.com/telefonicaid/fiware-sth-comet/blob/master/doc/manuals/running.md) to add this new env var.
A usage can be:

  sth-comet:
    image: fiware/sth-comet:${STH_COMET_VERSION}
    hostname: sth-comet
    container_name: fiware-sth-comet
    restart: always
    depends_on:
      - database-mongo
    networks:
      - monitoring-net
    ports:
      - ${STH_COMET_PORT}:${STH_COMET_PORT}
    environment:
      - STH_HOST=0.0.0.0
      - STH_PORT=${STH_COMET_PORT}
      - DB_PREFIX=sth_
      - DB_URI=database-mongo:${MONGO_DB_PORT}
      - LOGOPS_LEVEL=INFO
      - CORS_ENABLED=true

I don't know if it can be included in the test scripts, I only changed the sthConfiguration.js file in the lib/configuration folder.

@fgalan
Copy link
Member

fgalan commented Sep 26, 2023

Thank for the contribution!

In order to complete the PR, please take into account the following feedback:

  • Please add an entry to CHANGES_NEXT_RELEASE (future changelog) to briefly describe the modification done by this PR
  • The new functionality should be described in documentation. Please have a look to existing documents in doc/ and modify the relevants .md files.

@pasquy73
Copy link
Contributor Author

pasquy73 commented Oct 3, 2023

I added the entry regarding CORS_ENABLED in the CHANGES_NEXT_RELEASE. I also added a new line in the running.md (line 104) to define the CORS_ENABLED attribute.
Please note that inside the Cors configuration, there are other attributes that can be overwritten (i.e. origin, headers, etc.) when you enable Cors, but we can add them in the next releases.

CHANGES_NEXT_RELEASE Outdated Show resolved Hide resolved
Ok

Co-authored-by: Fermín Galán Márquez <[email protected]>
@@ -1 +1 @@
- Set Nodejs 14 as minimum version in packages.json (effectively removing Nodev12 from supported versions)
- Add: CORS_ENABLED env var (boolean) to enable the cors configuration (of config.js file) in your docker image (#608)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Add: CORS_ENABLED env var (boolean) to enable the cors configuration (of config.js file) in your docker image (#608)
- Add: CORS_ENABLED env var (boolean) to enable the cors configuration (of config.js file) in your docker image (#608)
- Set Nodejs 14 as minimum version in packages.json (effectively removing Nodev12 from supported versions)

This seems to be a wrongly solved merge conflict. You shouldn't delete existing lines in CHANGES_NEXT_RELEASE file, just add yours.

@pasquy73
Copy link
Contributor Author

Sorry, I didn't know! I added the first line in CHANGES_NEXT_RELEASE

Copy link
Member

@fgalan fgalan left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@fgalan fgalan merged commit f2d07dc into telefonicaid:master Oct 10, 2023
6 checks passed
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