helm-charts icon indicating copy to clipboard operation
helm-charts copied to clipboard

FIX remove web ui config in values.yaml

Open washanhanzi opened this issue 2 years ago • 5 comments

What was changed

  1. remove web ui config in values.yaml
  2. remove web config volume in web-deployment.yaml
  3. remove web-config config map
  4. remove line 280 since the install bash didn't configure web ui auth
  5. update the document for web ui configuration with env variable

Why?

#393

Checklist

  1. Closes #393

  2. How was this tested: manually, 100% craft by hand. since the repo didn't provide any testing tool.

  3. Any docs updates needed? already updated with the PR.

washanhanzi avatar Jun 02 '23 08:06 washanhanzi

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jun 02 '23 08:06 CLAassistant

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Jun 02 '23 08:06 CLAassistant

+1, spent ~hour trying to understand why OID does not work when finally found that config map is not mounted to any volume/file.

samm-git avatar Jul 20 '23 19:07 samm-git

Can we get this merged? Also just spent a ton of time on auth that wasn't necessary.

joshbranham avatar Sep 14 '23 20:09 joshbranham

@tsurdilo

washanhanzi avatar Oct 23 '23 11:10 washanhanzi

I can't understand why this was not merged; we all suffered when we tried to change the web values, and nothing was affected!

kamelj avatar May 28 '24 17:05 kamelj

@robholland

washanhanzi avatar May 29 '24 02:05 washanhanzi

All done. By the way, is there any test I can run? The PR has been open for a really long time, and I haven't touched the temporal charts since then. I'm not as confident as I was when I submitted the PR.

washanhanzi avatar May 29 '24 14:05 washanhanzi

Nothing automated. We internally use the helm charts to install the base app, but we don't modify the UI so wouldn't pick up on problems. I'll do a manual test before I merge it though.

robholland avatar May 29 '24 14:05 robholland