You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by cs...@apache.org on 2018/02/21 02:52:09 UTC
[incubator-openwhisk] branch master updated: Attempt to resolve
action name from conductor to a fully qualified name for added convenience.
(#3298)
This is an automated email from the ASF dual-hosted git repository.
csantanapr 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 18bb1b1 Attempt to resolve action name from conductor to a fully qualified name for added convenience. (#3298)
18bb1b1 is described below
commit 18bb1b1c3747ce7ce30117a9341dd31b8759a7c8
Author: rodric rabbah <ro...@gmail.com>
AuthorDate: Tue Feb 20 18:52:06 2018 -0800
Attempt to resolve action name from conductor to a fully qualified name for added convenience. (#3298)
---
.../core/entity/FullyQualifiedEntityName.scala | 32 +++++++++
.../src/main/scala/whisk/http/ErrorResponse.scala | 4 +-
.../core/controller/actions/PrimitiveActions.scala | 79 ++++++++++++----------
docs/conductors.md | 1 +
.../scala/system/basic/WskBasicNode6Tests.scala | 2 +-
.../scala/system/basic/WskConductorTests.scala | 4 +-
.../core/controller/test/ConductorsApiTests.scala | 6 +-
.../scala/whisk/core/entity/test/SchemaTests.scala | 25 +++++++
8 files changed, 111 insertions(+), 42 deletions(-)
diff --git a/common/scala/src/main/scala/whisk/core/entity/FullyQualifiedEntityName.scala b/common/scala/src/main/scala/whisk/core/entity/FullyQualifiedEntityName.scala
index c52af75..0abca40 100644
--- a/common/scala/src/main/scala/whisk/core/entity/FullyQualifiedEntityName.scala
+++ b/common/scala/src/main/scala/whisk/core/entity/FullyQualifiedEntityName.scala
@@ -96,4 +96,36 @@ protected[core] object FullyQualifiedEntityName extends DefaultJsonProtocol {
case Failure(t) => deserializationError("fully qualified name malformed")
}
}
+
+ /**
+ * Converts the name to a fully qualified name.
+ * There are 3 cases:
+ * - name is not a valid EntityPath => error
+ * - name is a valid single segment with a leading slash => error
+ * - name is a valid single segment without a leading slash => map it to user namespace, default package
+ * - name is a valid multi segment with a leading slash => treat it as fully qualified name (max segments allowed: 3)
+ * - name is a valid multi segment without a leading slash => treat it as package name and resolve it to the user namespace (max segments allowed: 3)
+ *
+ * The last case is ambiguous as '/namespace/action' and 'package/action' will be the same EntityPath value.
+ * The action should use a fully qualified result to avoid the ambiguity.
+ *
+ * @param name name of the action to fully qualify
+ * @param namespace the user namespace for the simple resolution
+ * @return Some(FullyQualifiedName) if the name is valid otherwise None
+ */
+ protected[core] def resolveName(name: JsValue, namespace: EntityName): Option[FullyQualifiedEntityName] = {
+ name match {
+ case v @ JsString(s) =>
+ Try(v.convertTo[EntityPath]).toOption
+ .flatMap { path =>
+ val n = path.segments
+ val leadingSlash = s.startsWith(EntityPath.PATHSEP)
+ if (n < 1 || n > 3 || (leadingSlash && n == 1) || (!leadingSlash && n > 3)) None
+ else if (leadingSlash || n == 3) Some(path)
+ else Some(namespace.toPath.addPath(path))
+ }
+ .map(_.toFullyQualifiedEntityName)
+ case _ => None
+ }
+ }
}
diff --git a/common/scala/src/main/scala/whisk/http/ErrorResponse.scala b/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
index e599a1f..0f466d3 100644
--- a/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
+++ b/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
@@ -129,9 +129,9 @@ object Messages {
def compositionComponentInvalid(value: JsValue) =
s"Failed to parse action name from json value $value during composition."
def compositionComponentNotFound(name: String) =
- s"""Failed to resolve action with name "$name" during composition."""
+ s"Failed to resolve action with name '$name' during composition."
def compositionComponentNotAccessible(name: String) =
- s"""Failed entitlement check for action with name "$name" during composition."""
+ s"Failed entitlement check for action with name '$name' during composition."
/** Error messages for bad requests where parameters do not conform. */
val parametersNotAllowed = "Request defines parameters that are not allowed (e.g., reserved properties)."
diff --git a/core/controller/src/main/scala/whisk/core/controller/actions/PrimitiveActions.scala b/core/controller/src/main/scala/whisk/core/controller/actions/PrimitiveActions.scala
index 2006a6b..24817f3 100644
--- a/core/controller/src/main/scala/whisk/core/controller/actions/PrimitiveActions.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/actions/PrimitiveActions.scala
@@ -28,7 +28,6 @@ import scala.concurrent.duration._
import scala.language.postfixOps
import scala.util.Failure
import scala.util.Success
-import scala.util.Try
import akka.actor.Actor
import akka.actor.ActorRef
import akka.actor.ActorSystem
@@ -355,41 +354,15 @@ protected[actions] trait PrimitiveActions {
// no next action, end composition execution, return to caller
Future.successful(ActivationResponse(activation.response.statusCode, Some(params.getOrElse(result))))
case Some(next) =>
- Try(next.convertTo[EntityPath]) match {
- case Failure(t) =>
- // parsing failure
- Future.successful(ActivationResponse.applicationError(compositionComponentInvalid(next)))
- case Success(_) if session.accounting.components >= actionSequenceLimit =>
- // composition is too long
+ FullyQualifiedEntityName.resolveName(next, user.namespace) match {
+ case Some(fqn) if session.accounting.components < actionSequenceLimit =>
+ tryInvokeNext(user, fqn, params, session)
+
+ case Some(_) => // composition is too long
Future.successful(ActivationResponse.applicationError(compositionIsTooLong))
- case Success(next) =>
- // resolve and invoke next action
- val fqn = (if (next.defaultPackage) EntityPath.DEFAULT.addPath(next) else next)
- .resolveNamespace(user.namespace)
- .toFullyQualifiedEntityName
- val resource = Resource(fqn.path, Collection(Collection.ACTIONS), Some(fqn.name.asString))
- entitlementProvider
- .check(user, Privilege.ACTIVATE, Set(resource), noThrottle = true)
- .flatMap { _ =>
- // successful entitlement check
- WhiskActionMetaData
- .resolveActionAndMergeParameters(entityStore, fqn)
- .flatMap {
- case next =>
- // successful resolution
- invokeComponent(user, action = next, payload = params, session)
- }
- .recover {
- case _ =>
- // resolution failure
- ActivationResponse.applicationError(compositionComponentNotFound(next.asString))
- }
- }
- .recover {
- case _ =>
- // failed entitlement check
- ActivationResponse.applicationError(compositionComponentNotAccessible(next.asString))
- }
+
+ case None => // parsing failure
+ Future.successful(ActivationResponse.applicationError(compositionComponentInvalid(next)))
}
}
}
@@ -398,6 +371,42 @@ protected[actions] trait PrimitiveActions {
}
/**
+ * Checks if the user is entitled to invoke the next action and invokes it.
+ *
+ * @param user the subject
+ * @param fqn the name of the action
+ * @param params parameters for the action
+ * @param session the session for the current activation
+ * @return promise for the eventual activation
+ */
+ private def tryInvokeNext(user: Identity, fqn: FullyQualifiedEntityName, params: Option[JsObject], session: Session)(
+ implicit transid: TransactionId): Future[ActivationResponse] = {
+ val resource = Resource(fqn.path, Collection(Collection.ACTIONS), Some(fqn.name.asString))
+ entitlementProvider
+ .check(user, Privilege.ACTIVATE, Set(resource), noThrottle = true)
+ .flatMap { _ =>
+ // successful entitlement check
+ WhiskActionMetaData
+ .resolveActionAndMergeParameters(entityStore, fqn)
+ .flatMap {
+ case next =>
+ // successful resolution
+ invokeComponent(user, action = next, payload = params, session)
+ }
+ .recover {
+ case _ =>
+ // resolution failure
+ ActivationResponse.applicationError(compositionComponentNotFound(fqn.asString))
+ }
+ }
+ .recover {
+ case _ =>
+ // failed entitlement check
+ ActivationResponse.applicationError(compositionComponentNotAccessible(fqn.asString))
+ }
+ }
+
+ /**
* A method that knows how to handle a component invocation.
*
* This method distinguishes primitive actions, sequences, and compositions.
diff --git a/docs/conductors.md b/docs/conductors.md
index 8ab888f..895efa7 100644
--- a/docs/conductors.md
+++ b/docs/conductors.md
@@ -235,6 +235,7 @@ If the _action_ field is defined in the output of the conductor action, the runt
- internal error (invocation failure or timeout).
In any of the first three failure scenarios, the conductor action invocation ends with an _application error_ status code and an error message describing the reason for the failure. In the latter, the status code is _internal error_.
+The action name should be a fully qualified name, which is of the form `/namespace/package-name/action-name` or `/namespace/action-name`. Failure to specify a fully qualified name may result in ambiguity or even a parsing error that terminates the activation.
If there is no error, _action_ is invoked on the _params_ dictionary if specified (auto boxed if necessary) or if not on the empty dictionary.
diff --git a/tests/src/test/scala/system/basic/WskBasicNode6Tests.scala b/tests/src/test/scala/system/basic/WskBasicNode6Tests.scala
index a97636f..f4a04e3 100644
--- a/tests/src/test/scala/system/basic/WskBasicNode6Tests.scala
+++ b/tests/src/test/scala/system/basic/WskBasicNode6Tests.scala
@@ -37,7 +37,7 @@ abstract class WskBasicNode6Tests extends TestHelpers with WskTestHelpers with J
val defaultAction = Some(TestUtils.getTestActionFilename("hello.js"))
lazy val currentNodeJsKind = "nodejs:6"
- behavior of "Runtime $currentNodeJsKind"
+ behavior of s"Runtime $currentNodeJsKind"
it should "Ensure that NodeJS actions can have a non-default entrypoint" in withAssetCleaner(wskprops) {
(wp, assetHelper) =>
diff --git a/tests/src/test/scala/system/basic/WskConductorTests.scala b/tests/src/test/scala/system/basic/WskConductorTests.scala
index 2ff00ff..7115104 100644
--- a/tests/src/test/scala/system/basic/WskConductorTests.scala
+++ b/tests/src/test/scala/system/basic/WskConductorTests.scala
@@ -121,10 +121,12 @@ abstract class WskConductorTests extends TestHelpers with WskTestHelpers with Js
// an undefined action
val undefinedrun = wsk.action.invoke(echo, Map("payload" -> testString.toJson, "action" -> missing.toJson))
+ val namespace = wsk.namespace.whois()
+
withActivation(wsk.activation, undefinedrun) { activation =>
activation.response.status shouldBe "application error"
activation.response.result.get.fields.get("error") shouldBe Some(
- JsString(compositionComponentNotFound(missing)))
+ JsString(compositionComponentNotFound(s"$namespace/$missing")))
checkConductorLogsAndAnnotations(activation, 1) // echo
}
}
diff --git a/tests/src/test/scala/whisk/core/controller/test/ConductorsApiTests.scala b/tests/src/test/scala/whisk/core/controller/test/ConductorsApiTests.scala
index f08d700..bacd912 100644
--- a/tests/src/test/scala/whisk/core/controller/test/ConductorsApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/ConductorsApiTests.scala
@@ -127,7 +127,7 @@ class ConductorsApiTests extends ControllerTestCommon with WhiskActionsApi {
val response = responseAs[JsObject]
response.fields("response").asJsObject.fields("status") shouldBe "application error".toJson
response.fields("response").asJsObject.fields("result") shouldBe JsObject(
- "error" -> compositionComponentNotFound(missing.toString).toJson)
+ "error" -> compositionComponentNotFound(s"$namespace/$missing").toJson)
response.fields("logs").convertTo[JsArray].elements.size shouldBe 1
}
}
@@ -137,7 +137,7 @@ class ConductorsApiTests extends ControllerTestCommon with WhiskActionsApi {
put(entityStore, WhiskAction(namespace, conductor, jsDefault("??"), annotations = Parameters("conductor", "true")))
put(entityStore, WhiskAction(namespace, step, jsDefault("??")))
put(entityStore, WhiskAction(alternateNamespace, step, jsDefault("??"))) // forbidden action
- val forbidden = s"$alternateNamespace/$step" // forbidden action name
+ val forbidden = s"/$alternateNamespace/$step" // forbidden action name
// dynamically invoke step action
Post(
@@ -179,7 +179,7 @@ class ConductorsApiTests extends ControllerTestCommon with WhiskActionsApi {
val response = responseAs[JsObject]
response.fields("response").asJsObject.fields("status") shouldBe "application error".toJson
response.fields("response").asJsObject.fields("result") shouldBe JsObject(
- "error" -> compositionComponentNotAccessible(forbidden).toJson)
+ "error" -> compositionComponentNotAccessible(forbidden.drop(1)).toJson)
response.fields("logs").convertTo[JsArray].elements.size shouldBe 1
}
diff --git a/tests/src/test/scala/whisk/core/entity/test/SchemaTests.scala b/tests/src/test/scala/whisk/core/entity/test/SchemaTests.scala
index 18bea38..b7bfb14 100644
--- a/tests/src/test/scala/whisk/core/entity/test/SchemaTests.scala
+++ b/tests/src/test/scala/whisk/core/entity/test/SchemaTests.scala
@@ -322,6 +322,31 @@ class SchemaTests extends FlatSpec with BeforeAndAfter with ExecHelpers with Mat
a[DeserializationException] should be thrownBy FullyQualifiedEntityName.serdesAsDocId.read(names(6))
}
+ it should "resolve names that may or may not be fully qualified" in {
+ FullyQualifiedEntityName.resolveName(JsString("a"), EntityName("ns")) shouldBe Some(
+ EntityPath("ns/a").toFullyQualifiedEntityName)
+
+ FullyQualifiedEntityName.resolveName(JsString("a/b"), EntityName("ns")) shouldBe Some(
+ EntityPath("ns/a/b").toFullyQualifiedEntityName)
+
+ FullyQualifiedEntityName.resolveName(JsString("a/b/c"), EntityName("ns")) shouldBe Some(
+ EntityPath("/a/b/c").toFullyQualifiedEntityName)
+
+ FullyQualifiedEntityName.resolveName(JsString("a/b/c/d"), EntityName("ns")) shouldBe None
+
+ FullyQualifiedEntityName.resolveName(JsString("/a"), EntityName("ns")) shouldBe None
+
+ FullyQualifiedEntityName.resolveName(JsString("/a/b"), EntityName("ns")) shouldBe Some(
+ EntityPath("/a/b").toFullyQualifiedEntityName)
+
+ FullyQualifiedEntityName.resolveName(JsString("/a/b/c"), EntityName("ns")) shouldBe Some(
+ EntityPath("/a/b/c").toFullyQualifiedEntityName)
+
+ FullyQualifiedEntityName.resolveName(JsString("/a/b/c/d"), EntityName("ns")) shouldBe None
+
+ FullyQualifiedEntityName.resolveName(JsString(""), EntityName("ns")) shouldBe None
+ }
+
behavior of "Binding"
it should "desiarilize legacy format" in {
--
To stop receiving notification emails like this one, please contact
csantanapr@apache.org.