From b3327d752270207fb4c02f5c09ee1d90136e120f Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Wed, 16 Sep 2020 15:03:54 -0500 Subject: [PATCH] Switch our client over to use the new TokenCredentialRequest API. Signed-off-by: Matt Moyer --- cmd/pinniped/cmd/exchange_credential.go | 9 +++- cmd/pinniped/cmd/exchange_credential_test.go | 20 +++++--- internal/client/client.go | 18 ++++--- internal/client/client_test.go | 50 +++++++++++++------- test/integration/client_test.go | 3 +- 5 files changed, 63 insertions(+), 37 deletions(-) diff --git a/cmd/pinniped/cmd/exchange_credential.go b/cmd/pinniped/cmd/exchange_credential.go index df910d16..d7cf16f9 100644 --- a/cmd/pinniped/cmd/exchange_credential.go +++ b/cmd/pinniped/cmd/exchange_credential.go @@ -75,7 +75,7 @@ func newExchangeCredentialCmd(args []string, stdout, stderr io.Writer) *exchange } type envGetter func(string) (string, bool) -type tokenExchanger func(ctx context.Context, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) +type tokenExchanger func(ctx context.Context, namespace, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) const ErrMissingEnvVar = constable.Error("failed to get credential: environment variable not set") @@ -91,6 +91,11 @@ func exchangeCredential(envGetter envGetter, tokenExchanger tokenExchanger, outp ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() + namespace, varExists := envGetter("PINNIPED_NAMESPACE") + if !varExists { + return envVarNotSetError("PINNIPED_NAMESPACE") + } + token, varExists := envGetter("PINNIPED_TOKEN") if !varExists { return envVarNotSetError("PINNIPED_TOKEN") @@ -106,7 +111,7 @@ func exchangeCredential(envGetter envGetter, tokenExchanger tokenExchanger, outp return envVarNotSetError("PINNIPED_K8S_API_ENDPOINT") } - cred, err := tokenExchanger(ctx, token, caBundle, apiEndpoint) + cred, err := tokenExchanger(ctx, namespace, token, caBundle, apiEndpoint) if err != nil { return fmt.Errorf("failed to get credential: %w", err) } diff --git a/cmd/pinniped/cmd/exchange_credential_test.go b/cmd/pinniped/cmd/exchange_credential_test.go index 672a064c..74ebb8c0 100644 --- a/cmd/pinniped/cmd/exchange_credential_test.go +++ b/cmd/pinniped/cmd/exchange_credential_test.go @@ -135,6 +135,7 @@ func TestExchangeCredential(t *testing.T) { r = require.New(t) buffer = new(bytes.Buffer) fakeEnv = map[string]string{ + "PINNIPED_NAMESPACE": "namespace from env", "PINNIPED_TOKEN": "token from env", "PINNIPED_CA_BUNDLE": "ca bundle from env", "PINNIPED_K8S_API_ENDPOINT": "k8s api from env", @@ -142,6 +143,12 @@ func TestExchangeCredential(t *testing.T) { }) when("env vars are missing", func() { + it("returns an error when PINNIPED_NAMESPACE is missing", func() { + delete(fakeEnv, "PINNIPED_NAMESPACE") + err := exchangeCredential(envGetter, tokenExchanger, buffer, 30*time.Second) + r.EqualError(err, "failed to get credential: environment variable not set: PINNIPED_NAMESPACE") + }) + it("returns an error when PINNIPED_TOKEN is missing", func() { delete(fakeEnv, "PINNIPED_TOKEN") err := exchangeCredential(envGetter, tokenExchanger, buffer, 30*time.Second) @@ -163,7 +170,7 @@ func TestExchangeCredential(t *testing.T) { when("the token exchange fails", func() { it.Before(func() { - tokenExchanger = func(ctx context.Context, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { + tokenExchanger = func(ctx context.Context, namespace, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { return nil, fmt.Errorf("some error") } }) @@ -176,7 +183,7 @@ func TestExchangeCredential(t *testing.T) { when("the JSON encoder fails", func() { it.Before(func() { - tokenExchanger = func(ctx context.Context, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { + tokenExchanger = func(ctx context.Context, namespace, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { return &clientauthenticationv1beta1.ExecCredential{ Status: &clientauthenticationv1beta1.ExecCredentialStatus{ Token: "some token", @@ -193,7 +200,7 @@ func TestExchangeCredential(t *testing.T) { when("the token exchange times out", func() { it.Before(func() { - tokenExchanger = func(ctx context.Context, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { + tokenExchanger = func(ctx context.Context, namespace, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { select { case <-time.After(100 * time.Millisecond): return &clientauthenticationv1beta1.ExecCredential{ @@ -214,11 +221,11 @@ func TestExchangeCredential(t *testing.T) { }) when("the token exchange succeeds", func() { - var actualToken, actualCaBundle, actualAPIEndpoint string + var actualNamespace, actualToken, actualCaBundle, actualAPIEndpoint string it.Before(func() { - tokenExchanger = func(ctx context.Context, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { - actualToken, actualCaBundle, actualAPIEndpoint = token, caBundle, apiEndpoint + tokenExchanger = func(ctx context.Context, namespace, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { + actualNamespace, actualToken, actualCaBundle, actualAPIEndpoint = namespace, token, caBundle, apiEndpoint now := metav1.NewTime(time.Date(2020, 7, 29, 1, 2, 3, 0, time.UTC)) return &clientauthenticationv1beta1.ExecCredential{ TypeMeta: metav1.TypeMeta{ @@ -238,6 +245,7 @@ func TestExchangeCredential(t *testing.T) { it("writes the execCredential to the given writer", func() { err := exchangeCredential(envGetter, tokenExchanger, buffer, 30*time.Second) r.NoError(err) + r.Equal(fakeEnv["PINNIPED_NAMESPACE"], actualNamespace) r.Equal(fakeEnv["PINNIPED_TOKEN"], actualToken) r.Equal(fakeEnv["PINNIPED_CA_BUNDLE"], actualCaBundle) r.Equal(fakeEnv["PINNIPED_K8S_API_ENDPOINT"], actualAPIEndpoint) diff --git a/internal/client/client.go b/internal/client/client.go index 15dcc1bd..bde9b2ca 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -14,7 +14,7 @@ import ( "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" - "github.com/suzerain-io/pinniped/generated/1.19/apis/pinniped/v1alpha1" + "github.com/suzerain-io/pinniped/generated/1.19/apis/login/v1alpha1" "github.com/suzerain-io/pinniped/generated/1.19/client/clientset/versioned" ) @@ -22,25 +22,23 @@ import ( var ErrLoginFailed = errors.New("login failed") // ExchangeToken exchanges an opaque token using the Pinniped CredentialRequest API, returning a client-go ExecCredential valid on the target cluster. -func ExchangeToken(ctx context.Context, token string, caBundle string, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { +func ExchangeToken(ctx context.Context, namespace string, token string, caBundle string, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { client, err := getClient(apiEndpoint, caBundle) if err != nil { return nil, fmt.Errorf("could not get API client: %w", err) } - resp, err := client.PinnipedV1alpha1().CredentialRequests().Create(ctx, &v1alpha1.CredentialRequest{ - Spec: v1alpha1.CredentialRequestSpec{ - Type: v1alpha1.TokenCredentialType, - Token: &v1alpha1.CredentialRequestTokenCredential{ - Value: token, - }, - }, + resp, err := client.LoginV1alpha1().TokenCredentialRequests(namespace).Create(ctx, &v1alpha1.TokenCredentialRequest{ + Spec: v1alpha1.TokenCredentialRequestSpec{Token: token}, }, metav1.CreateOptions{}) if err != nil { return nil, fmt.Errorf("could not login: %w", err) } if resp.Status.Credential == nil || resp.Status.Message != nil { - return nil, fmt.Errorf("%w: %s", ErrLoginFailed, *resp.Status.Message) + if resp.Status.Message != nil { + return nil, fmt.Errorf("%w: %s", ErrLoginFailed, *resp.Status.Message) + } + return nil, fmt.Errorf("%w: unknown", ErrLoginFailed) } return &clientauthenticationv1beta1.ExecCredential{ diff --git a/internal/client/client_test.go b/internal/client/client_test.go index 659b77ad..0874c6db 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -15,7 +15,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clientauthenticationv1beta1 "k8s.io/client-go/pkg/apis/clientauthentication/v1beta1" - "github.com/suzerain-io/pinniped/generated/1.19/apis/pinniped/v1alpha1" + "github.com/suzerain-io/pinniped/generated/1.19/apis/login/v1alpha1" "github.com/suzerain-io/pinniped/internal/testutil" ) @@ -25,7 +25,7 @@ func TestExchangeToken(t *testing.T) { t.Run("invalid configuration", func(t *testing.T) { t.Parallel() - got, err := ExchangeToken(ctx, "", "", "") + got, err := ExchangeToken(ctx, "test-namespace", "", "", "") require.EqualError(t, err, "could not get API client: invalid configuration: no configuration has been provided, try setting KUBERNETES_MASTER environment variable") require.Nil(t, got) }) @@ -38,8 +38,8 @@ func TestExchangeToken(t *testing.T) { _, _ = w.Write([]byte("some server error")) }) - got, err := ExchangeToken(ctx, "", caBundle, endpoint) - require.EqualError(t, err, `could not login: an error on the server ("some server error") has prevented the request from succeeding (post credentialrequests.pinniped.dev)`) + got, err := ExchangeToken(ctx, "test-namespace", "", caBundle, endpoint) + require.EqualError(t, err, `could not login: an error on the server ("some server error") has prevented the request from succeeding (post tokencredentialrequests.login.pinniped.dev)`) require.Nil(t, got) }) @@ -49,17 +49,32 @@ func TestExchangeToken(t *testing.T) { errorMessage := "some login failure" caBundle, endpoint := testutil.TLSTestServer(t, func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") - _ = json.NewEncoder(w).Encode(&v1alpha1.CredentialRequest{ - TypeMeta: metav1.TypeMeta{APIVersion: "pinniped.dev/v1alpha1", Kind: "CredentialRequest"}, - Status: v1alpha1.CredentialRequestStatus{Message: &errorMessage}, + _ = json.NewEncoder(w).Encode(&v1alpha1.TokenCredentialRequest{ + TypeMeta: metav1.TypeMeta{APIVersion: "login.pinniped.dev/v1alpha1", Kind: "TokenCredentialRequest"}, + Status: v1alpha1.TokenCredentialRequestStatus{Message: &errorMessage}, }) }) - got, err := ExchangeToken(ctx, "", caBundle, endpoint) + got, err := ExchangeToken(ctx, "test-namespace", "", caBundle, endpoint) require.EqualError(t, err, `login failed: some login failure`) require.Nil(t, got) }) + t.Run("login failure unknown error", func(t *testing.T) { + t.Parallel() + // Start a test server that returns without any error message but also without valid credentials + caBundle, endpoint := testutil.TLSTestServer(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("content-type", "application/json") + _ = json.NewEncoder(w).Encode(&v1alpha1.TokenCredentialRequest{ + TypeMeta: metav1.TypeMeta{APIVersion: "login.pinniped.dev/v1alpha1", Kind: "TokenCredentialRequest"}, + }) + }) + + got, err := ExchangeToken(ctx, "test-namespace", "", caBundle, endpoint) + require.EqualError(t, err, `login failed: unknown`) + require.Nil(t, got) + }) + t.Run("success", func(t *testing.T) { t.Parallel() expires := metav1.NewTime(time.Now().Truncate(time.Second)) @@ -67,21 +82,20 @@ func TestExchangeToken(t *testing.T) { // Start a test server that returns successfully and asserts various properties of the request. caBundle, endpoint := testutil.TLSTestServer(t, func(w http.ResponseWriter, r *http.Request) { require.Equal(t, http.MethodPost, r.Method) - require.Equal(t, "/apis/pinniped.dev/v1alpha1/credentialrequests", r.URL.Path) + require.Equal(t, "/apis/login.pinniped.dev/v1alpha1/namespaces/test-namespace/tokencredentialrequests", r.URL.Path) require.Equal(t, "application/json", r.Header.Get("content-type")) body, err := ioutil.ReadAll(r.Body) require.NoError(t, err) require.JSONEq(t, `{ - "kind": "CredentialRequest", - "apiVersion": "pinniped.dev/v1alpha1", + "kind": "TokenCredentialRequest", + "apiVersion": "login.pinniped.dev/v1alpha1", "metadata": { "creationTimestamp": null }, "spec": { - "type": "token", - "token": {} + "token": "test-token" }, "status": {} }`, @@ -89,10 +103,10 @@ func TestExchangeToken(t *testing.T) { ) w.Header().Set("content-type", "application/json") - _ = json.NewEncoder(w).Encode(&v1alpha1.CredentialRequest{ - TypeMeta: metav1.TypeMeta{APIVersion: "pinniped.dev/v1alpha1", Kind: "CredentialRequest"}, - Status: v1alpha1.CredentialRequestStatus{ - Credential: &v1alpha1.CredentialRequestCredential{ + _ = json.NewEncoder(w).Encode(&v1alpha1.TokenCredentialRequest{ + TypeMeta: metav1.TypeMeta{APIVersion: "login.pinniped.dev/v1alpha1", Kind: "TokenCredentialRequest"}, + Status: v1alpha1.TokenCredentialRequestStatus{ + Credential: &v1alpha1.ClusterCredential{ ExpirationTimestamp: expires, ClientCertificateData: "test-certificate", ClientKeyData: "test-key", @@ -101,7 +115,7 @@ func TestExchangeToken(t *testing.T) { }) }) - got, err := ExchangeToken(ctx, "", caBundle, endpoint) + got, err := ExchangeToken(ctx, "test-namespace", "test-token", caBundle, endpoint) require.NoError(t, err) require.Equal(t, &clientauthenticationv1beta1.ExecCredential{ TypeMeta: metav1.TypeMeta{ diff --git a/test/integration/client_test.go b/test/integration/client_test.go index 4fed2698..795d2f32 100644 --- a/test/integration/client_test.go +++ b/test/integration/client_test.go @@ -56,6 +56,7 @@ func TestClient(t *testing.T) { library.SkipUnlessIntegration(t) library.SkipUnlessClusterHasCapability(t, library.ClusterSigningKeyIsAvailable) token := library.GetEnv(t, "PINNIPED_TEST_USER_TOKEN") + namespace := library.GetEnv(t, "PINNIPED_NAMESPACE") ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() @@ -67,7 +68,7 @@ func TestClient(t *testing.T) { // Using the CA bundle and host from the current (admin) kubeconfig, do the token exchange. clientConfig := library.NewClientConfig(t) - resp, err := client.ExchangeToken(ctx, token, string(clientConfig.CAData), clientConfig.Host) + resp, err := client.ExchangeToken(ctx, namespace, token, string(clientConfig.CAData), clientConfig.Host) require.NoError(t, err) require.NotNil(t, resp.Status.ExpirationTimestamp) require.InDelta(t, time.Until(resp.Status.ExpirationTimestamp.Time), 1*time.Hour, float64(3*time.Minute))