feat(preflights): add arp filtering related preflights #1454

JGAntunes · 2024-11-05T18:49:32Z

What this PR does / why we need it:

~~Keeping the PR in draft mode as we try to create sysctl specifc preflights instead:~~

Changes have been merged, I'll work on releasing troubleshoot and testing the spec.

Docs for reference - https://www.kernel.org/doc/html/latest/networking/ip-sysctl.html

Which issue(s) this PR fixes:

https://app.shortcut.com/replicated/story/114999/add-preflights-for-arp-filter-and-arp-ignore-kernel-parameters

Does this PR require a test?

Simple spec change, but run a test locally:

root@node0:/replicatedhq/embedded-cluster# output/bin/embedded-cluster install run-preflights --no-prompt --license local-dev/Joao.yaml
✔  Host files materialized!
✔  Host preflights succeeded!
Host preflights completed successfully
root@node0:/replicatedhq/embedded-cluster# sysctl net.ipv4.conf.default.arp_filter=1
net.ipv4.conf.default.arp_filter = 1
root@node0:/replicatedhq/embedded-cluster# sysctl net.ipv4.conf.default.arp_ignore=1
net.ipv4.conf.default.arp_ignore = 1
root@node0:/replicatedhq/embedded-cluster# sysctl net.ipv4.conf.all.arp_filter=1
net.ipv4.conf.all.arp_filter = 1
root@node0:/replicatedhq/embedded-cluster# sysctl net.ipv4.conf.all.arp_ignore=1
net.ipv4.conf.all.arp_ignore = 1
root@node0:/replicatedhq/embedded-cluster# output/bin/embedded-cluster install run-preflights --no-prompt --license local-dev/Joao.yaml
✔  Host files materialized!
✗  4 host preflights failed

 •  ARP filtering is enabled by default for newly created interfaces on the host. Disable it by editing `/etc/sysctl.conf` and adding the line
    `net.ipv4.conf.default.arp_filter=0` followed by running `sudo sysctl -p`.
 •  ARP ignore is enabled by default for newly created interfaces on the host. Disable it by editing `/etc/sysctl.conf` and adding the line
    `net.ipv4.conf.default.arp_ignore=0` followed by running `sudo sysctl -p`.
 •  ARP filtering is enabled for all interfaces on the host. Disable it by editing `/etc/sysctl.conf` and adding the line
    `net.ipv4.conf.all.arp_filter=0` followed by running `sudo sysctl -p`.
 •  ARP ignore is enabled for all interfaces on the host. Disable it by editing `/etc/sysctl.conf` and adding the line
    `net.ipv4.conf.all.arp_ignore=0` followed by running `sudo sysctl -p`.

Please address these issues and try again.

Does this PR require a release note?

Adds preflight checks for the `arp_ignore` and `arp_filter` kernel parameters

Does this PR require documentation?

NONE

ricardomaraschini

Shouldn't we use the collector introduced by #1449 ?

JGAntunes · 2024-11-06T10:57:54Z

@ricardomaraschini we're keeping this on pause while we look into:

But yeah, if we end up going with this PR we should use the collector you introduced.

ricardomaraschini · 2024-11-06T11:01:56Z

@ricardomaraschini we're keeping this on pause while we look into:

Create a sysctl host collector troubleshoot#1674

Create a sysctl host analyzer troubleshoot#1675

But yeah, if we end up going with this PR we should use the collector you introduced.

Oh, nice. In the end we would migrate this (and mine) to use the new collector and analyzer. Thanks.

github-actions · 2024-11-08T14:57:53Z

This PR has been released (on staging) and is available for download with a embedded-cluster-smoke-test-staging-app license ID.

Online Installer:

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci/appver-dev-3735481" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Airgap Installer (may take a few minutes before the airgap bundle is built):

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci-airgap/appver-dev-3735481?airgap=true" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Happy debugging!

JGAntunes · 2024-11-08T19:33:02Z

@ajp-io I've updated the spec, feel free to 👀 the preflight text and push any changes that you deem worth. I'll work on updating troubleshoot and test the spec next week.

ajp-io · 2024-11-08T19:48:03Z

pkg/preflights/host-preflight.yaml

