search

temporalio / helm-charts

Temporal Helm charts
MIT License
294 stars 321 forks source link

add new options for TEMPORAL_CORS_ORIGINS and TEMPORAL_CSRF_COOKIE_INSECURE #402

Closed danthegoodman1 closed 2 months ago

danthegoodman1 commented 1 year ago

What was changed

Adds options for TEMPORAL_CORS_ORIGINS and TEMPORAL_CSRF_COOKIE_INSECURE

Why?

So they can be configured more explicitly than through the additionalEnvs, lacking documentation on them so surfacing to dedicated options makes it easier to find the functionality. Also this usage is quite common for privately-networked deployments.

Checklist

  1. Closes #401

  2. How was this tested:

helm template

  1. Any docs updates needed?

No

danthegoodman1 commented 1 year ago

@tsurdilo what's needed to merge this?

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

karelbilek commented 1 year ago

@tsurdilo this would be very helpful

karelbilek commented 1 year ago

How will this work with multiple cors origins? I am trying to decode how is UI config separating them and I don't really understand all the pieces involved

karelbilek commented 1 year ago

I see the env name is TEMPORAL_CORS_ORIGINS so I guess multiple is possible

but I also see this in the conf template in ui repo

  allowOrigins:
    # override framework's default that allows all origins "*"
    - {{ default .Env.TEMPORAL_CORS_ORIGINS "http://localhost:8080" }}

this seems like the value is just put into single value list? Not sure

karelbilek commented 12 months ago

It was fixed here (already merged)

https://github.com/temporalio/ui/pull/1572

jphilippelingrand commented 2 months ago

Should we close this MR?

robholland commented 2 months ago

A link to the web UI config docs is now included in the values.yaml. I don't think we should special case these.