fix: [NPM] add check for valid IPV4 addresses in TranslatePolicy (#1738)

* add check for ipv6 addresses in translatePolicy

* fix static error lint issue

* updated static error name and moved IPV4 logic

* moved check to ipBlockRule

* updated UT

---------

Co-authored-by: Hunter Gregory <[email protected]>

Author: ck319

npm/pkg/controlplane/controllers/v2/networkPolicyController.go

@@ -295,8 +295,8 @@ func (c *NetworkPolicyController) syncAddAndUpdateNetPol(netPolObj *networkingv1
 		}
 
 		klog.Errorf("Failed to translate podSelector in NetworkPolicy %s in namespace %s: %s", netPolObj.ObjectMeta.Name, netPolObj.ObjectMeta.Namespace, err.Error())
-		// The exec time isn't relevant here, so consider a no-op.
-		return metrics.NoOp, errNetPolTranslationFailure
+		// The exec time isn't relevant here, so consider a no-op. Returning nil to prevent re-queuing since this is not a transient error.
+		return metrics.NoOp, nil
 	}
 
 	_, policyExisted := c.rawNpSpecMap[netpolKey]

npm/pkg/controlplane/translation/translatePolicy.go

@@ -32,6 +32,8 @@ var (
 	ErrInvalidMatchExpressionValues = errors.New(
 		"matchExpression label values must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character",
 	)
+	// ErrUnsupportedIPAddress is returned when an unsupported IP address, such as IPV6, is used
+	ErrUnsupportedIPAddress = errors.New("unsupported IP address")
 )
 
 type podSelectorResult struct {
@@ -225,6 +227,10 @@ func ipBlockRule(policyName, ns string, direction policies.Direction, matchType
 		return nil, policies.SetInfo{}, nil
 	}
 
+	if !util.IsIPV4(ipBlockRule.CIDR) {
+		return nil, policies.SetInfo{}, ErrUnsupportedIPAddress
+	}
+
 	ipBlockIPSet, err := ipBlockIPSet(policyName, ns, direction, ipBlockSetIndex, ipBlockPeerIndex, ipBlockRule)
 	if err != nil {
 		return nil, policies.SetInfo{}, err
@@ -563,8 +569,8 @@ func egressPolicy(npmNetPol *policies.NPMNetworkPolicy, netPolName string, egres
 	return nil
 }
 
-// TranslatePolicy traslates networkpolicy object to NPMNetworkPolicy object
-// and return the NPMNetworkPolicy object.
+// TranslatePolicy translates networkpolicy object to NPMNetworkPolicy object
+// and returns the NPMNetworkPolicy object.
 func TranslatePolicy(npObj *networkingv1.NetworkPolicy) (*policies.NPMNetworkPolicy, error) {
 	netPolName := npObj.Name
 	npmNetPol := policies.NewNPMNetworkPolicy(netPolName, npObj.Namespace)

npm/pkg/controlplane/translation/translatePolicy_test.go

@@ -491,6 +491,7 @@ func TestIPBlockRule(t *testing.T) {
 		translatedIPSet *ipsets.TranslatedIPSet
 		setInfo         policies.SetInfo
 		skipWindows     bool
+		wantErr         bool
 	}{
 		{
 			name:            "empty ipblock rule ",
@@ -540,6 +541,26 @@ func TestIPBlockRule(t *testing.T) {
 			setInfo:         policies.NewSetInfo("test-network-policy-in-ns-default-0-0IN", ipsets.CIDRBlocks, included, policies.SrcMatch),
 			skipWindows:     true,
 		},
+		{
+			name:        "invalid ipv6",
+			ipBlockInfo: createIPBlockInfo("test", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0),
+			ipBlockRule: &networkingv1.IPBlock{
+				CIDR: "2002::1234:abcd:ffff:c0a8:101/64",
+			},
+			translatedIPSet: nil,
+			setInfo:         policies.SetInfo{},
+			wantErr:         true,
+		},
+		{
+			name:        "invalid cidr",
+			ipBlockInfo: createIPBlockInfo("test", defaultNS, policies.Ingress, policies.SrcMatch, 0, 0),
+			ipBlockRule: &networkingv1.IPBlock{
+				CIDR: "10.0.0.1/33",
+			},
+			translatedIPSet: nil,
+			setInfo:         policies.SetInfo{},
+			wantErr:         true,
+		},
 	}
 
 	for _, tt := range tests {
@@ -550,9 +571,15 @@ func TestIPBlockRule(t *testing.T) {
 			if tt.skipWindows && util.IsWindowsDP() {
 				require.Error(t, err)
 			} else {
-				require.NoError(t, err)
-				require.Equal(t, tt.translatedIPSet, translatedIPSet)
-				require.Equal(t, tt.setInfo, setInfo)
+				if tt.wantErr {
+					require.Error(t, err)
+					require.Equal(t, tt.translatedIPSet, translatedIPSet)
+					require.Equal(t, tt.setInfo, setInfo)
+				} else {
+					require.NoError(t, err)
+					require.Equal(t, tt.translatedIPSet, translatedIPSet)
+					require.Equal(t, tt.setInfo, setInfo)
+				}
 			}
 		})
 	}