Skip to content

Commit

Permalink
Add policy processed verification for networkpolicyanalysis
Browse files Browse the repository at this point in the history
Signed-off-by: Dyanngg <[email protected]>
  • Loading branch information
Dyanngg committed Dec 18, 2023
1 parent e859616 commit f29adb5
Show file tree
Hide file tree
Showing 19 changed files with 355 additions and 147 deletions.
27 changes: 14 additions & 13 deletions pkg/antctl/command_definition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
"antrea.io/antrea/pkg/antctl/transform/controllerinfo"
"antrea.io/antrea/pkg/antctl/transform/networkpolicy"
"antrea.io/antrea/pkg/antctl/transform/version"
"antrea.io/antrea/pkg/apis/controlplane"
cpv1beta "antrea.io/antrea/pkg/apis/controlplane/v1beta2"
"antrea.io/antrea/pkg/apis/crd/v1beta1"
controllernetworkpolicy "antrea.io/antrea/pkg/controller/networkpolicy"
Expand Down Expand Up @@ -909,8 +910,8 @@ func TestGetRequestErrorFallback(t *testing.T) {
}

func TestTableOutputForQueryEndpoint(t *testing.T) {
policyRef0 := controllernetworkpolicy.PolicyRef{Namespace: "testNamespace", Name: "test-ingress-egress", UID: "uid-1"}
policyRef1 := controllernetworkpolicy.PolicyRef{Namespace: "testNamespace", Name: "default-deny-egress", UID: "uid-2"}
policyRef0 := controlplane.NetworkPolicyReference{Namespace: "testNamespace", Name: "test-ingress-egress", UID: "uid-1"}
policyRef1 := controlplane.NetworkPolicyReference{Namespace: "testNamespace", Name: "default-deny-egress", UID: "uid-2"}
tc := []struct {
name string
rawResponseData interface{}
Expand Down Expand Up @@ -939,10 +940,10 @@ Ingress Rules: None
{
Namespace: "testNamespace",
Name: "podA",
Policies: []controllernetworkpolicy.Policy{{PolicyRef: policyRef0}},
Policies: []controllernetworkpolicy.Policy{{NetworkPolicyReference: policyRef0}},
Rules: []controllernetworkpolicy.Rule{
{PolicyRef: policyRef0, Direction: cpv1beta.DirectionOut, RuleIndex: 0},
{PolicyRef: policyRef0, Direction: cpv1beta.DirectionIn, RuleIndex: 0},
{NetworkPolicyReference: policyRef0, Direction: cpv1beta.DirectionOut, RuleIndex: 0},
{NetworkPolicyReference: policyRef0, Direction: cpv1beta.DirectionIn, RuleIndex: 0},
},
},
},
Expand Down Expand Up @@ -970,12 +971,12 @@ test-ingress-egress testNamespace 0 uid-1
Namespace: "testNamespace",
Name: "podA",
Policies: []controllernetworkpolicy.Policy{
{PolicyRef: policyRef0},
{PolicyRef: policyRef1},
{NetworkPolicyReference: policyRef0},
{NetworkPolicyReference: policyRef1},
},
Rules: []controllernetworkpolicy.Rule{
{PolicyRef: policyRef0, Direction: cpv1beta.DirectionOut, RuleIndex: 0},
{PolicyRef: policyRef0, Direction: cpv1beta.DirectionIn, RuleIndex: 0},
{NetworkPolicyReference: policyRef0, Direction: cpv1beta.DirectionOut, RuleIndex: 0},
{NetworkPolicyReference: policyRef0, Direction: cpv1beta.DirectionIn, RuleIndex: 0},
},
},
},
Expand Down Expand Up @@ -1009,7 +1010,7 @@ test-ingress-egress testNamespace 0 uid-1
}

