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

[release-4.15] OCPBUGS-43769: OCPBUGS-42244: Implement namespaceSelectors on ingress with all pods IPs #639

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 42 additions & 4 deletions pkg/network/node/networkpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package node
import (
"context"
"fmt"
"math"
"os"
"reflect"
"sort"
"strings"
Expand Down Expand Up @@ -35,6 +37,8 @@ const HostNetworkNamespace = "openshift-host-network"
// only used by network policy functions
const NetPolIsolationCookie = 1147582955

const migrationEnvVar = "NODE_CNI"

type networkPolicyPlugin struct {
node *OsdnNode
vnids *nodeVNIDMap
Expand Down Expand Up @@ -63,6 +67,9 @@ type networkPolicyPlugin struct {
// into unProcessedVNIDs in EnsureVNIDRules() method and it can be looked up later in
// AddNetNamespace() method so that required nw policy flow rules programmed into OVS.
unProcessedVNIDs sets.Int32

// indicate if the node is running in SDN live migration mode
inMigrationMode bool
}

// npNamespace tracks NetworkPolicy-related data for a Namespace
Expand Down Expand Up @@ -116,16 +123,17 @@ const (
)

func NewNetworkPolicyPlugin() osdnPolicy {
_, inMigration := os.LookupEnv(migrationEnvVar)
return &networkPolicyPlugin{
namespaces: make(map[uint32]*npNamespace),
namespacesByName: make(map[string]*npNamespace),

nsMatchCache: make(map[string]*npCacheEntry),

warnedPolicies: make(map[ktypes.UID]string),
skippedPolicies: make(map[ktypes.UID]string),
localPodIPs: make(map[ktypes.UID]string),

warnedPolicies: make(map[ktypes.UID]string),
skippedPolicies: make(map[ktypes.UID]string),
localPodIPs: make(map[ktypes.UID]string),
inMigrationMode: inMigration,
unProcessedVNIDs: sets.NewInt32(),
}
}
Expand Down Expand Up @@ -775,6 +783,12 @@ func (np *networkPolicyPlugin) parsePeerFlows(npns *npNamespace, npp *npPolicy,
if dir == ingressFlow && (len(peer.PodSelector.MatchLabels) == 0 && len(peer.PodSelector.MatchExpressions) == 0) {
// The PodSelector is empty, meaning it selects all pods in this namespace
peerFlows = append(peerFlows, fmt.Sprintf("reg0=%d, ", npns.vnid))
if np.inMigrationMode {
npp.watchesOwnPods = true
for _, ip := range np.selectPods(npns, peer.PodSelector) {
peerFlows = append(peerFlows, fmt.Sprintf("ip, nw_src=%s, ", ip))
}
}
} else {
npp.watchesOwnPods = true
for _, ip := range np.selectPods(npns, peer.PodSelector) {
Expand All @@ -794,6 +808,30 @@ func (np *networkPolicyPlugin) parsePeerFlows(npns *npNamespace, npp *npPolicy,
// checking the source VNID...
npp.watchesNamespaces = true
peerFlows = append(peerFlows, np.selectNamespaces(peer.NamespaceSelector, dir)...)
if np.inMigrationMode {
// In SDN live migration mode, we can't use source VNID to
// distinguish namespaces for traffic from OVN nodes.
// So instead we pretend the rule was a combined
// namespaceSelector+podSelector rule with a match-all
// podSelector, and generate per-pod-IP match rules.
npp.watchesAllPods = true
peerFlows = append(peerFlows, np.selectPodsFromNamespaces(peer.NamespaceSelector, &metav1.LabelSelector{}, dir)...)

// If the host network namespace is selected, Add rules for
// the OVN mp0 IP of each node.
sel, _ := metav1.LabelSelectorAsSelector(peer.NamespaceSelector)
if _, ok := np.nsMatchCache[sel.String()].matches[HostNetworkNamespace]; ok {
for _, network := range np.node.networkInfo.ClusterNetworks {
cidrIP := network.ClusterCIDR.IP
cidrIP[3] = cidrIP[3] | 0x2
maskLen, _ := network.ClusterCIDR.Mask.Size()
// use a mask to match the OVN mp0 IP which is the second IP of each host subnet.
mask := uint32(math.MaxUint32<<(32-maskLen) | math.MaxUint32>>(32-network.HostSubnetLength))
flow := fmt.Sprintf("ip, nw_src=%s/%d.%d.%d.%d, ", cidrIP, (mask>>24)&0xff, (mask>>16)&0xff, (mask>>8)&0xff, mask&0xff)
peerFlows = append(peerFlows, flow)
}
}
}
} else {
// ... but for namespaceSelectors on egress, we can't just
// check the destination VNID because we don't know that
Expand Down
158 changes: 158 additions & 0 deletions pkg/network/node/networkpolicy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package node
import (
"context"
"fmt"
"net"
"strings"
"sync/atomic"
"testing"
Expand All @@ -21,6 +22,7 @@ import (
"k8s.io/kubernetes/pkg/util/async"

osdnv1 "github.com/openshift/api/network/v1"
"github.com/openshift/sdn/pkg/network/common"
"github.com/openshift/sdn/pkg/util/ovs"
)

Expand Down Expand Up @@ -932,6 +934,162 @@ func TestNetworkPolicy(t *testing.T) {
}
}

func TestNetworkPolicyInMigrationMode(t *testing.T) {
var err error
np, _, synced, stopCh := newTestNPP()
np.inMigrationMode = true
_, clusterCIDR, _ := net.ParseCIDR("10.128.0.0/14")
np.node.networkInfo = &common.ParsedClusterNetwork{
ClusterNetworks: []common.ParsedClusterNetworkEntry{
{
ClusterCIDR: clusterCIDR,
HostSubnetLength: 9,
},
},
}
defer close(stopCh)

// Create some Namespaces
addNamespace(np, "default", 0, map[string]string{"default": "true"})
addNamespace(np, "one", 1, map[string]string{"parity": "odd"})
addNamespace(np, "two", 2, map[string]string{"parity": "even"})

npns1 := np.namespaces[1]
npns2 := np.namespaces[2]

addPods(np, npns1)
addPods(np, npns2)

// Allow client pods in the same namespace to connect to the server in namespace "one"
synced.Store(false)
addNetworkPolicy(np, &networkingv1.NetworkPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "allow-from-samenamespace",
UID: uid(npns1, "allow-from-samenamespace"),
Namespace: "one",
},
Spec: networkingv1.NetworkPolicySpec{
PodSelector: metav1.LabelSelector{
MatchLabels: map[string]string{},
},
PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress},
Ingress: []networkingv1.NetworkPolicyIngressRule{{
From: []networkingv1.NetworkPolicyPeer{{
PodSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{},
},
}},
}},
},
})
waitForSync(np, synced, "networkpolicy sync")

err = assertPolicies(np, npns1, 1, map[string]*npPolicy{
"allow-from-samenamespace": {
watchesNamespaces: false,
watchesAllPods: false,
watchesOwnPods: true,
ingressFlows: []string{
fmt.Sprintf("reg0=%d", npns1.vnid),
fmt.Sprintf("ip, nw_src=%s", clientIP(npns1)),
fmt.Sprintf("ip, nw_src=%s", serverIP(npns1)),
},
},
})
if err != nil {
t.Error(err.Error())
}

// Allow client pods in even namespaces to connect to the server in namespace "one"
synced.Store(false)
addNetworkPolicy(np, &networkingv1.NetworkPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "allow-from-even",
UID: uid(npns1, "allow-from-even"),
Namespace: "one",
},
Spec: networkingv1.NetworkPolicySpec{
PodSelector: metav1.LabelSelector{
MatchLabels: map[string]string{
"kind": "server",
},
},
PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress},
Ingress: []networkingv1.NetworkPolicyIngressRule{{
From: []networkingv1.NetworkPolicyPeer{{
NamespaceSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"parity": "even",
},
},
}},
}},
},
})
waitForSync(np, synced, "networkpolicy sync")

