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 <rod...@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" <commits@openwhisk.apache.org>'].