Update how integration tests which use LoginRequest make their clients

- When we call the LoginRequest endpoint in loginrequest_test.go,
  do it with an unauthenticated client, to make sure that endpoint works
  with unauthenticated clients.
- For tests which want to test using certs returned by LoginRequest to
  make API calls back to kube to check if those certs are working, make
  sure they start with a bare client and then add only those certs.
  Avoid accidentally picking up other kubeconfig configuration like
  tokens, etc.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>

test/integration/client_test.go

@@ -60,7 +60,7 @@ func TestClient(t *testing.T) {
 	defer cancel()
 
 	// Use an invalid certificate/key to validate that the ServerVersion API fails like we assume.
-	invalidClient := library.NewClientsetWithConfig(t, library.NewClientConfigWithCertAndKey(t, testCert, testKey))
+	invalidClient := library.NewClientsetWithCertAndKey(t, testCert, testKey)
 	_, err := invalidClient.Discovery().ServerVersion()
 	require.EqualError(t, err, "the server has asked for the client to provide credentials")
 
@@ -72,7 +72,7 @@ func TestClient(t *testing.T) {
 	require.InDelta(t, time.Until(*resp.ExpirationTimestamp), 1*time.Hour, float64(3*time.Minute))
 
 	// Create a client using the certificate and key returned by the token exchange.
-	validClient := library.NewClientsetWithConfig(t, library.NewClientConfigWithCertAndKey(t, resp.ClientCertificateData, resp.ClientKeyData))
+	validClient := library.NewClientsetWithCertAndKey(t, resp.ClientCertificateData, resp.ClientKeyData)
 
 	// Make a version request, which should succeed even without any authorization.
 	_, err = validClient.Discovery().ServerVersion()

test/integration/loginrequest_test.go

@@ -24,7 +24,7 @@ import (
 func makeRequest(t *testing.T, spec v1alpha1.LoginRequestSpec) (*v1alpha1.LoginRequest, error) {
 	t.Helper()
 
-	client := library.NewPlaceholderNameClientset(t)
+	client := library.NewAnonymousPlaceholderNameClientset(t)
 
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
@@ -88,13 +88,10 @@ func TestSuccessfulLoginRequest(t *testing.T) {
 	adminClient := library.NewClientset(t)
 
 	// Create a client using the certificate from the LoginRequest.
-	clientWithCert := library.NewClientsetWithConfig(
+	clientWithCertFromLoginRequest := library.NewClientsetWithCertAndKey(
 		t,
-		library.NewClientConfigWithCertAndKey(
+		response.Status.Credential.ClientCertificateData,
-			t,
+		response.Status.Credential.ClientKeyData,
-			response.Status.Credential.ClientCertificateData,
-			response.Status.Credential.ClientKeyData,
-		),
 	)
 
 	t.Run("access as user", func(t *testing.T) {
@@ -116,7 +113,7 @@ func TestSuccessfulLoginRequest(t *testing.T) {
 		})
 
 		// Use the client which is authenticated as the TMC user to list namespaces
-		listNamespaceResponse, err := clientWithCert.CoreV1().Namespaces().List(ctx, metav1.ListOptions{})
+		listNamespaceResponse, err := clientWithCertFromLoginRequest.CoreV1().Namespaces().List(ctx, metav1.ListOptions{})
 		require.NoError(t, err)
 		require.NotEmpty(t, listNamespaceResponse.Items)
 	})
@@ -140,7 +137,7 @@ func TestSuccessfulLoginRequest(t *testing.T) {
 		})
 
 		// Use the client which is authenticated as the TMC group to list namespaces
-		listNamespaceResponse, err := clientWithCert.CoreV1().Namespaces().List(ctx, metav1.ListOptions{})
+		listNamespaceResponse, err := clientWithCertFromLoginRequest.CoreV1().Namespaces().List(ctx, metav1.ListOptions{})
 		require.NoError(t, err)
 		require.NotEmpty(t, listNamespaceResponse.Items)
 	})

test/library/client.go

@@ -6,6 +6,8 @@ SPDX-License-Identifier: Apache-2.0
 package library
 
 import (
+	"io/ioutil"
+	"os"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -24,15 +26,34 @@ func NewClientConfig(t *testing.T) *rest.Config {
 	return newClientConfigWithOverrides(t, &clientcmd.ConfigOverrides{})
 }
 
-func NewClientConfigWithCertAndKey(t *testing.T, cert, key string) *rest.Config {
+func NewClientset(t *testing.T) kubernetes.Interface {
 	t.Helper()
 
-	return newClientConfigWithOverrides(t, &clientcmd.ConfigOverrides{
+	return newClientsetWithConfig(t, NewClientConfig(t))
-		AuthInfo: clientcmdapi.AuthInfo{
+}
-			ClientCertificateData: []byte(cert),
+
-			ClientKeyData:         []byte(key),
+func NewClientsetWithCertAndKey(t *testing.T, clientCertificateData, clientKeyData string) kubernetes.Interface {
-		},
+	t.Helper()
-	})
+
+	return newClientsetWithConfig(t, newAnonymousClientRestConfigWithCertAndKeyAdded(t, clientCertificateData, clientKeyData))
+}
+
+func NewPlaceholderNameClientset(t *testing.T) placeholdernameclientset.Interface {
+	t.Helper()
+
+	return placeholdernameclientset.NewForConfigOrDie(NewClientConfig(t))
+}
+
+func NewAnonymousPlaceholderNameClientset(t *testing.T) placeholdernameclientset.Interface {
+	t.Helper()
+
+	return placeholdernameclientset.NewForConfigOrDie(newAnonymousClientRestConfig(t))
+}
+
+func NewAggregatedClientset(t *testing.T) aggregatorclient.Interface {
+	t.Helper()
+
+	return aggregatorclient.NewForConfigOrDie(NewClientConfig(t))
 }
 
 func newClientConfigWithOverrides(t *testing.T, overrides *clientcmd.ConfigOverrides) *rest.Config {
@@ -45,13 +66,7 @@ func newClientConfigWithOverrides(t *testing.T, overrides *clientcmd.ConfigOverr
 	return config
 }
 
-func NewClientset(t *testing.T) kubernetes.Interface {
+func newClientsetWithConfig(t *testing.T, config *rest.Config) kubernetes.Interface {
-	t.Helper()
-
-	return NewClientsetWithConfig(t, NewClientConfig(t))
-}
-
-func NewClientsetWithConfig(t *testing.T, config *rest.Config) kubernetes.Interface {
 	t.Helper()
 
 	result, err := kubernetes.NewForConfig(config)
@@ -59,14 +74,47 @@ func NewClientsetWithConfig(t *testing.T, config *rest.Config) kubernetes.Interf
 	return result
 }
 
-func NewPlaceholderNameClientset(t *testing.T) placeholdernameclientset.Interface {
+// Returns a rest.Config without any user authentication info.
+// Ensures that we are not accidentally picking up any authentication info from the kube config file.
+// E.g. If your kube config were pointing at an Azure cluster, it would have both certs and a token,
+// and we don't want our tests to accidentally pick up that token.
+func newAnonymousClientRestConfig(t *testing.T) *rest.Config {
 	t.Helper()
 
-	return placeholdernameclientset.NewForConfigOrDie(NewClientConfig(t))
+	realConfig := NewClientConfig(t)
+
+	out, err := ioutil.TempFile("", "placeholder-name-anonymous-kubeconfig-test-*")
+	require.NoError(t, err)
+	defer os.Remove(out.Name())
+
+	anonConfig := clientcmdapi.NewConfig()
+	anonConfig.Clusters["anonymous-cluster"] = &clientcmdapi.Cluster{
+		Server:                   realConfig.Host,
+		CertificateAuthorityData: realConfig.CAData,
+	}
+	anonConfig.Contexts["anonymous"] = &clientcmdapi.Context{
+		Cluster: "anonymous-cluster",
+	}
+	anonConfig.CurrentContext = "anonymous"
+
+	data, err := clientcmd.Write(*anonConfig)
+	require.NoError(t, err)
+
+	_, err = out.Write(data)
+	require.NoError(t, err)
+
+	restConfig, err := clientcmd.BuildConfigFromFlags("", out.Name())
+	require.NoError(t, err)
+
+	return restConfig
 }
 
-func NewAggregatedClientset(t *testing.T) aggregatorclient.Interface {
+// Starting with an anonymous client config, add a cert and key to use for authentication in the API server.
+func newAnonymousClientRestConfigWithCertAndKeyAdded(t *testing.T, clientCertificateData, clientKeyData string) *rest.Config {
 	t.Helper()
 
-	return aggregatorclient.NewForConfigOrDie(NewClientConfig(t))
+	config := newAnonymousClientRestConfig(t)
+	config.CertData = []byte(clientCertificateData)
+	config.KeyData = []byte(clientKeyData)
+	return config
 }