Fix panic when Failsafe configs are set to empty list #10354
Conversation
sridhartigera
added
docs-not-required
| Expect(err).To(BeNil()) | ||
| } | ||
| }, | ||
| Entry("Empty", "", nil, true), |
There was a problem hiding this comment.
Interesting, I would have expected an empty string to be set the default - but I guess we have logic to detect "unset" vs "set but empty"?
If that's the case, I wonder if the right fix is just in documentation to set "" instead of "[]" - WDYT?
There was a problem hiding this comment.
Yeah. I am not sure. Given that FailsafeInboundHostPorts is a list of ProtoPort, [] does make sense.
There was a problem hiding this comment.
It doesn't really, based on the env var syntax.
tcp:22,udp:1.1.1.1:22,3033 - this is a valid value, and it has no [ or ] in it at all!
It's a valid YAML representation, but this isn't YAML we're talking about.
There was a problem hiding this comment.
Better to fix the docs.
There was a problem hiding this comment.
It makes sense to set the value as [] in the felix config but when configured as an env var it must be set to "".
Description
FailSafeInBoundPorts and FailSafeOutBoundPorts can be set to an empty list
"[]". This results in felix panicking.This PR fixes #10338
Related issues/PRs
Todos
Release Note
Reminder for the reviewer
Make sure that this PR has the correct labels and milestone set.
Every PR needs one
docs-*label.docs-pr-required: This change requires a change to the documentation that has not been completed yet.docs-completed: This change has all necessary documentation completed.docs-not-required: This change has no user-facing impact and requires no docs.Every PR needs one
release-note-*label.release-note-required: This PR has user-facing changes. Most PRs should have this label.release-note-not-required: This PR has no user-facing changes.Other optional labels:
cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.