flake: validation and client TLS e2e

changelog/v1.22.2/fix-flaky-validation-and-client-tls-e2e.yaml

@@ -0,0 +1,10 @@
+changelog:
+  - type: NON_USER_FACING
+    resolvesIssue: false
+    description: >-
+      Fix two flaky kube e2e tests. Run the ValidationSplitWebhook suite last, since it
+      cannot undo its changes to the ValidatingWebhookConfiguration (a helm hook resource
+      that `helm rollback` does not restore) and its matchConditions would otherwise
+      disable secret deletion validation for later suites. Retry the client_tls
+      VirtualService applies, which can be transiently rejected with a domain conflict
+      against a VirtualService that a prior test already deleted.

test/kubernetes/e2e/features/client_tls/suite.go

@@ -2,6 +2,7 @@ package client_tls
 
 import (
 	"context"
+	"time"
 
 	"github.com/onsi/gomega"
 	"github.com/stretchr/testify/suite"
@@ -75,8 +76,7 @@ func (s *clientTlsTestingSuite) TestRouteSecureRequestToUpstream() {
 
 	err := s.testInstallation.Actions.Kubectl().Apply(s.ctx, NginxUpstreamsYaml)
 	s.NoError(err, "can apply upstream manifest file")
-	err = s.testInstallation.Actions.Kubectl().Apply(s.ctx, VSTargetingUpstreamYaml, "-n", ns)
-	s.NoError(err, "can apply vs targeting upstream manifest file")
+	s.eventuallyApply(VSTargetingUpstreamYaml, "can apply vs targeting upstream manifest file", "-n", ns)
 
 	s.assertEventualResponseForPath("nginx", expectedHealthyResponse)
 
@@ -106,8 +106,7 @@ func (s *clientTlsTestingSuite) TestRouteSecureRequestToAnnotatedService() {
 
 	err := s.testInstallation.Actions.Kubectl().Apply(s.ctx, NginxAnnotatedServicesYaml)
 	s.NoError(err, "can apply services manifest file")
-	err = s.testInstallation.Actions.Kubectl().Apply(s.ctx, VSTargetingKubeYaml, "-n", ns)
-	s.NoError(err, "can apply vs targeting services manifest file")
+	s.eventuallyApply(VSTargetingKubeYaml, "can apply vs targeting services manifest file", "-n", ns)
 
 	s.assertEventualResponseForPath("nginx", expectedHealthyResponse)
 
@@ -188,6 +187,27 @@ func (s *clientTlsTestingSuite) TestOneWayTlsDoesNotRequestClientCertificate() {
 	}).WithContext(s.ctx).Should(gomega.Succeed())
 }
 
+// eventuallyApply applies a manifest, retrying for as long as the Edge validating admission
+// webhook rejects it.
+//
+// TestRouteSecureRequestToUpstream and TestRouteSecureRequestToAnnotatedService each create a
+// VirtualService that claims the nginx.example.com domain, and each deletes its VirtualService
+// during cleanup. Deleting a VirtualService removes it from the validator's in-memory snapshot,
+// but a snapshot that was already in flight when the delete happened can transiently re-add it
+// (see Sync in projects/gateway/pkg/validation/validator.go, which replaces latestSnapshot
+// wholesale). Until that settles, applying the other VirtualService is rejected with a domain
+// conflict against a VirtualService that no longer exists in the cluster.
+func (s *clientTlsTestingSuite) eventuallyApply(manifest []byte, description string, extraArgs ...string) {
+	s.testInstallation.AssertionsT(s.T()).Gomega.Eventually(func(g gomega.Gomega) {
+		err := s.testInstallation.Actions.Kubectl().Apply(s.ctx, manifest, extraArgs...)
+		g.Expect(err).NotTo(gomega.HaveOccurred(), description)
+	}).
+		WithContext(s.ctx).
+		WithTimeout(time.Minute).
+		WithPolling(time.Second).
+		Should(gomega.Succeed(), description)
+}
+
 func (s *clientTlsTestingSuite) assertEventualResponseForPath(path string, matcher *matchers.HttpResponse) {
 	s.testInstallation.AssertionsT(s.T()).AssertEventualCurlResponse(
 		s.ctx,

test/kubernetes/e2e/tests/validation_strict_tests.go

@@ -9,8 +9,17 @@ import (
 
 // ValidationStrictSuiteRunnerAll is used to run all the validation tests, including ones that depend on the helm chart/values/helpers
 // This is the function that should be used to run the validation tests in this repo
+//
+// These suites are ordered so that "ValidationSplitWebhook" always runs last. That suite mutates
+// the ValidatingWebhookConfiguration (failurePolicy, matchConditions) via `helm upgrade` and then
+// reverts it with `helm rollback`. However, the ValidatingWebhookConfiguration is a helm hook
+// resource ("helm.sh/hook": pre-install, pre-upgrade), so it is not part of the release manifest
+// and `helm rollback` does not restore it. The values it installs - notably the
+// `kubeCoreMatchConditions` that skip validation of secrets - therefore outlive the suite and
+// silently disable validation for any suite that runs after it.
 func ValidationStrictSuiteRunnerAll() e2e.SuiteRunner {
-	validationSuiteRunner := ValidationStrictSuiteRunner()
+	validationSuiteRunner := e2e.NewSuiteRunner(true)
+	registerValidationStrictSuites(validationSuiteRunner)
 	validationSuiteRunner.Register("ValidationSplitWebhook", split_webhook.NewTestingSuite)
 
 	return validationSuiteRunner
@@ -23,9 +32,12 @@ func ValidationStrictSuiteRunnerAll() e2e.SuiteRunner {
 // If more tests are added that depend on the helm chart/values/helpers, the above issue should be resolved instead of using this approach
 func ValidationStrictSuiteRunner() e2e.SuiteRunner {
 	validationSuiteRunner := e2e.NewSuiteRunner(false)
+	registerValidationStrictSuites(validationSuiteRunner)
 
+	return validationSuiteRunner
+}
+
+func registerValidationStrictSuites(validationSuiteRunner e2e.SuiteRunner) {
 	validationSuiteRunner.Register("ValidationStrictWarnings", validation_strict_warnings.NewTestingSuite)
 	validationSuiteRunner.Register("ValidationRejectInvalid", validation_reject_invalid.NewTestingSuite)
-
-	return validationSuiteRunner
 }