From 534eecec041342c9446d79c6f2dabddd804e878b Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Tue, 21 Aug 2018 16:24:06 +0530 Subject: [PATCH 1/8] Use attachments with BlackBoxExec --- .../scala/whisk/core/entity/Attachments.scala | 7 +++ .../main/scala/whisk/core/entity/Exec.scala | 58 +++++++++++------- .../scala/whisk/core/entity/WhiskAction.scala | 59 +++++++++++++++---- .../behavior/ArtifactStoreBehaviorBase.scala | 2 +- .../whisk/core/entity/test/ExecHelpers.scala | 9 +-- 5 files changed, 96 insertions(+), 39 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/entity/Attachments.scala b/common/scala/src/main/scala/whisk/core/entity/Attachments.scala index 2d00a03deb2..dc2c837456b 100644 --- a/common/scala/src/main/scala/whisk/core/entity/Attachments.scala +++ b/common/scala/src/main/scala/whisk/core/entity/Attachments.scala @@ -54,6 +54,13 @@ object Attachments { } } + implicit class OptionSizeAttachment[T <% SizeConversion](a: Option[Attachment[T]]) extends SizeConversion { + def sizeIn(unit: SizeUnits.Unit): ByteSize = a match { + case Some(Inline(v)) => (v: SizeConversion).sizeIn(unit) + case _ => 0.bytes + } + } + object Attached { implicit val serdes = { implicit val contentTypeSerdes = new RootJsonFormat[ContentType] { diff --git a/common/scala/src/main/scala/whisk/core/entity/Exec.scala b/common/scala/src/main/scala/whisk/core/entity/Exec.scala index c98e57188dc..8e5d8f62136 100644 --- a/common/scala/src/main/scala/whisk/core/entity/Exec.scala +++ b/common/scala/src/main/scala/whisk/core/entity/Exec.scala @@ -27,7 +27,6 @@ import spray.json.DefaultJsonProtocol._ import whisk.core.entity.Attachments._ import whisk.core.entity.ExecManifest._ import whisk.core.entity.size.SizeInt -import whisk.core.entity.size.SizeOptionString import whisk.core.entity.size.SizeString /** @@ -115,6 +114,11 @@ sealed abstract class ExecMetaData extends ExecMetaDataBase { override def size = 0.B } +trait AttachedCode { + def inline(bytes: Array[Byte]): Exec + def attach(attached: Attached): Exec +} + protected[core] case class CodeExecAsString(manifest: RuntimeManifest, override val code: String, override val entryPoint: Option[String]) @@ -142,7 +146,8 @@ protected[core] case class CodeExecAsAttachment(manifest: RuntimeManifest, override val code: Attachment[String], override val entryPoint: Option[String], override val binary: Boolean = false) - extends CodeExec[Attachment[String]] { + extends CodeExec[Attachment[String]] + with AttachedCode { override val kind = manifest.kind override val image = manifest.image override val sentinelledLogs = manifest.sentinelledLogs.getOrElse(true) @@ -150,12 +155,12 @@ protected[core] case class CodeExecAsAttachment(manifest: RuntimeManifest, override val pull = false override def codeAsJson = code.toJson - def inline(bytes: Array[Byte]): CodeExecAsAttachment = { + override def inline(bytes: Array[Byte]): CodeExecAsAttachment = { val encoded = new String(bytes, StandardCharsets.UTF_8) copy(code = Inline(encoded)) } - def attach(attached: Attached): CodeExecAsAttachment = { + override def attach(attached: Attached): CodeExecAsAttachment = { copy(code = attached) } } @@ -175,17 +180,27 @@ protected[core] case class CodeExecMetaDataAsAttachment(manifest: RuntimeManifes * @param code an optional script or zip archive (as base64 encoded) string */ protected[core] case class BlackBoxExec(override val image: ImageName, - override val code: Option[String], + override val code: Option[Attachment[String]], override val entryPoint: Option[String], - val native: Boolean) - extends CodeExec[Option[String]] { + val native: Boolean, + override val binary: Boolean) + extends CodeExec[Option[Attachment[String]]] + with AttachedCode { override val kind = Exec.BLACKBOX override val deprecated = false override def codeAsJson = code.toJson - override lazy val binary = code map { Exec.isBinaryCode(_) } getOrElse false override val sentinelledLogs = native override val pull = !native override def size = super.size + image.publicImageName.sizeInBytes + + override def inline(bytes: Array[Byte]): BlackBoxExec = { + val encoded = new String(bytes, StandardCharsets.UTF_8) + copy(code = Some(Inline(encoded))) + } + + override def attach(attached: Attached): BlackBoxExec = { + copy(code = Some(attached)) + } } protected[core] case class BlackBoxExecMetaData(override val image: ImageName, @@ -244,7 +259,7 @@ protected[core] object Exec extends ArgNormalizer[Exec] with DefaultJsonProtocol case b: BlackBoxExec => val base = Map("kind" -> JsString(b.kind), "image" -> JsString(b.image.publicImageName), "binary" -> JsBoolean(b.binary)) - val code = b.code.filter(_.trim.nonEmpty).map("code" -> JsString(_)) + val code = b.code.map("code" -> attFmt[String].write(_)) val main = b.entryPoint.map("main" -> JsString(_)) JsObject(base ++ code ++ main) case _ => JsObject.empty @@ -283,15 +298,13 @@ protected[core] object Exec extends ArgNormalizer[Exec] with DefaultJsonProtocol throw new DeserializationException( s"'image' must be a string defined in 'exec' for '${Exec.BLACKBOX}' actions") } - val code: Option[String] = obj.fields.get("code") match { - case Some(JsString(i)) => if (i.trim.nonEmpty) Some(i) else None - case Some(_) => - throw new DeserializationException( - s"if defined, 'code' must a string defined in 'exec' for '${Exec.BLACKBOX}' actions") - case None => None + val (codeOpt: Option[Attachment[String]], binary) = obj.fields.get("code") match { + case None => (None, false) + case Some(JsString(i)) if i.trim.isEmpty => (None, false) + case Some(code) => (Some(attFmt[String].read(code)), isBinary(code, obj)) } val native = execManifests.skipDockerPull(image) - BlackBoxExec(image, code, optMainField, native) + BlackBoxExec(image, codeOpt, optMainField, native, binary) case _ => // map "default" virtual runtime versions to the currently blessed actual runtime version @@ -307,10 +320,6 @@ protected[core] object Exec extends ArgNormalizer[Exec] with DefaultJsonProtocol throw new DeserializationException( s"'code' must be a string or attachment object defined in 'exec' for '$kind' actions") } - val binary: Boolean = code match { - case JsString(c) => isBinaryCode(c) - case _ => obj.fields.get("binary").map(_.convertTo[Boolean]).getOrElse(false) - } val main = optMainField.orElse { if (manifest.requireMain.exists(identity)) { @@ -318,7 +327,7 @@ protected[core] object Exec extends ArgNormalizer[Exec] with DefaultJsonProtocol } else None } - CodeExecAsAttachment(manifest, attFmt[String].read(code), main, binary) + CodeExecAsAttachment(manifest, attFmt[String].read(code), main, isBinary(code, obj)) } .getOrElse { val code: String = obj.fields.get("code") match { @@ -340,6 +349,13 @@ protected[core] object Exec extends ArgNormalizer[Exec] with DefaultJsonProtocol (t.length > 0) && (t.length % 4 == 0) && isBase64Pattern.matcher(t).matches() } else false } + + private def isBinary(code: JsValue, obj: JsObject): Boolean = { + code match { + case JsString(c) => isBinaryCode(c) + case _ => obj.fields.get("binary").map(_.convertTo[Boolean]).getOrElse(false) + } + } } protected[core] object ExecMetaDataBase extends ArgNormalizer[ExecMetaDataBase] with DefaultJsonProtocol { 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 fd4d5461a49..8dcad45c908 100644 --- a/common/scala/src/main/scala/whisk/core/entity/WhiskAction.scala +++ b/common/scala/src/main/scala/whisk/core/entity/WhiskAction.scala @@ -17,9 +17,8 @@ package whisk.core.entity -import java.io.ByteArrayInputStream -import java.io.ByteArrayOutputStream -import java.nio.charset.StandardCharsets +import java.io.{ByteArrayInputStream, ByteArrayOutputStream} +import java.nio.charset.StandardCharsets.UTF_8 import java.util.Base64 import akka.http.scaladsl.model.ContentTypes @@ -343,7 +342,7 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[ val (bytes, attachmentType) = if (binary) { (Base64.getDecoder.decode(code), ContentTypes.`application/octet-stream`) } else { - (code.getBytes(StandardCharsets.UTF_8), ContentTypes.`text/plain(UTF-8)`) + (code.getBytes(UTF_8), ContentTypes.`text/plain(UTF-8)`) } val stream = new ByteArrayInputStream(bytes) val oldAttachment = old @@ -352,17 +351,28 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[ case _ => None }) - super.putAndAttach( - db, - doc, - (d, a) => d.copy(exec = exec.attach(a)).revision[WhiskAction](d.rev), - attachmentType, - stream, - oldAttachment, - Some { a: WhiskAction => - a.copy(exec = exec.inline(code.getBytes("UTF-8"))) + super.putAndAttach(db, doc, attachmentUpdater, attachmentType, stream, oldAttachment, Some { a: WhiskAction => + a.copy(exec = exec.inline(code.getBytes(UTF_8))) + }) + case exec @ BlackBoxExec(_, Some(Inline(code)), _, _, binary) => + implicit val logger = db.logging + implicit val ec = db.executionContext + + val (bytes, attachmentType) = if (binary) { + (Base64.getDecoder.decode(code), ContentTypes.`application/octet-stream`) + } else { + (code.getBytes(UTF_8), ContentTypes.`text/plain(UTF-8)`) + } + val stream = new ByteArrayInputStream(bytes) + val oldAttachment = old + .flatMap(_.exec match { + case BlackBoxExec(_, Some(a: Attached), _, _, _) => Some(a) + case _ => None }) + super.putAndAttach(db, doc, attachmentUpdater, attachmentType, stream, oldAttachment, Some { a: WhiskAction => + a.copy(exec = exec.inline(code.getBytes(UTF_8))) + }) case _ => super.put(db, doc, old) } @@ -389,6 +399,16 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[ val boas = new ByteArrayOutputStream() val wrapped = if (binary) Base64.getEncoder().wrap(boas) else boas + getAttachment[A](db, action, attached, wrapped, Some { a: WhiskAction => + wrapped.close() + val newAction = a.copy(exec = exec.inline(boas.toByteArray)) + newAction.revision(a.rev) + newAction + }) + case exec @ BlackBoxExec(_, Some(attached: Attached), _, _, binary) => + val boas = new ByteArrayOutputStream() + val wrapped = if (binary) Base64.getEncoder().wrap(boas) else boas + getAttachment[A](db, action, attached, wrapped, Some { a: WhiskAction => wrapped.close() val newAction = a.copy(exec = exec.inline(boas.toByteArray)) @@ -409,11 +429,24 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[ attachmentName == attached.attachmentName, s"Attachment name '${attached.attachmentName}' does not match the expected name '$attachmentName'") exec.attach(attached) + case exec @ BlackBoxExec(_, Some(Attached(attachmentName, _, _, _)), _, _, _) => + require( + attachmentName == attached.attachmentName, + s"Attachment name '${attached.attachmentName}' does not match the expected name '$attachmentName'") + exec.attach(attached) case exec => exec } action.copy(exec = eu).revision[WhiskAction](action.rev) } + def attachmentUpdater(action: WhiskAction, updatedAttachment: Attached): WhiskAction = { + action.exec match { + case exec: AttachedCode => + action.copy(exec = exec.attach(updatedAttachment)).revision[WhiskAction](action.rev) + case _ => action + } + } + override def del[Wsuper >: WhiskAction](db: ArtifactStore[Wsuper], doc: DocInfo)( implicit transid: TransactionId, notifier: Option[CacheChangeNotification]): Future[Boolean] = { diff --git a/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala b/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala index d4bcd5c3d0f..8b6e86ee55c 100644 --- a/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala +++ b/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala @@ -125,7 +125,7 @@ trait ArtifactStoreBehaviorBase WhiskNamespace(Namespace(EntityName(name), uuid), BasicAuthenticationAuthKey(uuid, Secret())) } - private val exec = BlackBoxExec(ExecManifest.ImageName("image"), None, None, native = false) + private val exec = BlackBoxExec(ExecManifest.ImageName("image"), None, None, native = false, binary = false) protected def newAction(ns: EntityPath): WhiskAction = { WhiskAction(ns, aname(), exec) diff --git a/tests/src/test/scala/whisk/core/entity/test/ExecHelpers.scala b/tests/src/test/scala/whisk/core/entity/test/ExecHelpers.scala index fa8fd2ec77c..9fb96577a1b 100644 --- a/tests/src/test/scala/whisk/core/entity/test/ExecHelpers.scala +++ b/tests/src/test/scala/whisk/core/entity/test/ExecHelpers.scala @@ -19,7 +19,6 @@ package whisk.core.entity.test import org.scalatest.Matchers import org.scalatest.Suite - import common.StreamLogging import common.WskActorSystem import whisk.core.WhiskConfig @@ -27,7 +26,6 @@ import whisk.core.entity._ import whisk.core.entity.ArgNormalizer.trim import whisk.core.entity.ExecManifest._ import whisk.core.entity.size._ - import spray.json._ import spray.json.DefaultJsonProtocol._ @@ -136,10 +134,13 @@ trait ExecHelpers extends Matchers with WskActorSystem with StreamLogging { protected def sequenceMetaData(components: Vector[FullyQualifiedEntityName]) = SequenceExecMetaData(components) - protected def bb(image: String) = BlackBoxExec(ExecManifest.ImageName(trim(image)), None, None, false) + protected def bb(image: String) = BlackBoxExec(ExecManifest.ImageName(trim(image)), None, None, false, false) protected def bb(image: String, code: String, main: Option[String] = None) = { - BlackBoxExec(ExecManifest.ImageName(trim(image)), Some(trim(code)).filter(_.nonEmpty), main, false) + val (codeOpt, binary) = + if (code.trim.nonEmpty) (Some(attFmt[String].read(code.toJson)), Exec.isBinaryCode(code)) + else (None, false) + BlackBoxExec(ExecManifest.ImageName(trim(image)), codeOpt, main, false, binary) } protected def blackBoxMetaData(image: String, main: Option[String] = None, binary: Boolean) = { From a34e0fe671ffd9b5a2b417ef3d8fde7b090070d8 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Tue, 21 Aug 2018 17:08:07 +0530 Subject: [PATCH 2/8] Simplify code to avoid code duplication --- .../scala/whisk/core/entity/WhiskAction.scala | 108 ++++++++---------- 1 file changed, 47 insertions(+), 61 deletions(-) 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 8dcad45c908..55e02a777f0 100644 --- a/common/scala/src/main/scala/whisk/core/entity/WhiskAction.scala +++ b/common/scala/src/main/scala/whisk/core/entity/WhiskAction.scala @@ -330,49 +330,31 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[ implicit transid: TransactionId, notifier: Option[CacheChangeNotification]): Future[DocInfo] = { + def putWithAttachment(code: String, binary: Boolean, exec: AttachedCode) = { + implicit val logger = db.logging + implicit val ec = db.executionContext + + val oldAttachment = old.flatMap(getAttachment) + val (bytes, attachmentType) = if (binary) { + (Base64.getDecoder.decode(code), ContentTypes.`application/octet-stream`) + } else { + (code.getBytes(UTF_8), ContentTypes.`text/plain(UTF-8)`) + } + val stream = new ByteArrayInputStream(bytes) + super.putAndAttach(db, doc, attachmentUpdater, attachmentType, stream, oldAttachment, Some { a: WhiskAction => + a.copy(exec = exec.inline(code.getBytes(UTF_8))) + }) + } + Try { require(db != null, "db undefined") require(doc != null, "doc undefined") } map { _ => doc.exec match { case exec @ CodeExecAsAttachment(_, Inline(code), _, binary) => - implicit val logger = db.logging - implicit val ec = db.executionContext - - val (bytes, attachmentType) = if (binary) { - (Base64.getDecoder.decode(code), ContentTypes.`application/octet-stream`) - } else { - (code.getBytes(UTF_8), ContentTypes.`text/plain(UTF-8)`) - } - val stream = new ByteArrayInputStream(bytes) - val oldAttachment = old - .flatMap(_.exec match { - case CodeExecAsAttachment(_, a: Attached, _, _) => Some(a) - case _ => None - }) - - super.putAndAttach(db, doc, attachmentUpdater, attachmentType, stream, oldAttachment, Some { a: WhiskAction => - a.copy(exec = exec.inline(code.getBytes(UTF_8))) - }) + putWithAttachment(code, binary, exec) case exec @ BlackBoxExec(_, Some(Inline(code)), _, _, binary) => - implicit val logger = db.logging - implicit val ec = db.executionContext - - val (bytes, attachmentType) = if (binary) { - (Base64.getDecoder.decode(code), ContentTypes.`application/octet-stream`) - } else { - (code.getBytes(UTF_8), ContentTypes.`text/plain(UTF-8)`) - } - val stream = new ByteArrayInputStream(bytes) - val oldAttachment = old - .flatMap(_.exec match { - case BlackBoxExec(_, Some(a: Attached), _, _, _) => Some(a) - case _ => None - }) - - super.putAndAttach(db, doc, attachmentUpdater, attachmentType, stream, oldAttachment, Some { a: WhiskAction => - a.copy(exec = exec.inline(code.getBytes(UTF_8))) - }) + putWithAttachment(code, binary, exec) case _ => super.put(db, doc, old) } @@ -394,28 +376,23 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[ val fa = super.getWithAttachment(db, doc, rev, fromCache, Some(attachmentHandler _)) fa.flatMap { action => + def getWithAttachment(attached: Attached, binary: Boolean, exec: AttachedCode) = { + val boas = new ByteArrayOutputStream() + val wrapped = if (binary) Base64.getEncoder().wrap(boas) else boas + + getAttachment[A](db, action, attached, wrapped, Some { a: WhiskAction => + wrapped.close() + val newAction = a.copy(exec = exec.inline(boas.toByteArray)) + newAction.revision(a.rev) + newAction + }) + } + action.exec match { case exec @ CodeExecAsAttachment(_, attached: Attached, _, binary) => - val boas = new ByteArrayOutputStream() - val wrapped = if (binary) Base64.getEncoder().wrap(boas) else boas - - getAttachment[A](db, action, attached, wrapped, Some { a: WhiskAction => - wrapped.close() - val newAction = a.copy(exec = exec.inline(boas.toByteArray)) - newAction.revision(a.rev) - newAction - }) + getWithAttachment(attached, binary, exec) case exec @ BlackBoxExec(_, Some(attached: Attached), _, _, binary) => - val boas = new ByteArrayOutputStream() - val wrapped = if (binary) Base64.getEncoder().wrap(boas) else boas - - getAttachment[A](db, action, attached, wrapped, Some { a: WhiskAction => - wrapped.close() - val newAction = a.copy(exec = exec.inline(boas.toByteArray)) - newAction.revision(a.rev) - newAction - }) - + getWithAttachment(attached, binary, exec) case _ => Future.successful(action) } @@ -423,16 +400,17 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[ } def attachmentHandler(action: WhiskAction, attached: Attached): WhiskAction = { + def checkName(name: String) = { + require( + name == attached.attachmentName, + s"Attachment name '${attached.attachmentName}' does not match the expected name '$name'") + } val eu = action.exec match { case exec @ CodeExecAsAttachment(_, Attached(attachmentName, _, _, _), _, _) => - require( - attachmentName == attached.attachmentName, - s"Attachment name '${attached.attachmentName}' does not match the expected name '$attachmentName'") + checkName(attachmentName) exec.attach(attached) case exec @ BlackBoxExec(_, Some(Attached(attachmentName, _, _, _)), _, _, _) => - require( - attachmentName == attached.attachmentName, - s"Attachment name '${attached.attachmentName}' does not match the expected name '$attachmentName'") + checkName(attachmentName) exec.attach(attached) case exec => exec } @@ -447,6 +425,14 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[ } } + def getAttachment(action: WhiskAction): Option[Attached] = { + action.exec match { + case CodeExecAsAttachment(_, a: Attached, _, _) => Some(a) + case BlackBoxExec(_, Some(a: Attached), _, _, _) => Some(a) + case _ => None + } + } + override def del[Wsuper >: WhiskAction](db: ArtifactStore[Wsuper], doc: DocInfo)( implicit transid: TransactionId, notifier: Option[CacheChangeNotification]): Future[Boolean] = { From 02177304aa6e1547c74c9b36c51838068d5a06cb Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Tue, 21 Aug 2018 17:11:46 +0530 Subject: [PATCH 3/8] Fix test expectation --- .../scala/whisk/core/controller/test/ActionsApiTests.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 4b1401933bc..480d484e4fa 100644 --- a/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala +++ b/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala @@ -41,6 +41,7 @@ import java.util.Base64 import akka.http.scaladsl.model.headers.RawHeader import akka.stream.scaladsl._ +import whisk.core.entity.Attachments.Inline /** * Tests Actions API. @@ -591,7 +592,7 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi { action.annotations ++ Parameters(WhiskAction.execFieldName, Exec.BLACKBOX))) response.exec shouldBe an[BlackBoxExec] val bb = response.exec.asInstanceOf[BlackBoxExec] - bb.code shouldBe Some("cc") + bb.code shouldBe Some(Inline("cc")) bb.binary shouldBe false } } From d41d6148e45bd8f75d67c85cc0d4bb7e065c0c87 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Wed, 22 Aug 2018 11:02:38 +0530 Subject: [PATCH 4/8] Test for blackbox serialization --- .../whisk/core/entity/test/ExecTests.scala | 130 +++++++++++++++++- 1 file changed, 129 insertions(+), 1 deletion(-) diff --git a/tests/src/test/scala/whisk/core/entity/test/ExecTests.scala b/tests/src/test/scala/whisk/core/entity/test/ExecTests.scala index 48a4b0d7d3f..bd002ad48cd 100644 --- a/tests/src/test/scala/whisk/core/entity/test/ExecTests.scala +++ b/tests/src/test/scala/whisk/core/entity/test/ExecTests.scala @@ -17,6 +17,7 @@ package whisk.core.entity.test +import akka.http.scaladsl.model.ContentTypes import common.StreamLogging import spray.json._ import org.junit.runner.RunWith @@ -24,7 +25,8 @@ import org.scalatest.junit.JUnitRunner import org.scalatest.{BeforeAndAfterAll, FlatSpec, Matchers} import whisk.core.WhiskConfig import whisk.core.entity.Attachments.{Attached, Inline} -import whisk.core.entity.{CodeExecAsAttachment, CodeExecAsString, Exec, ExecManifest, WhiskAction} +import whisk.core.entity.ExecManifest.ImageName +import whisk.core.entity.{BlackBoxExec, CodeExecAsAttachment, CodeExecAsString, Exec, ExecManifest, WhiskAction} import scala.collection.mutable @@ -168,6 +170,132 @@ class ExecTests extends FlatSpec with Matchers with StreamLogging with BeforeAnd ExecManifest.initialize(config) } + behavior of "blackbox exec deserialization" + + it should "read existing code string as attachment" in { + val json = """{ + | "name": "action_tests_name2", + | "_id": "anon-Yzycx8QnIYDp3Tby0Fnj23KcMtH/action_tests_name2", + | "publish": false, + | "annotations": [], + | "version": "0.0.1", + | "updated": 1533623651650, + | "entityType": "action", + | "exec": { + | "kind": "blackbox", + | "image": "docker-custom.com/openwhisk-runtime/magic/nodejs:0.0.1", + | "code": "foo", + | "binary": false + | }, + | "parameters": [ + | { + | "key": "x", + | "value": "b" + | } + | ], + | "limits": { + | "timeout": 60000, + | "memory": 256, + | "logs": 10 + | }, + | "namespace": "anon-Yzycx8QnIYDp3Tby0Fnj23KcMtH" + |}""".stripMargin.parseJson.asJsObject + val action = WhiskAction.serdes.read(json) + action.exec should matchPattern { case BlackBoxExec(_, Some(Inline("foo")), None, false, false) => } + } + + it should "properly determine binary property" in { + val j1 = """{ + | "kind": "blackbox", + | "image": "docker-custom.com/openwhisk-runtime/magic/nodejs:0.0.1", + | "code": "SGVsbG8gT3BlbldoaXNr", + | "binary": false + |}""".stripMargin.parseJson.asJsObject + Exec.serdes.read(j1) should matchPattern { + case BlackBoxExec(_, Some(Inline("SGVsbG8gT3BlbldoaXNr")), None, false, true) => + } + + val j2 = """{ + | "kind": "blackbox", + | "image": "docker-custom.com/openwhisk-runtime/magic/nodejs:0.0.1", + | "code": "while (true)", + | "binary": false + |}""".stripMargin.parseJson.asJsObject + Exec.serdes.read(j2) should matchPattern { + case BlackBoxExec(_, Some(Inline("while (true)")), None, false, false) => + } + + //Empty code should resolve as None + val j3 = """{ + | "kind": "blackbox", + | "image": "docker-custom.com/openwhisk-runtime/magic/nodejs:0.0.1", + | "code": " " + |}""".stripMargin.parseJson.asJsObject + Exec.serdes.read(j3) should matchPattern { + case BlackBoxExec(_, None, None, false, false) => + } + + val j4 = """{ + | "kind": "blackbox", + | "image": "docker-custom.com/openwhisk-runtime/magic/nodejs:0.0.1", + | "code": { + | "attachmentName": "foo:bar", + | "attachmentType": "application/octet-stream", + | "length": 32768, + | "digest": "sha256-foo" + | }, + | "binary": true, + | "main": "hello" + |}""".stripMargin.parseJson.asJsObject + Exec.serdes.read(j4) should matchPattern { + case BlackBoxExec(_, Some(Attached("foo:bar", _, Some(32768), Some("sha256-foo"))), Some("hello"), false, true) => + } + } + + behavior of "blackbox exec serialization" + + it should "serialize with inline attachment" in { + val bb = BlackBoxExec( + ImageName.fromString("docker-custom.com/openwhisk-runtime/magic/nodejs:0.0.1").get, + Some(Inline("foo")), + None, + false, + false) + val js = Exec.serdes.write(bb) + + val js2 = """{ + | "kind": "blackbox", + | "image": "docker-custom.com/openwhisk-runtime/magic/nodejs:0.0.1", + | "binary": false, + | "code": "foo" + |}""".stripMargin.parseJson.asJsObject + js shouldBe js2 + } + + it should "serialize with attached attachment" in { + val bb = BlackBoxExec( + ImageName.fromString("docker-custom.com/openwhisk-runtime/magic/nodejs:0.0.1").get, + Some(Attached("foo", ContentTypes.`application/octet-stream`, Some(42), Some("sha1-42"))), + None, + false, + true) + val js = Exec.serdes.write(bb) + println(js) + + val js2 = """{ + | "kind": "blackbox", + | "image": "docker-custom.com/openwhisk-runtime/magic/nodejs:0.0.1", + | "binary": true, + | "code": { + | "attachmentName": "foo", + | "attachmentType": "application/octet-stream", + | "length": 42, + | "digest": "sha1-42" + | } + |}""".stripMargin.parseJson.asJsObject + js shouldBe js2 + } + private class TestConfig(val props: Map[String, String], requiredProperties: Map[String, String]) extends WhiskConfig(requiredProperties) { override protected def getProperties() = mutable.Map(props.toSeq: _*) From 6267121ab3ab4991266b1de3cd9b5f8de66a1aa9 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Wed, 22 Aug 2018 12:08:08 +0530 Subject: [PATCH 5/8] Compat test for blackbox action --- .../test/AttachmentCompatibilityTests.scala | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/src/test/scala/whisk/core/database/test/AttachmentCompatibilityTests.scala b/tests/src/test/scala/whisk/core/database/test/AttachmentCompatibilityTests.scala index 82b693259a3..685f27d1455 100644 --- a/tests/src/test/scala/whisk/core/database/test/AttachmentCompatibilityTests.scala +++ b/tests/src/test/scala/whisk/core/database/test/AttachmentCompatibilityTests.scala @@ -138,6 +138,21 @@ class AttachmentCompatibilityTests codeExec(action2).codeAsJson shouldBe JsString("while (true)") } + it should "read existing simple code string for blackbox action" in { + implicit val tid: TransactionId = transid() + val exec = """{ + | "kind": "blackbox", + | "image": "docker-custom.com/openwhisk-runtime/magic/nodejs:0.0.1", + | "code": "while (true)", + | "binary": false + |}""".stripMargin.parseJson.asJsObject + val (id, action) = makeActionJson(namespace, aname(), exec) + val info = putDoc(id, action) + + val action2 = WhiskAction.get(entityStore, info.id).futureValue + codeExec(action2).codeAsJson shouldBe JsString("while (true)") + } + private def codeExec(a: WhiskAction) = a.exec.asInstanceOf[CodeExec[_]] private def makeActionJson(namespace: EntityPath, name: EntityName, exec: JsObject): (String, JsObject) = { From 23e0d7c50cc77e200b79d8878f2518c4db5437d1 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Wed, 22 Aug 2018 12:08:34 +0530 Subject: [PATCH 6/8] Test for cache for blackbox action --- .../controller/test/ActionsApiTests.scala | 200 +++++++++--------- .../whisk/core/database/test/DbUtils.scala | 5 + .../whisk/core/entity/test/ExecHelpers.scala | 1 + 3 files changed, 101 insertions(+), 105 deletions(-) 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 480d484e4fa..1b95b8914e7 100644 --- a/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala +++ b/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala @@ -36,8 +36,6 @@ import whisk.core.entitlement.Collection import whisk.http.ErrorResponse import whisk.http.Messages import whisk.core.database.UserContext -import java.io.ByteArrayInputStream -import java.util.Base64 import akka.http.scaladsl.model.headers.RawHeader import akka.stream.scaladsl._ @@ -802,7 +800,8 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi { annotations = Parameters("exec", "java")) val nodeAction = WhiskAction(namespace, aname(), jsDefault(nonInlinedCode(entityStore)), Parameters("x", "b")) val swiftAction = WhiskAction(namespace, aname(), swift3(nonInlinedCode(entityStore)), Parameters("x", "b")) - val actions = Seq((javaAction, JAVA_DEFAULT), (nodeAction, NODEJS6), (swiftAction, SWIFT3)) + val bbAction = WhiskAction(namespace, aname(), bb("bb", nonInlinedCode(entityStore), Some("bbMain"))) + val actions = Seq((javaAction, JAVA_DEFAULT), (nodeAction, NODEJS6), (swiftAction, SWIFT3), (bbAction, BLACKBOX)) actions.foreach { case (action, kind) => @@ -960,121 +959,112 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi { it should "get an action with attachment that is not cached" in { implicit val tid = transid() - val code = nonInlinedCode(entityStore) - val action = - WhiskAction(namespace, aname(), javaDefault(code, Some("hello")), annotations = Parameters("exec", "java")) - val content = WhiskActionPut( - Some(action.exec), - Some(action.parameters), - Some(ActionLimitsOption(Some(action.limits.timeout), Some(action.limits.memory), Some(action.limits.logs)))) - val name = action.name - val cacheKey = s"${CacheKey(action)}".replace("(", "\\(").replace(")", "\\)") - val expectedGetLog = Seq( - s"finding document: 'id: ${action.namespace}/${action.name}", - s"finding attachment '[\\w-/:]+' of document 'id: ${action.namespace}/${action.name}").mkString("(?s).*") + val nodeAction = WhiskAction(namespace, aname(), jsDefault(nonInlinedCode(entityStore)), Parameters("x", "b")) + val swiftAction = WhiskAction(namespace, aname(), swift3(nonInlinedCode(entityStore)), Parameters("x", "b")) + val bbAction = WhiskAction(namespace, aname(), bb("bb", nonInlinedCode(entityStore), Some("bbMain"))) + val actions = Seq((nodeAction, NODEJS6), (swiftAction, SWIFT3), (bbAction, BLACKBOX)) - action.exec match { - case exec @ CodeExecAsAttachment(_, _, _, binary) => - val bytes = if (binary) Base64.getDecoder().decode(code) else code.getBytes("UTF-8") - val stream = new ByteArrayInputStream(bytes) - val manifest = exec.manifest.attached.get - val src = StreamConverters.fromInputStream(() => stream) - putAndAttach[WhiskAction, WhiskEntity]( - entityStore, - action, - (d, a) => d.copy(exec = exec.attach(a)).revision[WhiskAction](d.rev), - manifest.attachmentType, - src, - None) + actions.foreach { + case (action, kind) => + val content = WhiskActionPut( + Some(action.exec), + Some(action.parameters), + Some(ActionLimitsOption(Some(action.limits.timeout), Some(action.limits.memory), Some(action.limits.logs)))) + val name = action.name + val cacheKey = s"${CacheKey(action)}".replace("(", "\\(").replace(")", "\\)") + val expectedGetLog = Seq( + s"finding document: 'id: ${action.namespace}/${action.name}", + s"finding attachment '[\\w-/:]+' of document 'id: ${action.namespace}/${action.name}").mkString("(?s).*") - case _ => - } + Put(s"$collectionPath/$name", content) ~> Route.seal(routes(creds)(transid())) ~> check { + status should be(OK) + } - // second request should fetch from cache - Get(s"$collectionPath/$name") ~> Route.seal(routes(creds)(transid())) ~> check { - status should be(OK) - val response = responseAs[WhiskAction] - response should be( - WhiskAction( - action.namespace, - action.name, - action.exec, - action.parameters, - action.limits, - action.version, - action.publish, - action.annotations ++ Parameters(WhiskAction.execFieldName, JAVA_DEFAULT))) - } + removeFromCache(action, WhiskAction) - stream.toString should include regex (expectedGetLog) - stream.reset() + // second request should not fetch from cache + Get(s"$collectionPath/$name") ~> Route.seal(routes(creds)(transid())) ~> check { + status should be(OK) + val response = responseAs[WhiskAction] + response should be( + WhiskAction( + action.namespace, + action.name, + action.exec, + action.parameters, + action.limits, + action.version, + action.publish, + action.annotations ++ Parameters(WhiskAction.execFieldName, kind))) + } + + stream.toString should include regex (expectedGetLog) + stream.reset() + } } it should "update an existing action with attachment that is not cached" in { implicit val tid = transid() - val code = nonInlinedCode(entityStore) - val action = - WhiskAction(namespace, aname(), javaDefault(code, Some("hello")), annotations = Parameters("exec", "java")) - val content = WhiskActionPut( - Some(action.exec), - Some(action.parameters), - Some(ActionLimitsOption(Some(action.limits.timeout), Some(action.limits.memory), Some(action.limits.logs)))) - val name = action.name - val cacheKey = s"${CacheKey(action)}".replace("(", "\\(").replace(")", "\\)") - val expectedPutLog = - Seq(s"uploading attachment '[\\w-/:]+' of document 'id: ${action.namespace}/${action.name}", s"caching $cacheKey") - .mkString("(?s).*") + val nodeAction = WhiskAction(namespace, aname(), jsDefault(nonInlinedCode(entityStore)), Parameters("x", "b")) + val swiftAction = WhiskAction(namespace, aname(), swift3(nonInlinedCode(entityStore)), Parameters("x", "b")) + val bbAction = WhiskAction(namespace, aname(), bb("bb", nonInlinedCode(entityStore), Some("bbMain"))) + val actions = Seq((nodeAction, NODEJS6), (swiftAction, SWIFT3), (bbAction, BLACKBOX)) - action.exec match { - case exec @ CodeExecAsAttachment(_, _, _, _) => - val stream = new ByteArrayInputStream(code.getBytes) - val manifest = exec.manifest.attached.get - val src = StreamConverters.fromInputStream(() => stream) - putAndAttach[WhiskAction, WhiskEntity]( - entityStore, - action, - (d, a) => d.copy(exec = exec.attach(a)).revision[WhiskAction](d.rev), - manifest.attachmentType, - src, - None) + actions.foreach { + case (action, kind) => + val content = WhiskActionPut( + Some(action.exec), + Some(action.parameters), + Some(ActionLimitsOption(Some(action.limits.timeout), Some(action.limits.memory), Some(action.limits.logs)))) + val name = action.name + val cacheKey = s"${CacheKey(action)}".replace("(", "\\(").replace(")", "\\)") + val expectedPutLog = + Seq( + s"uploading attachment '[\\w-/:]+' of document 'id: ${action.namespace}/${action.name}", + s"caching $cacheKey") + .mkString("(?s).*") - case _ => - } + Put(s"$collectionPath/$name", content) ~> Route.seal(routes(creds)(transid())) ~> check { + status should be(OK) + } - Put(s"$collectionPath/$name?overwrite=true", content) ~> Route.seal(routes(creds)(transid())) ~> check { - status should be(OK) - val response = responseAs[WhiskAction] - response should be( - WhiskAction( - action.namespace, - action.name, - action.exec, - action.parameters, - action.limits, - action.version.upPatch, - action.publish, - action.annotations ++ Parameters(WhiskAction.execFieldName, JAVA_DEFAULT))) - } - stream.toString should include regex (expectedPutLog) - stream.reset() + removeFromCache(action, WhiskAction) - // delete should invalidate cache - Delete(s"$collectionPath/$name") ~> Route.seal(routes(creds)(transid())) ~> check { - status should be(OK) - val response = responseAs[WhiskAction] - response should be( - WhiskAction( - action.namespace, - action.name, - action.exec, - action.parameters, - action.limits, - action.version.upPatch, - action.publish, - action.annotations ++ Parameters(WhiskAction.execFieldName, JAVA_DEFAULT))) + Put(s"$collectionPath/$name?overwrite=true", content) ~> Route.seal(routes(creds)(transid())) ~> check { + status should be(OK) + val response = responseAs[WhiskAction] + response should be( + WhiskAction( + action.namespace, + action.name, + action.exec, + action.parameters, + action.limits, + action.version.upPatch, + action.publish, + action.annotations ++ Parameters(WhiskAction.execFieldName, kind))) + } + stream.toString should include regex (expectedPutLog) + stream.reset() + + // delete should invalidate cache + Delete(s"$collectionPath/$name") ~> Route.seal(routes(creds)(transid())) ~> check { + status should be(OK) + val response = responseAs[WhiskAction] + response should be( + WhiskAction( + action.namespace, + action.name, + action.exec, + action.parameters, + action.limits, + action.version.upPatch, + action.publish, + action.annotations ++ Parameters(WhiskAction.execFieldName, kind))) + } + stream.toString should include(s"invalidating ${CacheKey(action)}") + stream.reset() } - stream.toString should include(s"invalidating ${CacheKey(action)}") - stream.reset() } it should "ensure old and new action schemas are supported" in { diff --git a/tests/src/test/scala/whisk/core/database/test/DbUtils.scala b/tests/src/test/scala/whisk/core/database/test/DbUtils.scala index f4a1aac1d58..28a360dc2bf 100644 --- a/tests/src/test/scala/whisk/core/database/test/DbUtils.scala +++ b/tests/src/test/scala/whisk/core/database/test/DbUtils.scala @@ -350,6 +350,11 @@ trait DbUtils extends Assertions { def isMemoryStore(store: ArtifactStore[_]): Boolean = store.isInstanceOf[MemoryArtifactStore[_]] def isCouchStore(store: ArtifactStore[_]): Boolean = store.isInstanceOf[CouchDbRestStore[_]] + protected def removeFromCache[A <: DocumentRevisionProvider](entity: WhiskEntity, factory: DocumentFactory[A])( + implicit ec: ExecutionContext): Unit = { + factory.removeId(CacheKey(entity)) + } + protected def randomBytes(size: Int): Array[Byte] = { val arr = new Array[Byte](size) Random.nextBytes(arr) diff --git a/tests/src/test/scala/whisk/core/entity/test/ExecHelpers.scala b/tests/src/test/scala/whisk/core/entity/test/ExecHelpers.scala index 9fb96577a1b..7bf608ca5e1 100644 --- a/tests/src/test/scala/whisk/core/entity/test/ExecHelpers.scala +++ b/tests/src/test/scala/whisk/core/entity/test/ExecHelpers.scala @@ -39,6 +39,7 @@ trait ExecHelpers extends Matchers with WskActorSystem with StreamLogging { protected val NODEJS6 = "nodejs:6" protected val SWIFT = "swift" protected val SWIFT3 = "swift:3.1.1" + protected val BLACKBOX = "blackbox" protected val SWIFT3_IMAGE = "action-swift-v3.1.1" protected val JAVA_DEFAULT = "java" From 3c6084ca9b050752c7841fcbabcf9c571ec2a18d Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Fri, 24 Aug 2018 15:39:00 +0530 Subject: [PATCH 7/8] Remove unused import --- .../test/scala/whisk/core/controller/test/ActionsApiTests.scala | 1 - 1 file changed, 1 deletion(-) 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 1b95b8914e7..fe4d2171e24 100644 --- a/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala +++ b/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala @@ -38,7 +38,6 @@ import whisk.http.Messages import whisk.core.database.UserContext import akka.http.scaladsl.model.headers.RawHeader -import akka.stream.scaladsl._ import whisk.core.entity.Attachments.Inline /** From 00473fab8b53be35728a0d92495bef3d314d546d Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Wed, 29 Aug 2018 16:17:43 +0530 Subject: [PATCH 8/8] Drop println --- tests/src/test/scala/whisk/core/entity/test/ExecTests.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/src/test/scala/whisk/core/entity/test/ExecTests.scala b/tests/src/test/scala/whisk/core/entity/test/ExecTests.scala index bd002ad48cd..0a6397b1939 100644 --- a/tests/src/test/scala/whisk/core/entity/test/ExecTests.scala +++ b/tests/src/test/scala/whisk/core/entity/test/ExecTests.scala @@ -280,7 +280,6 @@ class ExecTests extends FlatSpec with Matchers with StreamLogging with BeforeAnd false, true) val js = Exec.serdes.write(bb) - println(js) val js2 = """{ | "kind": "blackbox",