Add an output-chain, needed for proxy-mode=ipvs

[uablrek]

If the receiving pod is local, then ipvs sends packet via the output hook (not forward). So both are needed

Fixes #46

Example:

# nft list table inet kube-network-policies
table inet kube-network-policies {
        comment "rules for kubernetes NetworkPolicy"
        set podips-v4 {
                type ipv4_addr
                comment "Local V4 Pod IPs with Network Policies"
                elements = { 11.0.2.2, 11.0.2.3,
                             11.0.2.4 }
        }

        set podips-v6 {
                type ipv6_addr
                comment "Local V6 Pod IPs with Network Policies"
                elements = { 1100::202,
                             1100::203,
                             1100::204 }
        }

        chain forward {
                type filter hook forward priority filter - 5; policy accept;
                ct state established,related accept
                ip saddr @podips-v4 queue to 100 comment "process IPv4 traffic with network policy enforcement"
                ip daddr @podips-v4 queue to 100 comment "process IPv4 traffic with network policy enforcement"
                ip6 saddr @podips-v6 queue to 100 comment "process IPv6 traffic with network policy enforcement"
                ip6 daddr @podips-v6 queue to 100 comment "process IPv6 traffic with network policy enforcement"
        }

        chain output {
                type filter hook output priority filter - 5; policy accept;
                ct state established,related accept
                ip saddr @podips-v4 queue to 100 comment "process IPv4 traffic with network policy enforcement"
                ip daddr @podips-v4 queue to 100 comment "process IPv4 traffic with network policy enforcement"
                ip6 saddr @podips-v6 queue to 100 comment "process IPv6 traffic with network policy enforcement"
                ip6 daddr @podips-v6 queue to 100 comment "process IPv6 traffic with network policy enforcement"
        }
}

The "chain output" is added by this PR.

Please see issue #46 for more info

[uablrek]

/cc @aojea @danwinship

[aojea]

/approve

I think this is ok, but let's wait if Dan have time for review

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aojea, uablrek

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [aojea]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[uablrek]

@aojea What does "npa" mean? Is it a flake? It seem to be sctp related. I didn't exclude sctp when I ran e2e, so the ~5 sctp e2e test-cases works for me

[danwinship]

"NPA" seems to mean "AdminNetworkPolicy"
Only the IPv6 run failed... let's see if it fails if we run it again

[danwinship]

/lgtm
other than the test failure; I believe Antonio and I had debated forward-only vs forward-and-output earlier...

[aojea]

hmm, it passed in the other CI https://github.com/kubernetes-sigs/network-policy-api/actions/runs/9740745124/job/26878488301

[uablrek]

Yeah, and failed 3 times with this PR. There is a link in the log to https://github.com/kubernetes-sigs/network-policy-api.
Perhaps @astoycos has any lead?

[aojea]

it has to be this PR then

[aojea]

2024-07-01T12:51:17.425650028Z stderr F I0701 12:51:17.425622 1 controller.go:390] Processing sync for packet 1783
2024-07-01T12:51:17.425660467Z stderr F I0701 12:51:17.425629 1 controller.go:394] Can not process packet 1783 applying default policy (failOpen: false): unknown protocol 58

it seems is blocking icmpv6 because does not recognize it

It seems that is better to just log and let the packet pass through rather than applying the default policy, if is not a protocol that is supported by the network policy APIs blocking them can cause issues, @uablrek can you append a commit to comment this and set the verdict to accept if parsePacket fails?

kube-network-policies/pkg/networkpolicy/controller.go

Lines 392 to 396 in db089bc

	packet, err := parsePacket(*a.Payload)
	if err != nil {
	klog.Infof("Can not process packet %d applying default policy (failOpen: %v): %v", *a.PacketID, c.config.FailOpen, err)
	c.nfq.SetVerdict(*a.PacketID, verdict) //nolint:errcheck
	return 0

[aojea]

hmm @BenTheElder points there are new changes on docker , another possible reason https://docs.docker.com/engine/release-notes/27.0/#ipv6

[BenTheElder]

We just picked up docker v27.x in CI recently, see the thread here kubernetes-sigs/kind#3677 and kubernetes/test-infra#32863 (comment)

Docker v27 has some behavior changes for ipv6.

[uablrek]

@uablrek can you append a commit to comment this and set the verdict to accept if parsePacket fails?

Sure

[k8s-ci-robot]

New changes are detected. LGTM label has been removed.

[uablrek]

It may be better to extend parsePacket(). I have intended to do that (#15), but haven't got around to do it yet

[aojea]

ok, progressing, different error now??

    helper.go:123: StatefulSet replicas in namespace network-policy-conformance-forbidden-forrest not rolled out yet. 1/2 replicas are available.
    helper.go:120: Error retrieving StatefulSet harry-potter from namespace network-policy-conformance-gryffindor: client rate limiter Wait returned an error: context deadline exceeded
    helper.go:123: StatefulSet replicas in namespace network-policy-conformance-gryffindor not rolled out yet. 0/2 replicas are available.
    suite.go:143: 

[uablrek]

This time the error seem unreated to the PR. How can allowing icmp make a StatefulSet fail to start??

[aojea]

This time the error seem unreated to the PR. How can allowing icmp make a StatefulSet fail to start??

the failure has to be related to the output rule we just added, it does not fail in #49

[aojea]

@uablrek please leave only first and last commit,

[uablrek]

@uablrek please leave only first and last commit

@aojea Done. But tests fail. I can't see why.

Local test works.

export FOCUS="\[sig-network\].*\[Feature:NetworkPolicy\].*"

[aojea]

interesting, anyway, this is the part we need to debug and troubleshoot

[aojea]

Once we get this sorted out I cut a new release so we can get this into kindnet

[aojea]

@uablrek can you rebase?

[aojea]

I see, so the problem most likely was that blocking unknown protocols we were blocking icmp, that in IPv6 is needed for discovery ... that may be the reason why adding lo to the rules made the test pass

[k8s-ci-robot]

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[uablrek]

@aojea This can be closed, right?

[aojea]

yes

[aojea]

we also have coverage now in CI for IPVS to avoid regressions