You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by ty...@apache.org on 2020/08/25 19:45:51 UTC
[openwhisk] branch master updated: Execute Only for Shared Actions
(#4935)
This is an automated email from the ASF dual-hosted git repository.
tysonnorris pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/openwhisk.git
The following commit(s) were added to refs/heads/master by this push:
new e255126 Execute Only for Shared Actions (#4935)
e255126 is described below
commit e255126b87eabb508b57caef78eba2a130a2e4df
Author: bkemburu <66...@users.noreply.github.com>
AuthorDate: Tue Aug 25 15:45:39 2020 -0400
Execute Only for Shared Actions (#4935)
* Initial Commit of Execute Only
---
common/scala/src/main/resources/application.conf | 1 +
.../org/apache/openwhisk/core/WhiskConfig.scala | 1 +
.../org/apache/openwhisk/http/ErrorResponse.scala | 9 +++
.../apache/openwhisk/core/controller/Actions.scala | 72 +++++++++++++++++-----
.../openwhisk/core/controller/Packages.scala | 32 ++++++++--
.../controller/test/PackageActionsApiTests.scala | 34 +++++++++-
.../core/controller/test/PackagesApiTests.scala | 54 ++++++++++++++++
7 files changed, 180 insertions(+), 23 deletions(-)
diff --git a/common/scala/src/main/resources/application.conf b/common/scala/src/main/resources/application.conf
index aa9fa08..22a9b57 100644
--- a/common/scala/src/main/resources/application.conf
+++ b/common/scala/src/main/resources/application.conf
@@ -99,6 +99,7 @@ kamon {
}
whisk {
+ shared-packages-execute-only = false
metrics {
# Enable/disable Prometheus support. If enabled then metrics would be exposed at `/metrics` endpoint
# If Prometheus is enabled then please review `kamon.metric.tick-interval` (set to 1 sec by default above).
diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/WhiskConfig.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/WhiskConfig.scala
index 0dd1c16..7aea661 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/WhiskConfig.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/WhiskConfig.scala
@@ -264,6 +264,7 @@ object ConfigKeys {
val featureFlags = "whisk.feature-flags"
val whiskConfig = "whisk.config"
+ val sharedPackageExecuteOnly = s"whisk.shared-packages-execute-only"
val swaggerUi = "whisk.swagger-ui"
val disableStoreResult = s"$activation.disable-store-result"
diff --git a/common/scala/src/main/scala/org/apache/openwhisk/http/ErrorResponse.scala b/common/scala/src/main/scala/org/apache/openwhisk/http/ErrorResponse.scala
index 1198cdc..5424dd3 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/http/ErrorResponse.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/http/ErrorResponse.scala
@@ -229,6 +229,15 @@ object Messages {
/** Indicates that the container for the action could not be started. */
val resourceProvisionError = "Failed to provision resources to run the action."
+
+ def forbiddenGetActionBinding(entityDocId: String) =
+ s"GET not permitted for '$entityDocId'. Resource does not exist or is an action in a shared package binding."
+ def forbiddenGetAction(entityPath: String) =
+ s"GET not permitted for '$entityPath' since it's an action in a shared package"
+ def forbiddenGetPackageBinding(packageName: String) =
+ s"GET not permitted since $packageName is a binding of a shared package"
+ def forbiddenGetPackage(packageName: String) =
+ s"GET not permitted for '$packageName' since it's a shared package"
}
/** Replaces rejections with Json object containing cause and transaction id. */
diff --git a/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Actions.scala b/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Actions.scala
index 4fc5312..a6e4554 100644
--- a/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Actions.scala
+++ b/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Actions.scala
@@ -43,6 +43,8 @@ import org.apache.openwhisk.http.Messages._
import org.apache.openwhisk.core.entitlement.Resource
import org.apache.openwhisk.core.entitlement.Collection
import org.apache.openwhisk.core.loadBalancer.LoadBalancerException
+import pureconfig._
+import org.apache.openwhisk.core.ConfigKeys
/**
* A singleton object which defines the properties that must be present in a configuration
@@ -102,6 +104,10 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
/** Database service to get activations. */
protected val activationStore: ActivationStore
+ /** Config flag for Execute Only for Actions in Shared Packages */
+ protected def executeOnly =
+ loadConfigOrThrow[Boolean](ConfigKeys.sharedPackageExecuteOnly)
+
/** Entity normalizer to JSON object. */
import RestApiCommons.emptyEntityToJsObject
@@ -330,6 +336,50 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
deleteEntity(WhiskAction, entityStore, entityName.toDocId, (a: WhiskAction) => Future.successful({}))
}
+ /** Checks for package binding case. we don't want to allow get for a package binding in shared package */
+ private def fetchEntity(entityName: FullyQualifiedEntityName, env: Option[Parameters], code: Boolean)(
+ implicit transid: TransactionId) = {
+ val resolvedPkg: Future[Either[String, FullyQualifiedEntityName]] = if (entityName.path.defaultPackage) {
+ Future.successful(Right(entityName))
+ } else {
+ WhiskPackage.resolveBinding(entityStore, entityName.path.toDocId, mergeParameters = true).map { pkg =>
+ val originalPackageLocation = pkg.fullyQualifiedName(withVersion = false).namespace
+ if (executeOnly && originalPackageLocation != entityName.namespace) {
+ Left(forbiddenGetActionBinding(entityName.toDocId.asString))
+ } else {
+ Right(entityName)
+ }
+ }
+ }
+ onComplete(resolvedPkg) {
+ case Success(pkgFuture) =>
+ pkgFuture match {
+ case Left(f) => terminate(Forbidden, f)
+ case Right(_) =>
+ if (code) {
+ getEntity(WhiskAction.resolveActionAndMergeParameters(entityStore, entityName), Some {
+ action: WhiskAction =>
+ val mergedAction = env map {
+ action inherit _
+ } getOrElse action
+ complete(OK, mergedAction)
+ })
+ } else {
+ getEntity(WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, entityName), Some {
+ action: WhiskActionMetaData =>
+ val mergedAction = env map {
+ action inherit _
+ } getOrElse action
+ complete(OK, mergedAction)
+ })
+ }
+ }
+ case Failure(t: Throwable) =>
+ logging.error(this, s"[GET] package ${entityName.path.toDocId} failed: ${t.getMessage}")
+ terminate(InternalServerError)
+ }
+ }
+
/**
* Gets action. The action name is prefixed with the namespace to create the primary index key.
*
@@ -341,22 +391,12 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
override def fetch(user: Identity, entityName: FullyQualifiedEntityName, env: Option[Parameters])(
implicit transid: TransactionId) = {
parameter('code ? true) { code =>
- code match {
- case true =>
- getEntity(WhiskAction.resolveActionAndMergeParameters(entityStore, entityName), Some { action: WhiskAction =>
- val mergedAction = env map {
- action inherit _
- } getOrElse action
- complete(OK, mergedAction)
- })
- case false =>
- getEntity(WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, entityName), Some {
- action: WhiskActionMetaData =>
- val mergedAction = env map {
- action inherit _
- } getOrElse action
- complete(OK, mergedAction)
- })
+ //check if execute only is enabled, and if there is a discrepancy between the current user's namespace
+ //and that of the entity we are trying to fetch
+ if (executeOnly && user.namespace.name != entityName.namespace) {
+ terminate(Forbidden, forbiddenGetAction(entityName.path.asString))
+ } else {
+ fetchEntity(entityName, env, code)
}
}
}
diff --git a/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Packages.scala b/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Packages.scala
index b1e555b..ffc992f 100644
--- a/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Packages.scala
+++ b/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Packages.scala
@@ -19,12 +19,10 @@ package org.apache.openwhisk.core.controller
import scala.concurrent.Future
import scala.util.{Failure, Success}
-
import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._
import akka.http.scaladsl.model.StatusCodes._
import akka.http.scaladsl.server.{RequestContext, RouteResult}
import akka.http.scaladsl.unmarshalling.Unmarshaller
-
import org.apache.openwhisk.common.TransactionId
import org.apache.openwhisk.core.controller.RestApiCommons.{ListLimit, ListSkip}
import org.apache.openwhisk.core.database.{CacheChangeNotification, DocumentTypeMismatchException, NoDocumentException}
@@ -33,6 +31,9 @@ import org.apache.openwhisk.core.entity._
import org.apache.openwhisk.core.entity.types.EntityStore
import org.apache.openwhisk.http.ErrorResponse.terminate
import org.apache.openwhisk.http.Messages
+import org.apache.openwhisk.http.Messages._
+import pureconfig._
+import org.apache.openwhisk.core.ConfigKeys
trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
services: WhiskServices =>
@@ -42,6 +43,10 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
/** Database service to CRUD packages. */
protected val entityStore: EntityStore
+ /** Config flag for Execute Only for Shared Packages */
+ protected def executeOnly =
+ loadConfigOrThrow[Boolean](ConfigKeys.sharedPackageExecuteOnly)
+
/** Notification service for cache invalidation. */
protected implicit val cacheChangeNotification: Some[CacheChangeNotification]
@@ -157,7 +162,14 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
*/
override def fetch(user: Identity, entityName: FullyQualifiedEntityName, env: Option[Parameters])(
implicit transid: TransactionId) = {
- getEntity(WhiskPackage.get(entityStore, entityName.toDocId), Some { mergePackageWithBinding() _ })
+ if (executeOnly && user.namespace.name != entityName.namespace) {
+ val value = entityName.toString
+ terminate(Forbidden, forbiddenGetPackage(entityName.asString))
+ } else {
+ getEntity(WhiskPackage.get(entityStore, entityName.toDocId), Some {
+ mergePackageWithBinding() _
+ })
+ }
}
/**
@@ -303,9 +315,17 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
logging.error(this, s"unexpected package binding refers to itself: $docid")
terminate(UnprocessableEntity, Messages.packageBindingCircularReference(b.fullyQualifiedName.toString))
} else {
- getEntity(WhiskPackage.get(entityStore, docid), Some {
- mergePackageWithBinding(Some { wp }) _
- })
+
+ /** Here's where I check package execute only case with package binding. */
+ if (executeOnly && wp.namespace.asString != b.namespace.asString) {
+ terminate(Forbidden, forbiddenGetPackageBinding(wp.name.asString))
+ } else {
+ getEntity(WhiskPackage.get(entityStore, docid), Some {
+ mergePackageWithBinding(Some {
+ wp
+ }) _
+ })
+ }
}
} getOrElse {
val pkg = ref map { _ inherit wp.parameters } getOrElse wp
diff --git a/tests/src/test/scala/org/apache/openwhisk/core/controller/test/PackageActionsApiTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/controller/test/PackageActionsApiTests.scala
index a645e87..71d821c 100644
--- a/tests/src/test/scala/org/apache/openwhisk/core/controller/test/PackageActionsApiTests.scala
+++ b/tests/src/test/scala/org/apache/openwhisk/core/controller/test/PackageActionsApiTests.scala
@@ -187,7 +187,6 @@ class PackageActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
status should be(BadRequest)
}
}
-
it should "reject put action in package binding" in {
implicit val tid = transid()
val provider = WhiskPackage(namespace, aname(), None, publish = true)
@@ -636,4 +635,37 @@ class PackageActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
responseAs[ErrorResponse].error shouldBe Messages.corruptedEntity
}
}
+
+ var testExecuteOnly = false
+ override def executeOnly = testExecuteOnly
+
+ it should ("allow access to get of action in binding of shared package when config option is disabled") in {
+ testExecuteOnly = false
+ implicit val tid = transid()
+ val auser = WhiskAuthHelpers.newIdentity()
+ val provider = WhiskPackage(namespace, aname(), None, Parameters("p", "P"), publish = true)
+ val binding = WhiskPackage(EntityPath(auser.subject.asString), aname(), provider.bind, Parameters("b", "B"))
+ val action = WhiskAction(provider.fullPath, aname(), jsDefault("??"), Parameters("a", "A"))
+ put(entityStore, provider)
+ put(entityStore, binding)
+ put(entityStore, action)
+ Get(s"$collectionPath/${binding.name}/${action.name}") ~> Route.seal(routes(auser)) ~> check {
+ status should be(OK)
+ }
+ }
+
+ it should ("deny access to get of action in binding of shared package when config option is enabled") in {
+ testExecuteOnly = true
+ implicit val tid = transid()
+ val auser = WhiskAuthHelpers.newIdentity()
+ val provider = WhiskPackage(namespace, aname(), None, Parameters("p", "P"), publish = true)
+ val binding = WhiskPackage(EntityPath(auser.subject.asString), aname(), provider.bind, Parameters("b", "B"))
+ val action = WhiskAction(provider.fullPath, aname(), jsDefault("??"), Parameters("a", "A"))
+ put(entityStore, provider)
+ put(entityStore, binding)
+ put(entityStore, action)
+ Get(s"$collectionPath/${binding.name}/${action.name}") ~> Route.seal(routes(auser)) ~> check {
+ status should be(Forbidden)
+ }
+ }
}
diff --git a/tests/src/test/scala/org/apache/openwhisk/core/controller/test/PackagesApiTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/controller/test/PackagesApiTests.scala
index e59121f..c52ba07 100644
--- a/tests/src/test/scala/org/apache/openwhisk/core/controller/test/PackagesApiTests.scala
+++ b/tests/src/test/scala/org/apache/openwhisk/core/controller/test/PackagesApiTests.scala
@@ -884,4 +884,58 @@ class PackagesApiTests extends ControllerTestCommon with WhiskPackagesApi {
responseAs[ErrorResponse].error shouldBe Messages.corruptedEntity
}
}
+
+ var testExecuteOnly = false
+ override def executeOnly = testExecuteOnly
+
+ it should ("allow access to get of shared package binding when config option is disabled") in {
+ testExecuteOnly = false
+ implicit val tid = transid()
+ val auser = WhiskAuthHelpers.newIdentity()
+ val provider = WhiskPackage(namespace, aname(), None, Parameters("p", "P"), publish = true)
+ val binding = WhiskPackage(EntityPath(auser.subject.asString), aname(), provider.bind, Parameters("b", "B"))
+ put(entityStore, provider)
+ put(entityStore, binding)
+ Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
+ status should be(OK)
+ }
+ }
+
+ it should ("allow access to get of shared package when config option is disabled") in {
+ testExecuteOnly = false
+ implicit val tid = transid()
+ val auser = WhiskAuthHelpers.newIdentity()
+ val provider = WhiskPackage(namespace, aname(), None, publish = true)
+ put(entityStore, provider)
+
+ Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
+ status should be(OK)
+ }
+ }
+
+ it should ("deny access to get of shared package binding when config option is enabled") in {
+ testExecuteOnly = true
+ implicit val tid = transid()
+ val auser = WhiskAuthHelpers.newIdentity()
+ val provider = WhiskPackage(namespace, aname(), None, Parameters("p", "P"), publish = true)
+ val binding = WhiskPackage(EntityPath(auser.subject.asString), aname(), provider.bind, Parameters("b", "B"))
+ put(entityStore, provider)
+ put(entityStore, binding)
+ Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
+ status should be(Forbidden)
+ }
+
+ }
+
+ it should ("deny access to get of shared package when config option is enabled") in {
+ testExecuteOnly = true
+ implicit val tid = transid()
+ val auser = WhiskAuthHelpers.newIdentity()
+ val provider = WhiskPackage(namespace, aname(), None, publish = true)
+ put(entityStore, provider)
+
+ Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
+ status should be(Forbidden)
+ }
+ }
}