diff --git a/cmd/kops/delete_cluster.go b/cmd/kops/delete_cluster.go index 1cdd175ac3c41..0bf76d75186ef 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 85472297fc1ed..d6720950528f6 100644 --- a/cmd/kops/export_kubecfg.go +++ b/cmd/kops/export_kubecfg.go @@ -144,7 +144,6 @@ func RunExportKubecfg(ctx context.Context, f *util.Factory, out io.Writer, optio keyStore, secretStore, &commands.CloudDiscoveryStatusStore{}, - buildPathOptions(options), options.admin, options.user, options.internal, @@ -154,7 +153,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 644cb3f55e11e..1049c6278af43 100644 --- a/cmd/kops/update_cluster.go +++ b/cmd/kops/update_cluster.go @@ -313,7 +313,6 @@ func RunUpdateCluster(ctx context.Context, f *util.Factory, clusterName string, keyStore, secretStore, &commands.CloudDiscoveryStatusStore{}, - clientcmd.NewDefaultPathOptions(), c.admin, c.user, c.internal, @@ -323,7 +322,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 b1782151551e0..87e173fe524a5 100644 --- a/go.mod +++ b/go.mod @@ -73,6 +73,7 @@ require ( github.com/go-ini/ini v1.51.0 github.com/go-logr/logr v0.2.1-0.20200730175230-ee2de8da5be6 github.com/gogo/protobuf v1.3.1 + github.com/google/go-cmp v0.4.0 github.com/google/uuid v1.1.1 github.com/gophercloud/gophercloud v0.11.1-0.20200518183226-7aec46f32c19 github.com/gorilla/mux v1.7.3 diff --git a/pkg/commands/helpers/BUILD.bazel b/pkg/commands/helpers/BUILD.bazel index 20e83b37d7d42..89d5a53400e83 100644 --- a/pkg/commands/helpers/BUILD.bazel +++ b/pkg/commands/helpers/BUILD.bazel @@ -13,7 +13,7 @@ go_library( "//upup/pkg/fi:go_default_library", "//vendor/github.com/spf13/cobra:go_default_library", "//vendor/k8s.io/client-go/util/homedir:go_default_library", - "//vendor/k8s.io/klog:go_default_library", + "//vendor/k8s.io/klog/v2:go_default_library", "//vendor/k8s.io/kubectl/pkg/util/i18n:go_default_library", ], ) diff --git a/pkg/commands/helpers/kubectl_auth.go b/pkg/commands/helpers/kubectl_auth.go index e8ced9ce2be77..5d29903a71ed6 100644 --- a/pkg/commands/helpers/kubectl_auth.go +++ b/pkg/commands/helpers/kubectl_auth.go @@ -33,7 +33,7 @@ import ( "github.com/spf13/cobra" "k8s.io/client-go/util/homedir" - "k8s.io/klog" + "k8s.io/klog/v2" "k8s.io/kops/cmd/kops/util" "k8s.io/kops/pkg/commands/commandutils" "k8s.io/kops/pkg/pki" diff --git a/pkg/kubeconfig/BUILD.bazel b/pkg/kubeconfig/BUILD.bazel index ef854141b9291..b2bbd6a489d40 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 72d181886b94c..f1ca9ec6e6930 100644 --- a/pkg/kubeconfig/create_kubecfg.go +++ b/pkg/kubeconfig/create_kubecfg.go @@ -23,7 +23,6 @@ import ( "sort" "time" - "k8s.io/client-go/tools/clientcmd" "k8s.io/klog/v2" "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/pkg/apis/kops/util" @@ -35,7 +34,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, configUser string, internal bool, kopsStateStore string, useKopsAuthenticationPlugin bool) (*KubeconfigBuilder, error) { +func BuildKubecfg(cluster *kops.Cluster, keyStore fi.Keystore, secretStore fi.SecretStore, status kops.StatusStore, admin time.Duration, configUser string, internal bool, kopsStateStore string, useKopsAuthenticationPlugin bool) (*KubeconfigBuilder, error) { clusterName := cluster.ObjectMeta.Name var master string @@ -98,7 +97,7 @@ func BuildKubecfg(cluster *kops.Cluster, keyStore fi.Keystore, secretStore fi.Se } } - b := NewKubeconfigBuilder(configAccess) + b := NewKubeconfigBuilder() b.Context = clusterName b.Server = server @@ -152,14 +151,20 @@ func BuildKubecfg(cluster *kops.Cluster, keyStore fi.Keystore, secretStore fi.Se } if useKopsAuthenticationPlugin { + // The api version is an internal implementation detail, the contract used with kubectl. + apiVersion := "v1beta1" + + // TODO(justinsb): rationalize with other kubectl lifetime flags + lifetime := time.Hour + b.AuthenticationExec = []string{ "kops", "helpers", "kubectl-auth", "--cluster=" + clusterName, "--state=" + kopsStateStore, - "--lifetime=1h", - "--api-version=" + "v1beta1", + "--lifetime=" + lifetime.String(), + "--api-version=" + apiVersion, } } diff --git a/pkg/kubeconfig/create_kubecfg_test.go b/pkg/kubeconfig/create_kubecfg_test.go index f88109be63655..54fa719815b55 100644 --- a/pkg/kubeconfig/create_kubecfg_test.go +++ b/pkg/kubeconfig/create_kubecfg_test.go @@ -18,11 +18,10 @@ package kubeconfig import ( "fmt" - "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" @@ -123,14 +122,13 @@ 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 - internal bool + cluster *kops.Cluster + secretStore fi.SecretStore + status fakeStatusStore + admin time.Duration + user string + internal bool + useKopsAuthenticationPlugin bool } publiccluster := buildMinimalCluster("testcluster", "testcluster.test.com") @@ -138,106 +136,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, - "", - false, + 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", - false, + 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, - "", - false, + 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{ { @@ -246,57 +203,76 @@ func TestBuildKubecfg(t *testing.T) { }, nil }, }, - nil, - 0, - "", - false, }, - &KubeconfigBuilder{ + want: &KubeconfigBuilder{ Context: "testgossipcluster.k8s.local", Server: "https://elbHostName", CACert: []byte(certData), User: "testgossipcluster.k8s.local", }, - false, + wantClientCert: false, }, { - "Test Kube Config Data For internal DNS name with admin", - args{ - publiccluster, - fakeKeyStore{ - FindKeypairFn: func(name string) (*pki.Certificate, *pki.PrivateKey, bool, error) { - return fakeCertificate(), - fakePrivateKey(), - true, - nil - }, + 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=1h0m0s", + "--api-version=v1beta1", }, - nil, - fakeStatusStore{}, - nil, - DefaultKubecfgAdminLifetime, - "", - true, }, - &KubeconfigBuilder{ - Context: "testcluster", - Server: "https://internal.testcluster.test.com", - CACert: []byte(certData), - ClientCert: []byte(certData), - ClientKey: []byte(privatekeyData), - User: "testcluster", + wantClientCert: false, + }, + { + name: "Test Kube Config Data For internal DNS name with admin", + args: args{ + cluster: publiccluster, + status: fakeStatusStore{}, + admin: DefaultKubecfgAdminLifetime, + internal: true, + }, + want: &KubeconfigBuilder{ + Context: "testcluster", + Server: "https://internal.testcluster.test.com", + CACert: []byte(certData), + User: "testcluster", }, - false, + wantClientCert: true, }, } 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, tt.args.internal) + 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, tt.args.internal, 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") } @@ -306,8 +282,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 c4013c1ba106f..0946bcc55aeca 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 daae35051ecf6..13eec25626505 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -279,6 +279,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