err = assertPolicies(np, npns1, 2, map[string]*npPolicy{
"allow-from-even": {
watchesNamespaces: true,
watchesAllPods: true,
watchesOwnPods: true,
ingressFlows: []string{
fmt.Sprintf("ip, nw_dst=%s, reg0=%d", serverIP(npns1), npns2.vnid),
fmt.Sprintf("ip, nw_dst=%s, ip, nw_src=%s", serverIP(npns1), clientIP(npns2)),
fmt.Sprintf("ip, nw_dst=%s, ip, nw_src=%s", serverIP(npns1), serverIP(npns2)),
},
},
})
if err != nil {
t.Error(err.Error())
}

// Create the special namespace that indicates host network traffic
addNamespace(np, "openshift-host-network", 200, map[string]string{"network.openshift.io/policy-group": "ingress"})
// Create the namespace to add network policy for
addNamespace(np, "host-network-target", 10, map[string]string{"foo": "bar"})
npns := np.namespaces[10]
synced.Store(false)
addNetworkPolicy(np, &networkingv1.NetworkPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "allow-from-host-network-ns",
UID: uid(npns, "allow-from-host-network-ns"),
Namespace: npns.name,
},
Spec: networkingv1.NetworkPolicySpec{
PodSelector: metav1.LabelSelector{},
PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress},
Ingress: []networkingv1.NetworkPolicyIngressRule{{
From: []networkingv1.NetworkPolicyPeer{{
NamespaceSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"network.openshift.io/policy-group": "ingress",
},
},
}},
}},
},
})
waitForSync(np, synced, "host-network NP addition")

// make sure we add the right flows
err = assertPolicies(np, npns, 1, map[string]*npPolicy{
"allow-from-host-network-ns": {
watchesNamespaces: true,
watchesAllPods: true,
watchesOwnPods: false,
ingressFlows: []string{
"reg0=0", //make sure host network namespace is classified into vnid 0
// allow the traffic from the second IP of the node subnet which is the onv-k mp0 interface IP
"ip, nw_src=10.128.0.2/255.252.1.255",
},
},
})
if err != nil {
t.Error(err.Error())
}
}

func TestNetworkPolicy_ipBlock(t *testing.T) {
np, _, synced, stopCh := newTestNPP()
defer close(stopCh)
Expand Down
2 changes: 1 addition & 1 deletion pkg/network/node/ovscontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ const (
Vxlan0 = "vxlan0"

// rule versioning; increment each time flow rules change
ruleVersion = 13
ruleVersion = 14

ruleVersionTable = 253
)
Expand Down
2 changes: 1 addition & 1 deletion pkg/network/node/ovscontroller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1060,7 +1060,7 @@ var expectedFlows = []string{
" cookie=0, table=111, priority=100, actions=move:NXM_NX_REG0[]->NXM_NX_TUN_ID[0..31],set_field:10.0.123.45->tun_dst,output:1,set_field:10.0.45.123->tun_dst,output:1,goto_table:120",
" cookie=0, table=120, priority=100, reg0=99, actions=output:4,output:5,output:6",
" cookie=0, table=120, priority=0, actions=drop",
" cookie=0, table=253, actions=note:00.0D",
" cookie=0, table=253, actions=note:00.0E",
}

// Ensure that we do not change the OVS flows without bumping ruleVersion
Expand Down