Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move basic auth username/password to ENV #2713

Merged
merged 16 commits into from
Mar 20, 2019
Merged

Move basic auth username/password to ENV #2713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
a96b22c
disable kustomize
gabrielwen Mar 15, 2019
af87116
remove username/password from params
gabrielwen Mar 15, 2019
91eb5e9
remove CliOption
gabrielwen Mar 15, 2019
175f4e6
add check in init
gabrielwen Mar 15, 2019
0b60900
use ENV
gabrielwen Mar 15, 2019
7f8e98f
remove from spec
gabrielwen Mar 15, 2019
0108c6c
revert comment
gabrielwen Mar 15, 2019
211401e
add GOPATH to fix test
gabrielwen Mar 15, 2019
853d8fb
rebase
gabrielwen Mar 18, 2019
c23ddc5
update test for ENV username/password
gabrielwen Mar 18, 2019
7b54e50
add some logs
gabrielwen Mar 18, 2019
c9b3b30
Merge branch 'master' of github.com:kubeflow/kubeflow into env
gabrielwen Mar 20, 2019
8b7502c
update
gabrielwen Mar 20, 2019
b0e44aa
update
gabrielwen Mar 20, 2019
973b19a
Merge branch 'master' into env
gabrielwen Mar 20, 2019
8d374e1
update naming
gabrielwen Mar 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 8 additions & 23 deletions bootstrap/cmd/kfctl/cmd/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
log "github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"github.com/spf13/viper"
"golang.org/x/crypto/bcrypt"
"os"
)

var initCfg = viper.New()
Expand Down Expand Up @@ -51,14 +51,13 @@ or a <name>. If just <name> a directory <name> will be created in the current di
init_gcp := initCfg.GetBool(string(kftypes.SKIP_INIT_GCP_PROJECT))

