Merge branch 'main' into dynamic_clients

test/integration/category_test.go

@@ -1,4 +1,4 @@
-// Copyright 2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package integration
@@ -32,17 +32,18 @@ func runTestKubectlCommand(t *testing.T, args ...string) (string, string) {
 func requireCleanKubectlStderr(t *testing.T, stderr string) {
 	// Every line must be empty or contain a known, innocuous warning.
 	for _, line := range strings.Split(stderr, "\n") {
-		if strings.TrimSpace(line) == "" {
-			continue
+		switch {
+		case strings.TrimSpace(line) == "",
+			strings.Contains(line, "Throttling request took"),
+			strings.Contains(line, "due to client-side throttling, not priority and fairness"),
+			strings.Contains(line, "the gcp auth plugin is deprecated in v1.22+, unavailable in v1.25+; use gcloud instead"),
+			strings.Contains(line, "To learn more, consult https://cloud.google.com/blog/products/containers-kubernetes/kubectl-auth-changes-in-gke"):
+			// ignore these allowed stderr lines
+		default:
+			// anything else is a failure
+			require.Failf(t, "unexpected kubectl stderr", "kubectl produced unexpected stderr:\n%s\n\n", stderr)
+			return
 		}
-		if strings.Contains(line, "Throttling request took") {
-			continue
-		}
-		if strings.Contains(line, "due to client-side throttling, not priority and fairness") {
-			continue
-		}
-		require.Failf(t, "unexpected kubectl stderr", "kubectl produced unexpected stderr:\n%s\n\n", stderr)
-		return
 	}
 }
 

test/integration/e2e_test.go

@@ -634,15 +634,13 @@ func TestE2EFullIntegration_Browser(t *testing.T) {
 	// Add an LDAP upstream IDP and try using it to authenticate during kubectl commands
 	// by interacting with the CLI's username and password prompts.
 	t.Run("with Supervisor LDAP upstream IDP using username and password prompts", func(t *testing.T) {
+		testlib.SkipTestWhenLDAPIsUnavailable(t, env)
+
 		testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 		t.Cleanup(cancel)
 
 		tempDir := testutil.TempDir(t) // per-test tmp dir to avoid sharing files between tests
 
-		if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) {
-			t.Skip("LDAP integration test requires connectivity to an LDAP server")
-		}
-
 		expectedUsername := env.SupervisorUpstreamLDAP.TestUserMailAttributeValue
 		expectedGroups := env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs
 
@@ -696,15 +694,13 @@ func TestE2EFullIntegration_Browser(t *testing.T) {
 	// Add an LDAP upstream IDP and try using it to authenticate during kubectl commands
 	// by passing username and password via environment variables, thus avoiding the CLI's username and password prompts.
 	t.Run("with Supervisor LDAP upstream IDP using PINNIPED_USERNAME and PINNIPED_PASSWORD env vars", func(t *testing.T) {
+		testlib.SkipTestWhenLDAPIsUnavailable(t, env)
+
 		testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 		t.Cleanup(cancel)
 
 		tempDir := testutil.TempDir(t) // per-test tmp dir to avoid sharing files between tests
 
-		if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) {
-			t.Skip("LDAP integration test requires connectivity to an LDAP server")
-		}
-
 		expectedUsername := env.SupervisorUpstreamLDAP.TestUserMailAttributeValue
 		expectedGroups := env.SupervisorUpstreamLDAP.TestUserDirectGroupsDNs
 
@@ -770,18 +766,13 @@ func TestE2EFullIntegration_Browser(t *testing.T) {
 	// Add an Active Directory upstream IDP and try using it to authenticate during kubectl commands
 	// by interacting with the CLI's username and password prompts.
 	t.Run("with Supervisor ActiveDirectory upstream IDP using username and password prompts", func(t *testing.T) {
+		testlib.SkipTestWhenActiveDirectoryIsUnavailable(t, env)
+
 		testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 		t.Cleanup(cancel)
 
 		tempDir := testutil.TempDir(t) // per-test tmp dir to avoid sharing files between tests
 
-		if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) {
-			t.Skip("Active Directory integration test requires connectivity to an LDAP server")
-		}
-		if env.SupervisorUpstreamActiveDirectory.Host == "" {
-			t.Skip("Active Directory hostname not specified")
-		}
-
 		expectedUsername := env.SupervisorUpstreamActiveDirectory.TestUserPrincipalNameValue
 		expectedGroups := env.SupervisorUpstreamActiveDirectory.TestUserIndirectGroupsSAMAccountPlusDomainNames
 
@@ -835,19 +826,13 @@ func TestE2EFullIntegration_Browser(t *testing.T) {
 	// Add an ActiveDirectory upstream IDP and try using it to authenticate during kubectl commands
 	// by passing username and password via environment variables, thus avoiding the CLI's username and password prompts.
 	t.Run("with Supervisor ActiveDirectory upstream IDP using PINNIPED_USERNAME and PINNIPED_PASSWORD env vars", func(t *testing.T) {
+		testlib.SkipTestWhenActiveDirectoryIsUnavailable(t, env)
+
 		testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 		t.Cleanup(cancel)
 
 		tempDir := testutil.TempDir(t) // per-test tmp dir to avoid sharing files between tests
 
-		if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) {
-			t.Skip("ActiveDirectory integration test requires connectivity to an LDAP server")
-		}
-
-		if env.SupervisorUpstreamActiveDirectory.Host == "" {
-			t.Skip("Active Directory hostname not specified")
-		}
-
 		expectedUsername := env.SupervisorUpstreamActiveDirectory.TestUserPrincipalNameValue
 		expectedGroups := env.SupervisorUpstreamActiveDirectory.TestUserIndirectGroupsSAMAccountPlusDomainNames
 
@@ -912,6 +897,8 @@ func TestE2EFullIntegration_Browser(t *testing.T) {
 
 	// Add an LDAP upstream IDP and try using it to authenticate during kubectl commands, using the browser flow.
 	t.Run("with Supervisor LDAP upstream IDP and browser flow with with form_post automatic authcode delivery to CLI", func(t *testing.T) {
+		testlib.SkipTestWhenLDAPIsUnavailable(t, env)
+
 		testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 		t.Cleanup(cancel)
 
@@ -966,6 +953,8 @@ func TestE2EFullIntegration_Browser(t *testing.T) {
 
 	// Add an Active Directory upstream IDP and try using it to authenticate during kubectl commands, using the browser flow.
 	t.Run("with Supervisor Active Directory upstream IDP and browser flow with with form_post automatic authcode delivery to CLI", func(t *testing.T) {
+		testlib.SkipTestWhenActiveDirectoryIsUnavailable(t, env)
+
 		testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 		t.Cleanup(cancel)
 
@@ -974,13 +963,6 @@ func TestE2EFullIntegration_Browser(t *testing.T) {
 		// Start a fresh browser driver because we don't want to share cookies between the various tests in this file.
 		page := browsertest.Open(t)
 
-		if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) {
-			t.Skip("Active Directory integration test requires connectivity to an LDAP server")
-		}
-		if env.SupervisorUpstreamActiveDirectory.Host == "" {
-			t.Skip("Active Directory hostname not specified")
-		}
-
 		expectedUsername := env.SupervisorUpstreamActiveDirectory.TestUserPrincipalNameValue
 		expectedGroups := env.SupervisorUpstreamActiveDirectory.TestUserIndirectGroupsSAMAccountPlusDomainNames
 
@@ -1027,6 +1009,8 @@ func TestE2EFullIntegration_Browser(t *testing.T) {
 
 	// Add an LDAP upstream IDP and try using it to authenticate during kubectl commands, using the env var to choose the browser flow.
 	t.Run("with Supervisor LDAP upstream IDP and browser flow selected by env var override with with form_post automatic authcode delivery to CLI", func(t *testing.T) {
+		testlib.SkipTestWhenLDAPIsUnavailable(t, env)
+
 		testCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 		t.Cleanup(cancel)
 

test/integration/supervisor_login_test.go

@@ -48,17 +48,12 @@ func TestSupervisorLogin_Browser(t *testing.T) {
 
 	skipLDAPTests := func(t *testing.T) {
 		t.Helper()
-		if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) {
-			t.Skip("LDAP integration test requires connectivity to an LDAP server")
-		}
+		testlib.SkipTestWhenLDAPIsUnavailable(t, env)
 	}
 
 	skipActiveDirectoryTests := func(t *testing.T) {
 		t.Helper()
-		skipLDAPTests(t)
-		if env.SupervisorUpstreamActiveDirectory.Host == "" {
-			t.Skip("Active Directory hostname not specified")
-		}
+		testlib.SkipTestWhenActiveDirectoryIsUnavailable(t, env)
 	}
 
 	basicOIDCIdentityProviderSpec := func() idpv1alpha1.OIDCIdentityProviderSpec {

test/integration/supervisor_warnings_test.go

@@ -103,9 +103,7 @@ func TestSupervisorWarnings_Browser(t *testing.T) {
 	)
 
 	t.Run("LDAP group refresh flow", func(t *testing.T) {
-		if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) {
-			t.Skip("LDAP integration test requires connectivity to an LDAP server")
-		}
+		testlib.SkipTestWhenLDAPIsUnavailable(t, env)
 
 		expectedUsername := env.SupervisorUpstreamLDAP.TestUserMailAttributeValue
 
@@ -242,13 +240,9 @@ func TestSupervisorWarnings_Browser(t *testing.T) {
 
 		t.Logf("second kubectl command took %s", time.Since(startTime2).String())
 	})
+
 	t.Run("Active Directory group refresh flow", func(t *testing.T) {
-		if len(env.ToolsNamespace) == 0 && !env.HasCapability(testlib.CanReachInternetLDAPPorts) {
-			t.Skip("LDAP integration test requires connectivity to an LDAP server")
-		}
-		if env.SupervisorUpstreamActiveDirectory.Host == "" {
-			t.Skip("Active Directory hostname not specified")
-		}
+		testlib.SkipTestWhenActiveDirectoryIsUnavailable(t, env)
 
 		expectedUsername, password := testlib.CreateFreshADTestUser(t, env)
 		t.Cleanup(func() {

test/testlib/skip.go

@@ -1,4 +1,4 @@
-// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package testlib
@@ -8,7 +8,28 @@ import "testing"
 // skipUnlessIntegration skips the current test if `-short` has been passed to `go test`.
 func skipUnlessIntegration(t *testing.T) {
 	t.Helper()
+
 	if testing.Short() {
 		t.Skip("skipping integration test because of '-short' flag")
 	}
 }
+
+func SkipTestWhenLDAPIsUnavailable(t *testing.T, env *TestEnv) {
+	t.Helper()
+
+	if len(env.ToolsNamespace) == 0 && !env.HasCapability(CanReachInternetLDAPPorts) {
+		t.Skip("LDAP integration test requires connectivity to an LDAP server")
+	}
+}
+
+func SkipTestWhenActiveDirectoryIsUnavailable(t *testing.T, env *TestEnv) {
+	t.Helper()
+
+	if !env.HasCapability(CanReachInternetLDAPPorts) {
+		t.Skip("Active Directory integration test requires network connectivity to an AD server")
+	}
+
+	if IntegrationEnv(t).SupervisorUpstreamActiveDirectory.Host == "" {
+		t.Skip("Active Directory hostname not specified")
+	}
+}