Skip to content

Commit

Permalink
kubeconfig generation: add tests for kops plugin
Browse files Browse the repository at this point in the history
Also slightly simplify the tests and Kubecfg Builder signature by
passing in the ConfigAccess only when needed.
  • Loading branch information
justinsb committed Aug 16, 2020
1 parent d3248d5 commit 4827817
Show file tree
Hide file tree
Showing 9 changed files with 99 additions and 111 deletions.
4 changes: 2 additions & 2 deletions cmd/kops/delete_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
3 changes: 1 addition & 2 deletions cmd/kops/export_kubecfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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
}
}
Expand Down
3 changes: 1 addition & 2 deletions cmd/kops/update_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubeconfig/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)
5 changes: 2 additions & 3 deletions pkg/kubeconfig/create_kubecfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
171 changes: 82 additions & 89 deletions pkg/kubeconfig/create_kubecfg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -121,117 +120,78 @@ 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")
emptyMasterPublicNameCluster := buildMinimalCluster("emptyMasterPublicNameCluster", "")
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{
{
Expand All @@ -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")
}
Expand All @@ -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)
}
})
}
Expand Down
20 changes: 8 additions & 12 deletions pkg/kubeconfig/kubecfg_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}

Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}

Expand Down
1 change: 1 addition & 0 deletions vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 4827817

Please sign in to comment.