useBasicAuth := initCfg.GetBool(string(kftypes.USE_BASIC_AUTH))
basicAuthUsername := initCfg.GetString(string(kftypes.BASIC_AUTH_USERNAME))
basicAuthPassword := initCfg.GetString(string(kftypes.BASIC_AUTH_PASSWORD))
if useBasicAuth && (basicAuthUsername == "" || basicAuthPassword == "") {
return fmt.Errorf("If using basic auth, need to specify username and password for it.")
}
passwordHash, err := bcrypt.GenerateFromPassword([]byte(basicAuthPassword), 10)
if err != nil {
return fmt.Errorf("error when hashing password: %v", err)
if useBasicAuth && (os.Getenv(kftypes.KUBEFLOW_USERNAME) == "" ||
os.Getenv(kftypes.KUBEFLOW_PASSWORD) == "") {
// Printing warning message instead of bailing out as both ENV are used in apply,
// not init.
log.Warnf("you need to set the environment variable %s to the username you "+
"want to use to login and variable %s to the password you want to use.",
kftypes.KUBEFLOW_USERNAME, kftypes.KUBEFLOW_PASSWORD)
}

options := map[string]interface{}{
Expand All @@ -70,8 +69,6 @@ or a <name>. If just <name> a directory <name> will be created in the current di
string(kftypes.PROJECT): project,
string(kftypes.SKIP_INIT_GCP_PROJECT): init_gcp,
string(kftypes.USE_BASIC_AUTH): useBasicAuth,
string(kftypes.BASIC_AUTH_USERNAME): basicAuthUsername,
string(kftypes.BASIC_AUTH_PASSWORD): string(passwordHash),
}
kfApp, kfAppErr := coordinator.NewKfApp(options)
if kfAppErr != nil || kfApp == nil {
Expand Down Expand Up @@ -159,16 +156,4 @@ func init() {
log.Errorf("couldn't set flag --%v: %v", string(kftypes.USE_BASIC_AUTH), bindErr)
return
}
initCmd.Flags().String(string(kftypes.BASIC_AUTH_USERNAME), "",
"Basic auth login username. Required if using basic auth.")
bindErr = initCfg.BindPFlag(string(kftypes.BASIC_AUTH_USERNAME), initCmd.Flags().Lookup(string(kftypes.BASIC_AUTH_USERNAME)))
if bindErr != nil {
log.Errorf("couldn't set flag --%v: %v", string(kftypes.BASIC_AUTH_USERNAME), bindErr)
}
initCmd.Flags().String(string(kftypes.BASIC_AUTH_PASSWORD), "",
"Basic auth login password. Required if using basic auth.")
bindErr = initCfg.BindPFlag(string(kftypes.BASIC_AUTH_PASSWORD), initCmd.Flags().Lookup(string(kftypes.BASIC_AUTH_PASSWORD)))
if bindErr != nil {
log.Errorf("couldn't set flag --%v: %v", string(kftypes.BASIC_AUTH_PASSWORD), bindErr)
}
}
2 changes: 0 additions & 2 deletions bootstrap/pkg/apis/apps/gcp/v1alpha1/application_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ type GcpSpec struct {
IpName string `json:"ipName,omitempty"`
Hostname string `json:"hostname,omitempty"`
Zone string `json:"zone,omitempty"`
BasicAuthUsername string `json:"basicAuthUsername,omitempty"`
BasicAuthPassword string `json:"basicAuthPassword,omitempty"`
UseBasicAuth bool `json:"useBasicAuth"`
SkipInitProject bool `json:"skipInitProject,omitempty"`
}
Expand Down
4 changes: 2 additions & 2 deletions bootstrap/pkg/apis/apps/group.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ const (
DefaultZone = "us-east1-d"
DefaultGkeApiVer = "v1beta1"
DefaultAppLabel = "app.kubernetes.io/name"
KUBEFLOW_USERNAME = "KUBEFLOW_USERNAME"
KUBEFLOW_PASSWORD = "KUBEFLOW_PASSWORD"
)

type ResourceEnum string
Expand Down Expand Up @@ -78,8 +80,6 @@ const (
USE_BASIC_AUTH CliOption = "use_basic_auth"
OAUTH_ID CliOption = "oauth_id"
OAUTH_SECRET CliOption = "oauth_secret"
BASIC_AUTH_USERNAME CliOption = "basic_auth_username"
BASIC_AUTH_PASSWORD CliOption = "basic_auth_password"
CONFIG CliOption = "config"
)

Expand Down
27 changes: 17 additions & 10 deletions bootstrap/pkg/client/gcp/gcp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
gcptypes "github.com/kubeflow/kubeflow/bootstrap/pkg/apis/apps/gcp/v1alpha1"
"github.com/kubeflow/kubeflow/bootstrap/pkg/utils"
log "github.com/sirupsen/logrus"
"golang.org/x/crypto/bcrypt"
"golang.org/x/net/context"
"golang.org/x/oauth2"
"golang.org/x/oauth2/google"
Expand Down Expand Up @@ -162,12 +163,6 @@ func GetKfApp(options map[string]interface{}) kftypes.KfApp {
if options[string(kftypes.USE_BASIC_AUTH)] != nil {
_gcp.GcpApp.Spec.UseBasicAuth = options[string(kftypes.USE_BASIC_AUTH)].(bool)
}
if options[string(kftypes.BASIC_AUTH_USERNAME)] != nil {
_gcp.GcpApp.Spec.BasicAuthUsername = options[string(kftypes.BASIC_AUTH_USERNAME)].(string)
}
if options[string(kftypes.BASIC_AUTH_PASSWORD)] != nil {
_gcp.GcpApp.Spec.BasicAuthPassword = options[string(kftypes.BASIC_AUTH_PASSWORD)].(string)
}
if options[string(kftypes.SKIP_INIT_GCP_PROJECT)] != nil {
skipInitProject := options[string(kftypes.SKIP_INIT_GCP_PROJECT)].(bool)
_gcp.GcpApp.Spec.SkipInitProject = skipInitProject
Expand Down Expand Up @@ -512,7 +507,8 @@ func (gcp *Gcp) updateDM(resources kftypes.ResourceEnum, options map[string]inte
"--zone="+gcp.GcpApp.Spec.Zone,
"--project="+gcp.GcpApp.Spec.Project)
cred_cmd.Stdout = os.Stdout
log.Infof("Running get-credentials ...")
log.Infof("Running get-credentials %v --zone=%v --project=%v ...", gcp.GcpApp.Name,
gcp.GcpApp.Spec.Zone, gcp.GcpApp.Spec.Project)
if err = cred_cmd.Run(); err != nil {
return fmt.Errorf("Error when running gcloud container clusters get-credentials: %v", err)
}
Expand Down Expand Up @@ -540,6 +536,11 @@ func (gcp *Gcp) updateDM(resources kftypes.ResourceEnum, options map[string]inte
}

func (gcp *Gcp) Apply(resources kftypes.ResourceEnum, options map[string]interface{}) error {
if gcp.GcpApp.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.",
kftypes.KUBEFLOW_USERNAME, kftypes.KUBEFLOW_PASSWORD)
}
updateDMErr := gcp.updateDM(resources, options)
if updateDMErr != nil {
return fmt.Errorf("gcp apply could not update deployment manager Error %v", updateDMErr)
Expand Down Expand Up @@ -811,18 +812,24 @@ func (gcp *Gcp) createIapSecret(ctx context.Context, client *clientset.Clientset

// Use username and password provided by user and create secret for basic auth.
func (gcp *Gcp) createBasicAuthSecret(client *clientset.Clientset, options map[string]interface{}) error {
encodedPasswordHash := base64.StdEncoding.EncodeToString([]byte(gcp.GcpApp.Spec.BasicAuthPassword))
username := os.Getenv(kftypes.KUBEFLOW_USERNAME)
password := os.Getenv(kftypes.KUBEFLOW_PASSWORD)
passwordHash, err := bcrypt.GenerateFromPassword([]byte(password), 10)
if err != nil {
return fmt.Errorf("Error when hashing password: %v", err)
}
encodedPasswordHash := base64.StdEncoding.EncodeToString(passwordHash)
secret := &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: BASIC_AUTH_SECRET,
Namespace: gcp.GcpApp.Namespace,
},
Data: map[string][]byte{
"username": []byte(gcp.GcpApp.Spec.BasicAuthUsername),
"username": []byte(username),
"passwordhash": []byte(encodedPasswordHash),
},
}
_, err := client.CoreV1().Secrets(gcp.GcpApp.Namespace).Update(secret)
_, err = client.CoreV1().Secrets(gcp.GcpApp.Namespace).Update(secret)
if err != nil {
log.Warnf("Updating basic auth login is failed, trying to create one: %v", err)
_, err = client.CoreV1().Secrets(gcp.GcpApp.Namespace).Create(secret)
Expand Down
8 changes: 5 additions & 3 deletions testing/kfctl/kfctl_go_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,15 @@ def test_build_kfctl_go(app_path, project):
run_with_retries(["make", "build-kfctl"], cwd=build_dir)
kfctl_path = os.path.join(build_dir, "bin", "kfctl")

# Set ENV for basic auth username/password.
os.environ["KUBEFLOW_USERNAME"] = "kf-test-user"
os.environ["KUBEFLOW_PASSWORD"] = str(uuid.uuid4().hex)

# We don't want the password to show up in the logs because the logs
# are public. So we use subprocess and not util.run
subprocess.check_call([kfctl_path, "init", app_path, "-V", "--platform=gcp",
"--use_basic_auth", "--skip-init-gcp-project",
"--project=" + project,
"--basic_auth_username=kf-test-user",
"--basic_auth_password=" + uuid.uuid4().hex])
"--project=" + project])

# TODO(jlewi): We need to specify a valid email otherwise we get an error
# when trying to apply the IAM policy.
Expand Down