From b04610ef9b9e9ae071be971df3319515232498ec Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Tue, 6 Oct 2020 16:14:44 -0700 Subject: [PATCH 1/3] simplify addon code by fixing gcp-auth failure policy --- deploy/addons/gcp-auth/gcp-auth-webhook.yaml | 7 +------ .../storage-provisioner.yaml.tmpl | 1 - pkg/addons/addons.go | 16 ---------------- 3 files changed, 1 insertion(+), 23 deletions(-) diff --git a/deploy/addons/gcp-auth/gcp-auth-webhook.yaml b/deploy/addons/gcp-auth/gcp-auth-webhook.yaml index 75134426038b..c91266a6e0b2 100644 --- a/deploy/addons/gcp-auth/gcp-auth-webhook.yaml +++ b/deploy/addons/gcp-auth/gcp-auth-webhook.yaml @@ -50,8 +50,6 @@ spec: template: metadata: name: gcp-auth-certs-create - labels: - gcp-auth-skip-secret: "true" spec: serviceAccountName: minikube-gcp-auth-certs containers: @@ -79,7 +77,6 @@ spec: labels: app: gcp-auth kubernetes.io/minikube-addons: gcp-auth - gcp-auth-skip-secret: "true" spec: containers: - name: gcp-auth @@ -112,8 +109,6 @@ spec: template: metadata: name: gcp-auth-certs-patch - labels: - gcp-auth-skip-secret: "true" spec: serviceAccountName: minikube-gcp-auth-certs containers: @@ -136,7 +131,7 @@ metadata: app: gcp-auth webhooks: - name: gcp-auth-mutate.k8s.io - failurePolicy: Fail + failurePolicy: Ignore objectSelector: matchExpressions: - key: gcp-auth-skip-secret diff --git a/deploy/addons/storage-provisioner/storage-provisioner.yaml.tmpl b/deploy/addons/storage-provisioner/storage-provisioner.yaml.tmpl index 6b27886c7580..60be98e4092b 100644 --- a/deploy/addons/storage-provisioner/storage-provisioner.yaml.tmpl +++ b/deploy/addons/storage-provisioner/storage-provisioner.yaml.tmpl @@ -95,7 +95,6 @@ metadata: labels: integration-test: storage-provisioner addonmanager.kubernetes.io/mode: Reconcile - gcp-auth-skip-secret: "true" spec: serviceAccountName: storage-provisioner hostNetwork: true diff --git a/pkg/addons/addons.go b/pkg/addons/addons.go index a7ca64a21ea2..14db356fac9b 100644 --- a/pkg/addons/addons.go +++ b/pkg/addons/addons.go @@ -406,18 +406,12 @@ func Start(wg *sync.WaitGroup, cc *config.ClusterConfig, toEnable map[string]boo var awg sync.WaitGroup enabledAddons := []string{} - deferredAddons := []string{} defer func() { // making it show after verifications (see #7613) register.Reg.SetStep(register.EnablingAddons) out.T(style.AddonEnable, "Enabled addons: {{.addons}}", out.V{"addons": strings.Join(enabledAddons, ", ")}) }() for _, a := range toEnableList { - if a == "gcp-auth" { - deferredAddons = append(deferredAddons, a) - continue - } - awg.Add(1) go func(name string) { err := RunCallbacks(cc, name, "true") @@ -433,16 +427,6 @@ func Start(wg *sync.WaitGroup, cc *config.ClusterConfig, toEnable map[string]boo // Wait until all of the addons are enabled before updating the config (not thread safe) awg.Wait() - // Now run the deferred addons - for _, a := range deferredAddons { - err := RunCallbacks(cc, a, "true") - if err != nil { - out.WarningT("Enabling '{{.name}}' returned an error: {{.error}}", out.V{"name": a, "error": err}) - } else { - enabledAddons = append(enabledAddons, a) - } - } - for _, a := range enabledAddons { if err := Set(cc, a, "true"); err != nil { glog.Errorf("store failed: %v", err) From 95f5bc6107492e5fc4c572a2d4259eafa4ed785e Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Thu, 8 Oct 2020 13:25:19 -0700 Subject: [PATCH 2/3] add integration test of gcp-auth addon --- test/integration/addons_test.go | 93 +++++++++++++++++++++++- test/integration/testdata/gcp-creds.json | 7 ++ 2 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 test/integration/testdata/gcp-creds.json diff --git a/test/integration/addons_test.go b/test/integration/addons_test.go index b0d03fe5d666..e7368001d0ab 100644 --- a/test/integration/addons_test.go +++ b/test/integration/addons_test.go @@ -19,12 +19,17 @@ limitations under the License. package integration import ( + "bytes" "context" + "encoding/json" "fmt" "net/http" "net/url" + "os" "os/exec" "path/filepath" + "reflect" + "runtime" "strings" "testing" "time" @@ -40,8 +45,21 @@ func TestAddons(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), Minutes(40)) defer Cleanup(t, profile, cancel) - args := append([]string{"start", "-p", profile, "--wait=false", "--memory=2600", "--alsologtostderr", "--addons=registry", "--addons=metrics-server", "--addons=helm-tiller", "--addons=olm", "--addons=volumesnapshots", "--addons=csi-hostpath-driver"}, StartArgs()...) - if !NoneDriver() { // none doesn't support ingress + // Set an env var to point to our dummy credentials file + err := os.Setenv("GOOGLE_APPLICATION_CREDENTIALS", filepath.Join(*testdataDir, "gcp-creds.json")) + defer os.Unsetenv("GOOGLE_APPLICATION_CREDENTIALS") + if err != nil { + t.Fatalf("Failed setting GOOGLE_APPLICATION_CREDENTIALS env var: %v", err) + } + + err = os.Setenv("GOOGLE_CLOUD_PROJECT", "this_is_fake") + defer os.Unsetenv("GOOGLE_CLOUD_PROJECT") + if err != nil { + t.Fatalf("Failed setting GOOGLE_CLOUD_PROJECT env var: %v", err) + } + + args := append([]string{"start", "-p", profile, "--wait=false", "--memory=2600", "--alsologtostderr", "--addons=registry", "--addons=metrics-server", "--addons=helm-tiller", "--addons=olm", "--addons=volumesnapshots", "--addons=csi-hostpath-driver", "--addons=gcp-auth"}, StartArgs()...) + if !NoneDriver() && !(runtime.GOOS == "darwin" && KicDriver()) { // none doesn't support ingress args = append(args, "--addons=ingress") } rr, err := Run(t, exec.CommandContext(ctx, Target(), args...)) @@ -61,6 +79,7 @@ func TestAddons(t *testing.T) { {"HelmTiller", validateHelmTillerAddon}, {"Olm", validateOlmAddon}, {"CSI", validateCSIDriverAndSnapshots}, + {"GCPAuth", validateGCPAuthAddon}, } for _, tc := range tests { tc := tc @@ -92,7 +111,7 @@ func TestAddons(t *testing.T) { func validateIngressAddon(ctx context.Context, t *testing.T, profile string) { defer PostMortemLogs(t, profile) - if NoneDriver() { + if NoneDriver() || (runtime.GOOS == "darwin" && KicDriver()) { t.Skipf("skipping: ssh unsupported by none") } @@ -504,3 +523,71 @@ func validateCSIDriverAndSnapshots(ctx context.Context, t *testing.T, profile st t.Errorf("failed to disable volumesnapshots addon: args %q: %v", rr.Command(), err) } } + +func validateGCPAuthAddon(ctx context.Context, t *testing.T, profile string) { + defer PostMortemLogs(t, profile) + + // schedule a pod to check environment variables + rr, err := Run(t, exec.CommandContext(ctx, "kubectl", "--context", profile, "create", "-f", filepath.Join(*testdataDir, "busybox.yaml"))) + if err != nil { + t.Fatalf("%s failed: %v", rr.Command(), err) + } + + // 8 minutes, because 4 is not enough for images to pull in all cases. + names, err := PodWait(ctx, t, profile, "default", "integration-test=busybox", Minutes(8)) + if err != nil { + t.Fatalf("wait: %v", err) + } + + // Use this pod to confirm that the env vars are set correctly + rr, err = Run(t, exec.CommandContext(ctx, "kubectl", "--context", profile, "exec", names[0], "--", "/bin/sh", "-c", "printenv GOOGLE_APPLICATION_CREDENTIALS")) + if err != nil { + t.Fatalf("printenv creds: %v", err) + } + + got := strings.TrimSpace(rr.Stdout.String()) + expected := "/google-app-creds.json" + if got != expected { + t.Errorf("'printenv GOOGLE_APPLICATION_CREDENTIALS' returned %s, expected %s", got, expected) + } + + // Make sure the file contents are correct + rr, err = Run(t, exec.CommandContext(ctx, "kubectl", "--context", profile, "exec", names[0], "--", "/bin/sh", "-c", "cat /google-app-creds.json")) + if err != nil { + t.Fatalf("cat creds: %v", err) + } + + var gotJSON map[string]string + err = json.Unmarshal(bytes.TrimSpace(rr.Stdout.Bytes()), &gotJSON) + if err != nil { + t.Fatalf("unmarshal json: %v", err) + } + expectedJSON := map[string]string{ + "client_id": "haha", + "client_secret": "nice_try", + "quota_project_id": "this_is_fake", + "refresh_token": "maybe_next_time", + "type": "authorized_user", + } + + if !reflect.DeepEqual(gotJSON, expectedJSON) { + t.Fatalf("unexpected creds file: got %v, expected %v", gotJSON, expectedJSON) + } + + // Check the GOOGLE_CLOUD_PROJECT env var as well + rr, err = Run(t, exec.CommandContext(ctx, "kubectl", "--context", profile, "exec", names[0], "--", "/bin/sh", "-c", "printenv GOOGLE_CLOUD_PROJECT")) + if err != nil { + t.Fatalf("print env project: %v", err) + } + + got = strings.TrimSpace(rr.Stdout.String()) + expected = "this_is_fake" + if got != expected { + t.Errorf("'printenv GOOGLE_APPLICATION_CREDENTIALS' returned %s, expected %s", got, expected) + } + + rr, err := Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "addons", "disable", "gcp-auth", "--alsologtostderr", "-v=1")) + if err != nil { + t.Errorf("failed disabling gcp-auth addon. arg %q.s %v", rr.Command(), err) + } +} diff --git a/test/integration/testdata/gcp-creds.json b/test/integration/testdata/gcp-creds.json new file mode 100644 index 000000000000..e2115d80622f --- /dev/null +++ b/test/integration/testdata/gcp-creds.json @@ -0,0 +1,7 @@ +{ + "client_id": "haha", + "client_secret": "nice_try", + "quota_project_id": "this_is_fake", + "refresh_token": "maybe_next_time", + "type": "authorized_user" +} From def94d1ccc5da9c004db8ad055c6468c4757c45e Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Thu, 8 Oct 2020 13:36:30 -0700 Subject: [PATCH 3/3] fix lint --- test/integration/addons_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/addons_test.go b/test/integration/addons_test.go index e7368001d0ab..d9c686dd2d73 100644 --- a/test/integration/addons_test.go +++ b/test/integration/addons_test.go @@ -586,7 +586,7 @@ func validateGCPAuthAddon(ctx context.Context, t *testing.T, profile string) { t.Errorf("'printenv GOOGLE_APPLICATION_CREDENTIALS' returned %s, expected %s", got, expected) } - rr, err := Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "addons", "disable", "gcp-auth", "--alsologtostderr", "-v=1")) + rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "addons", "disable", "gcp-auth", "--alsologtostderr", "-v=1")) if err != nil { t.Errorf("failed disabling gcp-auth addon. arg %q.s %v", rr.Command(), err) }