-
Notifications
You must be signed in to change notification settings - Fork 12
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
add kuma ATTACHMENT_ORIGIN setting #623
add kuma ATTACHMENT_ORIGIN setting #623
Conversation
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.
👍 with one issue around blank environments
I tested on the prod environments by modifying make k8s-web
to just run j2
. I then tried with the stage environment, which still had KUMA_ATTACHMENT_ORIGIN
defined from prod. I then ran export KUMA_ATTACHMENT_ORIGIN=
and tried again, and got a blank value for ATTACHMENT_ORIGIN
.
Also, I just remembered we had a staging environment where we could test this stuff.
@@ -33,6 +33,8 @@ | |||
value: "{{ KUMA_ALLOWED_HOSTS }}" | |||
- name: ATTACHMENT_HOST | |||
value: {{ KUMA_ATTACHMENT_HOST }} | |||
- name: ATTACHMENT_ORIGIN | |||
value: {{ KUMA_ATTACHMENT_ORIGIN | default(KUMA_ATTACHMENT_HOST) }} |
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.
Let's use:
value: {{ KUMA_ATTACHMENT_ORIGIN | default(KUMA_ATTACHMENT_HOST, True) }}
This way, a blank string also defaults to KUMA_ATTACHMENT_HOST
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.
Great catch! That issue has bit me in the past with these configs, so I should have been more vigilant about it!
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.
👍
Companion PR to mdn/kuma#4479.