+          - fail:
+              when: 'net.ipv4.conf.default.arp_filter > 0'
+              message: "ARP filtering is enabled by default for newly created interfaces on the host. Disable it by running 'sysctl net.ipv4.conf.default.arp_filter=0'."
+          - fail:
+              when: 'net.ipv4.conf.default.arp_ignore > 0'
+              message: "ARP ignore is enabled by default for newly created interfaces on the host. Disable it by running 'sysctl net.ipv4.conf.default.arp_ignore=0'."
+          - fail:
+              when: 'net.ipv4.conf.all.arp_filter > 0'
+              message: "ARP filtering is enabled for all interfaces on the host. Disable it by running 'sysctl net.ipv4.conf.all.arp_filter=0'."
+          - fail:
+              when: 'net.ipv4.conf.all.arp_ignore > 0'
+              message: "ARP ignore is enabled for all interfaces on the host. Disable it by running 'sysctl net.ipv4.conf.all.arp_ignore=0'."


I think you want a pass condition here too.

As I understand Troubleshoot, you hit a matching when and drop out there. So if someone has filter and ignore set incorrectly, they would only get the message that comes first in this list. Is that correct? If so, can we make this so that all failure conditions would show? Not sure if that needs to be two analyzers or something else.

You're right, I misunderstood troubleshoot in that aspect. Tested it out locally now and looks 👍

The separate analyzers looks good. I still think you need a pass for each in addition to the fail. See the other analyzers in there, like here.

Addressed via - 3735481

pkg/preflights/host-preflight.yaml

ajp-io · 2024-11-08T19:55:09Z

pkg/preflights/host-preflight.yaml

+        outcomes:
+          - fail:
+              when: 'net.ipv4.conf.default.arp_filter > 0'
+              message: "ARP filtering is enabled by default for newly created interfaces on the host. Disable it by running 'sysctl net.ipv4.conf.default.arp_filter=0'."


I'm not sure these instructions make the change permanent. I think you might want to edit /etc/sysctl.conf and then use sysctl -p.

True, changed the instructions in - 6c800ba

JGAntunes · 2024-11-11T15:12:03Z

@ajp-io addressed your review and have run this locally:

root@node0:/replicatedhq/embedded-cluster# output/bin/embedded-cluster install run-preflights --no-prompt --license local-dev/Joao.yaml
✔  Host files materialized!
✔  Host preflights succeeded!
Host preflights completed successfully
root@node0:/replicatedhq/embedded-cluster# sysctl net.ipv4.conf.default.arp_filter=1
net.ipv4.conf.default.arp_filter = 1
root@node0:/replicatedhq/embedded-cluster# sysctl net.ipv4.conf.default.arp_ignore=1
net.ipv4.conf.default.arp_ignore = 1
root@node0:/replicatedhq/embedded-cluster# sysctl net.ipv4.conf.all.arp_filter=1
net.ipv4.conf.all.arp_filter = 1
root@node0:/replicatedhq/embedded-cluster# sysctl net.ipv4.conf.all.arp_ignore=1
net.ipv4.conf.all.arp_ignore = 1
root@node0:/replicatedhq/embedded-cluster# output/bin/embedded-cluster install run-preflights --no-prompt --license local-dev/Joao.yaml
✔  Host files materialized!
✗  4 host preflights failed

 •  ARP filtering is enabled by default for newly created interfaces on the host. Disable it by editing `/etc/sysctl.conf` and adding the line
    `net.ipv4.conf.default.arp_filter=0` followed by running `sudo sysctl -p`.
 •  ARP ignore is enabled by default for newly created interfaces on the host. Disable it by editing `/etc/sysctl.conf` and adding the line
    `net.ipv4.conf.default.arp_ignore=0` followed by running `sudo sysctl -p`.
 •  ARP filtering is enabled for all interfaces on the host. Disable it by editing `/etc/sysctl.conf` and adding the line
    `net.ipv4.conf.all.arp_filter=0` followed by running `sudo sysctl -p`.
 •  ARP ignore is enabled for all interfaces on the host. Disable it by editing `/etc/sysctl.conf` and adding the line
    `net.ipv4.conf.all.arp_ignore=0` followed by running `sudo sysctl -p`.

Please address these issues and try again.

ajp-io · 2024-11-12T19:30:54Z

