Skip to content
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

Is there a way to mock interface? #257

Open
dzvancuks opened this issue Mar 25, 2024 · 3 comments
Open

Is there a way to mock interface? #257

dzvancuks opened this issue Mar 25, 2024 · 3 comments
Labels
question Further information is requested

Comments

@dzvancuks
Copy link

dzvancuks commented Mar 25, 2024

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

@dzvancuks dzvancuks changed the title Is there a way to mock interface Is there a way to mock interface? Mar 25, 2024
@stapelberg
Copy link
Collaborator

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:

nftables/nftables_test.go

Lines 233 to 286 in 8ffcbc2

func TestConfigureNAT(t *testing.T) {
// The want byte sequences come from stracing nft(8), e.g.:
// strace -f -v -x -s 2048 -eraw=sendto nft add table ip nat
//
// The nft(8) command sequence was taken from:
// https://wiki.nftables.org/wiki-nftables/index.php/Performing_Network_Address_Translation_(NAT)
want := [][]byte{
// batch begin
[]byte("\x00\x00\x00\x0a"),
// nft flush ruleset
[]byte("\x00\x00\x00\x00"),
// nft add table ip nat
[]byte("\x02\x00\x00\x00\x08\x00\x01\x00\x6e\x61\x74\x00\x08\x00\x02\x00\x00\x00\x00\x00"),
// nft add chain nat prerouting '{' type nat hook prerouting priority 0 \; '}'
[]byte("\x02\x00\x00\x00\x08\x00\x01\x00\x6e\x61\x74\x00\x0f\x00\x03\x00\x70\x72\x65\x72\x6f\x75\x74\x69\x6e\x67\x00\x00\x14\x00\x04\x80\x08\x00\x01\x00\x00\x00\x00\x00\x08\x00\x02\x00\x00\x00\x00\x00\x08\x00\x07\x00\x6e\x61\x74\x00"),
// nft add chain nat postrouting '{' type nat hook postrouting priority 100 \; '}'
[]byte("\x02\x00\x00\x00\x08\x00\x01\x00\x6e\x61\x74\x00\x10\x00\x03\x00\x70\x6f\x73\x74\x72\x6f\x75\x74\x69\x6e\x67\x00\x14\x00\x04\x80\x08\x00\x01\x00\x00\x00\x00\x04\x08\x00\x02\x00\x00\x00\x00\x64\x08\x00\x07\x00\x6e\x61\x74\x00"),
// nft add rule nat postrouting oifname uplink0 masquerade
[]byte("\x02\x00\x00\x00\x08\x00\x01\x00\x6e\x61\x74\x00\x10\x00\x02\x00\x70\x6f\x73\x74\x72\x6f\x75\x74\x69\x6e\x67\x00\x74\x00\x04\x80\x24\x00\x01\x80\x09\x00\x01\x00\x6d\x65\x74\x61\x00\x00\x00\x00\x14\x00\x02\x80\x08\x00\x02\x00\x00\x00\x00\x07\x08\x00\x01\x00\x00\x00\x00\x01\x38\x00\x01\x80\x08\x00\x01\x00\x63\x6d\x70\x00\x2c\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x08\x00\x02\x00\x00\x00\x00\x00\x18\x00\x03\x80\x14\x00\x01\x00\x75\x70\x6c\x69\x6e\x6b\x30\x00\x00\x00\x00\x00\x00\x00\x00\x00\x14\x00\x01\x80\x09\x00\x01\x00\x6d\x61\x73\x71\x00\x00\x00\x00\x04\x00\x02\x80"),
// nft add rule nat prerouting iif uplink0 tcp dport 4070 dnat 192.168.23.2:4080
[]byte("\x02\x00\x00\x00\x08\x00\x01\x00\x6e\x61\x74\x00\x0f\x00\x02\x00\x70\x72\x65\x72\x6f\x75\x74\x69\x6e\x67\x00\x00\x98\x01\x04\x80\x24\x00\x01\x80\x09\x00\x01\x00\x6d\x65\x74\x61\x00\x00\x00\x00\x14\x00\x02\x80\x08\x00\x02\x00\x00\x00\x00\x06\x08\x00\x01\x00\x00\x00\x00\x01\x38\x00\x01\x80\x08\x00\x01\x00\x63\x6d\x70\x00\x2c\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x08\x00\x02\x00\x00\x00\x00\x00\x18\x00\x03\x80\x14\x00\x01\x00\x75\x70\x6c\x69\x6e\x6b\x30\x00\x00\x00\x00\x00\x00\x00\x00\x00\x24\x00\x01\x80\x09\x00\x01\x00\x6d\x65\x74\x61\x00\x00\x00\x00\x14\x00\x02\x80\x08\x00\x02\x00\x00\x00\x00\x10\x08\x00\x01\x00\x00\x00\x00\x01\x2c\x00\x01\x80\x08\x00\x01\x00\x63\x6d\x70\x00\x20\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x08\x00\x02\x00\x00\x00\x00\x00\x0c\x00\x03\x80\x05\x00\x01\x00\x06\x00\x00\x00\x34\x00\x01\x80\x0c\x00\x01\x00\x70\x61\x79\x6c\x6f\x61\x64\x00\x24\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x08\x00\x02\x00\x00\x00\x00\x02\x08\x00\x03\x00\x00\x00\x00\x02\x08\x00\x04\x00\x00\x00\x00\x02\x2c\x00\x01\x80\x08\x00\x01\x00\x63\x6d\x70\x00\x20\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x08\x00\x02\x00\x00\x00\x00\x00\x0c\x00\x03\x80\x06\x00\x01\x00\x0f\xe6\x00\x00\x2c\x00\x01\x80\x0e\x00\x01\x00\x69\x6d\x6d\x65\x64\x69\x61\x74\x65\x00\x00\x00\x18\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x0c\x00\x02\x80\x08\x00\x01\x00\xc0\xa8\x17\x02\x2c\x00\x01\x80\x0e\x00\x01\x00\x69\x6d\x6d\x65\x64\x69\x61\x74\x65\x00\x00\x00\x18\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x02\x0c\x00\x02\x80\x06\x00\x01\x00\x0f\xf0\x00\x00\x30\x00\x01\x80\x08\x00\x01\x00\x6e\x61\x74\x00\x24\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x08\x00\x02\x00\x00\x00\x00\x02\x08\x00\x03\x00\x00\x00\x00\x01\x08\x00\x05\x00\x00\x00\x00\x02"),
// nft add rule nat prerouting iifname uplink0 udp dport 4070-4090 dnat 192.168.23.2:4070-4090
[]byte("\x02\x00\x00\x00\x08\x00\x01\x00\x6e\x61\x74\x00\x0f\x00\x02\x00\x70\x72\x65\x72\x6f\x75\x74\x69\x6e\x67\x00\x00\xf8\x01\x04\x80\x24\x00\x01\x80\x09\x00\x01\x00\x6d\x65\x74\x61\x00\x00\x00\x00\x14\x00\x02\x80\x08\x00\x02\x00\x00\x00\x00\x06\x08\x00\x01\x00\x00\x00\x00\x01\x38\x00\x01\x80\x08\x00\x01\x00\x63\x6d\x70\x00\x2c\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x08\x00\x02\x00\x00\x00\x00\x00\x18\x00\x03\x80\x14\x00\x01\x00\x75\x70\x6c\x69\x6e\x6b\x30\x00\x00\x00\x00\x00\x00\x00\x00\x00\x24\x00\x01\x80\x09\x00\x01\x00\x6d\x65\x74\x61\x00\x00\x00\x00\x14\x00\x02\x80\x08\x00\x02\x00\x00\x00\x00\x10\x08\x00\x01\x00\x00\x00\x00\x01\x2c\x00\x01\x80\x08\x00\x01\x00\x63\x6d\x70\x00\x20\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x08\x00\x02\x00\x00\x00\x00\x00\x0c\x00\x03\x80\x05\x00\x01\x00\x11\x00\x00\x00\x34\x00\x01\x80\x0c\x00\x01\x00\x70\x61\x79\x6c\x6f\x61\x64\x00\x24\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x08\x00\x02\x00\x00\x00\x00\x02\x08\x00\x03\x00\x00\x00\x00\x02\x08\x00\x04\x00\x00\x00\x00\x02\x2c\x00\x01\x80\x08\x00\x01\x00\x63\x6d\x70\x00\x20\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x08\x00\x02\x00\x00\x00\x00\x05\x0c\x00\x03\x80\x06\x00\x01\x00\x0f\xe6\x00\x00\x2c\x00\x01\x80\x08\x00\x01\x00\x63\x6d\x70\x00\x20\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x08\x00\x02\x00\x00\x00\x00\x03\x0c\x00\x03\x80\x06\x00\x01\x00\x0f\xfa\x00\x00\x2c\x00\x01\x80\x0e\x00\x01\x00\x69\x6d\x6d\x65\x64\x69\x61\x74\x65\x00\x00\x00\x18\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x0c\x00\x02\x80\x08\x00\x01\x00\xc0\xa8\x17\x02\x2c\x00\x01\x80\x0e\x00\x01\x00\x69\x6d\x6d\x65\x64\x69\x61\x74\x65\x00\x00\x00\x18\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x02\x0c\x00\x02\x80\x06\x00\x01\x00\x0f\xe6\x00\x00\x2c\x00\x01\x80\x0e\x00\x01\x00\x69\x6d\x6d\x65\x64\x69\x61\x74\x65\x00\x00\x00\x18\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x03\x0c\x00\x02\x80\x06\x00\x01\x00\x0f\xfa\x00\x00\x38\x00\x01\x80\x08\x00\x01\x00\x6e\x61\x74\x00\x2c\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x08\x00\x02\x00\x00\x00\x00\x02\x08\x00\x03\x00\x00\x00\x00\x01\x08\x00\x05\x00\x00\x00\x00\x02\x08\x00\x06\x00\x00\x00\x00\x03"),
// nft add rule nat prerouting ip daddr 10.0.0.0/24 dnat prefix to 20.0.0.0/24
[]byte("\x02\x00\x00\x00\x08\x00\x01\x00\x6e\x61\x74\x00\x0f\x00\x02\x00\x70\x72\x65\x72\x6f\x75\x74\x69\x6e\x67\x00\x00\x38\x01\x04\x80\x34\x00\x01\x80\x0c\x00\x01\x00\x70\x61\x79\x6c\x6f\x61\x64\x00\x24\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x08\x00\x02\x00\x00\x00\x00\x01\x08\x00\x03\x00\x00\x00\x00\x10\x08\x00\x04\x00\x00\x00\x00\x04\x44\x00\x01\x80\x0c\x00\x01\x00\x62\x69\x74\x77\x69\x73\x65\x00\x34\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x08\x00\x02\x00\x00\x00\x00\x01\x08\x00\x03\x00\x00\x00\x00\x04\x0c\x00\x04\x80\x08\x00\x01\x00\xff\xff\xff\x00\x0c\x00\x05\x80\x08\x00\x01\x00\x00\x00\x00\x00\x2c\x00\x01\x80\x08\x00\x01\x00\x63\x6d\x70\x00\x20\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x08\x00\x02\x00\x00\x00\x00\x00\x0c\x00\x03\x80\x08\x00\x01\x00\x0a\x00\x00\x00\x2c\x00\x01\x80\x0e\x00\x01\x00\x69\x6d\x6d\x65\x64\x69\x61\x74\x65\x00\x00\x00\x18\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x0c\x00\x02\x80\x08\x00\x01\x00\x14\x00\x00\x00\x2c\x00\x01\x80\x0e\x00\x01\x00\x69\x6d\x6d\x65\x64\x69\x61\x74\x65\x00\x00\x00\x18\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x02\x0c\x00\x02\x80\x08\x00\x01\x00\x14\x00\x00\xff\x38\x00\x01\x80\x08\x00\x01\x00\x6e\x61\x74\x00\x2c\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x08\x00\x02\x00\x00\x00\x00\x02\x08\x00\x03\x00\x00\x00\x00\x01\x08\x00\x04\x00\x00\x00\x00\x02\x08\x00\x07\x00\x00\x00\x00\x40"),
// batch end
[]byte("\x00\x00\x00\x0a"),
}
c, err := nftables.New(nftables.WithTestDial(
func(req []netlink.Message) ([]netlink.Message, error) {
for idx, msg := range req {
b, err := msg.MarshalBinary()
if err != nil {
t.Fatal(err)
}
if len(b) < 16 {
continue
}
b = b[16:]
if len(want) == 0 {
t.Errorf("no want entry for message %d: %x", idx, b)
continue
}
if got, want := b, want[0]; !bytes.Equal(got, want) {
t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want)))
}
want = want[1:]
}
return req, nil
}))
if err != nil {
t.Fatal(err)
}

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 :)

@stapelberg stapelberg added the question Further information is requested label Apr 1, 2024
@dzvancuks
Copy link
Author

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.

  1. 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
Copy link
Collaborator

  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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants