-
Notifications
You must be signed in to change notification settings - Fork 304
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
HPCC-28033 Document CORS Section of values.yaml #17817
Conversation
https://track.hpccsystems.com/browse/HPCC-28033 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of comments about example values. I know you just used what I put in the comments of the yaml file, but I'd like to start improving these examples.
If you want you could update the values.yaml comments as well. They are really internal documentation.
# origin starting with https will only allow https CORS | ||
- origin: https://*.my.com | ||
headers: | ||
- "X-X" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, for clarity, change "X-X" to "X-Custom-Header"
# | ||
corsAllowed: | ||
# origin starting with https will only allow https CORS | ||
- origin: https://*.my.com |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to slowly make examples more consistent over time, use "https://*.example2.com"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor suggestion.
<para>Cross-origin resource sharing (CORS) is a mechanism for | ||
integrating applications in different domains. CORS defines how client | ||
web applications in one domain can interact with resources in another | ||
domain. As a further enhancement to HPCC Systems we've added support |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested: The configuration settings for CORS access are in the ESP section of the values.yaml file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good from my POV
@g-pan please squash and I will merge |
Signed-off-by: g-pan <[email protected]>
Squashed and ready to merge. Thank you @ghalliday |
Type of change:
Checklist:
Smoketest:
Testing:
All Tests Successful http://10.224.20.18/view/Docs-gp/job/DocBuild-GPRepo3/