I left a comment on an outdated comment, which means you might not see it. Here it is #1454 (comment)

JGAntunes · 2024-11-13T15:33:04Z

@ajp-io addressed via 3735481

ajp-io · 2024-11-13T16:07:14Z

lgtm, though I don't know if you need an engineer's review too

JGAntunes self-assigned this Nov 5, 2024

ricardomaraschini reviewed Nov 6, 2024

View reviewed changes

JGAntunes force-pushed the jgantunes/sc-114999/add-preflights-for-arp-filter-and-arp-ignore branch from 25f513b to 0d909cf Compare November 8, 2024 15:03

JGAntunes requested a review from ajp-io November 8, 2024 19:31

ajp-io reviewed Nov 8, 2024

View reviewed changes

JGAntunes added 5 commits November 11, 2024 15:08

feat(preflights): add arp filtering related preflights

518af2d

chore: change to the soon to be added sysctl collector and analyzer

514b33a

chore: remove the e2e tests (which are now covered in troubleshoot)

0d58f62

fix: individual arp preflights

40faa3f

chore: bump troubleshoot

3bb1fef

JGAntunes force-pushed the jgantunes/sc-114999/add-preflights-for-arp-filter-and-arp-ignore branch from b93b64b to 3bb1fef Compare November 11, 2024 15:09

JGAntunes requested review from ajp-io and ricardomaraschini November 11, 2024 15:11

JGAntunes marked this pull request as ready for review November 11, 2024 15:11

JGAntunes added the type::feature label Nov 11, 2024

Update messages and slightly change order

a5c30aa

chore: add pass analyzers to arp preflights

3735481

laverya approved these changes Nov 13, 2024

View reviewed changes

JGAntunes merged commit f805a16 into main Nov 13, 2024
67 checks passed

JGAntunes deleted the jgantunes/sc-114999/add-preflights-for-arp-filter-and-arp-ignore branch November 13, 2024 17:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(preflights): add arp filtering related preflights #1454

feat(preflights): add arp filtering related preflights #1454

JGAntunes commented Nov 5, 2024 •

edited

Loading

ricardomaraschini left a comment

JGAntunes commented Nov 6, 2024

ricardomaraschini commented Nov 6, 2024

github-actions bot commented Nov 8, 2024 •

edited

Loading

JGAntunes commented Nov 8, 2024

ajp-io Nov 8, 2024

JGAntunes Nov 11, 2024

ajp-io Nov 12, 2024

JGAntunes Nov 13, 2024

ajp-io Nov 8, 2024

JGAntunes Nov 11, 2024

JGAntunes commented Nov 11, 2024

ajp-io commented Nov 12, 2024

JGAntunes commented Nov 13, 2024

ajp-io commented Nov 13, 2024 •

edited

Loading

feat(preflights): add arp filtering related preflights #1454

feat(preflights): add arp filtering related preflights #1454

Conversation

JGAntunes commented Nov 5, 2024 • edited Loading

What this PR does / why we need it:

Which issue(s) this PR fixes:

Does this PR require a test?

Does this PR require a release note?

Does this PR require documentation?

ricardomaraschini left a comment

Choose a reason for hiding this comment

JGAntunes commented Nov 6, 2024

ricardomaraschini commented Nov 6, 2024

github-actions bot commented Nov 8, 2024 • edited Loading

JGAntunes commented Nov 8, 2024

ajp-io Nov 8, 2024

Choose a reason for hiding this comment

JGAntunes Nov 11, 2024

Choose a reason for hiding this comment

ajp-io Nov 12, 2024

Choose a reason for hiding this comment

JGAntunes Nov 13, 2024

Choose a reason for hiding this comment

ajp-io Nov 8, 2024

Choose a reason for hiding this comment

JGAntunes Nov 11, 2024

Choose a reason for hiding this comment

JGAntunes commented Nov 11, 2024

ajp-io commented Nov 12, 2024

JGAntunes commented Nov 13, 2024

ajp-io commented Nov 13, 2024 • edited Loading

JGAntunes commented Nov 5, 2024 •

edited

Loading

github-actions bot commented Nov 8, 2024 •

edited

Loading

ajp-io commented Nov 13, 2024 •

edited

Loading