Skip to content

Commit 69602d9

Browse files
ningyougangningyougang
committed
Add protect feature to avoid update or delete actions by mistake
As more and more production system uses openwhisk, Users will need some features to protect their service safe from mistake. If we have this feature, user can protect their actions from updating or deletion by mistake.
1 parent 7ff1550 commit 69602d9

File tree

5 files changed

+140
-15
lines changed

5 files changed

+140
-15
lines changed

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

+4-2
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ case class WhiskActionPut(exec: Option[Exec] = None,
5555
limits: Option[ActionLimitsOption] = None,
5656
version: Option[SemVer] = None,
5757
publish: Option[Boolean] = None,
58-
annotations: Option[Parameters] = None) {
58+
annotations: Option[Parameters] = None,
59+
unlock: Option[Boolean] = None) {
5960

6061
protected[core] def replace(exec: Exec) = {
6162
WhiskActionPut(Some(exec), parameters, limits, version, publish, annotations)
@@ -308,6 +309,7 @@ case class ExecutableWhiskActionMetaData(namespace: EntityPath,
308309
object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[WhiskAction] with DefaultJsonProtocol {
309310

310311
val execFieldName = "exec"
312+
val lockFieldName = "lock"
311313
val finalParamsAnnotationName = "final"
312314

313315
override val collectionName = "actions"
@@ -591,5 +593,5 @@ object ActionLimitsOption extends DefaultJsonProtocol {
591593
}
592594

593595
object WhiskActionPut extends DefaultJsonProtocol {
594-
implicit val serdes = jsonFormat6(WhiskActionPut.apply)
596+
implicit val serdes = jsonFormat7(WhiskActionPut.apply)
595597
}

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

+5-2
Original file line numberDiff line numberDiff line change
@@ -190,12 +190,13 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
190190
val checkAdditionalPrivileges = entitleReferencedEntities(user, Privilege.READ, request.exec).flatMap {
191191
case _ => entitlementProvider.check(user, content.exec)
192192
}
193+
val unlock = content.unlock.getOrElse(false)
193194

194195
onComplete(checkAdditionalPrivileges) {
195196
case Success(_) =>
196197
putEntity(WhiskAction, entityStore, entityName.toDocId, overwrite, update(user, request) _, () => {
197198
make(user, entityName, request)
198-
})
199+
}, unlock = unlock)
199200
case Failure(f) =>
200201
super.handleEntitlementFailure(f)
201202
}
@@ -531,7 +532,9 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
531532
limits,
532533
content.version getOrElse action.version.upPatch,
533534
content.publish getOrElse action.publish,
534-
(content.annotations getOrElse action.annotations) ++ execAnnotation(exec))
535+
(content.annotations getOrElse action.annotations) ++ execAnnotation(exec) ++ content.unlock
536+
.map(u => Parameters(WhiskAction.lockFieldName, JsBoolean(!u)))
537+
.getOrElse(Parameters()))
535538
.revision[WhiskAction](action.docinfo.rev)
536539
}
537540

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

