You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by du...@apache.org on 2017/09/20 19:21:53 UTC
[incubator-openwhisk] branch master updated: Don't assume apihost
is https for sdk and action urls (#2748)
This is an automated email from the ASF dual-hosted git repository.
dubeejw pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-openwhisk.git
The following commit(s) were added to refs/heads/master by this push:
new ac99eed Don't assume apihost is https for sdk and action urls (#2748)
ac99eed is described below
commit ac99eed848e2f300e30ff7b0ff7d11f3aeb1852b
Author: Ben Browning <be...@gmail.com>
AuthorDate: Wed Sep 20 15:21:51 2017 -0400
Don't assume apihost is https for sdk and action urls (#2748)
* Don't assume apihost is https for sdk and action urls
Reuse the getURLBase utility method when computing the URL for sdk
downloads and action URLs.
This fixes #2720 and fixes #2719.
* Cleanup some trailing whitespace I missed
* Missed this import in last-second rebase
* Update debug messages to match `GetURLBase` method name
---
.../src/test/scala/system/basic/WskSdkTests.scala | 19 +++++++++-
.../whisk/core/cli/test/WskBasicUsageTests.scala | 42 ++++++++++++++++++----
tools/cli/go-whisk-cli/commands/action.go | 8 ++++-
tools/cli/go-whisk-cli/commands/commands.go | 4 +--
tools/cli/go-whisk-cli/commands/property.go | 8 ++---
tools/cli/go-whisk-cli/commands/util.go | 24 -------------
tools/cli/go-whisk/whisk/action.go | 20 +++++++----
tools/cli/go-whisk/whisk/sdk.go | 6 +++-
tools/cli/go-whisk/whisk/util.go | 24 +++++++++++++
9 files changed, 108 insertions(+), 47 deletions(-)
diff --git a/tests/src/test/scala/system/basic/WskSdkTests.scala b/tests/src/test/scala/system/basic/WskSdkTests.scala
index 62c8d60..df550b8 100644
--- a/tests/src/test/scala/system/basic/WskSdkTests.scala
+++ b/tests/src/test/scala/system/basic/WskSdkTests.scala
@@ -24,8 +24,8 @@ import scala.collection.JavaConversions.asScalaBuffer
import org.apache.commons.io.FileUtils
import org.junit.runner.RunWith
import org.scalatest.junit.JUnitRunner
-
import common.TestHelpers
+import common.TestUtils.ERROR_EXIT
import common.TestUtils.SUCCESS_EXIT
import common.WhiskProperties
import common.Wsk
@@ -40,6 +40,23 @@ class WskSdkTests extends TestHelpers with WskTestHelpers {
behavior of "Wsk SDK"
+ it should "prefix https to apihost if no scheme given" in {
+ val result = wsk.cli(Seq("--apihost", "localhost:54321", "sdk", "install", "docker"), expectedExitCode = ERROR_EXIT)
+ result.stderr should include regex ("""(?i)Get https://localhost:54321/""")
+ }
+
+ it should "not prefix https to http apihost" in {
+ val result =
+ wsk.cli(Seq("--apihost", "http://localhost:54321", "sdk", "install", "docker"), expectedExitCode = ERROR_EXIT)
+ result.stderr should include regex ("""(?i)Get http://localhost:54321/""")
+ }
+
+ it should "not double prefix https to https apihost" in {
+ val result =
+ wsk.cli(Seq("--apihost", "https://localhost:54321", "sdk", "install", "docker"), expectedExitCode = ERROR_EXIT)
+ result.stderr should include regex ("""(?i)Get https://localhost:54321/""")
+ }
+
it should "download docker action sdk" in {
val dir = File.createTempFile("wskinstall", ".tmp")
dir.delete()
diff --git a/tests/src/test/scala/whisk/core/cli/test/WskBasicUsageTests.scala b/tests/src/test/scala/whisk/core/cli/test/WskBasicUsageTests.scala
index c411705..c635959 100644
--- a/tests/src/test/scala/whisk/core/cli/test/WskBasicUsageTests.scala
+++ b/tests/src/test/scala/whisk/core/cli/test/WskBasicUsageTests.scala
@@ -652,9 +652,10 @@ class WskBasicUsageTests extends TestHelpers with WskTestHelpers {
val encodedPackageName = URLEncoder.encode(packageName, StandardCharsets.UTF_8.name).replace("+", "%20")
val encodedWebActionName = URLEncoder.encode(webActionName, StandardCharsets.UTF_8.name).replace("+", "%20")
val encodedNamespace = URLEncoder.encode(namespace, StandardCharsets.UTF_8.name).replace("+", "%20")
- val actionPath = "https://%s/api/%s/namespaces/%s/actions/%s"
+ val scheme = "https"
+ val actionPath = "%s://%s/api/%s/namespaces/%s/actions/%s"
val packagedActionPath = s"$actionPath/%s"
- val webActionPath = "https://%s/api/%s/web/%s/%s/%s"
+ val webActionPath = "%s://%s/api/%s/web/%s/%s/%s"
assetHelper.withCleaner(wsk.action, actionName) { (action, _) =>
action.create(actionName, defaultAction)
@@ -677,25 +678,52 @@ class WskBasicUsageTests extends TestHelpers with WskTestHelpers {
}
wsk.action.get(actionName, url = Some(true)).stdout should include(
- actionPath.format(wskprops.apihost, wskprops.apiversion, encodedNamespace, encodedActionName))
+ actionPath.format(scheme, wskprops.apihost, wskprops.apiversion, encodedNamespace, encodedActionName))
// Ensure url flag works when a field filter and summary flag are specified
wsk.action.get(actionName, url = Some(true), fieldFilter = Some("field"), summary = true).stdout should include(
- actionPath.format(wskprops.apihost, wskprops.apiversion, encodedNamespace, encodedActionName))
+ actionPath.format(scheme, wskprops.apihost, wskprops.apiversion, encodedNamespace, encodedActionName))
wsk.action.get(webActionName, url = Some(true)).stdout should include(
webActionPath
- .format(wskprops.apihost, wskprops.apiversion, encodedNamespace, defaultPackageName, encodedWebActionName))
+ .format(
+ scheme,
+ wskprops.apihost,
+ wskprops.apiversion,
+ encodedNamespace,
+ defaultPackageName,
+ encodedWebActionName))
wsk.action.get(packagedAction, url = Some(true)).stdout should include(
packagedActionPath
- .format(wskprops.apihost, wskprops.apiversion, encodedNamespace, encodedPackageName, encodedActionName))
+ .format(scheme, wskprops.apihost, wskprops.apiversion, encodedNamespace, encodedPackageName, encodedActionName))
wsk.action.get(packagedWebAction, url = Some(true)).stdout should include(
webActionPath
- .format(wskprops.apihost, wskprops.apiversion, encodedNamespace, encodedPackageName, encodedWebActionName))
+ .format(
+ scheme,
+ wskprops.apihost,
+ wskprops.apiversion,
+ encodedNamespace,
+ encodedPackageName,
+ encodedWebActionName))
wsk.action.get(nonExistentActionName, url = Some(true), expectedExitCode = NOT_FOUND)
+
+ val httpsProps = WskProps(apihost = "https://" + wskprops.apihost)
+ wsk.action.get(actionName, url = Some(true))(httpsProps).stdout should include(
+ actionPath
+ .format("https", wskprops.apihost, wskprops.apiversion, encodedNamespace, encodedActionName))
+ wsk.action.get(webActionName, url = Some(true))(httpsProps).stdout should include(
+ webActionPath
+ .format(
+ "https",
+ wskprops.apihost,
+ wskprops.apiversion,
+ encodedNamespace,
+ defaultPackageName,
+ encodedWebActionName))
+
}
it should "limit length of HTTP request and response bodies for --verbose" in withAssetCleaner(wskprops) {
(wp, assetHelper) =>
diff --git a/tools/cli/go-whisk-cli/commands/action.go b/tools/cli/go-whisk-cli/commands/action.go
index bfa6294..86b427b 100644
--- a/tools/cli/go-whisk-cli/commands/action.go
+++ b/tools/cli/go-whisk-cli/commands/action.go
@@ -229,10 +229,16 @@ var actionGetCmd = &cobra.Command{
}
if flags.action.url {
- actionURL := action.ActionURL(Properties.APIHost,
+ actionURL, err := action.ActionURL(Properties.APIHost,
DefaultOpenWhiskApiPath,
Properties.APIVersion,
qualifiedName.GetPackageName())
+ if err != nil {
+ errStr := wski18n.T("Invalid host address '{{.host}}': {{.err}}",
+ map[string]interface{}{"host": Properties.APIHost, "err": err})
+ werr := whisk.MakeWskError(errors.New(errStr), whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
+ return werr
+ }
printActionGetWithURL(qualifiedName.GetEntity(), actionURL)
} else if flags.common.summary {
printSummary(action)
diff --git a/tools/cli/go-whisk-cli/commands/commands.go b/tools/cli/go-whisk-cli/commands/commands.go
index 6a93ee6..63ee37a 100644
--- a/tools/cli/go-whisk-cli/commands/commands.go
+++ b/tools/cli/go-whisk-cli/commands/commands.go
@@ -33,7 +33,7 @@ const DefaultOpenWhiskApiPath string = "/api"
var UserAgent string = "OpenWhisk-CLI"
func setupClientConfig(cmd *cobra.Command, args []string) (error){
- baseURL, err := getURLBase(Properties.APIHost, DefaultOpenWhiskApiPath)
+ baseURL, err := whisk.GetURLBase(Properties.APIHost, DefaultOpenWhiskApiPath)
// Determine if the parent command will require the API host to be set
apiHostRequired := (cmd.Parent().Name() == "property" && cmd.Name() == "get" && (flags.property.auth ||
@@ -45,7 +45,7 @@ func setupClientConfig(cmd *cobra.Command, args []string) (error){
// Display an error if the parent command requires an API host to be set, and the current API host is not valid
if err != nil && !apiHostRequired {
- whisk.Debug(whisk.DbgError, "getURLBase(%s, %s) error: %s\n", Properties.APIHost, DefaultOpenWhiskApiPath, err)
+ whisk.Debug(whisk.DbgError, "whisk.GetURLBase(%s, %s) error: %s\n", Properties.APIHost, DefaultOpenWhiskApiPath, err)
errMsg := wski18n.T("The API host is not valid: {{.err}}", map[string]interface{}{"err": err})
whiskErr := whisk.MakeWskErrorFromWskError(errors.New(errMsg), err, whisk.EXIT_CODE_ERR_GENERAL,
whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
diff --git a/tools/cli/go-whisk-cli/commands/property.go b/tools/cli/go-whisk-cli/commands/property.go
index 27ad367..4a6f9b9 100644
--- a/tools/cli/go-whisk-cli/commands/property.go
+++ b/tools/cli/go-whisk-cli/commands/property.go
@@ -106,11 +106,11 @@ var propertySetCmd = &cobra.Command{
}
if apiHost := flags.property.apihostSet; len(apiHost) > 0 {
- baseURL, err := getURLBase(apiHost, DefaultOpenWhiskApiPath)
+ baseURL, err := whisk.GetURLBase(apiHost, DefaultOpenWhiskApiPath)
if err != nil {
// Not aborting now. Subsequent commands will result in error
- whisk.Debug(whisk.DbgError, "getURLBase(%s, %s) error: %s", apiHost, DefaultOpenWhiskApiPath, err)
+ whisk.Debug(whisk.DbgError, "whisk.GetURLBase(%s, %s) error: %s", apiHost, DefaultOpenWhiskApiPath, err)
errStr := fmt.Sprintf(
wski18n.T("Unable to set API host value; the API host value '{{.apihost}}' is invalid: {{.err}}",
map[string]interface{}{"apihost": apiHost, "err": err}))
@@ -537,10 +537,10 @@ func parseConfigFlags(cmd *cobra.Command, args []string) error {
if client != nil {
client.Config.Host = apiHost
- baseURL, err := getURLBase(apiHost, DefaultOpenWhiskApiPath)
+ baseURL, err := whisk.GetURLBase(apiHost, DefaultOpenWhiskApiPath)
if err != nil {
- whisk.Debug(whisk.DbgError, "getURLBase(%s, %s) failed: %s\n", apiHost, DefaultOpenWhiskApiPath, err)
+ whisk.Debug(whisk.DbgError, "whisk.GetURLBase(%s, %s) failed: %s\n", apiHost, DefaultOpenWhiskApiPath, err)
errStr := wski18n.T("Invalid host address '{{.host}}': {{.err}}",
map[string]interface{}{"host": Properties.APIHost, "err": err})
werr := whisk.MakeWskError(errors.New(errStr), whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
diff --git a/tools/cli/go-whisk-cli/commands/util.go b/tools/cli/go-whisk-cli/commands/util.go
index e8c422f..fa16168 100644
--- a/tools/cli/go-whisk-cli/commands/util.go
+++ b/tools/cli/go-whisk-cli/commands/util.go
@@ -34,7 +34,6 @@ import (
"compress/gzip"
"archive/zip"
"encoding/json"
- "net/url"
"io/ioutil"
"sort"
"reflect"
@@ -815,29 +814,6 @@ func checkArgs(args []string, minimumArgNumber int, maximumArgNumber int, comman
}
}
-func getURLBase(host string, path string) (*url.URL, error) {
- if len(host) == 0 {
- errMsg := wski18n.T("An API host must be provided.")
- whiskErr := whisk.MakeWskError(errors.New(errMsg), whisk.EXIT_CODE_ERR_GENERAL,
- whisk.DISPLAY_MSG, whisk.DISPLAY_USAGE)
- return nil, whiskErr
- }
-
- if !strings.HasPrefix(host, "http") {
- host = "https://" + host
- }
-
- urlBase := fmt.Sprintf("%s%s", host, path)
- url, err := url.Parse(urlBase)
-
- if len(url.Scheme) == 0 || len(url.Host) == 0 {
- urlBase = fmt.Sprintf("https://%s%s", host, path)
- url, err = url.Parse(urlBase)
- }
-
- return url, err
-}
-
func normalizeNamespace(namespace string) (string) {
if (namespace == "_") {
namespace = wski18n.T("default")
diff --git a/tools/cli/go-whisk/whisk/action.go b/tools/cli/go-whisk/whisk/action.go
index 8b17d23..7284ae6 100644
--- a/tools/cli/go-whisk/whisk/action.go
+++ b/tools/cli/go-whisk/whisk/action.go
@@ -122,33 +122,39 @@ func (action Action) WebAction() (webExportValue bool) {
Returns the URL of an action as a string. A valid API host, path and version must be passed. A package that contains the
action must be passed as well. An empty string must be passed if the action is not packaged.
*/
-func (action Action) ActionURL(apiHost string, apiPath string, apiVersion string, pkg string) (actionURL string) {
- webActionPath := "https://%s%s/%s/web/%s/%s/%s"
- actionPath := "https://%s%s/%s/namespaces/%s/actions/%s"
+func (action Action) ActionURL(apiHost string, apiPath string, apiVersion string, pkg string) (string, error) {
+ baseURL, err := GetURLBase(apiHost, apiPath)
+ if err != nil {
+ Debug(DbgError, "GetURLBase(%s, %s) failed: %s\n", apiHost, apiPath, err)
+ return "", err
+ }
+ webActionPath := "%s/%s/web/%s/%s/%s"
+ actionPath := "%s/%s/namespaces/%s/actions/%s"
packagedActionPath := actionPath + "/%s"
namespace := strings.Split(action.Namespace, "/")[0]
namespace = strings.Replace(url.QueryEscape(namespace), "+", "%20", -1)
name := strings.Replace(url.QueryEscape(action.Name), "+", "%20", -1)
pkg = strings.Replace(url.QueryEscape(pkg), "+", "%20", -1)
+ var actionURL string
if action.WebAction() {
if len(pkg) == 0 {
pkg = "default"
}
- actionURL = fmt.Sprintf(webActionPath, apiHost, apiPath, apiVersion, namespace, pkg, name)
+ actionURL = fmt.Sprintf(webActionPath, baseURL, apiVersion, namespace, pkg, name)
Debug(DbgInfo, "Web action URL: %s\n", actionURL)
} else {
if len(pkg) == 0 {
- actionURL = fmt.Sprintf(actionPath, apiHost, apiPath, apiVersion, namespace, name)
+ actionURL = fmt.Sprintf(actionPath, baseURL, apiVersion, namespace, name)
Debug(DbgInfo, "Packaged action URL: %s\n", actionURL)
} else {
- actionURL = fmt.Sprintf(packagedActionPath, apiHost, apiPath, apiVersion, namespace, pkg, name)
+ actionURL = fmt.Sprintf(packagedActionPath, baseURL, apiVersion, namespace, pkg, name)
Debug(DbgInfo, "Action URL: %s\n", actionURL)
}
}
- return actionURL
+ return actionURL, nil
}
////////////////////
diff --git a/tools/cli/go-whisk/whisk/sdk.go b/tools/cli/go-whisk/whisk/sdk.go
index 01e4f3b..df3178a 100644
--- a/tools/cli/go-whisk/whisk/sdk.go
+++ b/tools/cli/go-whisk/whisk/sdk.go
@@ -40,7 +40,11 @@ type SdkRequest struct {
// Install artifact {component = docker || swift || iOS}
func (s *SdkService) Install(relFileUrl string) (*http.Response, error) {
- urlStr := fmt.Sprintf("https://%s/%s", s.client.Config.BaseURL.Host, relFileUrl)
+ baseURL := s.client.Config.BaseURL
+ // Remove everything but the scheme, host, and port
+ baseURL.Path, baseURL.RawQuery, baseURL.Fragment = "", "", ""
+
+ urlStr := fmt.Sprintf("%s/%s", baseURL, relFileUrl)
req, err := http.NewRequest("GET", urlStr, nil)
if err != nil {
diff --git a/tools/cli/go-whisk/whisk/util.go b/tools/cli/go-whisk/whisk/util.go
index f29f1e0..cf576d8 100644
--- a/tools/cli/go-whisk/whisk/util.go
+++ b/tools/cli/go-whisk/whisk/util.go
@@ -22,6 +22,7 @@ import (
"fmt"
"net/url"
"reflect"
+ "strings"
"github.com/fatih/color"
"github.com/google/go-querystring/query"
@@ -80,3 +81,26 @@ func PrintJSON(v interface{}) {
output, _ := prettyjson.Marshal(v)
fmt.Fprintln(color.Output, string(output))
}
+
+func GetURLBase(host string, path string) (*url.URL, error) {
+ if len(host) == 0 {
+ errMsg := wski18n.T("An API host must be provided.")
+ whiskErr := MakeWskError(errors.New(errMsg), EXIT_CODE_ERR_GENERAL,
+ DISPLAY_MSG, DISPLAY_USAGE)
+ return nil, whiskErr
+ }
+
+ if !strings.HasPrefix(host, "http") {
+ host = "https://" + host
+ }
+
+ urlBase := fmt.Sprintf("%s%s", host, path)
+ url, err := url.Parse(urlBase)
+
+ if len(url.Scheme) == 0 || len(url.Host) == 0 {
+ urlBase = fmt.Sprintf("https://%s%s", host, path)
+ url, err = url.Parse(urlBase)
+ }
+
+ return url, err
+}
--
To stop receiving notification emails like this one, please contact
['"commits@openwhisk.apache.org" <co...@openwhisk.apache.org>'].