From a3fe0f5c499245802ee766f6e4a34349f657267c Mon Sep 17 00:00:00 2001 From: Kazuki Suda Date: Thu, 13 Aug 2020 11:33:55 +0900 Subject: [PATCH 1/4] Fix "Using reference to loop iterator variable" in t.Parallel() --- pkg/minikube/kubeconfig/kubeconfig_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/minikube/kubeconfig/kubeconfig_test.go b/pkg/minikube/kubeconfig/kubeconfig_test.go index 6eb05cdcff7b..e0ed479b888d 100644 --- a/pkg/minikube/kubeconfig/kubeconfig_test.go +++ b/pkg/minikube/kubeconfig/kubeconfig_test.go @@ -328,6 +328,7 @@ func TestUpdateIP(t *testing.T) { }, } for _, test := range tests { + test := test t.Run(test.description, func(t *testing.T) { t.Parallel() configFilename := tempFile(t, test.existing) From c41a5f19b778ea6fb57a063022c9caceb11fe8ed Mon Sep 17 00:00:00 2001 From: Kazuki Suda Date: Thu, 13 Aug 2020 12:19:42 +0900 Subject: [PATCH 2/4] minikube upate-cotext: Fix nil pointer dereference --- go.mod | 1 - pkg/minikube/kubeconfig/kubeconfig.go | 25 ++++------------------ pkg/minikube/kubeconfig/kubeconfig_test.go | 12 +++++++++++ 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/go.mod b/go.mod index 51b5d1276604..78f91ae7da2b 100644 --- a/go.mod +++ b/go.mod @@ -72,7 +72,6 @@ require ( github.com/zchee/go-vmnet v0.0.0-20161021174912-97ebf9174097 golang.org/x/build v0.0.0-20190927031335-2835ba2e683f golang.org/x/crypto v0.0.0-20200510223506-06a226fb4e37 - golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6 // indirect golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a golang.org/x/sys v0.0.0-20200523222454-059865788121 diff --git a/pkg/minikube/kubeconfig/kubeconfig.go b/pkg/minikube/kubeconfig/kubeconfig.go index 566353eb3241..42656e39fabe 100644 --- a/pkg/minikube/kubeconfig/kubeconfig.go +++ b/pkg/minikube/kubeconfig/kubeconfig.go @@ -21,7 +21,6 @@ import ( "io/ioutil" "net/url" "os" - "path" "path/filepath" "strconv" @@ -31,7 +30,6 @@ import ( "k8s.io/client-go/tools/clientcmd/api" "k8s.io/client-go/tools/clientcmd/api/latest" "k8s.io/minikube/pkg/minikube/constants" - "k8s.io/minikube/pkg/minikube/localpath" pkgutil "k8s.io/minikube/pkg/util" "k8s.io/minikube/pkg/util/lock" ) @@ -121,26 +119,11 @@ func UpdateEndpoint(contextName string, hostname string, port int, confpath stri return false, errors.Wrap(err, "read") } - address := "https://" + hostname + ":" + strconv.Itoa(port) - - // if the kubeconfig is missed, create new one - if len(cfg.Clusters) == 0 { - lp := localpath.Profile(contextName) - gp := localpath.MiniPath() - kcs := &Settings{ - ClusterName: contextName, - ClusterServerAddress: address, - ClientCertificate: path.Join(lp, "client.crt"), - ClientKey: path.Join(lp, "client.key"), - CertificateAuthority: path.Join(gp, "ca.crt"), - KeepContext: false, - } - err = PopulateFromSettings(kcs, cfg) - if err != nil { - return false, errors.Wrap(err, "populating kubeconfig") - } + if _, ok := cfg.Clusters[contextName]; !ok { + return false, errors.Errorf("%q does not appear in %s", contextName, confpath) } - cfg.Clusters[contextName].Server = address + + cfg.Clusters[contextName].Server = "https://" + hostname + ":" + strconv.Itoa(port) err = writeToFile(cfg, confpath) if err != nil { return false, errors.Wrap(err, "write") diff --git a/pkg/minikube/kubeconfig/kubeconfig_test.go b/pkg/minikube/kubeconfig/kubeconfig_test.go index e0ed479b888d..7b60e1b23eb7 100644 --- a/pkg/minikube/kubeconfig/kubeconfig_test.go +++ b/pkg/minikube/kubeconfig/kubeconfig_test.go @@ -343,6 +343,18 @@ func TestUpdateIP(t *testing.T) { if test.status != statusActual { t.Errorf("Expected status %t, but got %t", test.status, statusActual) } + + actual, err := readOrNew(configFilename) + if err != nil { + t.Fatal(err) + } + expected, err := decode(test.expCfg) + if err != nil { + t.Fatal(err) + } + if !configEquals(actual, expected) { + t.Errorf("Expected cfg %v, but got %v", expected, actual) + } }) } From 79d5641c5364c16dcc4d5e7cd64ceedfe78d04ab Mon Sep 17 00:00:00 2001 From: Kazuki Suda Date: Sun, 16 Aug 2020 09:36:09 +0900 Subject: [PATCH 3/4] Revert support for adding context if no clusters in the kubeconfig --- pkg/minikube/kubeconfig/kubeconfig.go | 23 +++++- pkg/minikube/kubeconfig/kubeconfig_test.go | 83 +++++++++++++++++++++- 2 files changed, 101 insertions(+), 5 deletions(-) diff --git a/pkg/minikube/kubeconfig/kubeconfig.go b/pkg/minikube/kubeconfig/kubeconfig.go index 42656e39fabe..5cda239f2ce5 100644 --- a/pkg/minikube/kubeconfig/kubeconfig.go +++ b/pkg/minikube/kubeconfig/kubeconfig.go @@ -21,6 +21,7 @@ import ( "io/ioutil" "net/url" "os" + "path" "path/filepath" "strconv" @@ -30,6 +31,7 @@ import ( "k8s.io/client-go/tools/clientcmd/api" "k8s.io/client-go/tools/clientcmd/api/latest" "k8s.io/minikube/pkg/minikube/constants" + "k8s.io/minikube/pkg/minikube/localpath" pkgutil "k8s.io/minikube/pkg/util" "k8s.io/minikube/pkg/util/lock" ) @@ -119,11 +121,28 @@ func UpdateEndpoint(contextName string, hostname string, port int, confpath stri return false, errors.Wrap(err, "read") } + address := "https://" + hostname + ":" + strconv.Itoa(port) + + // if the cluster setting is missed in the kubeconfig, create new one if _, ok := cfg.Clusters[contextName]; !ok { - return false, errors.Errorf("%q does not appear in %s", contextName, confpath) + lp := localpath.Profile(contextName) + gp := localpath.MiniPath() + kcs := &Settings{ + ClusterName: contextName, + ClusterServerAddress: address, + ClientCertificate: path.Join(lp, "client.crt"), + ClientKey: path.Join(lp, "client.key"), + CertificateAuthority: path.Join(gp, "ca.crt"), + KeepContext: false, + } + err = PopulateFromSettings(kcs, cfg) + if err != nil { + return false, errors.Wrap(err, "populating kubeconfig") + } + } else { + cfg.Clusters[contextName].Server = address } - cfg.Clusters[contextName].Server = "https://" + hostname + ":" + strconv.Itoa(port) err = writeToFile(cfg, confpath) if err != nil { return false, errors.Wrap(err, "write") diff --git a/pkg/minikube/kubeconfig/kubeconfig_test.go b/pkg/minikube/kubeconfig/kubeconfig_test.go index 7b60e1b23eb7..5507f8142be9 100644 --- a/pkg/minikube/kubeconfig/kubeconfig_test.go +++ b/pkg/minikube/kubeconfig/kubeconfig_test.go @@ -25,6 +25,7 @@ import ( "k8s.io/client-go/tools/clientcmd/api" "k8s.io/minikube/pkg/minikube/constants" + "k8s.io/minikube/pkg/minikube/localpath" "k8s.io/client-go/tools/clientcmd" ) @@ -51,6 +52,40 @@ users: client-key: /home/la-croix/apiserver.key `) +var kubeConfigWithoutHTTPSUpdated = []byte(` +apiVersion: v1 +clusters: +- cluster: + certificate-authority: /home/la-croix/apiserver.crt + server: 192.168.1.1:8080 + name: la-croix +- cluster: + certificate-authority: /home/la-croix/.minikube/ca.crt + server: https://192.168.10.100:8080 + name: minikube +contexts: +- context: + cluster: la-croix + user: la-croix + name: la-croix +- context: + cluster: minikube + user: minikube + name: minikube +current-context: minikube +kind: Config +preferences: {} +users: +- name: la-croix + user: + client-certificate: /home/la-croix/apiserver.crt + client-key: /home/la-croix/apiserver.key +- name: minikube + user: + client-certificate: /home/la-croix/.minikube/profiles/minikube/client.crt + client-key: /home/la-croix/.minikube/profiles/minikube/client.key +`) + var kubeConfig192 = []byte(` apiVersion: v1 clusters: @@ -117,6 +152,37 @@ users: client-key: /home/la-croix/apiserver.key `) +var kubeConfigNoClusters = []byte(` +apiVersion: v1 +clusters: +contexts: +kind: Config +preferences: {} +users: +`) + +var kubeConfigNoClustersUpdated = []byte(` +apiVersion: v1 +clusters: +- cluster: + certificate-authority: /home/la-croix/.minikube/ca.crt + server: https://192.168.10.100:8080 + name: minikube +contexts: +- context: + cluster: minikube + user: minikube + name: minikube +current-context: minikube +kind: Config +preferences: {} +users: +- name: minikube + user: + client-certificate: /home/la-croix/.minikube/profiles/minikube/client.crt + client-key: /home/la-croix/.minikube/profiles/minikube/client.key +`) + func TestUpdate(t *testing.T) { setupCfg := &Settings{ ClusterName: "test", @@ -300,8 +366,8 @@ func TestUpdateIP(t *testing.T) { hostname: "192.168.10.100", port: 8080, existing: kubeConfigWithoutHTTPS, - err: true, - expCfg: kubeConfigWithoutHTTPS, + status: true, + expCfg: kubeConfigWithoutHTTPSUpdated, }, { description: "same IP", @@ -326,7 +392,18 @@ func TestUpdateIP(t *testing.T) { status: true, expCfg: kubeConfigLocalhost12345, }, + { + description: "no clusters", + hostname: "192.168.10.100", + port: 8080, + existing: kubeConfigNoClusters, + status: true, + expCfg: kubeConfigNoClustersUpdated, + }, } + + os.Setenv(localpath.MinikubeHome, "/home/la-croix") + for _, test := range tests { test := test t.Run(test.description, func(t *testing.T) { @@ -353,7 +430,7 @@ func TestUpdateIP(t *testing.T) { t.Fatal(err) } if !configEquals(actual, expected) { - t.Errorf("Expected cfg %v, but got %v", expected, actual) + t.Fatal("configs did not match") } }) From bbf96e76185f8c0564ecb15d23695182b025b9e5 Mon Sep 17 00:00:00 2001 From: Kazuki Suda Date: Sun, 16 Aug 2020 14:03:41 +0900 Subject: [PATCH 4/4] Update an integration test --- test/integration/functional_test.go | 84 ++++++++++++++++++++++++++--- 1 file changed, 78 insertions(+), 6 deletions(-) diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index e2a82a726a0a..c9b6abb4b383 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -1007,14 +1007,86 @@ func validateCertSync(ctx context.Context, t *testing.T, profile string) { func validateUpdateContextCmd(ctx context.Context, t *testing.T, profile string) { defer PostMortemLogs(t, profile) - rr, err := Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "update-context", "--alsologtostderr", "-v=2")) - if err != nil { - t.Errorf("failed to run minikube update-context: args %q: %v", rr.Command(), err) + tests := []struct { + name string + kubeconfig []byte + want []byte + }{ + { + name: "no changes", + kubeconfig: nil, + want: []byte("No changes"), + }, + { + name: "no minikube cluster", + kubeconfig: []byte(` +apiVersion: v1 +clusters: +- cluster: + certificate-authority: /home/la-croix/apiserver.crt + server: 192.168.1.1:8080 + name: la-croix +contexts: +- context: + cluster: la-croix + user: la-croix + name: la-croix +current-context: la-croix +kind: Config +preferences: {} +users: +- name: la-croix + user: + client-certificate: /home/la-croix/apiserver.crt + client-key: /home/la-croix/apiserver.key +`), + want: []byte("context has been updated"), + }, + { + name: "no clusters", + kubeconfig: []byte(` +apiVersion: v1 +clusters: +contexts: +kind: Config +preferences: {} +users: +`), + want: []byte("context has been updated"), + }, } - want := []byte("No changes") - if !bytes.Contains(rr.Stdout.Bytes(), want) { - t.Errorf("update-context: got=%q, want=*%q*", rr.Stdout.Bytes(), want) + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + c := exec.CommandContext(ctx, Target(), "-p", profile, "update-context", "--alsologtostderr", "-v=2") + if tc.kubeconfig != nil { + tf, err := ioutil.TempFile("", "kubeconfig") + if err != nil { + t.Fatal(err) + } + + if err := ioutil.WriteFile(tf.Name(), tc.kubeconfig, 0644); err != nil { + t.Fatal(err) + } + + t.Cleanup(func() { + os.Remove(tf.Name()) + }) + + c.Env = append(os.Environ(), fmt.Sprintf("KUBECONFIG=%s", tf.Name())) + } + + rr, err := Run(t, c) + if err != nil { + t.Errorf("failed to run minikube update-context: args %q: %v", rr.Command(), err) + } + + if !bytes.Contains(rr.Stdout.Bytes(), tc.want) { + t.Errorf("update-context: got=%q, want=*%q*", rr.Stdout.Bytes(), tc.want) + } + }) } }