Add protect feature to avoid 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 deletion by mistake.

Author: ningyougang

core/controller/src/main/scala/whisk/core/controller/Actions.scala

@@ -184,7 +184,7 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
    * - 500 Internal Server Error
    */
   override def create(user: Identity, entityName: FullyQualifiedEntityName)(implicit transid: TransactionId) = {
-    parameter('overwrite ? false) { overwrite =>
+    parameter('overwrite ? false, 'protection ? false) { (overwrite, protection) =>
       entity(as[WhiskActionPut]) { content =>
         val request = content.resolve(user.namespace)
         val checkAdditionalPrivileges = entitleReferencedEntities(user, Privilege.READ, request.exec).flatMap {
@@ -193,9 +193,15 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
 
         onComplete(checkAdditionalPrivileges) {
           case Success(_) =>
-            putEntity(WhiskAction, entityStore, entityName.toDocId, overwrite, update(user, request) _, () => {
-              make(user, entityName, request)
-            })
+            putEntity(
+              WhiskAction,
+              entityStore,
+              entityName.toDocId,
+              overwrite,
+              update(user, request, protection) _,
+              () => {
+                make(user, entityName, request, protection)
+              })
           case Failure(f) =>
             super.handleEntitlementFailure(f)
         }
@@ -307,7 +313,21 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
    * - 500 Internal Server Error
    */
   override def remove(user: Identity, entityName: FullyQualifiedEntityName)(implicit transid: TransactionId) = {
-    deleteEntity(WhiskAction, entityStore, entityName.toDocId, (a: WhiskAction) => Future.successful({}))
+    getEntity(WhiskAction.get(entityStore, entityName.toDocId), Some {
+      action: WhiskAction =>
+        action.annotations.get("protection") match {
+          case Some(value) => {
+            if (value.convertTo[String] == "false") {
+              deleteEntity(WhiskAction, entityStore, entityName.toDocId, (a: WhiskAction) => Future.successful({}))
+            } else {
+              complete(MethodNotAllowed, "this action can't be deleted until protection annotation is updated to false")
+            }
+          }
+          case None => {
+            deleteEntity(WhiskAction, entityStore, entityName.toDocId, (a: WhiskAction) => Future.successful({}))
+          }
+        }
+    })
   }
 
   /**
@@ -387,7 +407,7 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
   /**
    * Creates a WhiskAction instance from the PUT request.
    */
-  private def makeWhiskAction(content: WhiskActionPut, entityName: FullyQualifiedEntityName)(
+  private def makeWhiskAction(content: WhiskActionPut, entityName: FullyQualifiedEntityName, protection: Boolean)(
     implicit transid: TransactionId) = {
     val exec = content.exec.get
     val limits = content.limits map { l =>
@@ -412,7 +432,9 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
       limits,
       content.version getOrElse SemVer(),
       content.publish getOrElse false,
-      (content.annotations getOrElse Parameters()) ++ execAnnotation(exec))
+      (content.annotations getOrElse Parameters()) ++ execAnnotation(exec) ++ Parameters(
+        "protection",
+        if (protection) "true" else "false"))
   }
 
   /** For a sequence action, gather referenced entities and authorize access. */
@@ -437,37 +459,38 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
   }
 
   /** Creates a WhiskAction from PUT content, generating default values where necessary. */
-  private def make(user: Identity, entityName: FullyQualifiedEntityName, content: WhiskActionPut)(
+  private def make(user: Identity, entityName: FullyQualifiedEntityName, content: WhiskActionPut, protection: Boolean)(
     implicit transid: TransactionId) = {
     content.exec map {
       case seq: SequenceExec =>
         // check that the sequence conforms to max length and no recursion rules
         checkSequenceActionLimits(entityName, seq.components) map { _ =>
-          makeWhiskAction(content.replace(seq), entityName)
+          makeWhiskAction(content.replace(seq), entityName, protection)
         }
       case supportedExec if !supportedExec.deprecated =>
-        Future successful makeWhiskAction(content, entityName)
+        Future successful makeWhiskAction(content, entityName, protection)
       case deprecatedExec =>
         Future failed RejectRequest(BadRequest, runtimeDeprecated(deprecatedExec))
 
     } getOrElse Future.failed(RejectRequest(BadRequest, "exec undefined"))
   }
 
   /** Updates a WhiskAction from PUT content, merging old action where necessary. */
-  private def update(user: Identity, content: WhiskActionPut)(action: WhiskAction)(implicit transid: TransactionId) = {
+  private def update(user: Identity, content: WhiskActionPut, protection: Boolean)(action: WhiskAction)(
+    implicit transid: TransactionId) = {
     content.exec map {
       case seq: SequenceExec =>
         // check that the sequence conforms to max length and no recursion rules
         checkSequenceActionLimits(FullyQualifiedEntityName(action.namespace, action.name), seq.components) map { _ =>
-          updateWhiskAction(content.replace(seq), action)
+          updateWhiskAction(content.replace(seq), action, protection)
         }
       case supportedExec if !supportedExec.deprecated =>
-        Future successful updateWhiskAction(content, action)
+        Future successful updateWhiskAction(content, action, protection)
       case deprecatedExec =>
         Future failed RejectRequest(BadRequest, runtimeDeprecated(deprecatedExec))
     } getOrElse {
       if (!action.exec.deprecated) {
-        Future successful updateWhiskAction(content, action)
+        Future successful updateWhiskAction(content, action, protection)
       } else {
         Future failed RejectRequest(BadRequest, runtimeDeprecated(action.exec))
       }
@@ -477,7 +500,8 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
   /**
    * Updates a WhiskAction instance from the PUT request.
    */
-  private def updateWhiskAction(content: WhiskActionPut, action: WhiskAction)(implicit transid: TransactionId) = {
+  private def updateWhiskAction(content: WhiskActionPut, action: WhiskAction, protection: Boolean)(
+    implicit transid: TransactionId) = {
     val limits = content.limits map { l =>
       ActionLimits(
         l.timeout getOrElse action.limits.timeout,
@@ -526,7 +550,9 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
       limits,
       content.version getOrElse action.version.upPatch,
       content.publish getOrElse action.publish,
-      (content.annotations getOrElse action.annotations) ++ execAnnotation(exec))
+      (content.annotations getOrElse action.annotations) ++ execAnnotation(exec) ++ Parameters(
+        "protection",
+        if (protection) "true" else "false"))
       .revision[WhiskAction](action.docinfo.rev)
   }
 

tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala

@@ -401,6 +401,27 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
     }
   }
 
+  it should "delete action not allowed when protection annotation is true" in {
+    implicit val tid = transid()
+    val action = WhiskAction(namespace, aname(), jsDefault(""))
+    val content = JsObject("exec" -> JsObject("code" -> "".toJson, "kind" -> action.exec.kind.toJson))
+    Put(s"$collectionPath/${action.name}?protection=true", content) ~> Route.seal(routes(creds)) ~> check {
+      status should be(OK)
+    }
+
+    Delete(s"$collectionPath/${action.name}") ~> Route.seal(routes(creds)) ~> check {
+      status should be(MethodNotAllowed)
+    }
+
+    Put(s"$collectionPath/${action.name}?overwrite=true&protection=false", content) ~> Route.seal(routes(creds)) ~> check {
+      status should be(OK)
+    }
+
+    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 {