From 9648db0837672700a1804b7d35a4be452afc222e Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Wed, 12 Aug 2020 14:29:46 -0700 Subject: [PATCH] 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 --- test/integration/client_test.go | 4 +- test/integration/loginrequest_test.go | 15 ++--- test/library/client.go | 84 +++++++++++++++++++++------ 3 files changed, 74 insertions(+), 29 deletions(-) diff --git a/test/integration/client_test.go b/test/integration/client_test.go index 105fc5abe..2008bb063 100644 --- a/test/integration/client_test.go +++ b/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() diff --git a/test/integration/loginrequest_test.go b/test/integration/loginrequest_test.go index 6dc2f191f..3bd3f5038 100644 --- a/test/integration/loginrequest_test.go +++ b/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( - t, - response.Status.Credential.ClientCertificateData, - 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) }) diff --git a/test/library/client.go b/test/library/client.go index 5524d919a..92b70c874 100644 --- a/test/library/client.go +++ b/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{ - AuthInfo: clientcmdapi.AuthInfo{ - ClientCertificateData: []byte(cert), - ClientKeyData: []byte(key), - }, - }) + return newClientsetWithConfig(t, NewClientConfig(t)) +} + +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 { - t.Helper() - - return NewClientsetWithConfig(t, NewClientConfig(t)) -} - -func NewClientsetWithConfig(t *testing.T, config *rest.Config) kubernetes.Interface { +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 }