func TestTableOutputForQueryNetworkPolicyAnalysis(t *testing.T) {
policyRef0 := controllernetworkpolicy.PolicyRef{Namespace: "testNamespace", Name: "test-default-deny", UID: "uid-1"}
policyRef0 := controlplane.NetworkPolicyReference{Namespace: "testNamespace", Name: "test-default-deny", UID: "uid-1"}
tc := []struct {
name string
rawResponseData interface{}
Expand All @@ -1023,9 +1024,9 @@ func TestTableOutputForQueryNetworkPolicyAnalysis(t *testing.T) {
{
name: "Matched KNP default drop rule",
rawResponseData: &controllernetworkpolicy.Rule{
PolicyRef: policyRef0,
Direction: cpv1beta.DirectionIn,
RuleIndex: -1,
NetworkPolicyReference: policyRef0,
Direction: cpv1beta.DirectionIn,
RuleIndex: -1,
},
expected: `Name Namespace RuleIndex PolicyUID Direction
test-default-deny testNamespace -1 uid-1 In
Expand Down
13 changes: 7 additions & 6 deletions pkg/apiserver/handlers/endpoint/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
v1 "k8s.io/api/admission/v1"
"k8s.io/apimachinery/pkg/api/errors"

"antrea.io/antrea/pkg/apis/controlplane"
"antrea.io/antrea/pkg/controller/networkpolicy"
queriermock "antrea.io/antrea/pkg/controller/networkpolicy/testing"
)
Expand Down Expand Up @@ -57,7 +58,7 @@ var responses = []response{
{
Policies: []networkpolicy.Policy{
{
PolicyRef: networkpolicy.PolicyRef{Name: "policy1"},
NetworkPolicyReference: controlplane.NetworkPolicyReference{Name: "policy1"},
},
},
},
Expand All @@ -70,10 +71,10 @@ var responses = []response{
{
Policies: []networkpolicy.Policy{
{
PolicyRef: networkpolicy.PolicyRef{Name: "policy1"},
NetworkPolicyReference: controlplane.NetworkPolicyReference{Name: "policy1"},
},
{
PolicyRef: networkpolicy.PolicyRef{Name: "policy2"},
NetworkPolicyReference: controlplane.NetworkPolicyReference{Name: "policy2"},
},
},
},
Expand Down Expand Up @@ -148,7 +149,7 @@ func TestSinglePolicyResponse(t *testing.T) {
{
Policies: []networkpolicy.Policy{
{
PolicyRef: networkpolicy.PolicyRef{Name: "policy1"},
NetworkPolicyReference: controlplane.NetworkPolicyReference{Name: "policy1"},
},
},
},
Expand Down Expand Up @@ -181,10 +182,10 @@ func TestMultiPolicyResponse(t *testing.T) {
{
Policies: []networkpolicy.Policy{
{
PolicyRef: networkpolicy.PolicyRef{Name: "policy1"},
NetworkPolicyReference: controlplane.NetworkPolicyReference{Name: "policy1"},
},
{
PolicyRef: networkpolicy.PolicyRef{Name: "policy2"},
NetworkPolicyReference: controlplane.NetworkPolicyReference{Name: "policy2"},
},
},
},
Expand Down
19 changes: 11 additions & 8 deletions pkg/apiserver/handlers/networkpolicyanalysis/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,9 @@ func predictEndpointsRules(srcEndpoints, dstEndpoints *networkpolicy.EndpointRul
return &networkpolicy.Rule{}
}
return &networkpolicy.Rule{
PolicyRef: networkpolicy.PolicyRef{
Namespace: commonRule.Policy.SourceRef.Namespace,
Name: commonRule.Policy.SourceRef.Name,
UID: commonRule.Policy.SourceRef.UID,
},
Direction: commonRule.Direction,
RuleIndex: commonRule.Index,
NetworkPolicyReference: *commonRule.Policy.SourceRef,
Direction: commonRule.Direction,
RuleIndex: commonRule.Index,
}
}

Expand All @@ -159,7 +155,14 @@ func HandleFunc(eq networkpolicy.EndpointQuerier) http.HandlerFunc {
http.Error(w, "invalid command argument format", http.StatusBadRequest)
return
}

if policyProcessed, err := eq.VerifyPoliciesProcessed(); !policyProcessed || err != nil {
if !policyProcessed {
http.Error(w, "policies in the cluster have not been fully processed by Antrea, please retry later", http.StatusTooEarly)
return
}
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}
// query endpoints and handle response errors
endpointAnalysisSource, err := eq.QueryNetworkPolicyRules(srcNS, srcPod)
if err != nil {
Expand Down
78 changes: 46 additions & 32 deletions pkg/apiserver/handlers/networkpolicyanalysis/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func TestIncompleteArguments(t *testing.T) {
},
{
name: "Default namespaces",
handlerRequest: "?source=pod1&destination=/pod2",
handlerRequest: "?source=pod1&destination=pod2",
argsMock: []string{"default", "pod1", "default", "pod2"},
mockQueryResponse: []mockResponse{{response: nil, error: nil}, {response: nil, error: nil}},
expectedStatus: http.StatusOK,
Expand Down Expand Up @@ -130,18 +130,29 @@ func TestNetworkPolicyAnalysis(t *testing.T) {
mockKNPEgressRules := generateRuleInfo(uid1, controlplane.K8sNetworkPolicy, controlplane.DirectionOut, nil, &defaultPriority, 1)
mockKNPIngressRules := generateRuleInfo(uid2, controlplane.K8sNetworkPolicy, controlplane.DirectionIn, nil, &defaultPriority, 1)
expectedRuleEgress := networkpolicy.Rule{
PolicyRef: networkpolicy.PolicyRef{Namespace: namespace, Name: "Policy111", UID: uid1},
Direction: v1beta2.DirectionOut,
RuleIndex: 0,
NetworkPolicyReference: controlplane.NetworkPolicyReference{Type: controlplane.AntreaNetworkPolicy, Namespace: namespace, Name: "Policy111", UID: uid1},
Direction: v1beta2.DirectionOut,
RuleIndex: 0,
}
expectedRuleIngress := networkpolicy.Rule{
PolicyRef: networkpolicy.PolicyRef{Namespace: namespace, Name: "Policy222", UID: uid2},
Direction: v1beta2.DirectionIn,
RuleIndex: 0,
expectedRuleAntreaPolicyIngress := networkpolicy.Rule{
NetworkPolicyReference: controlplane.NetworkPolicyReference{Type: controlplane.AntreaNetworkPolicy, Namespace: namespace, Name: "Policy222", UID: uid2},
Direction: v1beta2.DirectionIn,
RuleIndex: 0,
}
expectedRuleKNPEgressIsolation := expectedRuleEgress
expectedRuleKNPIngress := networkpolicy.Rule{
NetworkPolicyReference: controlplane.NetworkPolicyReference{Type: controlplane.K8sNetworkPolicy, Namespace: namespace, Name: "Policy222", UID: uid2},
Direction: v1beta2.DirectionIn,
RuleIndex: 0,
}
expectedRuleKNPEgress := networkpolicy.Rule{
NetworkPolicyReference: controlplane.NetworkPolicyReference{Type: controlplane.K8sNetworkPolicy, Namespace: namespace, Name: "Policy111", UID: uid1},
Direction: v1beta2.DirectionOut,
RuleIndex: 0,
}

expectedRuleKNPEgressIsolation := expectedRuleKNPEgress
expectedRuleKNPEgressIsolation.RuleIndex = -1
expectedRuleKNPIngressIsolation := expectedRuleIngress
expectedRuleKNPIngressIsolation := expectedRuleKNPIngress
expectedRuleKNPIngressIsolation.RuleIndex = -1

testCases := []TestCase{
Expand All @@ -154,7 +165,7 @@ func TestNetworkPolicyAnalysis(t *testing.T) {
{response: generateResponse(2, uid2, generateRuleInfo(uid1, controlplane.AntreaNetworkPolicy, controlplane.DirectionOut, &networkpolicy.DefaultTierPriority, nil, 1), nil)},
},
expectedStatus: http.StatusOK,
expectedResult: &expectedRuleIngress,
expectedResult: &expectedRuleAntreaPolicyIngress,
},
{
name: "Different policy priorities",
Expand All @@ -176,7 +187,7 @@ func TestNetworkPolicyAnalysis(t *testing.T) {
{response: generateResponse(2, uid2, generateRuleInfo(uid1, controlplane.AntreaNetworkPolicy, controlplane.DirectionOut, &networkpolicy.DefaultTierPriority, &priority1, 0), nil)},
},
expectedStatus: http.StatusOK,
expectedResult: &expectedRuleIngress,
expectedResult: &expectedRuleAntreaPolicyIngress,
},
{
name: "Different policy names",
Expand All @@ -199,7 +210,7 @@ func TestNetworkPolicyAnalysis(t *testing.T) {
mockKNPIngressRules)},
},
expectedStatus: http.StatusOK,
expectedResult: &expectedRuleIngress,
expectedResult: &expectedRuleKNPIngress,
},
{
name: "KNP and default isolation",
Expand All @@ -210,7 +221,7 @@ func TestNetworkPolicyAnalysis(t *testing.T) {
{response: generateResponse(2, uid2, mockKNPEgressRules, nil)},
},
expectedStatus: http.StatusOK,
expectedResult: &expectedRuleEgress,
expectedResult: &expectedRuleKNPEgress,
},
{
name: "KNP egress default isolation",
Expand Down Expand Up @@ -260,26 +271,29 @@ func TestNetworkPolicyAnalysis(t *testing.T) {
// argsMock has at least 4 entries and mockQueryResponse has at least 2 entries if not expecting bad request.
func evaluateTestCases(testCases []TestCase, mockCtrl *gomock.Controller, t *testing.T) {
for _, tc := range testCases {
mockQuerier := queriermock.NewMockEndpointQuerier(mockCtrl)
if tc.expectedStatus != http.StatusBadRequest {
mockQuerier.EXPECT().QueryNetworkPolicyRules(tc.argsMock[0], tc.argsMock[1]).Return(tc.mockQueryResponse[0].response, tc.mockQueryResponse[0].error)
mockQuerier.EXPECT().QueryNetworkPolicyRules(tc.argsMock[2], tc.argsMock[3]).Return(tc.mockQueryResponse[1].response, tc.mockQueryResponse[1].error)
}
t.Run(tc.name, func(t *testing.T) {
mockQuerier := queriermock.NewMockEndpointQuerier(mockCtrl)
if tc.expectedStatus != http.StatusBadRequest {
mockQuerier.EXPECT().VerifyPoliciesProcessed().Return(true, nil)
mockQuerier.EXPECT().QueryNetworkPolicyRules(tc.argsMock[0], tc.argsMock[1]).Return(tc.mockQueryResponse[0].response, tc.mockQueryResponse[0].error)
mockQuerier.EXPECT().QueryNetworkPolicyRules(tc.argsMock[2], tc.argsMock[3]).Return(tc.mockQueryResponse[1].response, tc.mockQueryResponse[1].error)
}

handler := HandleFunc(mockQuerier)
req, err := http.NewRequest(http.MethodGet, tc.handlerRequest, nil)
assert.Nil(t, err)
handler := HandleFunc(mockQuerier)
req, err := http.NewRequest(http.MethodGet, tc.handlerRequest, nil)
assert.Nil(t, err)

recorder := httptest.NewRecorder()
handler.ServeHTTP(recorder, req)
assert.Equal(t, tc.expectedStatus, recorder.Code)
if tc.expectedStatus != http.StatusOK {
return
}
recorder := httptest.NewRecorder()
handler.ServeHTTP(recorder, req)
assert.Equal(t, tc.expectedStatus, recorder.Code)
if tc.expectedStatus != http.StatusOK {
return
}

var received networkpolicy.Rule
err = json.Unmarshal(recorder.Body.Bytes(), &received)
assert.Nil(t, err)
assert.EqualValues(t, *tc.expectedResult, received)
var received networkpolicy.Rule
err = json.Unmarshal(recorder.Body.Bytes(), &received)
assert.Nil(t, err)
assert.EqualValues(t, *tc.expectedResult, received)
})
}
}
4 changes: 2 additions & 2 deletions pkg/controller/networkpolicy/adminnetworkpolicy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ func TestProcessAdminNetworkPolicy(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, features.DefaultFeatureGate, features.AdminNetworkPolicy, true)
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, c := newController(nil, nil)
_, c := newController(nil, nil, nil)
actualPolicy, actualAppliedToGroups, actualAddressGroups := c.processAdminNetworkPolicy(tt.inputPolicy)
assert.Equal(t, tt.expectedPolicy.UID, actualPolicy.UID)
assert.Equal(t, tt.expectedPolicy.Name, actualPolicy.Name)
Expand Down Expand Up @@ -712,7 +712,7 @@ func TestProcessBaselineAdminNetworkPolicy(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, features.DefaultFeatureGate, features.AdminNetworkPolicy, true)
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, c := newController(nil, nil)
_, c := newController(nil, nil, nil)
actualPolicy, actualAppliedToGroups, actualAddressGroups := c.processBaselineAdminNetworkPolicy(tt.inputPolicy)
assert.Equal(t, tt.expectedPolicy.UID, actualPolicy.UID)
assert.Equal(t, tt.expectedPolicy.Name, actualPolicy.Name)
Expand Down
8 changes: 4 additions & 4 deletions pkg/controller/networkpolicy/antreanetworkpolicy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -739,7 +739,7 @@ func TestProcessAntreaNetworkPolicy(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, c := newController(nil, nil)
_, c := newController(nil, nil, nil)
c.serviceStore.Add(&svcA)
actualPolicy, actualAppliedToGroups, actualAddressGroups := c.processAntreaNetworkPolicy(tt.inputPolicy)
assert.Equal(t, tt.expectedPolicy, actualPolicy)
Expand All @@ -750,7 +750,7 @@ func TestProcessAntreaNetworkPolicy(t *testing.T) {
}

func TestAddANNP(t *testing.T) {
_, npc := newController(nil, nil)
_, npc := newController(nil, nil, nil)
annp := getANNP()
npc.addANNP(annp)
require.Equal(t, 1, npc.internalNetworkPolicyQueue.Len())
Expand All @@ -761,7 +761,7 @@ func TestAddANNP(t *testing.T) {
}

func TestUpdateANNP(t *testing.T) {
_, npc := newController(nil, nil)
_, npc := newController(nil, nil, nil)
annp := getANNP()
newANNP := annp.DeepCopy()
// Make a change to the ANNP.
Expand All @@ -775,7 +775,7 @@ func TestUpdateANNP(t *testing.T) {
}

func TestDeleteANNP(t *testing.T) {
_, npc := newController(nil, nil)
_, npc := newController(nil, nil, nil)
annp := getANNP()
npc.deleteANNP(annp)
require.Equal(t, 1, npc.internalNetworkPolicyQueue.Len())
Expand Down
Loading

0 comments on commit f29adb5

Please sign in to comment.