You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@openwhisk.apache.org by GitBox <gi...@apache.org> on 2018/12/24 06:32:53 UTC
[GitHub] ningyougang closed pull request #4058: Add protect feature to avoid
update or delete actions by mistake
ningyougang closed pull request #4058: Add protect feature to avoid update or delete actions by mistake
URL: https://github.com/apache/incubator-openwhisk/pull/4058
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/common/scala/src/main/scala/whisk/core/entity/WhiskAction.scala b/common/scala/src/main/scala/whisk/core/entity/WhiskAction.scala
index 55e02a777f..ea4debced2 100644
--- a/common/scala/src/main/scala/whisk/core/entity/WhiskAction.scala
+++ b/common/scala/src/main/scala/whisk/core/entity/WhiskAction.scala
@@ -52,7 +52,8 @@ case class WhiskActionPut(exec: Option[Exec] = None,
limits: Option[ActionLimitsOption] = None,
version: Option[SemVer] = None,
publish: Option[Boolean] = None,
- annotations: Option[Parameters] = None) {
+ annotations: Option[Parameters] = None,
+ unlock: Option[Boolean] = None) {
protected[core] def replace(exec: Exec) = {
WhiskActionPut(Some(exec), parameters, limits, version, publish, annotations)
@@ -305,6 +306,7 @@ case class ExecutableWhiskActionMetaData(namespace: EntityPath,
object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[WhiskAction] with DefaultJsonProtocol {
val execFieldName = "exec"
+ val lockFieldName = "lock"
val finalParamsAnnotationName = "final"
override val collectionName = "actions"
@@ -589,5 +591,5 @@ object ActionLimitsOption extends DefaultJsonProtocol {
}
object WhiskActionPut extends DefaultJsonProtocol {
- implicit val serdes = jsonFormat6(WhiskActionPut.apply)
+ implicit val serdes = jsonFormat7(WhiskActionPut.apply)
}
diff --git a/core/controller/src/main/scala/whisk/core/controller/Actions.scala b/core/controller/src/main/scala/whisk/core/controller/Actions.scala
index 8828241ab7..801e4bfc69 100644
--- a/core/controller/src/main/scala/whisk/core/controller/Actions.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/Actions.scala
@@ -190,12 +190,13 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
val checkAdditionalPrivileges = entitleReferencedEntities(user, Privilege.READ, request.exec).flatMap {
case _ => entitlementProvider.check(user, content.exec)
}
+ val unlock = content.unlock.getOrElse(false)
onComplete(checkAdditionalPrivileges) {
case Success(_) =>
putEntity(WhiskAction, entityStore, entityName.toDocId, overwrite, update(user, request) _, () => {
make(user, entityName, request)
- })
+ }, unlock = unlock)
case Failure(f) =>
super.handleEntitlementFailure(f)
}
@@ -518,16 +519,32 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
val exec = content.exec getOrElse action.exec
- WhiskAction(
- action.namespace,
- action.name,
- exec,
- parameters,
- limits,
- content.version getOrElse action.version.upPatch,
- content.publish getOrElse action.publish,
- (content.annotations getOrElse action.annotations) ++ execAnnotation(exec))
- .revision[WhiskAction](action.docinfo.rev)
+ if (content.unlock.getOrElse(false)) {
+ WhiskAction(
+ action.namespace,
+ action.name,
+ exec,
+ parameters,
+ limits,
+ content.version getOrElse action.version.upPatch,
+ content.publish getOrElse action.publish,
+ (content.annotations getOrElse action.annotations) ++ execAnnotation(exec) ++ Parameters(
+ WhiskAction.lockFieldName,
+ JsBoolean(false)))
+ .revision[WhiskAction](action.docinfo.rev)
+ } else {
+ //keep original value not changed
+ WhiskAction(
+ action.namespace,
+ action.name,
+ exec,
+ parameters,
+ limits,
+ content.version getOrElse action.version.upPatch,
+ content.publish getOrElse action.publish,
+ (content.annotations getOrElse action.annotations) ++ execAnnotation(exec))
+ .revision[WhiskAction](action.docinfo.rev)
+ }
}
/**
diff --git a/core/controller/src/main/scala/whisk/core/controller/ApiUtils.scala b/core/controller/src/main/scala/whisk/core/controller/ApiUtils.scala
index 7bb0f2af7c..d89355662c 100644
--- a/core/controller/src/main/scala/whisk/core/controller/ApiUtils.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/ApiUtils.scala
@@ -23,10 +23,7 @@ import scala.util.Failure
import scala.util.Success
import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._
import akka.http.scaladsl.model.StatusCode
-import akka.http.scaladsl.model.StatusCodes.Conflict
-import akka.http.scaladsl.model.StatusCodes.InternalServerError
-import akka.http.scaladsl.model.StatusCodes.NotFound
-import akka.http.scaladsl.model.StatusCodes.OK
+import akka.http.scaladsl.model.StatusCodes._
import akka.http.scaladsl.server.{Directives, RequestContext, RouteResult}
import spray.json.DefaultJsonProtocol._
import spray.json.JsObject
@@ -36,8 +33,7 @@ import whisk.common.Logging
import whisk.common.TransactionId
import whisk.core.controller.PostProcess.PostProcessEntity
import whisk.core.database._
-import whisk.core.entity.DocId
-import whisk.core.entity.WhiskDocument
+import whisk.core.entity.{DocId, WhiskAction, WhiskDocument}
import whisk.http.ErrorResponse
import whisk.http.ErrorResponse.terminate
import whisk.http.Messages._
@@ -242,7 +238,8 @@ trait WriteOps extends Directives {
update: A => Future[A],
create: () => Future[A],
treatExistsAsConflict: Boolean = true,
- postProcess: Option[PostProcessEntity[A]] = None)(
+ postProcess: Option[PostProcessEntity[A]] = None,
+ unlock: Boolean = false)(
implicit transid: TransactionId,
format: RootJsonFormat[A],
notifier: Option[CacheChangeNotification],
@@ -267,8 +264,24 @@ trait WriteOps extends Directives {
} flatMap {
case (old, a) =>
logging.debug(this, s"[PUT] entity created/updated, writing back to datastore")
- factory.put(datastore, a, old) map { _ =>
- a
+ if (overwrite && !unlock && old.getOrElse(None).isInstanceOf[WhiskAction]) {
+ val oldWhiskAction = old.getOrElse(None).asInstanceOf[WhiskAction]
+ oldWhiskAction.annotations.get(WhiskAction.lockFieldName) match {
+ case Some(value) if (value.convertTo[Boolean]) => {
+ Future failed RejectRequest(
+ MethodNotAllowed,
+ s"this action can't be updated until ${WhiskAction.lockFieldName} annotation is updated to false")
+ }
+ case _ => {
+ factory.put(datastore, a, old) map { _ =>
+ a
+ }
+ }
+ }
+ } else {
+ factory.put(datastore, a, old) map { _ =>
+ a
+ }
}
}) {
case Success(entity) =>
@@ -322,11 +335,30 @@ trait WriteOps extends Directives {
notifier: Option[CacheChangeNotification],
ma: Manifest[A]) = {
onComplete(factory.get(datastore, docid) flatMap { entity =>
- confirm(entity) flatMap {
- case _ =>
- factory.del(datastore, entity.docinfo) map { _ =>
- entity
+ if (entity.isInstanceOf[WhiskAction]) {
+ val whiskAction = entity.asInstanceOf[WhiskAction]
+ whiskAction.annotations.get(WhiskAction.lockFieldName) match {
+ case Some(value) if (value.convertTo[Boolean]) => {
+ Future failed RejectRequest(
+ MethodNotAllowed,
+ s"this action can't be deleted until ${WhiskAction.lockFieldName} annotation is updated to false")
+ }
+ case _ => {
+ confirm(entity) flatMap {
+ case _ =>
+ factory.del(datastore, entity.docinfo) map { _ =>
+ entity
+ }
+ }
}
+ }
+ } else {
+ confirm(entity) flatMap {
+ case _ =>
+ factory.del(datastore, entity.docinfo) map { _ =>
+ entity
+ }
+ }
}
}) {
case Success(entity) =>
diff --git a/docs/actions.md b/docs/actions.md
index 658068716f..b45bcae6ad 100644
--- a/docs/actions.md
+++ b/docs/actions.md
@@ -596,6 +596,23 @@ You can clean up by deleting actions that you do not want to use.
actions
/guest/mySequence private sequence
```
+## Protect action updated or deleted by mistake
+
+If your action is very important, you can add `--annotation lock true` to protect it
+```
+wsk action create greeting greeting.js --annotation lock true
+```
+Then update or delete this action will be not allowed
+
+You can add `{"unlock":true}` to enable `update or delete operation`, for example:
+```
+curl -X PUT -H "Content-type: application/json"
+--user 23bc46b1-71f6-4ed5-8c54-816aa4f8c502:123zO3xZCLrMN6v2BKK1dXYFpXlPkccOFqm12CdAsMgRU4VrNZ9lyGVCGuMDGIwP
+-d '{"unlock":true}'
+'/api/v1/namespaces/guest/actions/hello?overwrite=true'
+```
+
+TODO(add unlock operation to wsk, for example: wsk action update hello --unlock)
## Accessing action metadata within the action body
diff --git a/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala b/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala
index fe4d2171e2..67c474ed0a 100644
--- a/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala
@@ -401,6 +401,70 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
}
}
+ it should "update action not allowed when lock annotation is true" in {
+ implicit val tid = transid()
+ val action =
+ WhiskAction(
+ namespace,
+ aname(),
+ jsDefault(""),
+ annotations = Parameters(WhiskAction.lockFieldName, JsBoolean(true)))
+ val content = JsObject(
+ "exec" -> JsObject("code" -> "".toJson, "kind" -> action.exec.kind.toJson),
+ "annotations" -> action.annotations.toJson)
+ Put(s"$collectionPath/${action.name}", content) ~> Route.seal(routes(creds)) ~> check {
+ status should be(OK)
+ }
+
+ //Update not allowed
+ Put(s"$collectionPath/${action.name}?overwrite=true", content) ~> Route.seal(routes(creds)) ~> check {
+ status should be(MethodNotAllowed)
+ }
+
+ //unlock the action
+ val unlockContent = s"""{"unlock":true}""".stripMargin.parseJson.asJsObject
+ Put(s"$collectionPath/${action.name}?overwrite=true", unlockContent) ~> Route.seal(routes(creds)) ~> check {
+ status should be(OK)
+ }
+
+ //Update allowed after unlock the action
+ Put(s"$collectionPath/${action.name}?overwrite=true", content) ~> Route.seal(routes(creds)) ~> check {
+ status should be(OK)
+ }
+ }
+
+ it should "delete action not allowed when lock annotation is true" in {
+ implicit val tid = transid()
+ val action =
+ WhiskAction(
+ namespace,
+ aname(),
+ jsDefault(""),
+ annotations = Parameters(WhiskAction.lockFieldName, JsBoolean(true)))
+ val content = JsObject(
+ "exec" -> JsObject("code" -> "".toJson, "kind" -> action.exec.kind.toJson),
+ "annotations" -> action.annotations.toJson)
+ Put(s"$collectionPath/${action.name}", content) ~> Route.seal(routes(creds)) ~> check {
+ status should be(OK)
+ }
+
+ //Delete not allowed
+ Delete(s"$collectionPath/${action.name}") ~> Route.seal(routes(creds)) ~> check {
+ status should be(MethodNotAllowed)
+ }
+
+ //unlock the action
+ val unlockContent = s"""{"unlock":true}""".stripMargin.parseJson.asJsObject
+ Put(s"$collectionPath/${action.name}?overwrite=true", unlockContent) ~> Route.seal(routes(creds)) ~> check {
+ status should be(OK)
+ }
+
+ //Delete allowed after unlock the action
+ Delete(s"$collectionPath/${action.name}") ~> Route.seal(routes(creds)) ~> check {
+ status should be(OK)
+ }
+ }
+
it should "report NotFound for delete non existent action" in {
implicit val tid = transid()
Delete(s"$collectionPath/xyz") ~> Route.seal(routes(creds)) ~> check {
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services