+45-11
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,7 @@ import scala.util.Failure
2323
import scala.util.Success
2424
import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._
2525
import akka.http.scaladsl.model.StatusCode
26-
import akka.http.scaladsl.model.StatusCodes.Conflict
27-
import akka.http.scaladsl.model.StatusCodes.InternalServerError
28-
import akka.http.scaladsl.model.StatusCodes.NotFound
29-
import akka.http.scaladsl.model.StatusCodes.OK
26+
import akka.http.scaladsl.model.StatusCodes._
3027
import akka.http.scaladsl.server.{Directives, RequestContext, RouteResult}
3128
import spray.json.DefaultJsonProtocol._
3229
import spray.json.JsObject
@@ -37,6 +34,7 @@ import org.apache.openwhisk.common.TransactionId
3734
import org.apache.openwhisk.core.controller.PostProcess.PostProcessEntity
3835
import org.apache.openwhisk.core.database._
3936
import org.apache.openwhisk.core.entity.DocId
37+
import org.apache.openwhisk.core.entity.WhiskAction
4038
import org.apache.openwhisk.core.entity.WhiskDocument
4139
import org.apache.openwhisk.http.ErrorResponse
4240
import org.apache.openwhisk.http.ErrorResponse.terminate
@@ -242,7 +240,8 @@ trait WriteOps extends Directives {
242240
update: A => Future[A],
243241
create: () => Future[A],
244242
treatExistsAsConflict: Boolean = true,
245-
postProcess: Option[PostProcessEntity[A]] = None)(
243+
postProcess: Option[PostProcessEntity[A]] = None,
244+
unlock: Boolean = false)(
246245
implicit transid: TransactionId,
247246
format: RootJsonFormat[A],
248247
notifier: Option[CacheChangeNotification],
@@ -267,8 +266,24 @@ trait WriteOps extends Directives {
267266
} flatMap {
268267
case (old, a) =>
269268
logging.debug(this, s"[PUT] entity created/updated, writing back to datastore")
270-
factory.put(datastore, a, old) map { _ =>
271-
a
269+
if (overwrite && !unlock && old.getOrElse(None).isInstanceOf[WhiskAction]) {
270+
val oldWhiskAction = old.getOrElse(None).asInstanceOf[WhiskAction]
271+
oldWhiskAction.annotations.get(WhiskAction.lockFieldName) match {
272+
case Some(value) if (value.convertTo[Boolean]) => {
273+
Future failed RejectRequest(
274+
MethodNotAllowed,
275+
s"this action can't be updated until ${WhiskAction.lockFieldName} annotation is updated to false")
276+
}
277+
case _ => {
278+
factory.put(datastore, a, old) map { _ =>
279+
a
280+
}
281+
}
282+
}
283+
} else {
284+
factory.put(datastore, a, old) map { _ =>
285+
a
286+
}
272287
}
273288
}) {
274289
case Success(entity) =>
@@ -322,11 +337,30 @@ trait WriteOps extends Directives {
322337
notifier: Option[CacheChangeNotification],
323338
ma: Manifest[A]) = {
324339
onComplete(factory.get(datastore, docid) flatMap { entity =>
325-
confirm(entity) flatMap {
326-
case _ =>
327-
factory.del(datastore, entity.docinfo) map { _ =>
328-
entity
340+
if (entity.isInstanceOf[WhiskAction]) {
341+
val whiskAction = entity.asInstanceOf[WhiskAction]
342+
whiskAction.annotations.get(WhiskAction.lockFieldName) match {
343+
case Some(value) if (value.convertTo[Boolean]) => {
344+
Future failed RejectRequest(
345+
MethodNotAllowed,
346+
s"this action can't be deleted until ${WhiskAction.lockFieldName} annotation is updated to false")
347+
}
348+
case _ => {
349+
confirm(entity) flatMap {
350+
case _ =>
351+
factory.del(datastore, entity.docinfo) map { _ =>
352+
entity
353+
}
354+
}
329355
}
356+
}
357+
} else {
358+
confirm(entity) flatMap {
359+
case _ =>
360+
factory.del(datastore, entity.docinfo) map { _ =>
361+
entity
362+
}
363+
}
330364
}
331365
}) {
332366
case Success(entity) =>

‎docs/actions.md

+22
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,28 @@ You can clean up by deleting actions that you do not want to use.
598598
actions
599599
/guest/mySequence private sequence
600600
```
601+
## Protect action updated or deleted by mistake
602+
603+
If your action is very important, you can add `--annotation lock true` to protect it
604+
```
605+
wsk action create greeting greeting.js --annotation lock true
606+
```
607+
Then update or delete this action will be not allowed
608+
609+
You can add `{"unlock":true}` to enable `update or delete operation`, for example:
610+
```
611+
curl -X PUT -H "Content-type: application/json"
612+
--user 23bc46b1-71f6-4ed5-8c54-816aa4f8c502:123zO3xZCLrMN6v2BKK1dXYFpXlPkccOFqm12CdAsMgRU4VrNZ9lyGVCGuMDGIwP
613+
-d '{"unlock":true}'
614+
'/api/v1/namespaces/guest/actions/hello?overwrite=true'
615+
```
616+
617+
TODO(add unlock operation to wsk, for example: wsk action update hello --unlock)
618+
619+
You can lock the existing unlocked action again
620+
```
621+
wsk action update greeting --annotation lock true
622+
```
601623

602624
## Accessing action metadata within the action body
603625

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

+64
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,70 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
405405
}
406406
}
407407

408+
it should "update action not allowed when lock annotation is true" in {
409+
implicit val tid = transid()
410+
val action =
411+
WhiskAction(
412+
namespace,
413+
aname(),
414+
jsDefault(""),
415+
annotations = Parameters(WhiskAction.lockFieldName, JsBoolean(true)))
416+
val content = JsObject(
417+
"exec" -> JsObject("code" -> "".toJson, "kind" -> action.exec.kind.toJson),
418+
"annotations" -> action.annotations.toJson)
419+
Put(s"$collectionPath/${action.name}", content) ~> Route.seal(routes(creds)) ~> check {
420+
status should be(OK)
421+
}
422+
423+
//Update not allowed
424+
Put(s"$collectionPath/${action.name}?overwrite=true", content) ~> Route.seal(routes(creds)) ~> check {
425+
status should be(MethodNotAllowed)
426+
}
427+
428+
//unlock the action
429+
val unlockContent = s"""{"unlock":true}""".stripMargin.parseJson.asJsObject
430+
Put(s"$collectionPath/${action.name}?overwrite=true", unlockContent) ~> Route.seal(routes(creds)) ~> check {
431+
status should be(OK)
432+
}
433+
434+
//Update allowed after unlock the action
435+
Put(s"$collectionPath/${action.name}?overwrite=true", content) ~> Route.seal(routes(creds)) ~> check {
436+
status should be(OK)
437+
}
438+
}
439+
440+
it should "delete action not allowed when lock annotation is true" in {
441+
implicit val tid = transid()
442+
val action =
443+
WhiskAction(
444+
namespace,
445+
aname(),
446+
jsDefault(""),
447+
annotations = Parameters(WhiskAction.lockFieldName, JsBoolean(true)))
448+
val content = JsObject(
449+
"exec" -> JsObject("code" -> "".toJson, "kind" -> action.exec.kind.toJson),
450+
"annotations" -> action.annotations.toJson)
451+
Put(s"$collectionPath/${action.name}", content) ~> Route.seal(routes(creds)) ~> check {
452+
status should be(OK)
453+
}
454+
455+
//Delete not allowed
456+
Delete(s"$collectionPath/${action.name}") ~> Route.seal(routes(creds)) ~> check {
457+
status should be(MethodNotAllowed)
458+
}
459+
460+
//unlock the action
461+
val unlockContent = s"""{"unlock":true}""".stripMargin.parseJson.asJsObject
462+
Put(s"$collectionPath/${action.name}?overwrite=true", unlockContent) ~> Route.seal(routes(creds)) ~> check {
463+
status should be(OK)
464+
}
465+
466+
//Delete allowed after unlock the action
467+
Delete(s"$collectionPath/${action.name}") ~> Route.seal(routes(creds)) ~> check {
468+
status should be(OK)
469+
}
470+
}
471+
408472
it should "report NotFound for delete non existent action" in {
409473
implicit val tid = transid()
410474
Delete(s"$collectionPath/xyz") ~> Route.seal(routes(creds)) ~> check {

0 commit comments

Comments
 (0)