Skip to content

Add nftables table#23941

Merged
zwass merged 3 commits intofleetdm:mainfrom
ilpianista:feature/nftables
Dec 4, 2024
Merged

Add nftables table#23941
zwass merged 3 commits intofleetdm:mainfrom
ilpianista:feature/nftables

Conversation

@ilpianista
Copy link
Copy Markdown
Contributor

@ilpianista ilpianista commented Nov 19, 2024

Following the discussion in #15651, this adds the nftables table by parsing the binary output.

Tests are missing, before I write them I would like to know if this is the correct way to implement it or if I should use the cryptsetup table as reference to implement it.

Checklist for submitter

  • Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
    See Changes files for more information.
  • Added/updated tests
  • Manual QA for all new/changed functionality
  • For Orbit and Fleet Desktop changes:
    • Orbit runs on macOS, Linux and Windows. Check if the orbit feature/bugfix should only apply to one platform (runtime.GOOS).

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 19, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 63.41%. Comparing base (ded196d) to head (8a57910).
Report is 285 commits behind head on main.

Files with missing lines Patch % Lines
orbit/pkg/table/extension_linux.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #23941      +/-   ##
==========================================
+ Coverage   63.12%   63.41%   +0.28%     
==========================================
  Files        1557     1580      +23     
  Lines      147693   150301    +2608     
  Branches     3687     3687              
==========================================
+ Hits        93236    95312    +2076     
- Misses      47073    47363     +290     
- Partials     7384     7626     +242     
Flag Coverage Δ
backend 64.28% <0.00%> (+0.29%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jahzielv
Copy link
Copy Markdown
Contributor

hi @ilpianista 👋

Thanks for the PR! We are taking a look internally and will follow up here with next steps!

@zayhanlon zayhanlon requested a review from zwass November 19, 2024 15:15
@lukeheath
Copy link
Copy Markdown
Member

@jahzielv @zwass Just following up to see what next steps is on this.

@zwass
Copy link
Copy Markdown
Member

zwass commented Dec 4, 2024

I added a changes file and updated to support nft binary in multiple paths (as I found a different path when testing on Ubuntu).

Because I made changes I can't approve the PR for merging. @lukeheath or @jahzielv can you please approve?

@lucasmrod lucasmrod added this to the Fleetd-1.37.0 milestone Dec 4, 2024
@zwass zwass merged commit 0028e2c into fleetdm:main Dec 4, 2024
@zwass
Copy link
Copy Markdown
Member

zwass commented Dec 4, 2024

Thank you @ilpianista for your contribution! This will go out in the next fleetd release.

@ilpianista ilpianista deleted the feature/nftables branch December 10, 2024 10:51
lucasmrod added a commit that referenced this pull request Dec 14, 2024
#15651

We missed to add the docs in the original PR:
#23941

---------

Co-authored-by: Eric <eashaw@sailsjs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants