openwhisk-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dube...@apache.org
Subject [incubator-openwhisk] branch master updated: Don't assume apihost is https for sdk and action urls (#2748)
Date Wed, 20 Sep 2017 19:21:53 GMT
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 <ben324@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" <commits@openwhisk.apache.org>'].

Mime
View raw message