-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add binding
annotation to record an action path not resolved
#4211
Changes from all commits
5b5708b
de5a09b
78aad4b
2b331a7
316ecc5
dc3006f
2e79409
210a5ff
b36b6f4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,12 +23,10 @@ import scala.language.postfixOps | |
import scala.util.{Failure, Success, Try} | ||
import org.apache.kafka.common.errors.RecordTooLargeException | ||
import akka.actor.ActorSystem | ||
import akka.http.scaladsl.model.HttpMethod | ||
import akka.http.scaladsl.model.StatusCodes._ | ||
import akka.http.scaladsl.server.RequestContext | ||
import akka.http.scaladsl.server.RouteResult | ||
import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._ | ||
import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport.sprayJsonMarshaller | ||
import akka.http.scaladsl.unmarshalling._ | ||
import spray.json._ | ||
import spray.json.DefaultJsonProtocol._ | ||
|
@@ -173,14 +171,10 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with | |
onComplete(entitlementProvider.check(user, right, packageResource)) { | ||
case Success(_) => | ||
getEntity(WhiskPackage.get(entityStore, packageDocId), Some { | ||
if (right == Privilege.READ || right == Privilege.ACTIVATE) { | ||
// need to merge package with action, hence authorize subject for package | ||
// access (if binding, then subject must be authorized for both the binding | ||
// and the referenced package) | ||
// | ||
// NOTE: it is an error if either the package or the action does not exist, | ||
// the former manifests as unauthorized and the latter as not found | ||
mergeActionWithPackageAndDispatch(m, user, EntityName(innername)) _ | ||
if (right == Privilege.READ || right == Privilege.ACTIVATE) { wp: WhiskPackage => | ||
val actionResource = Resource(wp.fullPath, collection, Some(innername)) | ||
dispatchOp(user, right, actionResource) | ||
|
||
} else { | ||
// these packaged action operations do not need merging with the package, | ||
// but may not be permitted if this is a binding, or if the subject does | ||
|
@@ -253,7 +247,7 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with | |
'result ? false, | ||
'timeout.as[FiniteDuration] ? WhiskActionsApi.maxWaitForBlockingActivation) { (blocking, result, waitOverride) => | ||
entity(as[Option[JsObject]]) { payload => | ||
getEntity(WhiskActionMetaData.get(entityStore, entityName.toDocId), Some { | ||
getEntity(WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, entityName), Some { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @style95 the action is resolved here after it's dispatched. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, this is the similar way which is taken for web action. |
||
act: WhiskActionMetaData => | ||
// resolve the action --- special case for sequences that may contain components with '_' as default package | ||
val action = act.resolve(user.namespace) | ||
|
@@ -355,18 +349,19 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with | |
parameter('code ? true) { code => | ||
code match { | ||
case true => | ||
getEntity(WhiskAction.get(entityStore, entityName.toDocId), Some { action: WhiskAction => | ||
getEntity(WhiskAction.resolveActionAndMergeParameters(entityStore, entityName), Some { action: WhiskAction => | ||
val mergedAction = env map { | ||
action inherit _ | ||
} getOrElse action | ||
complete(OK, mergedAction) | ||
}) | ||
case false => | ||
getEntity(WhiskActionMetaData.get(entityStore, entityName.toDocId), Some { action: WhiskActionMetaData => | ||
val mergedAction = env map { | ||
action inherit _ | ||
} getOrElse action | ||
complete(OK, mergedAction) | ||
getEntity(WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, entityName), Some { | ||
action: WhiskActionMetaData => | ||
val mergedAction = env map { | ||
action inherit _ | ||
} getOrElse action | ||
complete(OK, mergedAction) | ||
}) | ||
} | ||
} | ||
|
@@ -595,38 +590,6 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with | |
}) | ||
} | ||
|
||
/** | ||
* Constructs a WhiskPackage that is a merger of a package with its packing binding (if any). | ||
* This resolves a reference versus an actual package and merge parameters as needed. | ||
* Once the package is resolved, the operation is dispatched to the action in the package | ||
* namespace. | ||
*/ | ||
private def mergeActionWithPackageAndDispatch(method: HttpMethod, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @upgle Could you elaborate the reason to remove this method a little bit? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If an action is resolved before it's dispatched, there is no way to know its original name in the activate method because it's already resolved. override def activate(user: Identity, entityName: FullyQualifiedEntityName, env: Option[Parameters])(
implicit transid: TransactionId) = { implicit transid: TransactionId) |
||
user: Identity, | ||
action: EntityName, | ||
ref: Option[WhiskPackage] = None)(wp: WhiskPackage)( | ||
implicit transid: TransactionId): RequestContext => Future[RouteResult] = { | ||
wp.binding map { | ||
case b: Binding => | ||
val docid = b.fullyQualifiedName.toDocId | ||
logging.debug(this, s"fetching package '$docid' for reference") | ||
// already checked that subject is authorized for package and binding; | ||
// this fetch is redundant but should hit the cache to ameliorate cost | ||
getEntity(WhiskPackage.get(entityStore, docid), Some { | ||
mergeActionWithPackageAndDispatch(method, user, action, Some { wp }) _ | ||
}) | ||
} getOrElse { | ||
// a subject has implied rights to all resources in a package, so dispatch | ||
// operation without further entitlement checks | ||
val params = { ref map { _ inherit wp.parameters } getOrElse wp } parameters | ||
val ns = wp.namespace.addPath(wp.name) // the package namespace | ||
val resource = Resource(ns, collection, Some { action.asString }, Some { params }) | ||
val right = collection.determineRight(method, resource.entity) | ||
logging.debug(this, s"merged package parameters and rebased action to '$ns") | ||
dispatchOp(user, right, resource) | ||
} | ||
} | ||
|
||
/** | ||
* Checks that the sequence is not cyclic and that the number of atomic actions in the "inlined" sequence is lower than max allowed. | ||
* | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will check down below but this raises the opportunity to have the name in the activation be that of the binding, and the annotation is that of the package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry, I don't understand this.
In the example below, I think the name
triple
in the activation can not be the name of the bindingwhisk.system/p2
whisk.system/pkg1/triple
whisk.system/p2
is binding thewhisk.system/pkg1
whisk.system/p2/triple