You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@openwhisk.apache.org by gi...@git.apache.org on 2017/09/13 23:48:33 UTC
[GitHub] pritidesai commented on a change in pull request #476: Fixing the precedence order of retreiving credentials and adding unit tests
pritidesai commented on a change in pull request #476: Fixing the precedence order of retreiving credentials and adding unit tests
URL: https://github.com/apache/incubator-openwhisk-wskdeploy/pull/476#discussion_r138767866
##########
File path: deployers/whiskclient.go
##########
@@ -20,194 +20,220 @@ package deployers
import (
"bufio"
"fmt"
- "net/http"
- "os"
- "strings"
"github.com/apache/incubator-openwhisk-client-go/whisk"
"github.com/apache/incubator-openwhisk-wskdeploy/parsers"
- "github.com/apache/incubator-openwhisk-wskdeploy/wski18n"
"github.com/apache/incubator-openwhisk-wskdeploy/utils"
- "path"
+ "github.com/apache/incubator-openwhisk-wskdeploy/wski18n"
+ "net/http"
+ "os"
+ "path"
+ "strings"
)
const (
- COMMANDLINE = "wskdeploy command line"
+ COMMANDLINE = "wskdeploy command line"
DEFAULTVALUE = "default value"
- WSKPROPS = ".wskprops"
+ WSKPROPS = ".wskprops"
WHISKPROPERTY = "whisk.properties"
- INTERINPUT = "interactve input"
+ INTERINPUT = "interactve input"
)
type PropertyValue struct {
- Value string
+ Value string
Source string
}
-var GetPropertyValue = func (prop PropertyValue, newValue string, source string) PropertyValue {
+var GetPropertyValue = func(prop PropertyValue, newValue string, source string) PropertyValue {
if len(prop.Value) == 0 && len(newValue) > 0 {
prop.Value = newValue
prop.Source = source
}
return prop
}
-var GetWskPropFromWskprops = func (pi whisk.Properties, proppath string) (*whisk.Wskprops, error) {
+var GetWskPropFromWskprops = func(pi whisk.Properties, proppath string) (*whisk.Wskprops, error) {
return whisk.GetWskPropFromWskprops(pi, proppath)
}
-var GetWskPropFromWhiskProperty = func (pi whisk.Properties) (*whisk.Wskprops, error) {
+var GetWskPropFromWhiskProperty = func(pi whisk.Properties) (*whisk.Wskprops, error) {
return whisk.GetWskPropFromWhiskProperty(pi)
}
-var GetCommandLineFlags = func () (string, string, string) {
+var GetCommandLineFlags = func() (string, string, string) {
return utils.Flags.ApiHost, utils.Flags.Auth, utils.Flags.Namespace
}
-var CreateNewClient = func (httpClient *http.Client, config_input *whisk.Config) (*whisk.Client, error) {
+var CreateNewClient = func(httpClient *http.Client, config_input *whisk.Config) (*whisk.Client, error) {
return whisk.NewClient(http.DefaultClient, clientConfig)
}
+// we are reading openwhisk credentials (apihost, namespace, and auth) in the following precedence order:
+// (1) wskdeploy command line `wskdeploy --apihost --namespace --auth`
+// (2) deployment file
+// (3) manifest file
+// (4) .wskprops
+// (5) prompt for values in interactive mode if any of them are missing
func NewWhiskConfig(proppath string, deploymentPath string, manifestPath string, isInteractive bool) (*whisk.Config, error) {
- credential := PropertyValue {}
- namespace := PropertyValue {}
- apiHost := PropertyValue {}
-
- // First, we look up the above variables in the deployment file.
- if utils.FileExists(deploymentPath) {
- mm := parsers.NewYAMLParser()
- deployment, err := mm.ParseDeployment(deploymentPath)
- if err == nil {
- credential.Value = deployment.Application.Credential
- credential.Source = path.Base(deploymentPath)
- namespace.Value = deployment.Application.Namespace
- namespace.Source = path.Base(deploymentPath)
- apiHost.Value = deployment.Application.ApiHost
- apiHost.Source = path.Base(deploymentPath)
- }
- }
-
- if len(credential.Value) == 0 || len(namespace.Value) == 0 || len(apiHost.Value) == 0 {
- if utils.FileExists(manifestPath) {
- mm := parsers.NewYAMLParser()
- manifest, err := mm.ParseManifest(manifestPath)
- if err == nil {
- credential = GetPropertyValue(credential, manifest.Package.Credential, path.Base(manifestPath))
- namespace = GetPropertyValue(namespace, manifest.Package.Namespace, path.Base(manifestPath))
- apiHost = GetPropertyValue(apiHost, manifest.Package.ApiHost, path.Base(manifestPath))
- }
- }
- }
-
- // If the variables are not correctly assigned, we look up auth key and api host in the command line. The namespace
- // is currently not available in command line, which can be added later.
- apihost, auth, ns := GetCommandLineFlags()
- credential = GetPropertyValue(credential, auth, COMMANDLINE)
- namespace = GetPropertyValue(namespace, ns, COMMANDLINE)
- apiHost = GetPropertyValue(apiHost, apihost, COMMANDLINE)
-
- // Third, we need to look up the variables in .wskprops file.
- pi := whisk.PropertiesImp {
- OsPackage: whisk.OSPackageImp{},
- }
-
- // The error raised here can be neglected, because we will handle it in the end of this function.
- wskprops, _ := GetWskPropFromWskprops(pi, proppath)
- credential = GetPropertyValue(credential, wskprops.AuthKey, WSKPROPS)
- namespace = GetPropertyValue(namespace, wskprops.Namespace, WSKPROPS)
- apiHost = GetPropertyValue(apiHost, wskprops.APIHost, WSKPROPS)
-
- // Fourth, we look up the variables in whisk.properties on a local openwhisk deployment.
- if len(credential.Value) == 0 || len(apiHost.Value) == 0 {
- // No need to keep the default value for namespace, since both of auth and apihost are not set after .wskprops.
- // whisk.property will set the default value as well.
- apiHost.Value = ""
- apiHost.Source = DEFAULTVALUE
- }
-
- // The error raised here can be neglected, because we will handle it in the end of this function.
- whiskproperty, _ := GetWskPropFromWhiskProperty(pi)
- credential = GetPropertyValue(credential, whiskproperty.AuthKey, WHISKPROPERTY)
- namespace = GetPropertyValue(namespace, whiskproperty.Namespace, WHISKPROPERTY)
- apiHost = GetPropertyValue(apiHost, whiskproperty.APIHost, WHISKPROPERTY)
-
- // If we still can not find the variables we need, check if it is interactive mode. If so, we accept the input
- // from the user. The namespace will be set to a default value, when the code reaches this line, because WSKPROPS
- // has a default value for namespace.
- if len(apiHost.Value) == 0 && isInteractive == true {
- host := promptForValue("\nPlease provide the hostname for OpenWhisk [default value is openwhisk.ng.bluemix.net]: ")
- if host == "" {
- host = "openwhisk.ng.bluemix.net"
- }
- apiHost.Value = host
- apiHost.Source = INTERINPUT
- }
-
- if len(credential.Value) == 0 && isInteractive == true {
- cred := promptForValue("\nPlease provide an authentication token: ")
- credential.Value = cred
- credential.Source = INTERINPUT
-
- // The namespace is always associated with the credential. Both of them should be picked up from the same source.
- if len(namespace.Value) == 0 || namespace.Value == whisk.DEFAULT_NAMESPACE {
- ns := promptForValue("\nPlease provide a namespace [default value is guest]: ")
-
- source := INTERINPUT
- if ns == "" {
- ns = whisk.DEFAULT_NAMESPACE
- source = DEFAULTVALUE
- }
- namespace.Value = ns
- namespace.Source = source
- }
- }
-
- clientConfig = &whisk.Config{
- AuthToken: credential.Value, //Authtoken
- Namespace: namespace.Value, //Namespace
- Host: apiHost.Value,
- Version: "v1",
- Insecure: true, // true if you want to ignore certificate signing
- }
-
- if len(credential.Value) == 0 {
- errStr := wski18n.T("The authentication key is not configured.\n")
- whisk.Debug(whisk.DbgError, errStr)
- return clientConfig, utils.NewInvalidWskpropsError(errStr)
- }
-
- if len(apiHost.Value) == 0 {
- errStr := wski18n.T("The API host is not configured.\n")
- whisk.Debug(whisk.DbgError, errStr)
- return clientConfig, utils.NewInvalidWskpropsError(errStr)
- }
-
- if len(namespace.Value) == 0 {
- errStr := wski18n.T("The namespace is not configured.\n")
- whisk.Debug(whisk.DbgError, errStr)
- return clientConfig, utils.NewInvalidWskpropsError(errStr)
- }
-
- stdout := wski18n.T("The API host is {{.apihost}}, from {{.apisource}}.\n",
- map[string]interface{}{"apihost": apiHost.Value, "apisource": apiHost.Source})
- whisk.Debug(whisk.DbgInfo, stdout)
-
- stdout = wski18n.T("The auth key is set, from {{.authsource}}.\n",
- map[string]interface{}{"authsource": credential.Source})
- whisk.Debug(whisk.DbgInfo, stdout)
-
- stdout = wski18n.T("The namespace is {{.namespace}}, from {{.namespacesource}}.\n",
- map[string]interface{}{"namespace": namespace.Value, "namespacesource": namespace.Source})
- whisk.Debug(whisk.DbgInfo, stdout)
- return clientConfig, nil
+ // struct to store credential, namespace, and host with their respective source
+ credential := PropertyValue{}
+ namespace := PropertyValue{}
+ apiHost := PropertyValue{}
+
+ // read credentials from command line
+ apihost, auth, ns := GetCommandLineFlags()
+ credential = GetPropertyValue(credential, auth, COMMANDLINE)
+ namespace = GetPropertyValue(namespace, ns, COMMANDLINE)
+ apiHost = GetPropertyValue(apiHost, apihost, COMMANDLINE)
+
+ // now, read them from deployment file if not found on command line
+ if len(credential.Value) == 0 || len(namespace.Value) == 0 || len(apiHost.Value) == 0 {
+ if utils.FileExists(deploymentPath) {
+ mm := parsers.NewYAMLParser()
+ deployment, _ := mm.ParseDeployment(deploymentPath)
+ credential = GetPropertyValue(credential, deployment.Application.Credential, path.Base(deploymentPath))
+ namespace = GetPropertyValue(namespace, deployment.Application.Namespace, path.Base(deploymentPath))
+ apiHost = GetPropertyValue(apiHost, deployment.Application.ApiHost, path.Base(deploymentPath))
+ }
+ }
+
+ // read credentials from manifest file as didn't find them on command line and in deployment file
+ if len(credential.Value) == 0 || len(namespace.Value) == 0 || len(apiHost.Value) == 0 {
+ if utils.FileExists(manifestPath) {
+ mm := parsers.NewYAMLParser()
+ manifest, _ := mm.ParseManifest(manifestPath)
+ credential = GetPropertyValue(credential, manifest.Package.Credential, path.Base(manifestPath))
+ namespace = GetPropertyValue(namespace, manifest.Package.Namespace, path.Base(manifestPath))
+ apiHost = GetPropertyValue(apiHost, manifest.Package.ApiHost, path.Base(manifestPath))
+ }
+ }
+
+ // Third, we need to look up the variables in .wskprops file.
+ pi := whisk.PropertiesImp{
+ OsPackage: whisk.OSPackageImp{},
+ }
+
+ // The error raised here can be neglected, because we will handle it in the end of this function.
+ wskprops, _ := GetWskPropFromWskprops(pi, proppath)
+ credential = GetPropertyValue(credential, wskprops.AuthKey, WSKPROPS)
+ namespace = GetPropertyValue(namespace, wskprops.Namespace, WSKPROPS)
+ apiHost = GetPropertyValue(apiHost, wskprops.APIHost, WSKPROPS)
+
+ // now, read credentials from whisk.properties but this is only acceptable within Travis
+ // whisk.properties will soon be deprecated and should not be used for any production deployment
+ whiskproperty, _ := GetWskPropFromWhiskProperty(pi)
+ credential = GetPropertyValue(credential, whiskproperty.AuthKey, WHISKPROPERTY)
+ if credential.Source == WHISKPROPERTY {
+ fmt.Println("WARNING: The authentication key was retrieved from whisk.properties " +
+ "which will soon be deprecated please do not use it outside of Travis builds.")
+ }
+ namespace = GetPropertyValue(namespace, whiskproperty.Namespace, WHISKPROPERTY)
+ if namespace.Source == WHISKPROPERTY {
+ fmt.Println("WARNING: The namespace was retrieved from whisk.properties " +
+ "which will soon be deprecated please do not use it outside of Travis builds.")
+ }
+ apiHost = GetPropertyValue(apiHost, whiskproperty.APIHost, WHISKPROPERTY)
+ if apiHost.Source == WHISKPROPERTY {
+ fmt.Println("WARNING: The API host was retrieved from whisk.properties " +
+ "which will soon be deprecated please do not use it outside of Travis builds.")
+ }
+
+ // set namespace to default namespace if not yet found
+ if len(apiHost.Value) != 0 && len(credential.Value) != 0 && len(namespace.Value) == 0 {
+ namespace.Value = whisk.DEFAULT_NAMESPACE
+ namespace.Source = DEFAULTVALUE
+ }
+
+ // If we still can not find the values we need, check if it is interactive mode.
+ // If so, we prompt users for the input.
+ // The namespace is set to a default value at this point if not provided.
+ if len(apiHost.Value) == 0 && isInteractive == true {
+ host, err := promptForValue("\nPlease provide the hostname for OpenWhisk [default value is openwhisk.ng.bluemix.net]: ")
+ utils.Check(err)
+ if host == "" {
+ host = "openwhisk.ng.bluemix.net"
+ }
+ apiHost.Value = host
+ apiHost.Source = INTERINPUT
+ }
+
+ if len(credential.Value) == 0 && isInteractive == true {
Review comment:
Both `len(string) == 0` and `string == ""` are idiomatic ways of checking if string is empty. To find which option is faster and results showed `len()` is faster (for fun :) as the length of a string is stored in the variable itself during declaration.
```
package main
import (
"fmt"
"time"
"github.com/davecgh/go-spew/spew"
)
func main() {
// Length is 3.
string1 := "cat"
// Length is 13 times 5 which is 65.
string2 := "cat0123456789" +
"cat0123456789" +
"cat0123456789" +
"cat0123456789" +
"cat0123456789"
spew.Dump(string1)
spew.Dump(string2)
t0 := time.Now()
// Version 1: length of short string.
for i := 0; i < 10000000; i++ {
result := len(string1)
if result != 3 {
return
}
}
t1 := time.Now()
// Version 2: length of long string.
for i := 0; i < 10000000; i++ {
result := len(string2)
if result != 65 {
return
}
}
t2 := time.Now()
// Print results.
fmt.Println(t1.Sub(t0))
fmt.Println(t2.Sub(t1))
}
```
Results:
$ go run main.go
(string) (len=3) "cat"
(string) (len=65) "cat0123456789cat0123456789cat0123456789cat0123456789cat0123456789"
2.752905ms
2.761132ms
```
package main
import (
"fmt"
"time"
)
func main() {
// Length is 3.
string1 := "cat"
// Length is 13 times 5 which is 65.
string2 := "cat0123456789" +
"cat0123456789" +
"cat0123456789" +
"cat0123456789" +
"cat0123456789"
t0 := time.Now()
// Version 1: length of short string.
for i := 0; i < 10000000; i++ {
if string1 == "" {
return
}
}
t1 := time.Now()
// Version 2: length of long string.
for i := 0; i < 10000000; i++ {
if string2 == "" {
return
}
}
t2 := time.Now()
// Print results.
fmt.Println(t1.Sub(t0))
fmt.Println(t2.Sub(t1))
}
```
Results:
go run main.go
2.91229ms
3.12793ms
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services