Add cors settings #103
Add cors settings #103
Conversation
| allowed-origins: {{ artemis_CORS_allowed_origins }} | ||
| {% if artemis_cors is defined and artemis_cors is not none %} | ||
| artemis_cors: | ||
| {% for key, value in cors.items() %} |
There was a problem hiding this comment.
This probably should be artemis_cors instead of cors
| {% for key, value in cors.items() %} | |
| {% for key, value in artemis_cors.items() %} |
There was a problem hiding this comment.
Also why do you iterate through the dict and then convert each entry to yaml instead of just converting the entire dict directly to yaml?
There was a problem hiding this comment.
Another question: I see the point of not hard coding the keys and having the option of the dynamic configuration, but in the artemis.env.j2 file the keys are hard coded.
Therefore, I would prefer an homogeneous approach and that the keys are hard coded here as well.
There was a problem hiding this comment.
Well, I reiterated over that because I need a similar approach for something else (Theia configuration). Thus, I came up with a potential approach for a dynamic configuration in the environment file.
| # artemis_cors: | ||
| # allowed_methods: "*" | ||
| # allowed_headers: "*" | ||
| # exposed_headers: "Authorization,Link,X-Total-Count,Set-Cookie" | ||
| # allow_credentials: true | ||
| # max_age: 1800 | ||
| # allowed_origins: | ||
| # - "vscode-file://vscode-app" | ||
| # - "vscode-webview://*" |
There was a problem hiding this comment.
Would it makes sense to have this as a default if theia is defined?
| allowed-origins: {{ artemis_CORS_allowed_origins }} | ||
| {% if artemis_cors is defined and artemis_cors is not none %} | ||
| artemis_cors: | ||
| {% for key, value in cors.items() %} |
There was a problem hiding this comment.
Another question: I see the point of not hard coding the keys and having the option of the dynamic configuration, but in the artemis.env.j2 file the keys are hard coded.
Therefore, I would prefer an homogeneous approach and that the keys are hard coded here as well.
| JHIPSTER_CORS_ALLOWEDMETHODS='{{ artemis_cors.allowed_methods | join(',') }}' | ||
| JHIPSTER_CORS_ALLOWEDHEADERS='{{ artemis_cors.allowed_headers | join(',') }}' | ||
| JHIPSTER_CORS_EXPOSEDHEADERS='{{ artemis_cors.exposed_headers }}' | ||
| JHIPSTER_CORS_ALLOWCREDENTIALS='{{ artemis_cors.allow_credentials }}' | ||
| JHIPSTER_CORS_MAXAGE='{{ artemis_cors.max_age }}' | ||
| JHIPSTER_CORS_ALLOWEDORIGINS='{{ artemis_cors.allowed_origins | join(',') }}' |
There was a problem hiding this comment.
| JHIPSTER_CORS_ALLOWEDMETHODS='{{ artemis_cors.allowed_methods | join(',') }}' | |
| JHIPSTER_CORS_ALLOWEDHEADERS='{{ artemis_cors.allowed_headers | join(',') }}' | |
| JHIPSTER_CORS_EXPOSEDHEADERS='{{ artemis_cors.exposed_headers }}' | |
| JHIPSTER_CORS_ALLOWCREDENTIALS='{{ artemis_cors.allow_credentials }}' | |
| JHIPSTER_CORS_MAXAGE='{{ artemis_cors.max_age }}' | |
| JHIPSTER_CORS_ALLOWEDORIGINS='{{ artemis_cors.allowed_origins | join(',') }}' | |
| {% for key, value in artemis_cors.items() %} | |
| JHIPSTER_CORS_{{ key | upper | regex_replace ("[^A-Z0-9]","") }}='{{ value | join(',') }}' | |
| {% endfor %} |
No description provided.