google / nftables

This repository contains a Go module to interact with Linux nftables (the iptables successor).
Apache License 2.0
1.12k stars 140 forks source link

Is there a way to mock interface? #257

Open dzvancuks opened 8 months ago

dzvancuks commented 8 months ago

I would like to Unit Test my NFT code. Is there a way to mock interfaces of Conn, Set, Rule and expressions? I.e. I want to apply some rules to the chain and print out results so that it could be tested against expected rules.

In current implementation I encounter problem that UT must be executed using sudo as rules are applied directly to NFT: https://github.com/google/nftables/blob/main/nftables_test.go

stapelberg commented 8 months ago

I would like to Unit Test my NFT code. Is there a way to mock interfaces of Conn, Set, Rule and expressions? I.e. I want to apply some rules to the chain and print out results so that it could be tested against expected rules.

When you say “print out results”, what sort of output do you expect?

I’m asking because the nftables package is rather low-level and works directly with the wire format. Drill into the Conn.AddTable implementation, for example, to see that it directly appends to cc.messages.

Do I understand correctly that you are asking for some higher-level abstraction where a high-level version of the request gets stored (e.g. the *nftables.Table for an AddTable call) and can later be obtained for comparison?

The main problem I see with this approach is that it won’t work for code that queries the rulesets to construct the changes to the rulesets. If we wanted to make it work, we’d need to either run against the system nftables (which you don’t want as it requires privileges), or we’d need to somehow simulate what nftables does, and that’s not something I want to get into.

In current implementation I encounter problem that UT must be executed using sudo as rules are applied directly to NFT: https://github.com/google/nftables/blob/main/nftables_test.go

Just to spell it out in detail to avoid any misunderstandings: nftables_test.go uses the nftables.WithTestDial() option when creating the connection and then captures+compares the netlink messages at the wire format level: https://github.com/google/nftables/blob/8ffcbc2d360c941349ee7162cb04e855ac0ec28b/nftables_test.go#L233-L286

This doesn’t require sudo; only the tests which query the system ruleset require sudo, as explained above.


For now, I think if you want to ensure that you did the right nftables library calls in the right order, your best bet is to define your own interface and a recorder implementation that records the parameters for later comparison.

Maybe this approach is good enough for your use-case. Or maybe you have some insight into how to generalize it so that it could work for all use-cases (despite what I outline above), in which case I’d be happy to learn more :)

dzvancuks commented 8 months ago

Hi! Thanks for the hint!

  1. As for output, the ideal way would be the same as I'd see in NFT. I.e.
    table ip global {
    chain test_chain {
        ip saddr 1.2.3.4 tcp dport ssh accept
    }
    }

    this way I could compare string to string. It would be readable from UT perspective and easy to test.

But I know that NFT sometimes mangles output by optimizing code. So some deviations are expected. At least approximate output in string format would also do.

  1. Another approach is to have mockable NFT interface. I.e. for commands like AddTable, AddChain, etc. I could just UT that these commands calls have happened:

    func TestNFT(t *testing.T) {
    nftMock := mocked.NFTinterface{}
    myNFTcontroller := NewNFTcontroller(WithInjected(nftMock )) // injecting mocked interface instead of real one
    
    // check that following function calls were made with specific arguments
    nftMock.Expect("AddTable", "my_table")
    nftMock.Expect("AddChain", "my_table", "my_chain")
    nftMock.Expect("AddRule", "my_chain", "ip saddr 1.2.3.4 tcp dport ssh accept")
    // arguments might also be structs i.e.
    // nftMock.Expect("AddChain", &nftables.Chain{Name:"my_chain", Table: "my_table"}) 
    // it is just a matter on how interface is mocked. Not a problem to implement if interfaces are there
    
    myNFTcontroller.CreateMySuperDuperFirewall()
    
    nftMock.CheckExpectations(t) // check that all functions from NFT interface were called. Otherwise fail test
    }

    I could even create mocks on my own using https://github.com/vektra/mockery. All I need is an interface for those functions.

  2. And of course there is an option with WithTestDial(). But this makes testing unreadable. Yes, it is doable, but it requires binary format. Any change to my NFT code would require completely rewriting UT in binary. It is time consuming and quite hard to do every time.

stapelberg commented 8 months ago
  1. As for output, the ideal way would be the same as I'd see in NFT. I.e.
table ip global {
    chain test_chain {
        ip saddr 1.2.3.4 tcp dport ssh accept
    }
}

this way I could compare string to string.

I can understand the appeal of this suggestion from a user perspective, but please note that the textual representation you’re describing from the nft(8) command line tool does not exist in this nftables package, as this nftables package is more low-level.

We never translate from text to nftables concept or vice-versa in this Go nftables package.

Adding this additional layer would be a lot of complexity.

3. Another approach is to have mockable NFT interface. I.e. for commands like AddTable, AddChain, etc. I could just UT that these commands calls have happened: […]

Yes, but as I mentioned, this doesn’t work as soon as you call any functions that retrieve nftables state, unless you mock those, too.

The value of such a test seems relatively low in my eyes, as you only verify that you call functions in the right order, but not that your sequence of function calls can actually configure nftables. Hence I’m not convinced that it’s worthwhile adding mock test helpers to this module.

I could even create mocks on my own using https://github.com/vektra/mockery. All I need is an interface for those functions.

No, you don’t need an interface in the nftables package for that.

You can just define an interface yourself in your own package. In fact, in Go, interfaces should typically go into the package that uses them: https://go.dev/wiki/CodeReviewComments#interfaces