Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

fix: add CORS setup #55

Merged
merged 1 commit into from
Mar 19, 2021
Merged

Conversation

namit-chandwani
Copy link
Member

Changes Made:

I have added a basic CORS setup using the rs/cors package.

Further Comments:

However, to test these changes when containerized, I created a new docker image with the updated code (by executing the build.sh script present in the docker/ directory).
After running this latest image as a container, I noticed that the initial docker image issue which was pointed out in #39, still exists.

Do I need to make any further changes to the CORS config in order to fix it?

@wtrocki
Copy link
Contributor

wtrocki commented Mar 10, 2021

I will look into this

@wtrocki
Copy link
Contributor

wtrocki commented Mar 16, 2021

I'm not sure if makes sense to bring framework into this.

@chirino WDYT?

@wtrocki wtrocki requested a review from chirino March 18, 2021 18:06
@chirino
Copy link
Collaborator

chirino commented Mar 19, 2021

I think this is fine and I think overtime more options of the framework may need to be exposed so that admins can control who can connect to the endpoint.

@chirino
Copy link
Collaborator

chirino commented Mar 19, 2021

lgtm

@wtrocki
Copy link
Contributor

wtrocki commented Mar 19, 2021

@chirino thank you so much. Yes. This is just start. I wanted you to approve choice of the framework. If we pick some more top level framework we can now take number of improvements like cors wildcards etc.

@wtrocki
Copy link
Contributor

wtrocki commented Mar 19, 2021

@namit-chandwani Thank you for contribution. Verified. All good

@wtrocki wtrocki merged commit e237015 into aerogear:master Mar 19, 2021
@namit-chandwani namit-chandwani deleted the add-cors-setup branch March 19, 2021 17:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants