From 555832c973b1c29a3893d818891646fcf1199646 Mon Sep 17 00:00:00 2001 From: Hung-Ting Wen Date: Mon, 25 Mar 2019 17:51:17 -0700 Subject: [PATCH] check CLIENT_ID/CLIENT_SECRET before acutal Apply (#2803) --- bootstrap/cmd/kfctl/cmd/apply.go | 22 +--------------------- bootstrap/pkg/apis/apps/group.go | 28 +++++++++++++--------------- bootstrap/pkg/kfapp/gcp/gcp.go | 16 ++++++++-------- 3 files changed, 22 insertions(+), 44 deletions(-) diff --git a/bootstrap/cmd/kfctl/cmd/apply.go b/bootstrap/cmd/kfctl/cmd/apply.go index 2f359e20c0b..1e06796eeaf 100644 --- a/bootstrap/cmd/kfctl/cmd/apply.go +++ b/bootstrap/cmd/kfctl/cmd/apply.go @@ -42,11 +42,7 @@ var applyCmd = &cobra.Command{ if resourceErr != nil { return fmt.Errorf("invalid resource: %v", resourceErr) } - options := map[string]interface{}{ - string(kftypes.OAUTH_ID): applyCfg.GetString(string(kftypes.OAUTH_ID)), - string(kftypes.OAUTH_SECRET): applyCfg.GetString(string(kftypes.OAUTH_SECRET)), - } - kfApp, kfAppErr := coordinator.LoadKfApp(options) + kfApp, kfAppErr := coordinator.LoadKfApp(map[string]interface{}{}) if kfAppErr != nil { return fmt.Errorf("couldn't load KfApp: %v", kfAppErr) } @@ -72,20 +68,4 @@ func init() { log.Errorf("couldn't set flag --%v: %v", string(kftypes.VERBOSE), bindErr) return } - applyCmd.Flags().String(string(kftypes.OAUTH_ID), "", - "OAuth Client ID, GCP only. Required if using IAP but ENV CLIENT_ID is not set. "+ - "Value passed will take precedence to ENV.") - bindErr = applyCfg.BindPFlag(string(kftypes.OAUTH_ID), applyCmd.Flags().Lookup(string(kftypes.OAUTH_ID))) - if bindErr != nil { - log.Errorf("couldn't set flag --%v: %v", string(kftypes.OAUTH_ID), bindErr) - return - } - applyCmd.Flags().String(string(kftypes.OAUTH_SECRET), "", - "OAuth Client Secret, GCP only. Required if using IAP but ENV CLIENT_SECRET is not set. "+ - "Value passed will take precedence to ENV.") - bindErr = applyCfg.BindPFlag(string(kftypes.OAUTH_SECRET), applyCmd.Flags().Lookup(string(kftypes.OAUTH_SECRET))) - if bindErr != nil { - log.Errorf("couldn't set flag --%v: %v", string(kftypes.OAUTH_SECRET), bindErr) - return - } } diff --git a/bootstrap/pkg/apis/apps/group.go b/bootstrap/pkg/apis/apps/group.go index 9068cfd3cc5..93a64ff087b 100644 --- a/bootstrap/pkg/apis/apps/group.go +++ b/bootstrap/pkg/apis/apps/group.go @@ -38,19 +38,19 @@ import ( const ( DefaultNamespace = "kubeflow" // TODO: find the latest tag dynamically - DefaultVersion = "master" - DefaultGitRepo = "https://github.com/kubeflow/kubeflow/tarball" - KfConfigFile = "app.yaml" - DefaultCacheDir = ".cache" - DefaultConfigDir = "bootstrap/config" - DefaultConfigFile = "kfctl_default.yaml" - GcpIapConfig = "kfctl_iap.yaml" - GcpBasicAuth = "kfctl_basic_auth.yaml" - DefaultZone = "us-east1-d" - DefaultGkeApiVer = "v1beta1" - DefaultAppLabel = "app.kubernetes.io/name" - KUBEFLOW_USERNAME = "KUBEFLOW_USERNAME" - KUBEFLOW_PASSWORD = "KUBEFLOW_PASSWORD" + DefaultVersion = "master" + DefaultGitRepo = "https://github.com/kubeflow/kubeflow/tarball" + KfConfigFile = "app.yaml" + DefaultCacheDir = ".cache" + DefaultConfigDir = "bootstrap/config" + DefaultConfigFile = "kfctl_default.yaml" + GcpIapConfig = "kfctl_iap.yaml" + GcpBasicAuth = "kfctl_basic_auth.yaml" + DefaultZone = "us-east1-d" + DefaultGkeApiVer = "v1beta1" + DefaultAppLabel = "app.kubernetes.io/name" + KUBEFLOW_USERNAME = "KUBEFLOW_USERNAME" + KUBEFLOW_PASSWORD = "KUBEFLOW_PASSWORD" // TODO: switch to bootstrap/k8sSpec/v1.11.7/api/openapi-spec/swagger.json DefaultSwaggerFile = "releasing/releaser/lib/v1.9.7/swagger.json" ) @@ -81,8 +81,6 @@ const ( ZONE CliOption = "zone" USE_BASIC_AUTH CliOption = "use_basic_auth" USE_ISTIO CliOption = "use_istio" - OAUTH_ID CliOption = "oauth_id" - OAUTH_SECRET CliOption = "oauth_secret" DELETE_STORAGE CliOption = "delete_storage" DISABLE_USAGE_REPORT CliOption = "disable_usage_report" ) diff --git a/bootstrap/pkg/kfapp/gcp/gcp.go b/bootstrap/pkg/kfapp/gcp/gcp.go index a6fd1d67cb9..f8dde308711 100644 --- a/bootstrap/pkg/kfapp/gcp/gcp.go +++ b/bootstrap/pkg/kfapp/gcp/gcp.go @@ -436,6 +436,14 @@ func (gcp *Gcp) updateDM(resources kftypes.ResourceEnum) error { // Apply applies the gcp kfapp. func (gcp *Gcp) Apply(resources kftypes.ResourceEnum) error { + if os.Getenv(CLIENT_ID) == "" && !gcp.Spec.UseBasicAuth { + return fmt.Errorf("Need to set environment variable `%v` for IAP.", + CLIENT_ID) + } + if os.Getenv(CLIENT_SECRET) == "" && !gcp.Spec.UseBasicAuth { + return fmt.Errorf("Need to set environment variable `%v` for IAP.", + CLIENT_SECRET) + } if gcp.KfDef.Spec.UseBasicAuth && (os.Getenv(kftypes.KUBEFLOW_USERNAME) == "" || os.Getenv(kftypes.KUBEFLOW_PASSWORD) == "") { return fmt.Errorf("gcp apply needs ENV %v and %v set when using basic auth", @@ -732,15 +740,7 @@ func (gcp *Gcp) createIapSecret(ctx context.Context, client *clientset.Clientset return nil } oauthId := os.Getenv(CLIENT_ID) - if oauthId == "" && !gcp.Spec.UseBasicAuth { - return fmt.Errorf("At least one of --%v or ENV `%v` needs to be set.", - string(kftypes.OAUTH_ID), CLIENT_ID) - } oauthSecret := os.Getenv(CLIENT_SECRET) - if oauthSecret == "" && !gcp.Spec.UseBasicAuth { - return fmt.Errorf("At least one of --%v or ENV `%v` needs to be set.", - string(kftypes.OAUTH_SECRET), CLIENT_SECRET) - } return insertSecret(client, KUBEFLOW_OAUTH, oauthSecretNamespace, map[string][]byte{ strings.ToLower(CLIENT_ID): []byte(oauthId),