Tidup sidekiq queues in case they're multiline
What does this MR do?
Makes the sidekiq container a little more robust against the sort of inputs that might come through helm unintentionally, in this case multi-block lines with
Related issues
These issues are where we found this edge case:
- https://gitlab.com/gitlab-com/gl-infra/infrastructure/-/issues/14002
- gitlab-com/gl-infra/production#5357 (closed)
Discussion
To be clear:
- I'm still not entirely sure where the extra single quote came from in the above issue, but I'm not sure that ultimately matters; the trailing newline was the trigger.
- Yes, technically the problem here lies in the usage of the helmchart (not even a bug in the helmchart itself, unless there's some way to enforce non-multi-line strings), but it's entirely too easy to do and the results were a bit subtle and hard to spot.
This MR is belt'n'braces to protect users from themselves, and as such it's more of a politeness (and not mandatory) than a fatal flaw that absolutely must be fixed. So I'm open to further discussion.
For validation:
- what should be passed to SIDEKIQ_QUEUES is a comma-separated list, e.g. "foo,bar,baz".
- what was passed in the problem case was "foo,bar,baz\n" (not sure if it was an escaped \n or a literal, nor how to reasonably tell)
- this should also handle embedded multi-lines, e.g. "foo,bar\n,baz,bug\n" and so on
There are still a plethora of ways to misconfigure it, e.g. "foo,bar\nbaz" with no trailing comma on a non-last line, would come out as "foo,barbaz", but that is harder to fix but also should be a little more obvious if debugging a failed deployment. But the fact this remains so brittle may suggest that this is a losing battle and we shouldn't even do this MR. Again, open to discussion.
Checklist
See Definition of done.
For anything in this list which will not be completed, please provide a reason in the MR discussion
Required
-
Merge Request Title, and Description are up to date, accurate, and descriptive -
MR targeting the appropriate branch -
MR has a green pipeline on GitLab.com
Expected (please provide an explanation if not completing)
-
Test plan indicating conditions for success has been posted and passes -
Documentation created/updated -
Integration tests added to GitLab QA -
The impact any change in container size has should be evaluated