-
Notifications
You must be signed in to change notification settings - Fork 4
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
Create default public key endpoint for radar-gateway #290
Conversation
Great PR! Please pay attention to the following items before merging: Files matching
This is an automatically generated QA checklist based on modified files. |
@@ -57,6 +57,7 @@ data: | |||
managementPortalUrl: {{ .Values.managementportal_url }} | |||
checkSourceId: {{ .Values.checkSourceId }} | |||
publicKeyUrls: | |||
- {{ printf "%s://%s/managementportal/oauth/token_key" .Values.advertised_protocol .Values.serverName | quote }} |
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.
Should this line always be present?
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.
@keyvaann I think so yes.
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.
@keyvaann Ah I think that I understand where your question comes from. This is related to the error that is thrown by gateway when using this config option. Shall I enable this via config option and have it off by default?
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.
@keyvaann I have added the option to disable the publicKeyUrls config option.
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.
Thanks!
61c0b74
to
c91cf51
Compare
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.
LGTM
@pvannierop The lint is failing. |
Fixes a config error introduced by prior PR.
c91cf51
to
6dd64fb
Compare
Problem
Previous PR introduced public key endpoint for radar-gateway. This implementation did not include config for a sensible default value.
Solution
This PR will provide a default value that respects the protocol (http, https) which is needed for local testing.