From 482781743d7a0306f90096298ff0a7c233d31317 Mon Sep 17 00:00:00 2001 From: Justin SB Date: Sun, 16 Aug 2020 14:29:01 -0400 Subject: [PATCH] kubeconfig generation: add tests for kops plugin Also slightly simplify the tests and Kubecfg Builder signature by passing in the ConfigAccess only when needed. --- cmd/kops/delete_cluster.go | 4 +- cmd/kops/export_kubecfg.go | 3 +- cmd/kops/update_cluster.go | 3 +- go.mod | 1 + pkg/kubeconfig/BUILD.bazel | 2 +- pkg/kubeconfig/create_kubecfg.go | 5 +- pkg/kubeconfig/create_kubecfg_test.go | 171 ++++++++++++-------------- pkg/kubeconfig/kubecfg_builder.go | 20 ++- vendor/modules.txt | 1 + 9 files changed, 99 insertions(+), 111 deletions(-) diff --git a/cmd/kops/delete_cluster.go b/cmd/kops/delete_cluster.go index 90e54caff4244..b527f5b5e8015 100644 --- a/cmd/kops/delete_cluster.go +++ b/cmd/kops/delete_cluster.go @@ -204,9 +204,9 @@ func RunDeleteCluster(ctx context.Context, f *util.Factory, out io.Writer, optio } } - b := kubeconfig.NewKubeconfigBuilder(clientcmd.NewDefaultPathOptions()) + b := kubeconfig.NewKubeconfigBuilder() b.Context = clusterName - err = b.DeleteKubeConfig() + err = b.DeleteKubeConfig(clientcmd.NewDefaultPathOptions()) if err != nil { klog.Warningf("error removing kube config: %v", err) } diff --git a/cmd/kops/export_kubecfg.go b/cmd/kops/export_kubecfg.go index 5c74109c00569..a6b1036eaaa88 100644 --- a/cmd/kops/export_kubecfg.go +++ b/cmd/kops/export_kubecfg.go @@ -139,7 +139,6 @@ func RunExportKubecfg(ctx context.Context, f *util.Factory, out io.Writer, optio keyStore, secretStore, &commands.CloudDiscoveryStatusStore{}, - buildPathOptions(options), options.admin, options.user, f.KopsStateStore(), @@ -148,7 +147,7 @@ func RunExportKubecfg(ctx context.Context, f *util.Factory, out io.Writer, optio return err } - if err := conf.WriteKubecfg(); err != nil { + if err := conf.WriteKubecfg(buildPathOptions(options)); err != nil { return err } } diff --git a/cmd/kops/update_cluster.go b/cmd/kops/update_cluster.go index b4f7713f600b4..c706553542376 100644 --- a/cmd/kops/update_cluster.go +++ b/cmd/kops/update_cluster.go @@ -305,7 +305,6 @@ func RunUpdateCluster(ctx context.Context, f *util.Factory, clusterName string, keyStore, secretStore, &commands.CloudDiscoveryStatusStore{}, - clientcmd.NewDefaultPathOptions(), c.admin, c.user, f.KopsStateStore(), @@ -314,7 +313,7 @@ func RunUpdateCluster(ctx context.Context, f *util.Factory, clusterName string, return nil, err } - err = conf.WriteKubecfg() + err = conf.WriteKubecfg(clientcmd.NewDefaultPathOptions()) if err != nil { return nil, err } diff --git a/go.mod b/go.mod index ea88aa693410b..c8e16a4736562 100644 --- a/go.mod +++ b/go.mod @@ -75,6 +75,7 @@ require ( github.com/go-logr/logr v0.1.0 github.com/gogo/protobuf v1.3.1 github.com/golang/protobuf v1.4.2 // indirect + github.com/google/go-cmp v0.4.0 github.com/google/uuid v1.1.1 github.com/gophercloud/gophercloud v0.7.1-0.20200116011225-46fdd1830e9a github.com/gorilla/mux v1.7.3 diff --git a/pkg/kubeconfig/BUILD.bazel b/pkg/kubeconfig/BUILD.bazel index c5fb5ebc472b9..3d519a698fa40 100644 --- a/pkg/kubeconfig/BUILD.bazel +++ b/pkg/kubeconfig/BUILD.bazel @@ -32,6 +32,6 @@ go_test( "//pkg/pki:go_default_library", "//upup/pkg/fi:go_default_library", "//util/pkg/vfs:go_default_library", - "//vendor/k8s.io/client-go/tools/clientcmd:go_default_library", + "//vendor/github.com/google/go-cmp/cmp:go_default_library", ], ) diff --git a/pkg/kubeconfig/create_kubecfg.go b/pkg/kubeconfig/create_kubecfg.go index d5a3c305b97e7..c53c8a57c20e3 100644 --- a/pkg/kubeconfig/create_kubecfg.go +++ b/pkg/kubeconfig/create_kubecfg.go @@ -22,7 +22,6 @@ import ( "sort" "time" - "k8s.io/client-go/tools/clientcmd" "k8s.io/klog" "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/pkg/apis/kops/util" @@ -34,7 +33,7 @@ import ( const DefaultKubecfgAdminLifetime = 18 * time.Hour -func BuildKubecfg(cluster *kops.Cluster, keyStore fi.Keystore, secretStore fi.SecretStore, status kops.StatusStore, configAccess clientcmd.ConfigAccess, admin time.Duration, user string, kopsStateStore string, useKopsAuthenticationPlugin bool) (*KubeconfigBuilder, error) { +func BuildKubecfg(cluster *kops.Cluster, keyStore fi.Keystore, secretStore fi.SecretStore, status kops.StatusStore, admin time.Duration, user string, kopsStateStore string, useKopsAuthenticationPlugin bool) (*KubeconfigBuilder, error) { clusterName := cluster.ObjectMeta.Name master := cluster.Spec.MasterPublicName @@ -89,7 +88,7 @@ func BuildKubecfg(cluster *kops.Cluster, keyStore fi.Keystore, secretStore fi.Se } } - b := NewKubeconfigBuilder(configAccess) + b := NewKubeconfigBuilder() b.Context = clusterName b.Server = server diff --git a/pkg/kubeconfig/create_kubecfg_test.go b/pkg/kubeconfig/create_kubecfg_test.go index aa4e6e40ed36e..662144f61b3cd 100644 --- a/pkg/kubeconfig/create_kubecfg_test.go +++ b/pkg/kubeconfig/create_kubecfg_test.go @@ -17,11 +17,10 @@ limitations under the License. package kubeconfig import ( - "reflect" "testing" "time" - "k8s.io/client-go/tools/clientcmd" + "github.com/google/go-cmp/cmp" "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/pkg/pki" "k8s.io/kops/upup/pkg/fi" @@ -121,13 +120,12 @@ func TestBuildKubecfg(t *testing.T) { }() type args struct { - cluster *kops.Cluster - keyStore fakeKeyStore - secretStore fi.SecretStore - status fakeStatusStore - configAccess clientcmd.ConfigAccess - admin time.Duration - user string + cluster *kops.Cluster + secretStore fi.SecretStore + status fakeStatusStore + admin time.Duration + user string + useKopsAuthenticationPlugin bool } publiccluster := buildMinimalCluster("testcluster", "testcluster.test.com") @@ -135,103 +133,65 @@ func TestBuildKubecfg(t *testing.T) { gossipCluster := buildMinimalCluster("testgossipcluster.k8s.local", "") tests := []struct { - name string - args args - want *KubeconfigBuilder - wantErr bool + name string + args args + want *KubeconfigBuilder + wantErr bool + wantClientCert bool }{ { - "Test Kube Config Data For Public DNS with admin", - args{ - publiccluster, - fakeKeyStore{ - FindKeypairFn: func(name string) (*pki.Certificate, *pki.PrivateKey, bool, error) { - return fakeCertificate(), - fakePrivateKey(), - true, - nil - }, - }, - nil, - fakeStatusStore{}, - nil, - DefaultKubecfgAdminLifetime, - "", + name: "Test Kube Config Data For Public DNS with admin", + args: args{ + cluster: publiccluster, + status: fakeStatusStore{}, + admin: DefaultKubecfgAdminLifetime, + user: "", }, - &KubeconfigBuilder{ + want: &KubeconfigBuilder{ Context: "testcluster", Server: "https://testcluster.test.com", CACert: []byte(certData), User: "testcluster", }, - false, + wantClientCert: true, }, { - "Test Kube Config Data For Public DNS without admin", - args{ - publiccluster, - fakeKeyStore{ - FindKeypairFn: func(name string) (*pki.Certificate, *pki.PrivateKey, bool, error) { - return fakeCertificate(), - fakePrivateKey(), - true, - nil - }, - }, - nil, - fakeStatusStore{}, - nil, - 0, - "myuser", + name: "Test Kube Config Data For Public DNS without admin", + args: args{ + cluster: publiccluster, + status: fakeStatusStore{}, + admin: 0, + user: "myuser", }, - &KubeconfigBuilder{ + want: &KubeconfigBuilder{ Context: "testcluster", Server: "https://testcluster.test.com", CACert: []byte(certData), User: "myuser", }, - false, + wantClientCert: false, }, { - "Test Kube Config Data For Public DNS with Empty Master Name", - args{ - emptyMasterPublicNameCluster, - fakeKeyStore{ - FindKeypairFn: func(name string) (*pki.Certificate, *pki.PrivateKey, bool, error) { - return fakeCertificate(), - fakePrivateKey(), - true, - nil - }, - }, - nil, - fakeStatusStore{}, - nil, - 0, - "", + name: "Test Kube Config Data For Public DNS with Empty Master Name", + args: args{ + cluster: emptyMasterPublicNameCluster, + status: fakeStatusStore{}, + admin: 0, + user: "", }, - &KubeconfigBuilder{ + want: &KubeconfigBuilder{ Context: "emptyMasterPublicNameCluster", Server: "https://api.emptyMasterPublicNameCluster", CACert: []byte(certData), User: "emptyMasterPublicNameCluster", }, - false, + wantClientCert: false, }, { - "Test Kube Config Data For Gossip cluster", - args{ - gossipCluster, - fakeKeyStore{ - FindKeypairFn: func(name string) (*pki.Certificate, *pki.PrivateKey, bool, error) { - return fakeCertificate(), - fakePrivateKey(), - true, - nil - }, - }, - nil, - fakeStatusStore{ + name: "Test Kube Config Data For Gossip cluster", + args: args{ + cluster: gossipCluster, + status: fakeStatusStore{ GetApiIngressStatusFn: func(cluster *kops.Cluster) ([]kops.ApiIngressStatus, error) { return []kops.ApiIngressStatus{ { @@ -240,27 +200,60 @@ func TestBuildKubecfg(t *testing.T) { }, nil }, }, - nil, - 0, - "", }, - &KubeconfigBuilder{ + want: &KubeconfigBuilder{ Context: "testgossipcluster.k8s.local", Server: "https://elbHostName", CACert: []byte(certData), User: "testgossipcluster.k8s.local", }, - false, + wantClientCert: false, + }, + { + name: "Public DNS with kops auth plugin", + args: args{ + cluster: publiccluster, + status: fakeStatusStore{}, + admin: 0, + useKopsAuthenticationPlugin: true, + }, + want: &KubeconfigBuilder{ + Context: "testcluster", + Server: "https://testcluster.test.com", + CACert: []byte(certData), + User: "testcluster", + AuthenticationExec: []string{ + "kops", + "helpers", + "kubectl-auth", + "--cluster=testcluster", + "--state=memfs://example-state-store", + "--lifetime=1h", + "--api-version=v1beta1", + }, + }, + wantClientCert: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := BuildKubecfg(tt.args.cluster, tt.args.keyStore, tt.args.secretStore, tt.args.status, tt.args.configAccess, tt.args.admin, tt.args.user) + kopsStateStore := "memfs://example-state-store" + + keyStore := fakeKeyStore{ + FindKeypairFn: func(name string) (*pki.Certificate, *pki.PrivateKey, bool, error) { + return fakeCertificate(), + fakePrivateKey(), + true, + nil + }, + } + + got, err := BuildKubecfg(tt.args.cluster, keyStore, tt.args.secretStore, tt.args.status, tt.args.admin, tt.args.user, kopsStateStore, tt.args.useKopsAuthenticationPlugin) if (err != nil) != tt.wantErr { t.Errorf("BuildKubecfg() error = %v, wantErr %v", err, tt.wantErr) return } - if tt.args.admin != 0 { + if tt.wantClientCert { if got.ClientCert == nil { t.Errorf("Expected ClientCert, got nil") } @@ -270,8 +263,8 @@ func TestBuildKubecfg(t *testing.T) { tt.want.ClientCert = got.ClientCert tt.want.ClientKey = got.ClientKey } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("BuildKubecfg() = %v, want %v", got, tt.want) + if diff := cmp.Diff(got, tt.want); diff != "" { + t.Errorf("BuildKubecfg() diff (+got, -want): %s", diff) } }) } diff --git a/pkg/kubeconfig/kubecfg_builder.go b/pkg/kubeconfig/kubecfg_builder.go index adc7d6330d8db..b7962fa9542bd 100644 --- a/pkg/kubeconfig/kubecfg_builder.go +++ b/pkg/kubeconfig/kubecfg_builder.go @@ -42,20 +42,16 @@ type KubeconfigBuilder struct { ClientCert []byte ClientKey []byte - configAccess clientcmd.ConfigAccess - AuthenticationExec []string } // Create new KubeconfigBuilder -func NewKubeconfigBuilder(configAccess clientcmd.ConfigAccess) *KubeconfigBuilder { - c := &KubeconfigBuilder{} - c.configAccess = configAccess - return c +func NewKubeconfigBuilder() *KubeconfigBuilder { + return &KubeconfigBuilder{} } -func (b *KubeconfigBuilder) DeleteKubeConfig() error { - config, err := b.configAccess.GetStartingConfig() +func (b *KubeconfigBuilder) DeleteKubeConfig(configAccess clientcmd.ConfigAccess) error { + config, err := configAccess.GetStartingConfig() if err != nil { return fmt.Errorf("error loading kubeconfig: %v", err) } @@ -74,7 +70,7 @@ func (b *KubeconfigBuilder) DeleteKubeConfig() error { config.CurrentContext = "" } - if err := clientcmd.ModifyConfig(b.configAccess, *config, false); err != nil { + if err := clientcmd.ModifyConfig(configAccess, *config, false); err != nil { return fmt.Errorf("error writing kubeconfig: %v", err) } @@ -103,8 +99,8 @@ func (c *KubeconfigBuilder) BuildRestConfig() (*rest.Config, error) { } // Write out a new kubeconfig -func (b *KubeconfigBuilder) WriteKubecfg() error { - config, err := b.configAccess.GetStartingConfig() +func (b *KubeconfigBuilder) WriteKubecfg(configAccess clientcmd.ConfigAccess) error { + config, err := configAccess.GetStartingConfig() if err != nil { return fmt.Errorf("error reading kubeconfig: %v", err) } @@ -208,7 +204,7 @@ func (b *KubeconfigBuilder) WriteKubecfg() error { config.CurrentContext = b.Context - if err := clientcmd.ModifyConfig(b.configAccess, *config, true); err != nil { + if err := clientcmd.ModifyConfig(configAccess, *config, true); err != nil { return err } diff --git a/vendor/modules.txt b/vendor/modules.txt index 691fa9a641968..0c57579fef02f 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -282,6 +282,7 @@ github.com/golang/snappy # github.com/google/btree v1.0.0 github.com/google/btree # github.com/google/go-cmp v0.4.0 +## explicit github.com/google/go-cmp/cmp github.com/google/go-cmp/cmp/internal/diff github.com/google/go-cmp/cmp/internal/flags