Skip to content

Commit 63a2aaf

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 cb02bdb commit 63a2aaf

5 files changed

Lines changed: 158 additions & 24 deletions

File tree

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

Lines changed: 4 additions & 2 deletions
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‎

Lines changed: 28 additions & 11 deletions
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
}
@@ -523,16 +524,32 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
523524

524525
val exec = content.exec getOrElse action.exec
525526

526-
WhiskAction(
527-
action.namespace,
528-
action.name,
529-
exec,
530-
parameters,
531-
limits,
532-
content.version getOrElse action.version.upPatch,
533-
content.publish getOrElse action.publish,
534-
(content.annotations getOrElse action.annotations) ++ execAnnotation(exec))
535-
.revision[WhiskAction](action.docinfo.rev)
527+
if (content.unlock.getOrElse(false)) {
528+
WhiskAction(
529+
action.namespace,
530+
action.name,
531+
exec,
532+
parameters,
533+
limits,
534+
content.version getOrElse action.version.upPatch,
535+
content.publish getOrElse action.publish,
536+
(content.annotations getOrElse action.annotations) ++ execAnnotation(exec) ++ Parameters(
537+
WhiskAction.lockFieldName,
538+
JsBoolean(false)))
539+
.revision[WhiskAction](action.docinfo.rev)
540+
} else {
541+
//keep original value not changed
542+
WhiskAction(
543+
action.namespace,
544+
action.name,
545+
exec,
546+
parameters,
547+
limits,
548+
content.version getOrElse action.version.upPatch,
549+
content.publish getOrElse action.publish,
550+
(content.annotations getOrElse action.annotations) ++ execAnnotation(exec))
551+
.revision[WhiskAction](action.docinfo.rev)
552+
}
536553
}
537554

538555
/**

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

Lines changed: 45 additions & 11 deletions
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‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,23 @@ 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)
601618

602619
## Accessing action metadata within the action body
603620

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

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,70 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
401401
}
402402
}
403403

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

0 commit comments

Comments
 (0)