Skip to content

Commit 38880d9

Browse files
bkemburuBhavesh Kemburu
bkemburu
authored and
Bhavesh Kemburu
committed
Update application.conf
Changed Flag to shared-packages-execute-only with default value set to false.
1 parent 989ad61 commit 38880d9

File tree

7 files changed

+130
-118
lines changed

7 files changed

+130
-118
lines changed

‎common/scala/src/main/resources/application.conf

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ kamon {
9999
}
100100

101101
whisk {
102-
#packages-execute-only = true
102+
shared-packages-execute-only = false
103103
metrics {
104104
# Enable/disable Prometheus support. If enabled then metrics would be exposed at `/metrics` endpoint
105105
# If Prometheus is enabled then please review `kamon.metric.tick-interval` (set to 1 sec by default above).

‎common/scala/src/main/scala/org/apache/openwhisk/core/WhiskConfig.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,4 +273,4 @@ object ConfigKeys {
273273
val apacheClientConfig = "whisk.apache-client"
274274

275275
val parameterStorage = "whisk.parameter-storage"
276-
}
276+
}

‎common/scala/src/main/scala/org/apache/openwhisk/http/ErrorResponse.scala

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,15 @@ object Messages {
229229

230230
/** Indicates that the container for the action could not be started. */
231231
val resourceProvisionError = "Failed to provision resources to run the action."
232+
233+
def forbiddenGetActionBinding(entityDocId: String) =
234+
s"GET not permitted for '$entityDocId'. Resource does not exist or an action in a shared package binding."
235+
def forbiddenGetAction(entityPath: String) =
236+
s"GET not permitted for '$entityPath' since it's an action in a shared package"
237+
def forbiddenGetPackageBinding(packageName: String) =
238+
s"GET not permitted since $packageName is a binding of a shared package"
239+
def forbiddenGetPackage(packageName: String) =
240+
s"GET not permitted for '$packageName' since it's a shared package"
232241
}
233242

234243
/** Replaces rejections with Json object containing cause and transaction id. */

‎core/controller/src/main/scala/org/apache/openwhisk/core/controller/Actions.scala

Lines changed: 50 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import akka.http.scaladsl.model.StatusCodes._
2626
import akka.http.scaladsl.server.RequestContext
2727
import akka.http.scaladsl.server.RouteResult
2828
import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._
29-
import akka.http.scaladsl.model.StatusCode
3029
import akka.http.scaladsl.unmarshalling._
3130
import spray.json._
3231
import spray.json.DefaultJsonProtocol._
@@ -46,6 +45,7 @@ import org.apache.openwhisk.core.entitlement.Collection
4645
import org.apache.openwhisk.core.loadBalancer.LoadBalancerException
4746
import pureconfig._
4847
import org.apache.openwhisk.core.ConfigKeys
48+
4949
/**
5050
* A singleton object which defines the properties that must be present in a configuration
5151
* in order to implement the actions API.
@@ -105,9 +105,10 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
105105
protected val activationStore: ActivationStore
106106

107107
/** Config flag for Execute Only for Actions in Shared Packages */
108-
protected def executeOnly = Try({
109-
loadConfigOrThrow[Boolean](ConfigKeys.sharedPackageExecuteOnly)
110-
}).getOrElse(false)
108+
protected def executeOnly =
109+
Try({
110+
loadConfigOrThrow[Boolean](ConfigKeys.sharedPackageExecuteOnly)
111+
}).getOrElse(false)
111112

112113
/** Entity normalizer to JSON object. */
113114
import RestApiCommons.emptyEntityToJsObject
@@ -337,8 +338,6 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
337338
deleteEntity(WhiskAction, entityStore, entityName.toDocId, (a: WhiskAction) => Future.successful({}))
338339
}
339340

