-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
daemon: Enable configuration of iptables --random-fully #13383
Conversation
This comment has been minimized.
This comment has been minimized.
7581cfa
to
57bde2f
Compare
988ce2b
to
3a29b62
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Have you considered setting Please ignore my comment.--random-fully
if iptables supports it instead of controlling with the flag? The former approach is implemented by k8s: https://github.com/kubernetes/kubernetes/blob/112dbd55860e600af525cedc255f2664e3f286aa/pkg/util/iptables/iptables.go#L228.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, just one minor request on the Helm side.
install/kubernetes/cilium/charts/config/templates/configmap.yaml
Outdated
Show resolved
Hide resolved
3a29b62
to
e9a5673
Compare
test-me-please |
As discussed with @kh34 offline, I will add another commit on top of this branch to test the new flag (probably tomorrow). We will likely need to update the iptables version during backports somehow so I'd prefer if we have a test that fails when that is needed. |
In generate_commit_list_for_pr, the commit subject is used to determine the upstream commit ID from $REMOTE/master. However, if in the meantime another commit with e.g. a Fixes tag that mentions this commit subject, it appears first and leads to the original commit not being found. This can be demonstrated using #13383: ``` * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- #13383 Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200! Branch: master (!) refs/pull/13383/head ---------- ------------------- v (start) | Warning: No commit correlation found! via dbac86c ("daemon: Enable configuration of iptables --random-fully") | 350f0b3 via 22d4554 ("test: Test iptables masquerading with --random-fully") v (end) $ # this is the git log command (with the subject added) from $ # contrib/backporting/check-stable that should extract a single $ # upstream commit $ git log -F --since="1year" --pretty="%H %s" --no-merges --grep "daemon: Enable configuration of iptables --random-fully" origin/master 078ec54 install/kubernetes: consistent case spelling of iptables related values 4e39def daemon: Enable configuration of iptables --random-fully $ git show 078ec54 commit 078ec54 Author: Tobias Klauser <tklauser@distanz.ch> Date: Wed Oct 14 11:58:29 2020 +0200 install/kubernetes: consistent case spelling of iptables related values Make the case spelling of the newly introduced "ipTablesRandomFully" value consistent with other iptables option values which use the "iptables" spelling. Fixes: 4e39def ("daemon: Enable configuration of iptables --random-fully") Signed-off-by: Tobias Klauser <tklauser@distanz.ch> ``` Note the `Fixes: ...` line in commit 078ec54 above. Fix this behavior by grepping for the subject line from start of line: ``` $ git log -F --since="1year" --pretty="%H %s" --no-merges --extended-regexp --grep "^daemon: Enable configuration of iptables --random-fully" origin/master 4e39def daemon: Enable configuration of iptables --random-fully * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- #13383 Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200! Branch: master (!) refs/pull/13383/head ---------- ------------------- v (start) | 4e39def via dbac86c ("daemon: Enable configuration of iptables --random-fully") | 350f0b3 via 22d4554 ("test: Test iptables masquerading with --random-fully") v (end) ``` Reported-by: Robin Hahling <robin.hahling@gw-computing.net> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
In generate_commit_list_for_pr, the commit subject is used to determine the upstream commit ID from $REMOTE/master. However, if in the meantime another commit with e.g. a Fixes tag that mentions this commit subject, it appears first and leads to the original commit not being found. This can be demonstrated using #13383: ``` * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- #13383 Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200! Branch: master (!) refs/pull/13383/head ---------- ------------------- v (start) | Warning: No commit correlation found! via dbac86c ("daemon: Enable configuration of iptables --random-fully") | 350f0b3 via 22d4554 ("test: Test iptables masquerading with --random-fully") v (end) $ # this is the git log command (with the subject added) from $ # contrib/backporting/check-stable that should extract a single $ # upstream commit $ git log -F --since="1year" --pretty="%H %s" --no-merges --grep "daemon: Enable configuration of iptables --random-fully" origin/master 078ec54 install/kubernetes: consistent case spelling of iptables related values 4e39def daemon: Enable configuration of iptables --random-fully $ git show 078ec54 commit 078ec54 Author: Tobias Klauser <tklauser@distanz.ch> Date: Wed Oct 14 11:58:29 2020 +0200 install/kubernetes: consistent case spelling of iptables related values Make the case spelling of the newly introduced "ipTablesRandomFully" value consistent with other iptables option values which use the "iptables" spelling. Fixes: 4e39def ("daemon: Enable configuration of iptables --random-fully") Signed-off-by: Tobias Klauser <tklauser@distanz.ch> ``` Note the `Fixes: ...` line in commit 078ec54 above. Fix this behavior by grepping for the subject line from start of line: ``` $ git log -F --since="1year" --pretty="%H %s" --no-merges --extended-regexp --grep "^daemon: Enable configuration of iptables --random-fully" origin/master 4e39def daemon: Enable configuration of iptables --random-fully * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- #13383 Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200! Branch: master (!) refs/pull/13383/head ---------- ------------------- v (start) | 4e39def via dbac86c ("daemon: Enable configuration of iptables --random-fully") | 350f0b3 via 22d4554 ("test: Test iptables masquerading with --random-fully") v (end) ``` Reported-by: Robin Hahling <robin.hahling@gw-computing.net> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
[ upstream commit 6557f75 ] In generate_commit_list_for_pr, the commit subject is used to determine the upstream commit ID from $REMOTE/master. However, if in the meantime another commit with e.g. a Fixes tag that mentions this commit subject, it appears first and leads to the original commit not being found. This can be demonstrated using #13383: ``` * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- #13383 Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200! Branch: master (!) refs/pull/13383/head ---------- ------------------- v (start) | Warning: No commit correlation found! via dbac86c ("daemon: Enable configuration of iptables --random-fully") | 350f0b3 via 22d4554 ("test: Test iptables masquerading with --random-fully") v (end) $ # this is the git log command (with the subject added) from $ # contrib/backporting/check-stable that should extract a single $ # upstream commit $ git log -F --since="1year" --pretty="%H %s" --no-merges --grep "daemon: Enable configuration of iptables --random-fully" origin/master 078ec54 install/kubernetes: consistent case spelling of iptables related values 4e39def daemon: Enable configuration of iptables --random-fully $ git show 078ec54 commit 078ec54 Author: Tobias Klauser <tklauser@distanz.ch> Date: Wed Oct 14 11:58:29 2020 +0200 install/kubernetes: consistent case spelling of iptables related values Make the case spelling of the newly introduced "ipTablesRandomFully" value consistent with other iptables option values which use the "iptables" spelling. Fixes: 4e39def ("daemon: Enable configuration of iptables --random-fully") Signed-off-by: Tobias Klauser <tklauser@distanz.ch> ``` Note the `Fixes: ...` line in commit 078ec54 above. Fix this behavior by grepping for the subject line from start of line: ``` $ git log -F --since="1year" --pretty="%H %s" --no-merges --extended-regexp --grep "^daemon: Enable configuration of iptables --random-fully" origin/master 4e39def daemon: Enable configuration of iptables --random-fully * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- #13383 Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200! Branch: master (!) refs/pull/13383/head ---------- ------------------- v (start) | 4e39def via dbac86c ("daemon: Enable configuration of iptables --random-fully") | 350f0b3 via 22d4554 ("test: Test iptables masquerading with --random-fully") v (end) ``` Reported-by: Robin Hahling <robin.hahling@gw-computing.net> Signed-off-by: Tobias Klauser <tklauser@distanz.ch> Signed-off-by: Nate Sweet <nathanjsweet@pm.me>
[ upstream commit 6557f75 ] In generate_commit_list_for_pr, the commit subject is used to determine the upstream commit ID from $REMOTE/master. However, if in the meantime another commit with e.g. a Fixes tag that mentions this commit subject, it appears first and leads to the original commit not being found. This can be demonstrated using #13383: ``` * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- #13383 Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200! Branch: master (!) refs/pull/13383/head ---------- ------------------- v (start) | Warning: No commit correlation found! via dbac86c ("daemon: Enable configuration of iptables --random-fully") | 350f0b3 via 22d4554 ("test: Test iptables masquerading with --random-fully") v (end) $ # this is the git log command (with the subject added) from $ # contrib/backporting/check-stable that should extract a single $ # upstream commit $ git log -F --since="1year" --pretty="%H %s" --no-merges --grep "daemon: Enable configuration of iptables --random-fully" origin/master 078ec54 install/kubernetes: consistent case spelling of iptables related values 4e39def daemon: Enable configuration of iptables --random-fully $ git show 078ec54 commit 078ec54 Author: Tobias Klauser <tklauser@distanz.ch> Date: Wed Oct 14 11:58:29 2020 +0200 install/kubernetes: consistent case spelling of iptables related values Make the case spelling of the newly introduced "ipTablesRandomFully" value consistent with other iptables option values which use the "iptables" spelling. Fixes: 4e39def ("daemon: Enable configuration of iptables --random-fully") Signed-off-by: Tobias Klauser <tklauser@distanz.ch> ``` Note the `Fixes: ...` line in commit 078ec54 above. Fix this behavior by grepping for the subject line from start of line: ``` $ git log -F --since="1year" --pretty="%H %s" --no-merges --extended-regexp --grep "^daemon: Enable configuration of iptables --random-fully" origin/master 4e39def daemon: Enable configuration of iptables --random-fully * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- #13383 Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200! Branch: master (!) refs/pull/13383/head ---------- ------------------- v (start) | 4e39def via dbac86c ("daemon: Enable configuration of iptables --random-fully") | 350f0b3 via 22d4554 ("test: Test iptables masquerading with --random-fully") v (end) ``` Reported-by: Robin Hahling <robin.hahling@gw-computing.net> Signed-off-by: Tobias Klauser <tklauser@distanz.ch> Signed-off-by: Nate Sweet <nathanjsweet@pm.me>
[ upstream commit 6557f75 ] In generate_commit_list_for_pr, the commit subject is used to determine the upstream commit ID from $REMOTE/master. However, if in the meantime another commit with e.g. a Fixes tag that mentions this commit subject, it appears first and leads to the original commit not being found. This can be demonstrated using #13383: ``` * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- #13383 Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200! Branch: master (!) refs/pull/13383/head ---------- ------------------- v (start) | Warning: No commit correlation found! via dbac86c ("daemon: Enable configuration of iptables --random-fully") | 350f0b3 via 22d4554 ("test: Test iptables masquerading with --random-fully") v (end) $ # this is the git log command (with the subject added) from $ # contrib/backporting/check-stable that should extract a single $ # upstream commit $ git log -F --since="1year" --pretty="%H %s" --no-merges --grep "daemon: Enable configuration of iptables --random-fully" origin/master 078ec54 install/kubernetes: consistent case spelling of iptables related values 4e39def daemon: Enable configuration of iptables --random-fully $ git show 078ec54 commit 078ec54 Author: Tobias Klauser <tklauser@distanz.ch> Date: Wed Oct 14 11:58:29 2020 +0200 install/kubernetes: consistent case spelling of iptables related values Make the case spelling of the newly introduced "ipTablesRandomFully" value consistent with other iptables option values which use the "iptables" spelling. Fixes: 4e39def ("daemon: Enable configuration of iptables --random-fully") Signed-off-by: Tobias Klauser <tklauser@distanz.ch> ``` Note the `Fixes: ...` line in commit 078ec54 above. Fix this behavior by grepping for the subject line from start of line: ``` $ git log -F --since="1year" --pretty="%H %s" --no-merges --extended-regexp --grep "^daemon: Enable configuration of iptables --random-fully" origin/master 4e39def daemon: Enable configuration of iptables --random-fully * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- #13383 Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200! Branch: master (!) refs/pull/13383/head ---------- ------------------- v (start) | 4e39def via dbac86c ("daemon: Enable configuration of iptables --random-fully") | 350f0b3 via 22d4554 ("test: Test iptables masquerading with --random-fully") v (end) ``` Reported-by: Robin Hahling <robin.hahling@gw-computing.net> Signed-off-by: Tobias Klauser <tklauser@distanz.ch> Signed-off-by: Nate Sweet <nathanjsweet@pm.me>
[ upstream commit 6557f75 ] In generate_commit_list_for_pr, the commit subject is used to determine the upstream commit ID from $REMOTE/master. However, if in the meantime another commit with e.g. a Fixes tag that mentions this commit subject, it appears first and leads to the original commit not being found. This can be demonstrated using #13383: ``` * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- #13383 Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200! Branch: master (!) refs/pull/13383/head ---------- ------------------- v (start) | Warning: No commit correlation found! via dbac86c ("daemon: Enable configuration of iptables --random-fully") | 350f0b3 via 22d4554 ("test: Test iptables masquerading with --random-fully") v (end) $ # this is the git log command (with the subject added) from $ # contrib/backporting/check-stable that should extract a single $ # upstream commit $ git log -F --since="1year" --pretty="%H %s" --no-merges --grep "daemon: Enable configuration of iptables --random-fully" origin/master 078ec54 install/kubernetes: consistent case spelling of iptables related values 4e39def daemon: Enable configuration of iptables --random-fully $ git show 078ec54 commit 078ec54 Author: Tobias Klauser <tklauser@distanz.ch> Date: Wed Oct 14 11:58:29 2020 +0200 install/kubernetes: consistent case spelling of iptables related values Make the case spelling of the newly introduced "ipTablesRandomFully" value consistent with other iptables option values which use the "iptables" spelling. Fixes: 4e39def ("daemon: Enable configuration of iptables --random-fully") Signed-off-by: Tobias Klauser <tklauser@distanz.ch> ``` Note the `Fixes: ...` line in commit 078ec54 above. Fix this behavior by grepping for the subject line from start of line: ``` $ git log -F --since="1year" --pretty="%H %s" --no-merges --extended-regexp --grep "^daemon: Enable configuration of iptables --random-fully" origin/master 4e39def daemon: Enable configuration of iptables --random-fully * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- #13383 Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200! Branch: master (!) refs/pull/13383/head ---------- ------------------- v (start) | 4e39def via dbac86c ("daemon: Enable configuration of iptables --random-fully") | 350f0b3 via 22d4554 ("test: Test iptables masquerading with --random-fully") v (end) ``` Reported-by: Robin Hahling <robin.hahling@gw-computing.net> Signed-off-by: Tobias Klauser <tklauser@distanz.ch> Signed-off-by: Nate Sweet <nathanjsweet@pm.me>
[ upstream commit 6557f75 ] In generate_commit_list_for_pr, the commit subject is used to determine the upstream commit ID from $REMOTE/master. However, if in the meantime another commit with e.g. a Fixes tag that mentions this commit subject, it appears first and leads to the original commit not being found. This can be demonstrated using #13383: ``` * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- #13383 Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200! Branch: master (!) refs/pull/13383/head ---------- ------------------- v (start) | Warning: No commit correlation found! via dbac86c ("daemon: Enable configuration of iptables --random-fully") | 350f0b3 via 22d4554 ("test: Test iptables masquerading with --random-fully") v (end) $ # this is the git log command (with the subject added) from $ # contrib/backporting/check-stable that should extract a single $ # upstream commit $ git log -F --since="1year" --pretty="%H %s" --no-merges --grep "daemon: Enable configuration of iptables --random-fully" origin/master 078ec54 install/kubernetes: consistent case spelling of iptables related values 4e39def daemon: Enable configuration of iptables --random-fully $ git show 078ec54 commit 078ec54 Author: Tobias Klauser <tklauser@distanz.ch> Date: Wed Oct 14 11:58:29 2020 +0200 install/kubernetes: consistent case spelling of iptables related values Make the case spelling of the newly introduced "ipTablesRandomFully" value consistent with other iptables option values which use the "iptables" spelling. Fixes: 4e39def ("daemon: Enable configuration of iptables --random-fully") Signed-off-by: Tobias Klauser <tklauser@distanz.ch> ``` Note the `Fixes: ...` line in commit 078ec54 above. Fix this behavior by grepping for the subject line from start of line: ``` $ git log -F --since="1year" --pretty="%H %s" --no-merges --extended-regexp --grep "^daemon: Enable configuration of iptables --random-fully" origin/master 4e39def daemon: Enable configuration of iptables --random-fully * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- #13383 Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200! Branch: master (!) refs/pull/13383/head ---------- ------------------- v (start) | 4e39def via dbac86c ("daemon: Enable configuration of iptables --random-fully") | 350f0b3 via 22d4554 ("test: Test iptables masquerading with --random-fully") v (end) ``` Reported-by: Robin Hahling <robin.hahling@gw-computing.net> Signed-off-by: Tobias Klauser <tklauser@distanz.ch> Signed-off-by: Nate Sweet <nathanjsweet@pm.me>
[ upstream commit 6557f75 ] In generate_commit_list_for_pr, the commit subject is used to determine the upstream commit ID from $REMOTE/master. However, if in the meantime another commit with e.g. a Fixes tag that mentions this commit subject, it appears first and leads to the original commit not being found. This can be demonstrated using #13383: ``` * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- #13383 Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200! Branch: master (!) refs/pull/13383/head ---------- ------------------- v (start) | Warning: No commit correlation found! via dbac86c ("daemon: Enable configuration of iptables --random-fully") | 350f0b3 via 22d4554 ("test: Test iptables masquerading with --random-fully") v (end) $ # this is the git log command (with the subject added) from $ # contrib/backporting/check-stable that should extract a single $ # upstream commit $ git log -F --since="1year" --pretty="%H %s" --no-merges --grep "daemon: Enable configuration of iptables --random-fully" origin/master 078ec54 install/kubernetes: consistent case spelling of iptables related values 4e39def daemon: Enable configuration of iptables --random-fully $ git show 078ec54 commit 078ec54 Author: Tobias Klauser <tklauser@distanz.ch> Date: Wed Oct 14 11:58:29 2020 +0200 install/kubernetes: consistent case spelling of iptables related values Make the case spelling of the newly introduced "ipTablesRandomFully" value consistent with other iptables option values which use the "iptables" spelling. Fixes: 4e39def ("daemon: Enable configuration of iptables --random-fully") Signed-off-by: Tobias Klauser <tklauser@distanz.ch> ``` Note the `Fixes: ...` line in commit 078ec54 above. Fix this behavior by grepping for the subject line from start of line: ``` $ git log -F --since="1year" --pretty="%H %s" --no-merges --extended-regexp --grep "^daemon: Enable configuration of iptables --random-fully" origin/master 4e39def daemon: Enable configuration of iptables --random-fully * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- #13383 Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200! Branch: master (!) refs/pull/13383/head ---------- ------------------- v (start) | 4e39def via dbac86c ("daemon: Enable configuration of iptables --random-fully") | 350f0b3 via 22d4554 ("test: Test iptables masquerading with --random-fully") v (end) ``` Reported-by: Robin Hahling <robin.hahling@gw-computing.net> Signed-off-by: Tobias Klauser <tklauser@distanz.ch> Signed-off-by: Nate Sweet <nathanjsweet@pm.me>
Please ensure your pull request adheres to the following guidelines:
description and a
Fixes: #XXX
line if the commit addresses a particularGitHub issue.
Fixes: #13037