Skip to content

Commit fb0bab6

Browse files
rabbahdubee
rabbah
authored and
dubee
committed
Add an annotations to inject the API key into the action context. (#4284)
* Factor out some annotation names to WhiskAction singleton. * Omit API key unless requested explicitly. * Add an annotation provide-api-key which causes an API key to be passed to the action context. * Modify Parameters.isTruthy to allow for caller to specify how it wants to treat a missing key. Default is missing key -> false (preserving behavior). Add test for isTruthy. * Treat a missing provide-key-annotation as truthy annotation. Actions that already exist without the annotation will receive the key as they might already expect. * Add container proxy test. * Update tests for new system annotation. * Allow pre-existing actions to be exempt from new system annotations. * Update docs.
1 parent 063cb66 commit fb0bab6

16 files changed

Lines changed: 360 additions & 114 deletions

File tree

‎common/scala/src/main/scala/org/apache/openwhisk/core/entity/Parameter.scala‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ protected[core] class Parameters protected[entity] (private val params: Map[Para
6565
}
6666

6767
/** Remove parameter by name. */
68-
protected[core] def -(p: String) = {
68+
protected[core] def -(p: String): Parameters = {
6969
// wrap with try since parameter name may throw an exception for illegal p
7070
Try(new Parameters(params - new ParameterName(p))) getOrElse this
7171
}
@@ -103,15 +103,20 @@ protected[core] class Parameters protected[entity] (private val params: Map[Para
103103
.fold[Try[JsValue]](Failure(new IllegalStateException(s"key '$p' does not exist")))(Success.apply)
104104
.flatMap(js => Try(js.convertTo[T]))
105105

106-
/** Retrieves parameter by name if it exist. Returns true if parameter exists and has truthy value. */
107-
protected[core] def isTruthy(p: String): Boolean = {
106+
/**
107+
* Retrieves parameter by name if it exist.
108+
* @param p the parameter to check for a truthy value
109+
* @param valueForNonExistent the value to return for a missing parameter (default false)
110+
* @return true if parameter exists and has truthy value, otherwise returns the specified value for non-existent keys
111+
*/
112+
protected[core] def isTruthy(p: String, valueForNonExistent: Boolean = false): Boolean = {
108113
get(p) map {
109114
case JsBoolean(b) => b
110115
case JsNumber(n) => n != 0
111116
case JsString(s) => s.nonEmpty
112117
case JsNull => false
113118
case _ => true
114-
} getOrElse false
119+
} getOrElse valueForNonExistent
115120
}
116121
}
117122

‎common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ case class WhiskAction(namespace: EntityPath,
143143
* Merges parameters (usually from package) with existing action parameters.
144144
* Existing parameters supersede those in p.
145145
*/
146-
def inherit(p: Parameters) = copy(parameters = p ++ parameters).revision[WhiskAction](rev)
146+
def inherit(p: Parameters): WhiskAction = copy(parameters = p ++ parameters).revision[WhiskAction](rev)
147147

148148
/**
149149
* Resolves sequence components if they contain default namespace.
@@ -166,7 +166,7 @@ case class WhiskAction(namespace: EntityPath,
166166
}
167167
}
168168

169-
def toExecutableWhiskAction = exec match {
169+
def toExecutableWhiskAction: Option[ExecutableWhiskAction] = exec match {
170170
case codeExec: CodeExec[_] =>
171171
Some(
172172
ExecutableWhiskAction(namespace, name, codeExec, parameters, limits, version, publish, annotations)
@@ -178,7 +178,7 @@ case class WhiskAction(namespace: EntityPath,
178178
* This the action summary as computed by the database view.
179179
* Strictly used in view testing to enforce alignment.
180180
*/
181-
override def summaryAsJson = {
181+
override def summaryAsJson: JsObject = {
182182
val binary = exec match {
183183
case c: CodeExec[_] => c.binary
184184
case _ => false
@@ -309,6 +309,12 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[
309309

310310
val execFieldName = "exec"
311311
val finalParamsAnnotationName = "final"
312+
val webActionAnnotationName = "web-export"
313+
val webCustomOptionsAnnotationName = "web-custom-options"
314+
val rawHttpAnnotationName = "raw-http"
315+
val requireWhiskAuthAnnotation = "require-whisk-auth"
316+
val requireWhiskAuthHeader = "x-require-whisk-auth"
317+
val provideApiKeyAnnotationName = "provide-api-key"
312318

313319
override val collectionName = "actions"
314320

@@ -325,9 +331,6 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[
325331

326332
override val cacheEnabled = true
327333

328-
val requireWhiskAuthAnnotation = "require-whisk-auth"
329-
val requireWhiskAuthHeader = "x-require-whisk-auth"
330-
331334
// overriden to store attached code
332335
override def put[A >: WhiskAction](db: ArtifactStore[A], doc: WhiskAction, old: Option[WhiskAction])(
333336
implicit transid: TransactionId,
@@ -512,9 +515,6 @@ object WhiskActionMetaData
512515
with WhiskEntityQueries[WhiskActionMetaData]
513516
with DefaultJsonProtocol {
514517

515-
val execFieldName = "exec"
516-
val finalParamsAnnotationName = "final"
517-
518518
override val collectionName = "actions"
519519

520520
override implicit val serdes = jsonFormat(

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

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,38 @@ object WhiskActionsApi {
5959
* This is the default timeout on a POST request.
6060
*/
6161
protected[core] val maxWaitForBlockingActivation = 60 seconds
62+
63+
/**
64+
* Amends annotations on an action create/update with system defined values.
65+
* This method currently adds the following annotations:
66+
* 1. [[WhiskAction.provideApiKeyAnnotationName]] with the value false iff the annotation is not already defined in the action annotations
67+
* 2. An [[execAnnotation]] consistent with the action kind; this annotation is always added and overrides a pre-existing value
68+
*/
69+
protected[core] def amendAnnotations(annotations: Parameters, exec: Exec, create: Boolean = true): Parameters = {
70+
val newAnnotations = if (create) {
71+
// these annotations are only added on newly created actions
72+
// since they can break existing actions created before the
73+
// annotation was created
74+
annotations
75+
.get(WhiskAction.provideApiKeyAnnotationName)
76+
.map(_ => annotations)
77+
.getOrElse {
78+
annotations ++ Parameters(WhiskAction.provideApiKeyAnnotationName, JsBoolean(false))
79+
}
80+
} else annotations
81+
newAnnotations ++ execAnnotation(exec)
82+
}
83+
84+
/**
85+
* Constructs an "exec" annotation. This is redundant with the exec kind
86+
* information available in WhiskAction but necessary for some clients which
87+
* fetch action lists but cannot determine action kinds without fetching them.
88+
* An alternative is to include the exec in the action list "view" but this
89+
* will require an API change. So using an annotation instead.
90+
*/
91+
private def execAnnotation(exec: Exec): Parameters = {
92+
Parameters(WhiskAction.execFieldName, exec.kind)
93+
}
6294
}
6395

6496
/** A trait implementing the actions API. */
@@ -416,7 +448,7 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
416448
limits,
417449
content.version getOrElse SemVer(),
418450
content.publish getOrElse false,
419-
(content.annotations getOrElse Parameters()) ++ execAnnotation(exec))
451+
WhiskActionsApi.amendAnnotations(content.annotations getOrElse Parameters(), exec))
420452
}
421453

422454
/** For a sequence action, gather referenced entities and authorize access. */
@@ -531,7 +563,7 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
531563
limits,
532564
content.version getOrElse action.version.upPatch,
533565
content.publish getOrElse action.publish,
534-
(content.annotations getOrElse action.annotations) ++ execAnnotation(exec))
566+
WhiskActionsApi.amendAnnotations(content.annotations getOrElse action.annotations, exec, create = false))
535567
.revision[WhiskAction](action.docinfo.rev)
536568
}
537569

@@ -685,17 +717,6 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
685717
}
686718
}
687719

688-
/**
689-
* Constructs an "exec" annotation. This is redundant with the exec kind
690-
* information available in WhiskAction but necessary for some clients which
691-
* fetch action lists but cannot determine action kinds without fetching them.
692-
* An alternative is to include the exec in the action list "view" but this
693-
* will require an API change. So using an annotation instead.
694-
*/
695-
private def execAnnotation(exec: Exec): Parameters = {
696-
Parameters(WhiskAction.execFieldName, exec.kind)
697-
}
698-
699720
/** Max atomic action count allowed for sequences. */
700721
private lazy val actionSequenceLimit = whiskConfig.actionSequenceLimit.toInt
701722

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,9 @@ trait WhiskWebActionsApi
490490
this,
491491
"web action with require-whisk-auth was invoked without a matching x-require-whisk-auth header value")
492492
terminate(Unauthorized)
493-
} else if (!action.annotations.getAs[Boolean]("web-custom-options").getOrElse(false)) {
493+
} else if (!action.annotations
494+
.getAs[Boolean](WhiskAction.webCustomOptionsAnnotationName)
495+
.getOrElse(false)) {
494496
respondWithHeaders(defaultCorsResponse(context.headers)) {
495497
if (context.method == OPTIONS) {
496498
complete(OK, HttpEntity.Empty)
@@ -554,7 +556,7 @@ trait WhiskWebActionsApi
554556
processRequest(actionOwnerIdentity, action, extension, onBehalfOf, context.withBody(body), isRawHttpAction)
555557
}
556558

557-
provide(action.annotations.getAs[Boolean]("raw-http").getOrElse(false)) { isRawHttpAction =>
559+
provide(action.annotations.getAs[Boolean](WhiskAction.rawHttpAnnotationName).getOrElse(false)) { isRawHttpAction =>
558560
httpEntity match {
559561
case Empty =>
560562
process(None, isRawHttpAction)
@@ -705,7 +707,7 @@ trait WhiskWebActionsApi
705707
actionLookup flatMap { action =>
706708
val requiresAuthenticatedUser =
707709
action.annotations.getAs[Boolean](WhiskAction.requireWhiskAuthAnnotation).getOrElse(false)
708-
val isExported = action.annotations.getAs[Boolean]("web-export").getOrElse(false)
710+
val isExported = action.annotations.getAs[Boolean](WhiskAction.webActionAnnotationName).getOrElse(false)
709711

710712
if ((isExported && requiresAuthenticatedUser && authenticated) ||
711713
(isExported && !requiresAuthenticatedUser)) {

‎core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,13 @@ class ContainerProxy(
560560
}
561561
val parameters = job.msg.content getOrElse JsObject.empty
562562

563-
val authEnvironment = job.msg.user.authkey.toEnvironment
563+
// if the action requests the api key to be injected into the action context, add it here;
564+
// treat a missing annotation as requesting the api key for backward compatibility
565+
val authEnvironment = {
566+
if (job.action.annotations.isTruthy(WhiskAction.provideApiKeyAnnotationName, valueForNonExistent = true)) {
567+
job.msg.user.authkey.toEnvironment
568+
} else JsObject.empty
569+
}
564570

565571
val environment = JsObject(
566572
"namespace" -> job.msg.user.namespace.name.toJson,

‎docs/actions.md‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -616,9 +616,9 @@ or set an internal alarm when the action is about to use up its allotted time bu
616616
The properties are accessible via the system environment for all supported runtimes:
617617
Node.js, Python, Swift, Java and Docker actions when using the OpenWhisk Docker skeleton.
618618

619-
* `__OW_API_HOST` the API host for the OpenWhisk deployment running this action
620-
* `__OW_API_KEY` the API key for the subject invoking the action, this key may be a restricted API key
621-
* `__OW_NAMESPACE` the namespace for the _activation_ (this may not be the same as the namespace for the action)
622-
* `__OW_ACTION_NAME` the fully qualified name of the running action
623-
* `__OW_ACTIVATION_ID` the activation id for this running action instance
624-
* `__OW_DEADLINE` the approximate time when this action will have consumed its entire duration quota (measured in epoch milliseconds)
619+
* `__OW_API_HOST` the API host for the OpenWhisk deployment running this action.
620+
* `__OW_API_KEY` the API key for the subject invoking the action, this key may be a restricted API key. This property is absent unless explicitly [requested](./annotations.md#annotations-for-all-actions).
621+
* `__OW_NAMESPACE` the namespace for the _activation_ (this may not be the same as the namespace for the action).
622+
* `__OW_ACTION_NAME` the fully qualified name of the running action.
623+
* `__OW_ACTIVATION_ID` the activation id for this running action instance.
624+
* `__OW_DEADLINE` the approximate time when this action will have consumed its entire duration quota (measured in epoch milliseconds).

‎docs/annotations.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,15 @@ The annotations we have used for describing parameters include:
5757

5858
The annotations are _not_ checked. So while it is conceivable to use the annotations to infer if a composition of two actions into a sequence is legal, for example, the system does not yet do that.
5959

60+
# Annotations for all actions
61+
62+
The following annotations on an action are available.
63+
64+
* `provide-api-key`: This annotation may be attached to actions which require an API key, for example to make REST API calls to the OpenWhisk host.
65+
The absence of this annotation, or its presence with a value that is not _falsy_ (i.e., a value that is different from zero, null, false, and the empty string)
66+
will cause an API key to be present in the [action execution context](./actions.md#accessing-action-metadata-within-the-action-body). This annotation is added
67+
to newly created actions, if not already specified, with a default false value.
68+
6069
# Annotations specific to web actions
6170

6271
Web actions are enabled with explicit annotations which decorate individual actions. The annotations only apply to the [web actions](webactions.md) API,

‎tests/src/test/scala/org/apache/openwhisk/core/cli/test/WskRestBasicUsageTests.scala‎

Lines changed: 50 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,10 @@ class WskRestBasicUsageTests extends TestHelpers with WskTestHelpers with WskAct
300300
it should "invoke an action using npm openwhisk" in withAssetCleaner(wskprops) { (wp, assetHelper) =>
301301
val name = "hello npm openwhisk"
302302
assetHelper.withCleaner(wsk.action, name, confirmDelete = false) { (action, _) =>
303-
action.create(name, Some(TestUtils.getTestActionFilename("helloOpenwhiskPackage.js")))
303+
action.create(
304+
name,
305+
Some(TestUtils.getTestActionFilename("helloOpenwhiskPackage.js")),
306+
annotations = Map(WhiskAction.provideApiKeyAnnotationName -> JsBoolean(true)))
304307
}
305308

306309
val run = wsk.action.invoke(name, Map("ignore_certs" -> true.toJson, "name" -> name.toJson))
@@ -313,25 +316,51 @@ class WskRestBasicUsageTests extends TestHelpers with WskTestHelpers with WskAct
313316
wsk.action.delete(name, expectedExitCode = NotFound.intValue)
314317
}
315318

316-
it should "invoke an action receiving context properties" in withAssetCleaner(wskprops) { (wp, assetHelper) =>
317-
val namespace = wsk.namespace.whois()
318-
val name = "context"
319-
assetHelper.withCleaner(wsk.action, name) { (action, _) =>
320-
action.create(name, Some(TestUtils.getTestActionFilename("helloContext.js")))
321-
}
319+
it should "invoke an action receiving context properties excluding api key" in withAssetCleaner(wskprops) {
320+
(wp, assetHelper) =>
321+
val namespace = wsk.namespace.whois()
322+
val name = "context"
323+
assetHelper.withCleaner(wsk.action, name) { (action, _) =>
324+
action.create(name, Some(TestUtils.getTestActionFilename("helloContext.js")))
325+
}
322326

323-
val start = Instant.now(Clock.systemUTC()).toEpochMilli
324-
val run = wsk.action.invoke(name)
325-
withActivation(wsk.activation, run) { activation =>
326-
activation.response.status shouldBe "success"
327-
val fields = activation.response.result.get.convertTo[Map[String, String]]
328-
fields("api_host") shouldBe WhiskProperties.getApiHostForAction
329-
fields("api_key") shouldBe wskprops.authKey
330-
fields("namespace") shouldBe namespace
331-
fields("action_name") shouldBe s"/$namespace/$name"
332-
fields("activation_id") shouldBe activation.activationId
333-
fields("deadline").toLong should be >= start
334-
}
327+
val start = Instant.now(Clock.systemUTC()).toEpochMilli
328+
val run = wsk.action.invoke(name)
329+
withActivation(wsk.activation, run) { activation =>
330+
activation.response.status shouldBe "success"
331+
val fields = activation.response.result.get.convertTo[Map[String, String]]
332+
fields("api_host") shouldBe WhiskProperties.getApiHostForAction
333+
fields.get("api_key") shouldBe empty
334+
fields("namespace") shouldBe namespace
335+
fields("action_name") shouldBe s"/$namespace/$name"
336+
fields("activation_id") shouldBe activation.activationId
337+
fields("deadline").toLong should be >= start
338+
}
339+
}
340+
341+
it should "invoke an action receiving context properties including api key" in withAssetCleaner(wskprops) {
342+
(wp, assetHelper) =>
343+
val namespace = wsk.namespace.whois()
344+
val name = "context"
345+
assetHelper.withCleaner(wsk.action, name) { (action, _) =>
346+
action.create(
347+
name,
348+
Some(TestUtils.getTestActionFilename("helloContext.js")),
349+
annotations = Map(WhiskAction.provideApiKeyAnnotationName -> JsBoolean(true)))
350+
}
351+
352+
val start = Instant.now(Clock.systemUTC()).toEpochMilli
353+
val run = wsk.action.invoke(name)
354+
withActivation(wsk.activation, run) { activation =>
355+
activation.response.status shouldBe "success"
356+
val fields = activation.response.result.get.convertTo[Map[String, String]]
357+
fields("api_host") shouldBe WhiskProperties.getApiHostForAction
358+
fields("api_key") shouldBe wskprops.authKey
359+
fields("namespace") shouldBe namespace
360+
fields("action_name") shouldBe s"/$namespace/$name"
361+
fields("activation_id") shouldBe activation.activationId
362+
fields("deadline").toLong should be >= start
363+
}
335364
}
336365

337366
it should "invoke an action successfully with options --blocking and --result" in withAssetCleaner(wskprops) {
@@ -404,6 +433,7 @@ class WskRestBasicUsageTests extends TestHelpers with WskTestHelpers with WskAct
404433
val action = wsk.action.get(name)
405434
action.getFieldJsValue("annotations").convertTo[Set[JsObject]] shouldBe Set(
406435
JsObject("key" -> JsString("exec"), "value" -> JsString("nodejs:6")),
436+
JsObject("key" -> WhiskAction.provideApiKeyAnnotationName.toJson, "value" -> JsBoolean(false)),
407437
JsObject("key" -> JsString("web-export"), "value" -> JsBoolean(webEnabled || rawEnabled)),
408438
JsObject("key" -> JsString("raw-http"), "value" -> JsBoolean(rawEnabled)),
409439
JsObject("key" -> JsString("final"), "value" -> JsBoolean(webEnabled || rawEnabled)))
@@ -424,6 +454,7 @@ class WskRestBasicUsageTests extends TestHelpers with WskTestHelpers with WskAct
424454
JsObject("key" -> JsString("web-export"), "value" -> JsBoolean(true)),
425455
JsObject("key" -> JsString("raw-http"), "value" -> JsBoolean(false)),
426456
JsObject("key" -> JsString("final"), "value" -> JsBoolean(true)),
457+
JsObject("key" -> WhiskAction.provideApiKeyAnnotationName.toJson, "value" -> JsBoolean(false)),
427458
JsObject("key" -> JsString("exec"), "value" -> JsString("nodejs:6")))
428459
}
429460

0 commit comments

Comments
 (0)