340-
341-
342341
/**
343342
* Gets action. The action name is prefixed with the namespace to create the primary index key.
344343
*
@@ -352,56 +351,60 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
352351
parameter('code ? true) { code =>
353352
//check if execute only is enabled, and if there is a discrepancy between the current user's namespace
354353
//and that of the entity we are trying to fetch
355-
if (executeOnly && user.namespace.name.toString != entityName.namespace.toString) {
356-
val value = entityName.path
357-
terminate(StatusCode.int2StatusCode(403), s"GET not permitted for '$value' since it's an action in a shared package")
354+
355+
if (executeOnly && user.namespace.name != entityName.namespace) {
356+
terminate(Forbidden, forbiddenGetAction(entityName.path.asString))
358357
} else {
359-
code match {
360-
case true =>
361-
//Resolve Binding(Package) of the action
362-
getEntity(
363-
WhiskPackage.resolveBinding(entityStore, entityName.path.toDocId, mergeParameters = true),
364-
Some {pkg: WhiskPackage =>
365-
val originalPackageLocation = pkg.fullyQualifiedName(withVersion = false).namespace
366-
if (executeOnly && originalPackageLocation != entityName.namespace){
367-
terminate(StatusCode.int2StatusCode(403),
368-
s"GET not permitted for '${entityName.toDocId}'. Resource does not exist or an action in a shared package binding")
369-
}else{
370-
getEntity(WhiskAction.resolveActionAndMergeParameters(entityStore, entityName), Some {
371-
action: WhiskAction =>
372-
val mergedAction = env map {
373-
action inherit _
374-
} getOrElse action
375-
complete(OK, mergedAction)
376-
})
377-
}
358+
val getEntityWhiskActionCase =
359+
getEntity(WhiskAction.resolveActionAndMergeParameters(entityStore, entityName), Some { action: WhiskAction =>
360+
val mergedAction = env map {
361+
action inherit _
362+
} getOrElse action
363+
complete(OK, mergedAction)
364+
})
365+
val getEntityWhiskMetaDataCase =
366+
getEntity(WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, entityName), Some {
367+
action: WhiskActionMetaData =>
368+
val mergedAction = env map {
369+
action inherit _
370+
} getOrElse action
371+
complete(OK, mergedAction)
372+
})
373+
if (code) {
374+
if (entityName.path.defaultPackage) {
375+
getEntityWhiskActionCase
376+
} else {
377+
getEntity(
378+
WhiskPackage.resolveBinding(entityStore, entityName.path.toDocId, mergeParameters = true),
379+
Some { pkg: WhiskPackage =>
380+
val originalPackageLocation = pkg.fullyQualifiedName(withVersion = false).namespace
381+
if (executeOnly && originalPackageLocation != entityName.namespace) {
382+
terminate(Forbidden, forbiddenGetActionBinding(entityName.toDocId.asString))
383+
} else {
384+
getEntityWhiskActionCase
378385
}
379-
)
380-
case false =>
386+
})
387+
}
388+
} else {
389+
if (entityName.path.defaultPackage) {
390+
getEntityWhiskMetaDataCase
391+
} else {
381392
getEntity(
382393
WhiskPackage.resolveBinding(entityStore, entityName.path.toDocId, mergeParameters = true),
383-
Some {pkg: WhiskPackage =>
394+
Some { pkg: WhiskPackage =>
384395
val originalPackageLocation = pkg.fullyQualifiedName(withVersion = false).namespace
385-
if (executeOnly && originalPackageLocation != entityName.namespace){
386-
terminate(StatusCode.int2StatusCode(403),
387-
s"GET not permitted for '${entityName.toDocId}'. Resource does not exist or an action in a shared package binding")
388-
}else{
389-
getEntity(WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, entityName), Some {
390-
action: WhiskActionMetaData =>
391-
val mergedAction = env map {
392-
action inherit _
393-
} getOrElse action
394-
complete(OK, mergedAction)
395-
})
396+
if (executeOnly && originalPackageLocation != entityName.namespace) {
397+
terminate(Forbidden, forbiddenGetActionBinding(entityName.toDocId.asString))
398+
} else {
399+
getEntityWhiskMetaDataCase
396400
}
397-
}
398-
)
399-
}
400-
}
401+
})
402+
}
403+
}
404+
}
401405
}
402406
}
403407

404-
405408
/**
406409
* Gets all actions in a path.
407410
*

‎core/controller/src/main/scala/org/apache/openwhisk/core/controller/Packages.scala

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ package org.apache.openwhisk.core.controller
2020
import scala.concurrent.Future
2121
import scala.util.{Failure, Success, Try}
2222
import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._
23-
import akka.http.scaladsl.model.StatusCode
2423
import akka.http.scaladsl.model.StatusCodes._
2524
import akka.http.scaladsl.server.{RequestContext, RouteResult}
2625
import akka.http.scaladsl.unmarshalling.Unmarshaller
@@ -32,6 +31,7 @@ import org.apache.openwhisk.core.entity._
3231
import org.apache.openwhisk.core.entity.types.EntityStore
3332
import org.apache.openwhisk.http.ErrorResponse.terminate
3433
import org.apache.openwhisk.http.Messages
34+
import org.apache.openwhisk.http.Messages._
3535
import pureconfig._
3636
import org.apache.openwhisk.core.ConfigKeys
3737

@@ -44,10 +44,10 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
4444
protected val entityStore: EntityStore
4545

4646
/** Config flag for Execute Only for Shared Packages */
47-
48-
protected def executeOnly = Try({
49-
loadConfigOrThrow[Boolean](ConfigKeys.sharedPackageExecuteOnly)
50-
}).getOrElse(false)
47+
protected def executeOnly =
48+
Try({
49+
loadConfigOrThrow[Boolean](ConfigKeys.sharedPackageExecuteOnly)
50+
}).getOrElse(false)
5151

