Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Treat blackbox action code as attachment #3979

Merged
merged 8 commits into from
Aug 29, 2018
Merged

Treat blackbox action code as attachment #3979

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
534eece
Use attachments with BlackBoxExec
chetanmeh Aug 21, 2018
a34e0fe
Simplify code to avoid code duplication
chetanmeh Aug 21, 2018
0217730
Fix test expectation
chetanmeh Aug 21, 2018
d41d614
Test for blackbox serialization
chetanmeh Aug 22, 2018
6267121
Compat test for blackbox action
chetanmeh Aug 22, 2018
23e0d7c
Test for cache for blackbox action
chetanmeh Aug 22, 2018
3c6084c
Remove unused import
chetanmeh Aug 24, 2018
00473fa
Drop println
chetanmeh Aug 29, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions common/scala/src/main/scala/whisk/core/entity/Attachments.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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] {
Expand Down
58 changes: 37 additions & 21 deletions common/scala/src/main/scala/whisk/core/entity/Exec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand Down Expand Up @@ -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])
Expand Down Expand Up @@ -142,20 +146,21 @@ 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)
override val deprecated = manifest.deprecated.getOrElse(false)
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)
}
}
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -307,18 +320,14 @@ 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)) {
throw new DeserializationException(s"'main' must be a string defined in 'exec' for '$kind' actions")
} 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 {
Expand All @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i know you preserved the previous behavior... but should be check for the presence of the binary field first and only if it's missing (indicating an old schema), compute the binary bit?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure on that - Current read flow would happen for 2 cases

  1. Deserializing the json obtained from ArtifactStore where the binary flag would have been set at time of serialization
  2. Deserializing the user providing json

Would it fine to trust the binary field if provided by user? Probably yes ... but wanted to check on that

case _ => obj.fields.get("binary").map(_.convertTo[Boolean]).getOrElse(false)
}
}
}

protected[core] object ExecMetaDataBase extends ArgNormalizer[ExecMetaDataBase] with DefaultJsonProtocol {
Expand Down
103 changes: 61 additions & 42 deletions common/scala/src/main/scala/whisk/core/entity/WhiskAction.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -331,38 +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(StandardCharsets.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,
(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")))
})

putWithAttachment(code, binary, exec)
case exec @ BlackBoxExec(_, Some(Inline(code)), _, _, binary) =>
putWithAttachment(code, binary, exec)
case _ =>
super.put(db, doc, old)
}
Expand All @@ -384,36 +376,63 @@ 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) =>
getWithAttachment(attached, binary, exec)
case _ =>
Future.successful(action)
}
}
}

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, _, _, _)), _, _, _) =>
checkName(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
}
}

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] = {
Expand Down
Loading