You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by du...@apache.org on 2018/01/05 18:23:14 UTC

[incubator-openwhisk] branch master updated: Disallow create and update of package with reserved names. (#3147)

This is an automated email from the ASF dual-hosted git repository.

dubeejw pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-openwhisk.git


The following commit(s) were added to refs/heads/master by this push:
     new d10a83f  Disallow create and update of package with reserved names. (#3147)
d10a83f is described below

commit d10a83f81ada055513e31c5ab004e8093be7769a
Author: rodric rabbah <ro...@gmail.com>
AuthorDate: Fri Jan 5 13:23:10 2018 -0500

    Disallow create and update of package with reserved names. (#3147)
    
    * A get and delete are still allowed.
---
 .../src/main/scala/whisk/http/ErrorResponse.scala  |  4 +--
 .../scala/whisk/core/controller/Packages.scala     | 11 +++---
 .../core/controller/test/PackagesApiTests.scala    | 39 +++++++++++++++-------
 3 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/common/scala/src/main/scala/whisk/http/ErrorResponse.scala b/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
index 8dcfe20..733c8db 100644
--- a/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
+++ b/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
@@ -109,9 +109,7 @@ object Messages {
   val bindingCannotReferenceBinding = "Cannot bind to another package binding."
   val requestedBindingIsNotValid = "Cannot bind to a resource that is not a package."
   val notAllowedOnBinding = "Operation not permitted on package binding."
-  def packageNameIsReserved(name: String) = {
-    s"Package name '$name' is reserved."
-  }
+  def packageNameIsReserved(name: String) = s"Package name '$name' is reserved."
 
   /** Error messages for sequence activations. */
   def sequenceRetrieveActivationTimeout(id: ActivationId) =
diff --git a/core/controller/src/main/scala/whisk/core/controller/Packages.scala b/core/controller/src/main/scala/whisk/core/controller/Packages.scala
index e97f316..74b2d93 100644
--- a/core/controller/src/main/scala/whisk/core/controller/Packages.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/Packages.scala
@@ -43,8 +43,6 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
 
   protected override val collection = Collection(Collection.PACKAGES)
 
-  protected[core] val RESERVED_NAMES = Array("default")
-
   /** Database service to CRUD packages. */
   protected val entityStore: EntityStore
 
@@ -60,6 +58,9 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
   /** JSON response formatter. */
   import RestApiCommons.jsonDefaultResponsePrinter
 
+  /** Reserved package names. */
+  protected[core] val RESERVED_NAMES = Set("default")
+
   /**
    * Creates or updates package/binding if it already exists. The PUT content is deserialized into a
    * WhiskPackagePut which is a subset of WhiskPackage (it eschews the namespace and entity name since
@@ -78,9 +79,7 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
    */
   override def create(user: Identity, entityName: FullyQualifiedEntityName)(implicit transid: TransactionId) = {
     parameter('overwrite ? false) { overwrite =>
-      if (!overwrite && (RESERVED_NAMES contains entityName.name.asString)) {
-        terminate(BadRequest, Messages.packageNameIsReserved(entityName.name.asString))
-      } else {
+      if (!RESERVED_NAMES.contains(entityName.name.asString)) {
         entity(as[WhiskPackagePut]) { content =>
           val request = content.resolve(entityName.namespace)
 
@@ -102,6 +101,8 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
               rewriteEntitlementFailure(f)
           }
         }
+      } else {
+        terminate(BadRequest, Messages.packageNameIsReserved(entityName.name.asString))
       }
     }
   }
diff --git a/tests/src/test/scala/whisk/core/controller/test/PackagesApiTests.scala b/tests/src/test/scala/whisk/core/controller/test/PackagesApiTests.scala
index aad482d..1e4c389 100644
--- a/tests/src/test/scala/whisk/core/controller/test/PackagesApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/PackagesApiTests.scala
@@ -369,30 +369,45 @@ class PackagesApiTests extends ControllerTestCommon with WhiskPackagesApi {
     }
   }
 
-  it should "reject create package when package name is a reserved name" in {
+  it should "reject create/update package when package name is reserved" in {
+    implicit val tid = transid()
+    Set(true, false) foreach { overwrite =>
+      RESERVED_NAMES foreach { reservedName =>
+        val provider = WhiskPackage(namespace, EntityName(reservedName), None)
+        val content = WhiskPackagePut()
+        Put(s"$collectionPath/${provider.name}?overwrite=$overwrite", content) ~> Route.seal(routes(creds)) ~> check {
+          status should be(BadRequest)
+          responseAs[ErrorResponse].error shouldBe Messages.packageNameIsReserved(reservedName)
+        }
+      }
+    }
+  }
+
+  it should "not allow package update of pre-existing package with a reserved" in {
     implicit val tid = transid()
     RESERVED_NAMES foreach { reservedName =>
-      val provider = WhiskPackage(namespace, EntityName(s"$reservedName"), None)
+      val provider = WhiskPackage(namespace, EntityName(reservedName), None)
+      put(entityStore, provider)
       val content = WhiskPackagePut()
-      Put(s"$collectionPath/${provider.name}", content) ~> Route.seal(routes(creds)) ~> check {
+      Put(s"$collectionPath/${provider.name}?overwrite=true", content) ~> Route.seal(routes(creds)) ~> check {
         status should be(BadRequest)
-        val response = responseAs[String]
-        response should include {
-          Messages.packageNameIsReserved(reservedName)
-        }
+        responseAs[ErrorResponse].error shouldBe Messages.packageNameIsReserved(reservedName)
       }
     }
   }
 
-  it should "allow package update even when package name a reserved name" in {
+  it should "allow package get/delete for pre-existing package with a reserved name" in {
     implicit val tid = transid()
     RESERVED_NAMES foreach { reservedName =>
-      val provider = WhiskPackage(namespace, EntityName(s"$reservedName"), None)
+      val provider = WhiskPackage(namespace, EntityName(reservedName), None)
+      put(entityStore, provider, garbageCollect = false)
       val content = WhiskPackagePut()
-      Put(s"$collectionPath/${provider.name}?overwrite=true", content) ~> Route.seal(routes(creds)) ~> check {
+      Get(s"$collectionPath/${provider.name}") ~> Route.seal(routes(creds)) ~> check {
+        status should be(OK)
+        responseAs[WhiskPackage] shouldBe provider
+      }
+      Delete(s"$collectionPath/${provider.name}") ~> Route.seal(routes(creds)) ~> check {
         status should be(OK)
-        val response = responseAs[WhiskPackage]
-        response should be(provider)
       }
     }
   }

-- 
To stop receiving notification emails like this one, please contact
['"commits@openwhisk.apache.org" <co...@openwhisk.apache.org>'].