diff --git a/pkg/controllers/securitypolicy/pod_handler.go b/pkg/controllers/securitypolicy/pod_handler.go index 48d95ae30..bab18b7fc 100644 --- a/pkg/controllers/securitypolicy/pod_handler.go +++ b/pkg/controllers/securitypolicy/pod_handler.go @@ -102,8 +102,9 @@ var PredicateFuncsPod = predicate.Funcs{ oldObj := e.ObjectOld.(*v1.Pod) newObj := e.ObjectNew.(*v1.Pod) log.V(1).Info("receive pod update event", "namespace", oldObj.Namespace, "name", oldObj.Name) - if reflect.DeepEqual(oldObj.ObjectMeta.Labels, newObj.ObjectMeta.Labels) { - log.V(1).Info("label of pod is not changed, ignore it", "name", oldObj.Name) + // The NSX operator should handle the case when the pod phase is changed from Pending to Running. + if reflect.DeepEqual(oldObj.ObjectMeta.Labels, newObj.ObjectMeta.Labels) && oldObj.Status.Phase == newObj.Status.Phase { + log.V(1).Info("pod label and phase are not changed, ignore it", "name", oldObj.Name) return false } if util.CheckPodHasNamedPort(*newObj, "update") { diff --git a/pkg/nsx/services/securitypolicy/expand.go b/pkg/nsx/services/securitypolicy/expand.go index 18e3ad15f..f6af9e159 100644 --- a/pkg/nsx/services/securitypolicy/expand.go +++ b/pkg/nsx/services/securitypolicy/expand.go @@ -169,24 +169,19 @@ func (service *SecurityPolicyService) resolveNamedPort(obj *v1alpha1.SecurityPol return nil, err } for _, pod := range podsList.Items { - addr, err := service.resolvePodPort(pod, &spPort) - if errors.As(err, &nsxutil.PodIPNotFound{}) || errors.As(err, &nsxutil.PodNotRunning{}) { - return nil, err - } + addr := service.resolvePodPort(pod, &spPort) portAddress = append(portAddress, addr...) } } if len(portAddress) == 0 { - return nil, nsxutil.NoEffectiveOption{ - Desc: "no pod has the corresponding named port", - } + log.Info("no pod has the corresponding named port", "port", spPort) } return nsxutil.MergeAddressByPort(portAddress), nil } // Check port name and protocol, only when the pod is really running, and it does have effective ip. -func (service *SecurityPolicyService) resolvePodPort(pod v1.Pod, spPort *v1alpha1.SecurityPolicyPort) ([]nsxutil.PortAddress, error) { +func (service *SecurityPolicyService) resolvePodPort(pod v1.Pod, spPort *v1alpha1.SecurityPolicyPort) []nsxutil.PortAddress { var addr []nsxutil.PortAddress for _, c := range pod.Spec.Containers { container := c @@ -196,12 +191,12 @@ func (service *SecurityPolicyService) resolvePodPort(pod v1.Pod, spPort *v1alpha "protocol", port.Protocol, "podIP", pod.Status.PodIP) if port.Name == spPort.Port.String() && port.Protocol == spPort.Protocol { if pod.Status.Phase != "Running" { - errMsg := fmt.Sprintf("pod %s/%s is not running", pod.Namespace, pod.Name) - return nil, nsxutil.PodNotRunning{Desc: errMsg} + log.Info("pod with named port is not running", "pod.Namespace", pod.Namespace, "pod.Name", pod.Name) + return addr } if pod.Status.PodIP == "" { - errMsg := fmt.Sprintf("pod %s/%s ip not initialized", pod.Namespace, pod.Name) - return nil, nsxutil.PodIPNotFound{Desc: errMsg} + log.Info("pod with named port doesn't have initialized IP", "pod.Namespace", pod.Namespace, "pod.Name", pod.Name) + return addr } addr = append( addr, @@ -210,7 +205,7 @@ func (service *SecurityPolicyService) resolvePodPort(pod v1.Pod, spPort *v1alpha } } } - return addr, nil + return addr } func (service *SecurityPolicyService) buildRuleIPSetGroupID(ruleModel *model.Rule) string { diff --git a/test/e2e/framework.go b/test/e2e/framework.go index fc7d07f50..a321d135b 100644 --- a/test/e2e/framework.go +++ b/test/e2e/framework.go @@ -404,6 +404,31 @@ func (data *TestData) getCRResource(timeout time.Duration, cr string, namespace return crs, nil } +// deploymentWaitForNames polls the K8s apiServer once the specific pods are created, no matter they are running or not. +func (data *TestData) deploymentWaitForNames(timeout time.Duration, namespace, deployment string) ([]string, error) { + var podNames []string + opt := metav1.ListOptions{ + LabelSelector: "deployment=" + deployment, + } + err := wait.PollUntilContextTimeout(context.TODO(), 1*time.Second, timeout, false, func(ctx context.Context) (bool, error) { + if pods, err := data.clientset.CoreV1().Pods(namespace).List(context.TODO(), opt); err != nil { + if errors.IsNotFound(err) { + return false, nil + } + return false, fmt.Errorf("error when getting Pod %v", err) + } else { + for _, p := range pods.Items { + podNames = append(podNames, p.Name) + } + return true, nil + } + }) + if err != nil { + return nil, err + } + return podNames, nil +} + //Temporarily disable traffic check /* // podWaitFor polls the K8s apiServer until the specified Pod is found (in the test Namespace) and diff --git a/test/e2e/manifest/testSecurityPolicy/named-port-without-pod.yaml b/test/e2e/manifest/testSecurityPolicy/named-port-without-pod.yaml new file mode 100644 index 000000000..6d5a18d66 --- /dev/null +++ b/test/e2e/manifest/testSecurityPolicy/named-port-without-pod.yaml @@ -0,0 +1,50 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: tcp-deployment + namespace: web +spec: + replicas: 2 + selector: + matchLabels: + role: web + template: + metadata: + labels: + deployment: tcp-deployment + role: web + spec: + hostname: web-deployment + containers: + - name: web + image: "harbor-repo.vmware.com/dockerhub-proxy-cache/humanux/http_https_echo:latest" + imagePullPolicy: IfNotPresent + ports: + - containerPort: 80 + name: web-port + resources: + requests: # make sure that the pod cannot be running + cpu: "10000m" + memory: "10000Gi" +--- +apiVersion: nsx.vmware.com/v1alpha1 +kind: SecurityPolicy +metadata: + name: named-port-policy-without-pod + namespace: web +spec: + priority: 10 + appliedTo: + - podSelector: + matchLabels: + role: web + rules: + - direction: in + action: allow + ports: + - protocol: TCP + port: web-port + - direction: in + action: drop + - direction: out + action: drop \ No newline at end of file diff --git a/test/e2e/nsx_security_policy_test.go b/test/e2e/nsx_security_policy_test.go index a338edc15..5bef4affc 100644 --- a/test/e2e/nsx_security_policy_test.go +++ b/test/e2e/nsx_security_policy_test.go @@ -15,6 +15,7 @@ package e2e import ( + "fmt" "path/filepath" "testing" @@ -233,6 +234,46 @@ func TestSecurityPolicyMatchExpression(t *testing.T) { */ } +// TestSecurityPolicyNamedPortWithoutPod verifies that the traffic of security policy when named port applied. +// This test is to verify the named port feature of security policy. +// When appliedTo is in policy level and there's no pod holding the related named ports. +func TestSecurityPolicyNamedPortWithoutPod(t *testing.T) { + nsClient := "client" + nsWeb := "web" + securityPolicyCRName := "named-port-policy-without-pod" + securityPolicyNSXDisplayName := fmt.Sprintf("sp-%s-%s", nsWeb, securityPolicyCRName) + webA := "web" + labelWeb := "tcp-deployment" + ruleName0 := "all-ingress-isolation" + ruleName1 := "all-egress-isolation" + + testData.deleteNamespace(nsClient, defaultTimeout) + testData.deleteNamespace(nsWeb, defaultTimeout) + _ = testData.createNamespace(nsClient) + _ = testData.createNamespace(nsWeb) + defer testData.deleteNamespace(nsClient, defaultTimeout) + defer testData.deleteNamespace(nsWeb, defaultTimeout) + + // Create all + yamlPath, _ := filepath.Abs("./manifest/testSecurityPolicy/named-port-without-pod.yaml") + _ = applyYAML(yamlPath, "") + defer deleteYAML(yamlPath, "") + + psb, err := testData.deploymentWaitForNames(defaultTimeout, nsWeb, labelWeb) + t.Logf("Pods are %v", psb) + assertNil(t, err, "Error when waiting for IP for Pod %s", webA) + err = testData.waitForCRReadyOrDeleted(defaultTimeout, SP, nsWeb, securityPolicyCRName, Ready) + assertNil(t, err, "Error when waiting for Security Policy %s", securityPolicyCRName) + + // Check NSX resource existing + err = testData.waitForResourceExistOrNot(nsWeb, common.ResourceTypeSecurityPolicy, securityPolicyNSXDisplayName, true) + assertNil(t, err) + err = testData.waitForResourceExistOrNot(nsWeb, common.ResourceTypeRule, ruleName0, true) + assertNil(t, err) + err = testData.waitForResourceExistOrNot(nsWeb, common.ResourceTypeRule, ruleName1, true) + assertNil(t, err) +} + /* // TestSecurityPolicyNamedPort0 verifies that the traffic of security policy when named port applied. // This test is to verify the named port feature of security policy.