Fix invocation of iptables if redirection fails - #11
markusboehme wants to merge 2 commits into
Conversation
Fix the invocation of iptables/nftables if the redirection fails. As os.Args[0] can still contain e.g. the full path of the symlink the wrapper was invoked as, the leading path components must be stripped for the actual iptables/nftables multi-call binary to know how to behave.
Do not log an error if redirection failed because the target already exists. This can happen when multiple iptables-wrappers processes race each other, or the filesystem is mounted read-only (and removing the existing symlink silently failed). There is nothing to be done in these cases and the logs are just noise.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: markusboehme The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @markusboehme! |
k8s-ci-robot
added
cncf-cla: no
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
k8s-ci-robot
added
the
lifecycle/stale
danwinship
left a comment
danwinship
left a comment
There was a problem hiding this comment.
Sorry, I never noticed this before
| // fake it, though this will probably also fail if they aren't root | ||
| binaryPath = iptables.XtablesPath(sbinPath, mode) | ||
| args = os.Args | ||
| args[0] = path.Base(args[0]) |
There was a problem hiding this comment.
What problem are you seeing exactly?
xtables already handles this:
const char *cmd = basename(*argv);
| fmt.Fprintf(os.Stderr, "Unable to redirect iptables binaries. (Are you running in an unprivileged pod?): %s\n", err) | ||
| // Ignore redirection error if target already exists. This may happen when | ||
| // multiple iptables-wrappers are racing each other or the filesystem is | ||
| // mounted read-only. |
There was a problem hiding this comment.
If the target already exists and the filesystem is read-only then why was iptables-wrapper present at all?
Leave the comment about racing but the part about read-only doesn't seem like it makes sense to me.
|
/remove-lifecycle stale |
k8s-ci-robot
removed
the
lifecycle/stale
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
k8s-ci-robot
added
lifecycle/stale
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
kubernetes-prow
Bot
added
lifecycle/rotten
4 participants
Fix the invocation of iptables/nftables if the redirection fails. As os.Args[0] can still contain e.g. the full path of the symlink the wrapper was invoked as, the leading path components must be stripped for the actual iptables/nftables multi-call binary to know how to behave.
Also do not log an error if redirection failed because the target already exists. This can happen when multiple iptables-wrappers processes race each other, or the filesystem is mounted read-only (and removing the existing symlink silently failed). There is nothing to be done in these cases and the logs are just noise.