Skip to content

Commit

Permalink
Make Peer validation more comprehensive
Browse files Browse the repository at this point in the history 
Signed-off-by: wgrayson <[email protected]>
  • Loading branch information
@GraysonWu
GraysonWu committed Dec 8, 2021
1 parent b87b76c commit ba1ed4c
Show file tree
Hide file tree
Showing 2 changed files with 129 additions and 4 deletions.
23 changes: 19 additions & 4 deletions pkg/controller/networkpolicy/validate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package networkpolicy
import (
"encoding/json"
"fmt"
"reflect"
"regexp"
"strconv"
"strings"
Expand Down Expand Up @@ -461,6 +462,11 @@ func (v *antreaPolicyValidator) validateAppliedTo(ingress, egress []crdv1alpha1.
if numAppliedToInRules > 0 && (numAppliedToInRules != len(ingress)+len(egress)) {
return "appliedTo field should either be set in all rules or in none of them", false
}
for _, eachSpecAppliedTo := range specAppliedTo {
if eachSpecAppliedTo.Group != "" && numFieldsSetInPeer(eachSpecAppliedTo) > 1 {
return "group cannot be set with other peers in appliedTo", false
}
}
return "", true
}

Expand All @@ -472,10 +478,7 @@ func (v *antreaPolicyValidator) validatePeers(ingress, egress []crdv1alpha1.Rule
if peer.NamespaceSelector != nil && peer.Namespaces != nil {
return "namespaces and namespaceSelector cannot be set at the same time for a single NetworkPolicyPeer", false
}
if peer.Group == "" {
continue
}
if peer.PodSelector != nil || peer.IPBlock != nil || peer.NamespaceSelector != nil {
if peer.Group != "" && numFieldsSetInPeer(peer) > 1 {
return "group cannot be set with other peers in rules", false
}
}
Expand Down Expand Up @@ -504,6 +507,18 @@ func (v *antreaPolicyValidator) validatePeers(ingress, egress []crdv1alpha1.Rule
return "", true
}

// numFieldsSetInPeer returns the number of fields in use of a peer.
func numFieldsSetInPeer(peer crdv1alpha1.NetworkPolicyPeer) int {
num := 0
v := reflect.ValueOf(peer)
for i := 0; i < v.NumField(); i++ {
if !v.Field(i).IsZero() {
num++
}
}
return num
}

// validateTierForPolicy validates whether a referenced Tier exists.
func (v *antreaPolicyValidator) validateTierForPolicy(tier string) (string, bool) {
// "tier" must exist before referencing
Expand Down
110 changes: 110 additions & 0 deletions test/e2e/antreapolicy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,46 @@ func testInvalidACNPAppliedToNotSetInAllRules(t *testing.T) {
}
}

func testInvalidACNPAppliedToGroupSetWithPodSelector(t *testing.T) {
invalidNpErr := fmt.Errorf("invalid Antrea ClusterNetworkPolicy with appliedTo set group with podSelector")
appTo := ACNPAppliedToSpec{
PodSelector: map[string]string{"pod": "b"},
Group: "cgA",
}
builder := &ClusterNetworkPolicySpecBuilder{}
builder = builder.SetName("acnp-appto-set-group-with-psel").
SetPriority(1.0).
SetAppliedToGroup([]ACNPAppliedToSpec{appTo})
builder = builder.AddIngress(v1.ProtocolTCP, &p80, nil, nil, nil, map[string]string{"pod": "b"}, map[string]string{"ns": "x"},
nil, nil, false, nil, crdv1alpha1.RuleActionAllow, "", "")
acnp := builder.Get()
log.Debugf("creating ACNP %v", acnp.Name)
if _, err := k8sUtils.CreateOrUpdateACNP(acnp); err == nil {
// Above creation of ACNP must fail as it is an invalid spec.
failOnError(invalidNpErr, t)
}
}

func testInvalidACNPAppliedToGroupSetWithNSSelector(t *testing.T) {
invalidNpErr := fmt.Errorf("invalid Antrea ClusterNetworkPolicy with appliedTo set group with nsSelector")
appTo := ACNPAppliedToSpec{
NSSelector: map[string]string{"ns": "x"},
Group: "cgA",
}
builder := &ClusterNetworkPolicySpecBuilder{}
builder = builder.SetName("acnp-appto-set-group-with-nssel").
SetPriority(1.0).
SetAppliedToGroup([]ACNPAppliedToSpec{appTo})
builder = builder.AddIngress(v1.ProtocolTCP, &p80, nil, nil, nil, map[string]string{"pod": "b"}, map[string]string{"ns": "x"},
nil, nil, false, nil, crdv1alpha1.RuleActionAllow, "", "")
acnp := builder.Get()
log.Debugf("creating ACNP %v", acnp.Name)
if _, err := k8sUtils.CreateOrUpdateACNP(acnp); err == nil {
// Above creation of ACNP must fail as it is an invalid spec.
failOnError(invalidNpErr, t)
}
}

func testInvalidACNPIngressPeerCGSetWithPodSelector(t *testing.T) {
cgA := "cgA"
selectorA := metav1.LabelSelector{MatchLabels: map[string]string{"foo1": "bar1"}}
Expand Down Expand Up @@ -489,6 +529,71 @@ func testInvalidACNPIngressPeerCGSetWithIPBlock(t *testing.T) {
}
}

func testInvalidACNPEgressPeerCGSetWithFQDN(t *testing.T) {
cgA := "cgA"
selectorA := metav1.LabelSelector{MatchLabels: map[string]string{"foo1": "bar1"}}
k8sUtils.CreateCG(cgA, &selectorA, nil, nil)
invalidNpErr := fmt.Errorf("invalid Antrea ClusterNetworkPolicy with group and fqdn in NetworkPolicyPeer set")
builder := &ClusterNetworkPolicySpecBuilder{}
builder = builder.SetName("acnp-egress-group-fqdn-set").
SetPriority(1.0).
SetAppliedToGroup([]ACNPAppliedToSpec{{PodSelector: map[string]string{"pod": "b"}}})
builder = builder.AddEgress(v1.ProtocolTCP, &p80, nil, nil, nil, nil, nil,
nil, nil, false, nil, crdv1alpha1.RuleActionAllow, "cgA", "")
builder.Spec.Egress[0].To[0].FQDN = "google.com"

acnp := builder.Get()
log.Debugf("creating ACNP %v", acnp.Name)
if _, err := k8sUtils.CreateOrUpdateACNP(acnp); err == nil {
// Above creation of ACNP must fail as it is an invalid spec.
failOnError(invalidNpErr, t)
}
failOnError(k8sUtils.CleanCGs(), t)
}

func testInvalidACNPIngressPeerCGSetWithNamespace(t *testing.T) {
cgA := "cgA"
selectorA := metav1.LabelSelector{MatchLabels: map[string]string{"foo1": "bar1"}}
k8sUtils.CreateCG(cgA, &selectorA, nil, nil)
invalidNpErr := fmt.Errorf("invalid Antrea ClusterNetworkPolicy with group and namespace in NetworkPolicyPeer set")
builder := &ClusterNetworkPolicySpecBuilder{}
builder = builder.SetName("acnp-ingress-group-ns-set").
SetPriority(1.0).
SetAppliedToGroup([]ACNPAppliedToSpec{{PodSelector: map[string]string{"pod": "b"}}})
builder = builder.AddIngress(v1.ProtocolTCP, &p80, nil, nil, nil, nil, nil,
nil, nil, true, nil, crdv1alpha1.RuleActionAllow, "cgA", "")
acnp := builder.Get()
log.Debugf("creating ACNP %v", acnp.Name)
if _, err := k8sUtils.CreateOrUpdateACNP(acnp); err == nil {
// Above creation of ACNP must fail as it is an invalid spec.
failOnError(invalidNpErr, t)
}
failOnError(k8sUtils.CleanCGs(), t)
}

func testInvalidACNPIngressPeerCGSetWithEESelector(t *testing.T) {
cgA := "cgA"
selectorA := metav1.LabelSelector{MatchLabels: map[string]string{"foo1": "bar1"}}
k8sUtils.CreateCG(cgA, &selectorA, nil, nil)
invalidNpErr := fmt.Errorf("invalid Antrea ClusterNetworkPolicy with group and external entity selector in NetworkPolicyPeer set")
builder := &ClusterNetworkPolicySpecBuilder{}
builder = builder.SetName("acnp-ingress-group-eeselector-set").
SetPriority(1.0).
SetAppliedToGroup([]ACNPAppliedToSpec{{PodSelector: map[string]string{"pod": "b"}}})
builder = builder.AddIngress(v1.ProtocolTCP, &p80, nil, nil, nil, nil, nil,
nil, nil, true, nil, crdv1alpha1.RuleActionAllow, "cgA", "")
builder.Spec.Ingress[0].From[0].ExternalEntitySelector = &metav1.LabelSelector{
MatchLabels: map[string]string{"foo2": "bar2"},
}
acnp := builder.Get()
log.Debugf("creating ACNP %v", acnp.Name)
if _, err := k8sUtils.CreateOrUpdateACNP(acnp); err == nil {
// Above creation of ACNP must fail as it is an invalid spec.
failOnError(invalidNpErr, t)
}
failOnError(k8sUtils.CleanCGs(), t)
}

func testInvalidACNPIngressPeerNamespacesSetWithNSSelector(t *testing.T) {
invalidNpErr := fmt.Errorf("invalid Antrea ClusterNetworkPolicy with namespaces and namespaceSelector in NetworkPolicyPeer set")
builder := &ClusterNetworkPolicySpecBuilder{}
Expand Down Expand Up @@ -2989,9 +3094,14 @@ func TestAntreaPolicy(t *testing.T) {
t.Run("Case=ACNPIngressPeerCGSetWithIPBlock", func(t *testing.T) { testInvalidACNPIngressPeerCGSetWithIPBlock(t) })
t.Run("Case=ACNPIngressPeerCGSetWithPodSelector", func(t *testing.T) { testInvalidACNPIngressPeerCGSetWithPodSelector(t) })
t.Run("Case=ACNPIngressPeerCGSetWithNSSelector", func(t *testing.T) { testInvalidACNPIngressPeerCGSetWithNSSelector(t) })
t.Run("Case=ACNPEgressPeerCGSetWithFQDN", func(t *testing.T) { testInvalidACNPEgressPeerCGSetWithFQDN(t) })
t.Run("Case=ACNPIngressPeerCGSetWithNamespace", func(t *testing.T) { testInvalidACNPIngressPeerCGSetWithNamespace(t) })
t.Run("Case=ACNPIngressPeerCGSetWithEESelector", func(t *testing.T) { testInvalidACNPIngressPeerCGSetWithEESelector(t) })
t.Run("Case=ACNPIngressPeerNamespaceSetWithNSSelector", func(t *testing.T) { testInvalidACNPIngressPeerNamespacesSetWithNSSelector(t) })
t.Run("Case=ACNPSpecAppliedToRuleAppliedToSet", func(t *testing.T) { testInvalidACNPSpecAppliedToRuleAppliedToSet(t) })
t.Run("Case=ACNPAppliedToNotSetInAllRules", func(t *testing.T) { testInvalidACNPAppliedToNotSetInAllRules(t) })
t.Run("Case=ACNPAppliedToGroupSetWithPodSelector", func(t *testing.T) { testInvalidACNPAppliedToGroupSetWithPodSelector(t) })
t.Run("Case=ACNPAppliedToGroupSetWithNSSelector", func(t *testing.T) { testInvalidACNPAppliedToGroupSetWithNSSelector(t) })
t.Run("Case=ANPNoPriority", func(t *testing.T) { testInvalidANPNoPriority(t) })
t.Run("Case=ANPRuleNameNotUniqueDenied", func(t *testing.T) { testInvalidANPRuleNameNotUnique(t) })
t.Run("Case=ANPTierDoesNotExistDenied", func(t *testing.T) { testInvalidANPTierDoesNotExist(t) })
Expand Down

0 comments on commit ba1ed4c

Please sign in to comment.