check CLIENT_ID/CLIENT_SECRET before acutal Apply (kubeflow#2803)

StatCan · Mar 26, 2019 · 555832c · 555832c
1 parent 5ad3e79
commit 555832c
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 44 deletions.
diff --git 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
@@ -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
@@ -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),