http: fix a bug that would cause runtimeConfig to be cached #9923
Conversation
This bug would result in the UI not having the correct settings in Consul enterprise, which could produce many warnings in the logs. This bug occured because the index page, which includes a map of configuration was rendered when the HTTPHandler is first created. This PR changes the UIServer to instead render the index page when the page is requested. The rendering does not appear to be all that expensive, so rendering it when requested should not cause much extra latency.
| // Reload config in case it's changed | ||
| state := h.getState() | ||
| cfg := h.getRuntimeConfig() |
There was a problem hiding this comment.
This may be nil and prior code forced a quick panic in that case which is no longer present. Just wanted to verify that was intended.
There was a problem hiding this comment.
Ya, some callers did explicitly panic with a custom message, and others did not. In practice it should never be nil, but if there is a bug, it will panic on the next line either way. A panic on line 140 should be unambiguous because the UIConfig field is not a pointer, so only cfg could be nil.
|
|
||
| var fs http.FileSystem | ||
| func (h *Handler) handleIndex() (http.Handler, error) { | ||
| cfg := h.getRuntimeConfig() |
There was a problem hiding this comment.
same comment about this returning nil
|
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/342452. |
|
🍒✅ Cherry pick of commit 8d25f9a onto |
http: fix a bug that would cause runtimeConfig to be cached
http: fix a bug that would cause runtimeConfig to be cached
This bug would result in the UI not having the correct settings in Consul enterprise, which could produce many warnings in the logs.
This bug occurred because the index page, which includes a map of configuration was rendered when the HTTPHandler is first created. This PR changes the UIServer to instead render the index page when the page is requested.
The rendering does not appear to be all that expensive, so rendering it when requested should not cause much extra latency.
Also includes a bit of cleanup to remove some unnecessary args, and unnecessary function wrapping.
This bug only exists in 1.9.x, so I think we only need one backport.