From 42616e7d8a08ac09df7b563bfe090e01ccb5a945 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Tue, 28 Jul 2020 15:59:16 -0500 Subject: [PATCH] Fix a bug in placeholder-name CLI (wrong API version). This is kind of a subtle bug, but we were using the unversioned Kubernetes type package here, where we should have been using the v1beta1 version. They have the same fields, but they serialize to JSON differently. Signed-off-by: Matt Moyer --- cmd/placeholder-name/main.go | 4 +-- cmd/placeholder-name/main_test.go | 42 ++++++++++++++++++++----------- pkg/client/client.go | 8 +++--- pkg/client/client_test.go | 6 ++--- 4 files changed, 36 insertions(+), 24 deletions(-) diff --git a/cmd/placeholder-name/main.go b/cmd/placeholder-name/main.go index e6f5d881..3930fc91 100644 --- a/cmd/placeholder-name/main.go +++ b/cmd/placeholder-name/main.go @@ -13,7 +13,7 @@ import ( "os" "time" - "k8s.io/client-go/pkg/apis/clientauthentication" + clientauthenticationv1beta1 "k8s.io/client-go/pkg/apis/clientauthentication/v1beta1" "github.com/suzerain-io/placeholder-name/internal/constable" "github.com/suzerain-io/placeholder-name/pkg/client" @@ -28,7 +28,7 @@ func main() { } type envGetter func(string) (string, bool) -type tokenExchanger func(ctx context.Context, token, caBundle, apiEndpoint string) (*clientauthentication.ExecCredential, error) +type tokenExchanger func(ctx context.Context, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) const ErrMissingEnvVar = constable.Error("failed to login: environment variable not set") diff --git a/cmd/placeholder-name/main_test.go b/cmd/placeholder-name/main_test.go index 1276d6fd..5ae315d7 100644 --- a/cmd/placeholder-name/main_test.go +++ b/cmd/placeholder-name/main_test.go @@ -12,10 +12,12 @@ import ( "testing" "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/sclevine/spec" "github.com/sclevine/spec/report" "github.com/stretchr/testify/require" - "k8s.io/client-go/pkg/apis/clientauthentication" + clientauthenticationv1beta1 "k8s.io/client-go/pkg/apis/clientauthentication/v1beta1" "github.com/suzerain-io/placeholder-name/test/library" ) @@ -74,7 +76,7 @@ func TestRun(t *testing.T) { when("the token exchange fails", func() { it.Before(func() { - tokenExchanger = func(ctx context.Context, token, caBundle, apiEndpoint string) (*clientauthentication.ExecCredential, error) { + tokenExchanger = func(ctx context.Context, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { return nil, fmt.Errorf("some error") } }) @@ -87,9 +89,9 @@ func TestRun(t *testing.T) { when("the JSON encoder fails", func() { it.Before(func() { - tokenExchanger = func(ctx context.Context, token, caBundle, apiEndpoint string) (*clientauthentication.ExecCredential, error) { - return &clientauthentication.ExecCredential{ - Status: &clientauthentication.ExecCredentialStatus{Token: "some token"}, + tokenExchanger = func(ctx context.Context, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { + return &clientauthenticationv1beta1.ExecCredential{ + Status: &clientauthenticationv1beta1.ExecCredentialStatus{Token: "some token"}, }, nil } }) @@ -102,11 +104,11 @@ func TestRun(t *testing.T) { when("the token exchange times out", func() { it.Before(func() { - tokenExchanger = func(ctx context.Context, token, caBundle, apiEndpoint string) (*clientauthentication.ExecCredential, error) { + tokenExchanger = func(ctx context.Context, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { select { case <-time.After(100 * time.Millisecond): - return &clientauthentication.ExecCredential{ - Status: &clientauthentication.ExecCredentialStatus{Token: "some token"}, + return &clientauthenticationv1beta1.ExecCredential{ + Status: &clientauthenticationv1beta1.ExecCredentialStatus{Token: "some token"}, }, nil case <-ctx.Done(): return nil, ctx.Err() @@ -124,10 +126,14 @@ func TestRun(t *testing.T) { var actualToken, actualCaBundle, actualAPIEndpoint string it.Before(func() { - tokenExchanger = func(ctx context.Context, token, caBundle, apiEndpoint string) (*clientauthentication.ExecCredential, error) { + tokenExchanger = func(ctx context.Context, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { actualToken, actualCaBundle, actualAPIEndpoint = token, caBundle, apiEndpoint - return &clientauthentication.ExecCredential{ - Status: &clientauthentication.ExecCredentialStatus{Token: "some token"}, + return &clientauthenticationv1beta1.ExecCredential{ + TypeMeta: metav1.TypeMeta{ + Kind: "ExecCredential", + APIVersion: "client.authentication.k8s.io/v1beta1", + }, + Status: &clientauthenticationv1beta1.ExecCredentialStatus{Token: "some token"}, }, nil } }) @@ -138,10 +144,16 @@ func TestRun(t *testing.T) { require.Equal(t, fakeEnv["PLACEHOLDER_NAME_TOKEN"], actualToken) require.Equal(t, fakeEnv["PLACEHOLDER_NAME_CA_BUNDLE"], actualCaBundle) require.Equal(t, fakeEnv["PLACEHOLDER_NAME_K8S_API_ENDPOINT"], actualAPIEndpoint) - expected := `{ - "Spec": {"Interactive": false, "Response": null}, - "Status": {"ClientCertificateData": "", "ClientKeyData": "", "ExpirationTimestamp": null, "Token": "some token"} - }` + expected := ` + { + "kind": "ExecCredential", + "apiVersion": "client.authentication.k8s.io/v1beta1", + "spec": {}, + "status": { + "token": "some token" + } + } + ` require.JSONEq(t, expected, buffer.String()) }) }, spec.Parallel()) diff --git a/pkg/client/client.go b/pkg/client/client.go index b9f372f6..279859f9 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -10,7 +10,7 @@ import ( "fmt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/pkg/apis/clientauthentication" + clientauthenticationv1beta1 "k8s.io/client-go/pkg/apis/clientauthentication/v1beta1" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" @@ -22,7 +22,7 @@ import ( // ErrLoginFailed is returned by ExchangeToken when the server rejects the login request. const ErrLoginFailed = constable.Error("login failed") -func ExchangeToken(ctx context.Context, token, caBundle, apiEndpoint string) (*clientauthentication.ExecCredential, error) { +func ExchangeToken(ctx context.Context, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { clientset, err := getClient(apiEndpoint, caBundle) if err != nil { return nil, fmt.Errorf("could not get API client: %w", err) @@ -43,12 +43,12 @@ func ExchangeToken(ctx context.Context, token, caBundle, apiEndpoint string) (*c return nil, fmt.Errorf("%w: %s", ErrLoginFailed, resp.Status.Message) } - return &clientauthentication.ExecCredential{ + return &clientauthenticationv1beta1.ExecCredential{ TypeMeta: metav1.TypeMeta{ Kind: "ExecCredential", APIVersion: "client.authentication.k8s.io/v1beta1", }, - Status: &clientauthentication.ExecCredentialStatus{ + Status: &clientauthenticationv1beta1.ExecCredentialStatus{ ExpirationTimestamp: resp.Status.Credential.ExpirationTimestamp, ClientCertificateData: resp.Status.Credential.ClientCertificateData, ClientKeyData: resp.Status.Credential.ClientKeyData, diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 251f56e9..a6de4ac5 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -16,7 +16,7 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/pkg/apis/clientauthentication" + clientauthenticationv1beta1 "k8s.io/client-go/pkg/apis/clientauthentication/v1beta1" placeholderv1alpha1 "github.com/suzerain-io/placeholder-name-api/pkg/apis/placeholder/v1alpha1" ) @@ -112,12 +112,12 @@ func TestExchangeToken(t *testing.T) { got, err := ExchangeToken(ctx, "", caBundle, endpoint) require.NoError(t, err) - require.Equal(t, &clientauthentication.ExecCredential{ + require.Equal(t, &clientauthenticationv1beta1.ExecCredential{ TypeMeta: metav1.TypeMeta{ Kind: "ExecCredential", APIVersion: "client.authentication.k8s.io/v1beta1", }, - Status: &clientauthentication.ExecCredentialStatus{ + Status: &clientauthenticationv1beta1.ExecCredentialStatus{ ClientCertificateData: "test-certificate", ClientKeyData: "test-key", },