Skip to content

CORS configurations for score server allowing multiple origins at once#368

Closed
dahiyaAD wants to merge 2 commits into
developfrom
CORS-configurations
Closed

CORS configurations for score server allowing multiple origins at once#368
dahiyaAD wants to merge 2 commits into
developfrom
CORS-configurations

Conversation

@dahiyaAD

@dahiyaAD dahiyaAD commented Jun 2, 2023

Copy link
Copy Markdown
Contributor

No description provided.

@dahiyaAD dahiyaAD requested a review from joneubank June 2, 2023 17:42
@joneubank joneubank requested review from Azher2Ali and leoraba June 2, 2023 17:42
endpoints:
web:
cors:
allowedOrigins: http://example:8080, http://localhost:8081, http://hello:8080

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we leave only localhost origin by default? I guess others are just an example, also would be nice to add a comment to describe the value could contain multiple origins separated by commas.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done!

@leoraba leoraba left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@leoraba leoraba self-requested a review June 5, 2023 20:09
@dahiyaAD dahiyaAD force-pushed the CORS-configurations branch from e640d99 to 4589f8f Compare June 5, 2023 20:30

@joneubank joneubank left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looking for feedback on my suggestion to reorganize the new properties added. Good idea? Bad idea?

Comment on lines +86 to +90
management:
endpoints:
web:
cors:
allowedOrigins: http://localhost:8081

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does it make sense to include the cors settings in the server section of the yaml (line 25)?

would look like:

server:
  port: 5431
  compression:
    enabled: true
    mime-types: application/json
  cors:
    allowedOrigins:
      - http://localhost:8081
      - http://example.com

@dahiyaAD

dahiyaAD commented Jun 6, 2023

Copy link
Copy Markdown
Contributor Author

Opened new PR : #369 since the job pr-merge stuck.

@dahiyaAD dahiyaAD closed this Jun 6, 2023
@joneubank joneubank deleted the CORS-configurations branch November 22, 2024 16:23
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.

3 participants