Skip to content

Commit

Permalink
Fix L7 NetworkPolicy e2e test failure (#6138)
Browse files Browse the repository at this point in the history
After switching from `wait.PollImmediate` to `assert.Eventually` in #5843,
the probe used to validate L7 NP enforcement was no longer correct.
We improve the validation logic so that each iteration of the condition
function in `assert.Eventually` only sends an HTTP probe once, instead
of using a probe with its own retry mechanism. This fixes the issue.

Fixes #6129

Signed-off-by: Hongliang Liu <[email protected]>
  • Loading branch information
hongliangl authored Mar 25, 2024
1 parent e4aff61 commit 91f374b
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 10 deletions.
11 changes: 7 additions & 4 deletions test/e2e/l7networkpolicy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@ func probeL7NetworkPolicyHTTP(t *testing.T, data *TestData, serverPodName, clien

// Verify that access to path /clientip is as expected.
assert.Eventually(t, func() bool {
_, err := probeClientIPFromPod(data, clientPodName, agnhostContainerName, baseURL)
cmd := []string{"wget", "-O", "-", fmt.Sprintf("%s/%s", baseURL, "clientip"), "-T", "1"}
_, _, err := data.RunCommandFromPod(data.testNamespace, clientPodName, agnhostContainerName, cmd)
if (allowHTTPPathClientIP && err != nil) || (!allowHTTPPathClientIP && err == nil) {
return false
}
Expand All @@ -138,7 +139,8 @@ func probeL7NetworkPolicyHTTP(t *testing.T, data *TestData, serverPodName, clien

// Verify that access to path /hostname is as expected.
assert.Eventually(t, func() bool {
hostname, err := probeHostnameFromPod(data, clientPodName, agnhostContainerName, baseURL)
cmd := []string{"wget", "-O", "-", fmt.Sprintf("%s/%s", baseURL, "hostname"), "-T", "1"}
hostname, _, err := data.RunCommandFromPod(data.testNamespace, clientPodName, agnhostContainerName, cmd)
if (allowHTTPPathHostname && err != nil) || (!allowHTTPPathHostname && err == nil) {
return false
}
Expand Down Expand Up @@ -171,7 +173,8 @@ func probeL7NetworkPolicyHTTP(t *testing.T, data *TestData, serverPodName, clien
func probeL7NetworkPolicyTLS(t *testing.T, data *TestData, clientPodName string, serverName string, canAccess bool) {
url := fmt.Sprintf("https://%s", serverName)
assert.Eventually(t, func() bool {
stdout, stderr, err := data.runWgetCommandFromTestPodWithRetry(clientPodName, data.testNamespace, agnhostContainerName, url, 5)
cmd := []string{"wget", "-O", "-", url, "-T", "5"}
stdout, stderr, err := data.RunCommandFromPod(data.testNamespace, clientPodName, agnhostContainerName, cmd)
if canAccess && err != nil {
t.Logf("Failed to access %s: %v\nStdout: %s\nStderr: %s\n", url, err, stdout, stderr)
return false
Expand All @@ -180,7 +183,7 @@ func probeL7NetworkPolicyTLS(t *testing.T, data *TestData, clientPodName string,
return false
}
return true
}, 5*time.Second, time.Second)
}, 10*time.Second, time.Second)
}

func testL7NetworkPolicyHTTP(t *testing.T, data *TestData) {
Expand Down
6 changes: 0 additions & 6 deletions test/e2e/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,6 @@ func probeFromPod(data *TestData, pod, container string, url string) error {
return err
}

func probeHostnameFromPod(data *TestData, pod, container string, baseUrl string) (string, error) {
url := fmt.Sprintf("%s/%s", baseUrl, "hostname")
hostname, _, err := data.runWgetCommandFromTestPodWithRetry(pod, data.testNamespace, container, url, 5)
return hostname, err
}

func probeClientIPFromPod(data *TestData, pod, container string, baseUrl string) (string, error) {
url := fmt.Sprintf("%s/%s", baseUrl, "clientip")
hostPort, _, err := data.runWgetCommandFromTestPodWithRetry(pod, data.testNamespace, container, url, 5)
Expand Down

0 comments on commit 91f374b

Please sign in to comment.