5252
/** Notification service for cache invalidation. */
5353
protected implicit val cacheChangeNotification: Some[CacheChangeNotification]
@@ -153,7 +153,6 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
153153
})
154154
}
155155

156-
157156
/**
158157
* Gets package/binding.
159158
* The package/binding name is prefixed with the namespace to create the primary index key.
@@ -167,10 +166,10 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
167166
//within the mergePackageWithBinding() method
168167
override def fetch(user: Identity, entityName: FullyQualifiedEntityName, env: Option[Parameters])(
169168
implicit transid: TransactionId) = {
170-
if (executeOnly && user.namespace.name.toString != entityName.namespace.toString) {
171-
val value = entityName.toString
172-
terminate(StatusCode.int2StatusCode(403), s"GET not permitted for '$value' since it's a shared package")
173-
}else {
169+
if (executeOnly && user.namespace.name != entityName.namespace) {
170+
val value = entityName.toString
171+
terminate(Forbidden, forbiddenGetPackage(entityName.asString))
172+
} else {
174173
getEntity(WhiskPackage.get(entityStore, entityName.toDocId), Some {
175174
mergePackageWithBinding() _
176175
})
@@ -320,13 +319,14 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
320319
logging.error(this, s"unexpected package binding refers to itself: $docid")
321320
terminate(UnprocessableEntity, Messages.packageBindingCircularReference(b.fullyQualifiedName.toString))
322321
} else {
322+
323323
/** Here's where I check package execute only case with package binding. */
324-
val packagePath = wp.namespace.toString()
325-
val bindingPath = wp.binding.iterator.next().toString()
324+
val packagePath = wp.namespace.asString
325+
val bindingPath = wp.binding.iterator.next().toString
326326
val indexOfSlash = bindingPath.indexOf('/')
327-
if (executeOnly && packagePath != bindingPath.take(indexOfSlash)){
328-
terminate(StatusCode.int2StatusCode(403), s"GET not permitted since ${wp.name.toString} is a binding of a shared package")
329-
}else {
327+
if (executeOnly && packagePath != bindingPath.take(indexOfSlash)) {
328+
terminate(Forbidden, forbiddenGetPackageBinding(wp.name.asString))
329+
} else {
330330
getEntity(WhiskPackage.get(entityStore, docid), Some {
331331
mergePackageWithBinding(Some {
332332
wp

‎tests/src/test/scala/org/apache/openwhisk/core/controller/test/PackageActionsApiTests.scala

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -639,7 +639,7 @@ class PackageActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
639639
var testExecuteOnly = false
640640
override def executeOnly = testExecuteOnly
641641

642-
it should("allow access to get of action in binding of shared package when config option is disabled") in {
642+
it should ("allow access to get of action in binding of shared package when config option is disabled") in {
643643
testExecuteOnly = false
644644
implicit val tid = transid()
645645
val auser = WhiskAuthHelpers.newIdentity()
@@ -654,18 +654,18 @@ class PackageActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
654654
}
655655
}
656656

657-
it should("deny access to get of action in binding of shared package when config option is enabled") in {
658-
testExecuteOnly = true
659-
implicit val tid = transid()
660-
val auser = WhiskAuthHelpers.newIdentity()
661-
val provider = WhiskPackage(namespace, aname(), None, Parameters("p", "P"), publish = true)
662-
val binding = WhiskPackage(EntityPath(auser.subject.asString), aname(), provider.bind, Parameters("b", "B"))
663-
val action = WhiskAction(provider.fullPath, aname(), jsDefault("??"), Parameters("a", "A"))
664-
put(entityStore, provider)
665-
put(entityStore, binding)
666-
put(entityStore, action)
667-
Get(s"$collectionPath/${binding.name}/${action.name}") ~> Route.seal(routes(auser)) ~> check {
668-
status should be(Forbidden)
669-
}
657+
it should ("deny access to get of action in binding of shared package when config option is enabled") in {
658+
testExecuteOnly = true
659+
implicit val tid = transid()
660+
val auser = WhiskAuthHelpers.newIdentity()
661+
val provider = WhiskPackage(namespace, aname(), None, Parameters("p", "P"), publish = true)
662+
val binding = WhiskPackage(EntityPath(auser.subject.asString), aname(), provider.bind, Parameters("b", "B"))
663+
val action = WhiskAction(provider.fullPath, aname(), jsDefault("??"), Parameters("a", "A"))
664+
put(entityStore, provider)
665+
put(entityStore, binding)
666+
put(entityStore, action)
667+
Get(s"$collectionPath/${binding.name}/${action.name}") ~> Route.seal(routes(auser)) ~> check {
668+
status should be(Forbidden)
669+
}
670670
}
671671
}

‎tests/src/test/scala/org/apache/openwhisk/core/controller/test/PackagesApiTests.scala

Lines changed: 40 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -888,54 +888,54 @@ class PackagesApiTests extends ControllerTestCommon with WhiskPackagesApi {
888888
var testExecuteOnly = false
889889
override def executeOnly = testExecuteOnly
890890

891-
it should("allow access to get of shared package binding when config option is disabled") in {
892-
testExecuteOnly = false
893-
implicit val tid = transid()
894-
val auser = WhiskAuthHelpers.newIdentity()
895-
val provider = WhiskPackage(namespace, aname(), None, Parameters("p", "P"), publish = true)
896-
val binding = WhiskPackage(EntityPath(auser.subject.asString), aname(), provider.bind, Parameters("b", "B"))
897-
put(entityStore, provider)
898-
put(entityStore, binding)
899-
Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
900-
status should be(OK)
901-
}
891+
it should ("allow access to get of shared package binding when config option is disabled") in {
892+
testExecuteOnly = false
893+
implicit val tid = transid()
894+
val auser = WhiskAuthHelpers.newIdentity()
895+
val provider = WhiskPackage(namespace, aname(), None, Parameters("p", "P"), publish = true)
896+
val binding = WhiskPackage(EntityPath(auser.subject.asString), aname(), provider.bind, Parameters("b", "B"))
897+
put(entityStore, provider)
898+
put(entityStore, binding)
899+
Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
900+
status should be(OK)
901+
}
902902
}
903903

904-
it should("allow access to get of shared package when config option is disabled") in {
905-
testExecuteOnly = false
906-
implicit val tid = transid()
907-
val auser = WhiskAuthHelpers.newIdentity()
908-
val provider = WhiskPackage(namespace, aname(), None, publish = true)
909-
put(entityStore, provider)
904+
it should ("allow access to get of shared package when config option is disabled") in {
905+
testExecuteOnly = false
906+
implicit val tid = transid()
907+
val auser = WhiskAuthHelpers.newIdentity()
908+
val provider = WhiskPackage(namespace, aname(), None, publish = true)
909+
put(entityStore, provider)
910910

911-
Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
912-
status should be(OK)
913-
}
911+
Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
912+
status should be(OK)
913+
}
914914
}
915915

916916
it should ("deny access to get of shared package binding when config option is enabled") in {
917-
testExecuteOnly = true
918-
implicit val tid = transid()
919-
val auser = WhiskAuthHelpers.newIdentity()
920-
val provider = WhiskPackage(namespace, aname(), None, Parameters("p", "P"), publish = true)
921-
val binding = WhiskPackage(EntityPath(auser.subject.asString), aname(), provider.bind, Parameters("b", "B"))
922-
put(entityStore, provider)
923-
put(entityStore, binding)
924-
Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
925-
status should be(Forbidden)
926-
}
917+
testExecuteOnly = true
918+
implicit val tid = transid()
919+
val auser = WhiskAuthHelpers.newIdentity()
920+
val provider = WhiskPackage(namespace, aname(), None, Parameters("p", "P"), publish = true)
921+
val binding = WhiskPackage(EntityPath(auser.subject.asString), aname(), provider.bind, Parameters("b", "B"))
922+
put(entityStore, provider)
923+
put(entityStore, binding)
924+
Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
925+
status should be(Forbidden)
926+
}
927927

928928
}
929929

930-
it should("deny access to get of shared package when config option is enabled") in {
931-
testExecuteOnly = true
932-
implicit val tid = transid()
933-
val auser = WhiskAuthHelpers.newIdentity()
934-
val provider = WhiskPackage(namespace, aname(), None, publish = true)
935-
put(entityStore, provider)
930+
it should ("deny access to get of shared package when config option is enabled") in {
931+
testExecuteOnly = true
932+
implicit val tid = transid()
933+
val auser = WhiskAuthHelpers.newIdentity()
934+
val provider = WhiskPackage(namespace, aname(), None, publish = true)
935+
put(entityStore, provider)
936936

937-
Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
938-
status should be(Forbidden)
939-
}
937+
Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
938+
status should be(Forbidden)
939+
}
940940
}
941-
}
941+
}

0 commit comments

Comments
 (0)