Enable Web Secure token for sequences on API create (#1087)

* Enable Web Secure token for sequences on API create * Enable Web Secure token for sequences on API create * Fix non-working code that attempted to add web annotations * refine procedural logic for reuse * refine procedural logic for reuse * refine procedural logic for reuse * refine procedural logic for reuse * refine procedural logic for reuse
apache · Feb 1, 2020 · db5e34e · db5e34e
1 parent 0a6491c
commit db5e34e
Show file tree

Hide file tree

Showing 16 changed files with 326 additions and 51 deletions.
diff --git a/deployers/manifestreader.go b/deployers/manifestreader.go
@@ -108,7 +108,7 @@ func (reader *ManifestReader) HandleYaml(manifestParser *parsers.YAMLParser, man
 		return wskderrors.NewYAMLFileFormatError(manifestName, err)
 	}
 
-	apis, responses, err := manifestParser.ComposeApiRecordsFromAllPackages(reader.serviceDeployer.ClientConfig, manifest)
+	apis, responses, err := manifestParser.ComposeApiRecordsFromAllPackages(reader.serviceDeployer.ClientConfig, manifest, actions, sequences)
 	if err != nil {
 		return wskderrors.NewYAMLFileFormatError(manifestName, err)
 	}

diff --git a/deployers/servicedeployer.go b/deployers/servicedeployer.go
@@ -1012,15 +1012,18 @@ func (deployer *ServiceDeployer) createAction(pkgname string, action *whisk.Acti
 	return nil
 }
 
-func (deployer *ServiceDeployer) getAnnotationsFromPackageAction(packageActionName string) *whisk.KeyValueArr {
+func (deployer *ServiceDeployer) getAnnotationsFromPackageActionOrSequence(packageActionName string) *whisk.KeyValueArr {
 
 	if len(packageActionName)!=0 {
 		// Split the package name and action name being searched for
-		aActionName := strings.Split(packageActionName,"/")
+		aActionName := strings.Split(packageActionName,	parsers.PATH_SEPARATOR)
 
+		// Attempt to locate the named action (or sequence) to return its annotations
 		if pkg, found := deployer.Deployment.Packages[aActionName[0]]; found {
 			if atemp, found := pkg.Actions[aActionName[1]]; found {
 				return &(atemp.Action.Annotations)
+			} else if atemp, found := pkg.Sequences[aActionName[1]]; found {
+				return &(atemp.Action.Annotations)
 			}
 		}
 	}
@@ -1041,9 +1044,11 @@ func (deployer *ServiceDeployer) createApi(api *whisk.ApiCreateRequest) error {
 
 	// Retrieve annotations on the action we are attempting to create an API for
 	var actionAnnotations *whisk.KeyValueArr
-	actionAnnotations = deployer.getAnnotationsFromPackageAction(api.ApiDoc.Action.Name)
+	actionAnnotations = deployer.getAnnotationsFromPackageActionOrSequence(api.ApiDoc.Action.Name)
 
 	// Process any special annotations (e.g., "require-whisk-auth") on the associated Action
+	// NOTE: we do not throw an error if annotations are NOT found (nil) since this is already done in
+	// the parsing phase and would be redundant.
 	if actionAnnotations != nil {
 		wskprint.PrintlnOpenWhiskVerbose(utils.Flags.Verbose, fmt.Sprintf("Processing action annotations: %v", actionAnnotations))
 

diff --git a/parsers/manifest_parser.go b/parsers/manifest_parser.go
@@ -1131,7 +1131,9 @@ func (dm *YAMLParser) ComposeRules(pkg Package, packageName string, managedAnnot
 	return rules, nil
 }
 
-func (dm *YAMLParser) ComposeApiRecordsFromAllPackages(client *whisk.Config, manifest *YAML) ([]*whisk.ApiCreateRequest, map[string]*whisk.ApiCreateRequestOptions, error) {
+func (dm *YAMLParser) ComposeApiRecordsFromAllPackages(client *whisk.Config, manifest *YAML,
+	actionrecords []utils.ActionRecord,
+	sequencerecords []utils.ActionRecord) ([]*whisk.ApiCreateRequest, map[string]*whisk.ApiCreateRequestOptions, error) {
 	var requests = make([]*whisk.ApiCreateRequest, 0)
 	var responses = make(map[string]*whisk.ApiCreateRequestOptions, 0)
 	manifestPackages := make(map[string]Package)
@@ -1143,7 +1145,8 @@ func (dm *YAMLParser) ComposeApiRecordsFromAllPackages(client *whisk.Config, man
 	}
 
 	for packageName, p := range manifestPackages {
-		r, response, err := dm.ComposeApiRecords(client, packageName, p, manifest.Filepath)
+		r, response, err := dm.ComposeApiRecords(client, packageName, p, manifest.Filepath,
+			actionrecords, sequencerecords)
 		if err == nil {
 			requests = append(requests, r...)
 			for k, v := range response {
@@ -1181,7 +1184,8 @@ func (dm *YAMLParser) ComposeApiRecordsFromAllPackages(client *whisk.Config, man
  * 	}
  * }
  */
-func (dm *YAMLParser) ComposeApiRecords(client *whisk.Config, packageName string, pkg Package, manifestPath string) ([]*whisk.ApiCreateRequest, map[string]*whisk.ApiCreateRequestOptions, error) {
+func (dm *YAMLParser) ComposeApiRecords(client *whisk.Config, packageName string, pkg Package, manifestPath string,
+	actionrecords []utils.ActionRecord, sequencerecords []utils.ActionRecord) ([]*whisk.ApiCreateRequest, map[string]*whisk.ApiCreateRequestOptions, error) {
 	var requests = make([]*whisk.ApiCreateRequest, 0)
 
 	// supply a dummy API GW token as it is optional
@@ -1217,41 +1221,24 @@ func (dm *YAMLParser) ComposeApiRecords(client *whisk.Config, packageName string
 					gatewayRelPath = PATH_SEPARATOR + gatewayRelPath
 				}
 				for actionName, gatewayMethodResponse := range gatewayRelPathMap {
-					// verify that the action is defined under actions sections
+					// verify that the action is defined under action records
 					if _, ok := pkg.Actions[actionName]; ok {
-						// verify that the action is defined as web action
-						// web or web-export set to any of [true, yes, raw]
-						a := pkg.Actions[actionName]
-						if !webaction.IsWebAction(a.GetWeb()) {
-							warningString := wski18n.T(wski18n.ID_WARN_API_MISSING_WEB_ACTION_X_action_X_api_X,
-								map[string]interface{}{
-									wski18n.KEY_ACTION: actionName,
-									wski18n.KEY_API:    apiName})
-							wskprint.PrintOpenWhiskWarning(warningString)
-							if a.Annotations == nil {
-								a.Annotations = make(map[string]interface{}, 0)
-							}
-							a.Annotations[webaction.WEB_EXPORT_ANNOT] = true
-							pkg.Actions[actionName] = a
+						// verify that the action is defined as web action;
+						// web or web-export set to any of [true, yes, raw]; if not,
+						// we will try to add it (if no strict" flag) and warn user that we did so
+						if err := webaction.TryUpdateAPIsActionToWebAction(actionrecords, packageName,
+							apiName, actionName, false); err!=nil {
+							return requests, requestOptions, err
 						}
-						// verify that the sequence is defined under sequences sections
+						// verify that the sequence action is defined under sequence records
 					} else if _, ok := pkg.Sequences[actionName]; ok {
-						// verify that the sequence is defined as web sequence
-						// web set to any of [true, yes, raw]
-						a := pkg.Sequences[actionName]
-						if !webaction.IsWebSequence(a.Web) {
-							warningString := wski18n.T(wski18n.ID_WARN_API_MISSING_WEB_SEQUENCE_X_sequence_X_api_X,
-								map[string]interface{}{
-									wski18n.KEY_SEQUENCE: actionName,
-									wski18n.KEY_API:      apiName})
-							wskprint.PrintOpenWhiskWarning(warningString)
-							if a.Annotations == nil {
-								a.Annotations = make(map[string]interface{}, 0)
-							}
-							a.Annotations[webaction.WEB_EXPORT_ANNOT] = true
-							pkg.Sequences[actionName] = a
+						// verify that the sequence action is defined as web sequence
+						// web or web-export set to any of [true, yes, raw]; if not,
+						// we will try to add it (if no strict" flag) and warn user that we did so
+						if err := webaction.TryUpdateAPIsActionToWebAction(sequencerecords, packageName,
+							apiName, actionName, true); err!=nil {
+							return requests, requestOptions, err
 						}
-						// return failure since action or sequence are not defined in the manifest
 					} else {
 						return nil, nil, wskderrors.NewYAMLFileFormatError(manifestPath,
 							wski18n.T(wski18n.ID_ERR_API_MISSING_ACTION_OR_SEQUENCE_X_action_or_sequence_X_api_X,
@@ -1275,7 +1262,7 @@ func (dm *YAMLParser) ComposeApiRecords(client *whisk.Config, packageName string
 						gatewayMethodResponse.Response = utils.HTTP_FILE_EXTENSION
 					}
 
-					// Chekc if API verb is valid, it must be one of (GET, PUT, POST, DELETE)
+					// Check if API verb is valid, it must be one of (GET, PUT, POST, DELETE)
 					if _, ok := whisk.ApiVerbs[strings.ToUpper(gatewayMethodResponse.Method)]; !ok {
 						return nil, nil, wskderrors.NewInvalidAPIGatewayMethodError(manifestPath,
 							gatewayBasePath+gatewayRelPath,

diff --git a/parsers/manifest_parser_test.go b/parsers/manifest_parser_test.go
@@ -1576,7 +1576,7 @@ func TestComposeApiRecords(t *testing.T) {
 		ApigwAccessToken: "token",
 	}
 
-	apiList, apiRequestOptions, err := p.ComposeApiRecordsFromAllPackages(&config, m)
+	apiList, apiRequestOptions, err := p.ComposeApiRecordsFromAllPackages(&config, m, nil, nil)
 	if err != nil {
 		assert.Fail(t, "Failed to compose api records: "+err.Error())
 	}

diff --git a/specification/html/spec_sequences.md b/specification/html/spec_sequences.md
@@ -67,6 +67,12 @@ sequences:
   <sequence name>:
      <Entity schema>
      actions: <ordered list of action names>
+     web: <boolean> | yes | no | raw
+  annotations:
+    <map of annotation key-values>
+    web-export: <boolean> | yes | no | raw # optional
+    web-custom-options: <boolean> # optional, only valid when `web-export` enabled
+    require-whisk-auth: <boolean> | <string> | <positive integer> # optional, only valid when `web-export` enabled
   ...
 ```
 
@@ -75,6 +81,7 @@ sequences:
 sequences:
   newbot:
     actions: newbot-create, newbot-select-persona, newbot-greeting
+    web: true
 ```
 
 <!--

diff --git a/...s/src/integration/webaction/manifest_missing_web_annotation_on_api_action_or_sequence.yml b/...s/src/integration/webaction/manifest_missing_web_annotation_on_api_action_or_sequence.yml
@@ -0,0 +1,53 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# TODO automate this testcase
+# Test automatically adding web annotations on Actions that do not have them declared
+# when these actions are used to create APIs.
+# 1) Test an Action missing web-export, but with existing Annotations declared
+# 2) Test a Sequence missing web-export with NO existing Annotations (to verify we allocate)
+# 3) Test if web-action is set to false that we error out as we cannot override explicit value
+packages:
+    TestMissingWebAnnotation:
+        actions:
+            hello_validate:
+                function: src/hello_http_validate.js
+            hello:
+                function: src/hello.js
+                annotations:
+                    foo: bar
+            hello_wrap:
+                function: src/hello_http_wrap.js
+            helloWebFalse:
+                function: src/hello.js
+                web: false
+        sequences:
+            hellosequence:
+                actions: hello_validate, hello, hello_wrap
+        apis:
+            actionmissingweb:
+                helloworlds:
+                    helloweb:
+                        hello:
+                            method: GET
+                            response: http
+                        hellosequence:
+                            method: GET
+                            response: http
+                        helloWebFalse:
+                            method: GET
+                            response: http
diff --git a/tests/src/integration/webaction/manifest_require_whisk_auth_sequence_valid.yaml b/tests/src/integration/webaction/manifest_require_whisk_auth_sequence_valid.yaml
@@ -0,0 +1,41 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+packages:
+  hellosequencespkg:
+    version: 1.0
+    license: Apache-2.0
+    actions:
+      hello_validate:
+        function: src/hello_http_validate.js
+      hello:
+        function: src/hello.js
+      hello_wrap:
+        function: src/hello_http_wrap.js
+    sequences:
+      hellosequence:
+        actions: hello_validate, hello, hello_wrap
+        web: true
+        annotations:
+          require-whisk-auth: "mytoken"
+    apis:
+      web-secure-sequences:
+        helloworlds:
+          hellosequence:
+            hellosequence:
+              method: GET
+              response: http
diff --git a/tests/src/integration/webaction/src/hello.js b/tests/src/integration/webaction/src/hello.js
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/*
+ * Hello, world
+ */
+function main(params) {
+    msg = "Hello, " + params.name + " from " + params.place;
+    return { greeting:  msg };
+}
diff --git a/tests/src/integration/webaction/src/hello_http_validate.js b/tests/src/integration/webaction/src/hello_http_validate.js
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/*
+ * This action validates the parameters passed to the hello world action and
+ * returns an error if any of them is missing.
+ */
+function main(params) {
+    if(params.name && params.place) {
+        return params;
+    } else {
+        return {
+            error: {
+                body: {
+                    message: 'Attributes name and place are mandatory'
+                },
+                statusCode: 400,
+                headers: {'Content-Type': 'application/json'}
+            }
+        }
+    }
+}
diff --git a/tests/src/integration/webaction/src/hello_http_wrap.js b/tests/src/integration/webaction/src/hello_http_wrap.js
@@ -0,0 +1,28 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/*
+ * This action take a result from the previous action and wraps it into a
+ * HTTP response structure.
+ */
+function main(params) {
+    return {
+        body: params,
+        statusCode: 200,
+        headers: {'Content-Type': 'application/json'}
+    };
+}
diff --git a/utils/misc.go b/utils/misc.go
@@ -187,3 +187,14 @@ func getKeyValueFormattedJSON(data map[string]interface{}) whisk.KeyValueArr {
 	}
 	return keyValueArr
 }
+
+func GetActionFromActionRecords( records []ActionRecord, packageName string, actionName string ) *whisk.Action {
+	for _, record := range records {
+		if record.Packagename == packageName {
+			if record.Action.Name == actionName {
+				return record.Action
+			}
+		}
+	}
+	return nil
+}