Skip to content

Commit

Permalink
Add protect feature to avoid delete actions by mistake
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@ningyougang
ningyougang committed Oct 8, 2018
1 parent 5da62ed commit 3b7a7ed
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 16 deletions.
58 changes: 42 additions & 16 deletions core/controller/src/main/scala/whisk/core/controller/Actions.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}
Expand Down Expand Up @@ -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({}))
}
}
})
}

/**
Expand Down Expand Up @@ -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 =>
Expand All @@ -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. */
Expand All @@ -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))
}
Expand All @@ -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,
Expand Down Expand Up @@ -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)
}

Expand Down
21 changes: 21 additions & 0 deletions tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 3b7a7ed

Please sign in to comment.