diff --git a/changelog/v1.22.2/fix-flaky-validation-and-client-tls-e2e.yaml b/changelog/v1.22.2/fix-flaky-validation-and-client-tls-e2e.yaml new file mode 100644 index 00000000000..8936e00f4cf --- /dev/null +++ b/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. diff --git a/test/kubernetes/e2e/features/client_tls/suite.go b/test/kubernetes/e2e/features/client_tls/suite.go index df1b96ab481..27faa3f6ea9 100644 --- a/test/kubernetes/e2e/features/client_tls/suite.go +++ b/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, diff --git a/test/kubernetes/e2e/tests/validation_strict_tests.go b/test/kubernetes/e2e/tests/validation_strict_tests.go index 63bcb005674..242f07c6df8 100644 --- a/test/kubernetes/e2e/tests/validation_strict_tests.go +++ b/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 }