You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@openwhisk.apache.org by GitBox <gi...@apache.org> on 2018/08/08 17:36:00 UTC

[GitHub] csantanapr closed pull request #73: Don't override runtime npm packages when user provides their own

csantanapr closed pull request #73: Don't override runtime npm packages when user provides their own
URL: https://github.com/apache/incubator-openwhisk-runtime-nodejs/pull/73
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/.gitignore b/.gitignore
index 52f108d..0bc5144 100644
--- a/.gitignore
+++ b/.gitignore
@@ -65,6 +65,7 @@ ansible/roles/nginx/files/*cert.pem
 *.zip
 !tests/dat/actions/blackbox.zip
 !tests/dat/actions/helloSwift.zip
+!tests/dat/actions/nodejs-test.zip
 !tests/dat/actions/python.zip
 !tests/dat/actions/python2_virtualenv.zip
 !tests/dat/actions/python3_virtualenv.zip
diff --git a/core/nodejs6Action/Dockerfile b/core/nodejs6Action/Dockerfile
index 3cb7ac0..bd55807 100644
--- a/core/nodejs6Action/Dockerfile
+++ b/core/nodejs6Action/Dockerfile
@@ -33,9 +33,9 @@ WORKDIR /nodejsAction
 
 # Install app dependencies
 RUN rm -rf .project .settings build.xml Dockerfile README node_modules logs
-RUN npm install .
-
-RUN npm install \
+RUN cd / && npm install .
+# In nodejsActionBase we copied the package.json file to the root of the container, this is what we are installing here, as to not be overriden by user set packages
+RUN cd / && npm install --no-package-lock \
 apn@2.1.2 \
 async@2.1.4 \
 body-parser@1.15.2 \
diff --git a/core/nodejs8Action/Dockerfile b/core/nodejs8Action/Dockerfile
index 3f56b96..c497a56 100644
--- a/core/nodejs8Action/Dockerfile
+++ b/core/nodejs8Action/Dockerfile
@@ -22,7 +22,9 @@ RUN apt-get update && apt-get install -y \
     && rm -rf /var/lib/apt/lists/*
 WORKDIR /nodejsAction
 COPY . .
-RUN npm install \
+# COPY the package.json to root container, so we can install npm packages a level up from user's packages, so user's packages take precedence
+COPY ./package.json /
+RUN cd / && npm install --no-package-lock \
     && npm cache clean --force
 EXPOSE 8080
 CMD node --expose-gc app.js
diff --git a/core/nodejsActionBase/.dockerignore b/core/nodejsActionBase/.dockerignore
deleted file mode 100644
index 6df164e..0000000
--- a/core/nodejsActionBase/.dockerignore
+++ /dev/null
@@ -1,2 +0,0 @@
-Dockerfile
-build.gradle
diff --git a/core/nodejsActionBase/Dockerfile b/core/nodejsActionBase/Dockerfile
index a46360e..2c8c04d 100644
--- a/core/nodejsActionBase/Dockerfile
+++ b/core/nodejsActionBase/Dockerfile
@@ -23,3 +23,5 @@ ENV DEBIAN_FRONTEND noninteractive
 RUN apt-get update && apt-get install -y imagemagick && apt-get install -y unzip
 
 ADD . /nodejsAction
+# COPY the package.json to root container, so we can install npm packages a level up from user's packages, so user's packages take precedence
+COPY ./package.json /
diff --git a/settings.gradle b/settings.gradle
index c2bf195..e0f26b4 100644
--- a/settings.gradle
+++ b/settings.gradle
@@ -19,8 +19,9 @@ include 'tests'
 
 include 'core:nodejsActionBase'
 include 'core:nodejs6Action'
-
 include 'core:nodejs8Action'
+include 'tests:dat:docker:nodejs6docker'
+include 'tests:dat:docker:nodejs8docker'
 
 rootProject.name = 'runtime-nodejs'
 
diff --git a/tests/dat/actions/nodejs-test.zip b/tests/dat/actions/nodejs-test.zip
new file mode 100644
index 0000000..a0bfa23
Binary files /dev/null and b/tests/dat/actions/nodejs-test.zip differ
diff --git a/tests/dat/docker/nodejs6docker/Dockerfile b/tests/dat/docker/nodejs6docker/Dockerfile
new file mode 100644
index 0000000..63a26e1
--- /dev/null
+++ b/tests/dat/docker/nodejs6docker/Dockerfile
@@ -0,0 +1,3 @@
+FROM nodejs6action
+COPY package.json .
+RUN npm install --production
\ No newline at end of file
diff --git a/tests/dat/docker/nodejs6docker/build.gradle b/tests/dat/docker/nodejs6docker/build.gradle
new file mode 100644
index 0000000..4e4b018
--- /dev/null
+++ b/tests/dat/docker/nodejs6docker/build.gradle
@@ -0,0 +1,19 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+ext.dockerImageName = 'nodejs6docker'
+apply from: '../../../../gradle/docker.gradle'
diff --git a/tests/dat/docker/nodejs6docker/package.json b/tests/dat/docker/nodejs6docker/package.json
new file mode 100644
index 0000000..ebfb5a4
--- /dev/null
+++ b/tests/dat/docker/nodejs6docker/package.json
@@ -0,0 +1,8 @@
+{
+  "name": "testdocker",
+  "version": "1.0.0",
+  "main": "index.js",
+  "dependencies": {
+    "openwhisk": "2.0.0"
+  }
+}
\ No newline at end of file
diff --git a/tests/dat/docker/nodejs8docker/Dockerfile b/tests/dat/docker/nodejs8docker/Dockerfile
new file mode 100644
index 0000000..d1f6ada
--- /dev/null
+++ b/tests/dat/docker/nodejs8docker/Dockerfile
@@ -0,0 +1,3 @@
+FROM action-nodejs-v8
+COPY package.json .
+RUN npm install --production
\ No newline at end of file
diff --git a/tests/dat/docker/nodejs8docker/build.gradle b/tests/dat/docker/nodejs8docker/build.gradle
new file mode 100644
index 0000000..a3a1d74
--- /dev/null
+++ b/tests/dat/docker/nodejs8docker/build.gradle
@@ -0,0 +1,19 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+ext.dockerImageName = 'nodejs8docker'
+apply from: '../../../../gradle/docker.gradle'
diff --git a/tests/dat/docker/nodejs8docker/package.json b/tests/dat/docker/nodejs8docker/package.json
new file mode 100644
index 0000000..ebfb5a4
--- /dev/null
+++ b/tests/dat/docker/nodejs8docker/package.json
@@ -0,0 +1,8 @@
+{
+  "name": "testdocker",
+  "version": "1.0.0",
+  "main": "index.js",
+  "dependencies": {
+    "openwhisk": "2.0.0"
+  }
+}
\ No newline at end of file
diff --git a/tests/src/test/scala/runtime/actionContainers/NodeJs6ActionContainerTests.scala b/tests/src/test/scala/runtime/actionContainers/NodeJs6ActionContainerTests.scala
index 77fea66..a8a26fb 100644
--- a/tests/src/test/scala/runtime/actionContainers/NodeJs6ActionContainerTests.scala
+++ b/tests/src/test/scala/runtime/actionContainers/NodeJs6ActionContainerTests.scala
@@ -24,5 +24,5 @@ import org.scalatest.junit.JUnitRunner
 class NodeJs6ActionContainerTests extends NodeJsNonConcurrentTests {
 
   override lazy val nodejsContainerImageName = "nodejs6action"
-
+  override lazy val nodejsTestDockerImageName = "nodejs6docker"
 }
diff --git a/tests/src/test/scala/runtime/actionContainers/NodeJs8ActionContainerTests.scala b/tests/src/test/scala/runtime/actionContainers/NodeJs8ActionContainerTests.scala
index f4092c6..61fa8df 100644
--- a/tests/src/test/scala/runtime/actionContainers/NodeJs8ActionContainerTests.scala
+++ b/tests/src/test/scala/runtime/actionContainers/NodeJs8ActionContainerTests.scala
@@ -25,6 +25,7 @@ import spray.json.JsObject
 class NodeJs8ActionContainerTests extends NodeJsNonConcurrentTests {
 
   override lazy val nodejsContainerImageName = "action-nodejs-v8"
+  override lazy val nodejsTestDockerImageName = "nodejs8docker"
 
   it should "support async and await" in {
     withNodeJsContainer { c =>
diff --git a/tests/src/test/scala/runtime/actionContainers/NodeJsActionContainerTests.scala b/tests/src/test/scala/runtime/actionContainers/NodeJsActionContainerTests.scala
index 3f9694d..49c900b 100644
--- a/tests/src/test/scala/runtime/actionContainers/NodeJsActionContainerTests.scala
+++ b/tests/src/test/scala/runtime/actionContainers/NodeJsActionContainerTests.scala
@@ -17,8 +17,10 @@
 
 package runtime.actionContainers
 
+import java.io.File
+
 import common.WskActorSystem
-import actionContainers.{ActionContainer, BasicActionRunnerTests}
+import actionContainers.{ActionContainer, BasicActionRunnerTests, ResourceHelpers}
 import actionContainers.ActionContainer.withContainer
 import actionContainers.ResourceHelpers.ZipBuilder
 import spray.json._
@@ -26,6 +28,7 @@ import spray.json._
 abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with WskActorSystem {
 
   val nodejsContainerImageName: String
+  val nodejsTestDockerImageName: String
 
   override def withActionContainer(env: Map[String, String] = Map.empty)(code: ActionContainer => Unit) = {
     withContainer(nodejsContainerImageName, env)(code)
@@ -586,4 +589,39 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws
     })
   }
 
+  it should "use user provided npm packages in a zip file" in {
+    val zipPath = new File("tests/dat/actions/nodejs-test.zip").toPath
+    val code = ResourceHelpers.readAsBase64(zipPath)
+    withNodeJsContainer { c =>
+      c.init(initPayload(code))._1 should be(200)
+
+      val (runCode, runRes) = c.run(runPayload(JsObject()))
+      runRes.get.fields.get("message") shouldBe Some(JsString("success"))
+    }
+  }
+
+  it should "use user provided packages in Docker Actions" in {
+    withContainer(nodejsTestDockerImageName) { c =>
+      val code =
+        """
+                   | function main(args) {
+                   |  var ow = require('openwhisk');
+                   |  // actions only exists on 2.* versions of openwhisk, not 3.*, so if this was 3.* it would throw an error,
+                   |  var actions = ow().actions;
+                   |
+                   |  return { "message": "success" };
+                   |}
+                 """.stripMargin
+      // Initialization of the code should be successful
+      val (initCode, err) = c.init(initPayload(code))
+      initCode should be(200)
+
+      // Running the code should be successful and return a 200 status
+      val (runCode, runRes) = c.run(runPayload(JsObject()))
+      runCode should be(200)
+
+      runRes shouldBe defined
+      runRes.get.fields.get("message") shouldBe Some(JsString("success"))
+    }
+  }
 }
diff --git a/tests/src/test/scala/runtime/actionContainers/Nodejs6ConcurrentTests.scala b/tests/src/test/scala/runtime/actionContainers/Nodejs6ConcurrentTests.scala
index 0a218f6..27dfbfb 100644
--- a/tests/src/test/scala/runtime/actionContainers/Nodejs6ConcurrentTests.scala
+++ b/tests/src/test/scala/runtime/actionContainers/Nodejs6ConcurrentTests.scala
@@ -23,4 +23,5 @@ import org.scalatest.junit.JUnitRunner
 @RunWith(classOf[JUnitRunner])
 class Nodejs6ConcurrentTests extends NodeJsConcurrentTests {
   override lazy val nodejsContainerImageName = "nodejs6action"
+  override lazy val nodejsTestDockerImageName = "nodejs6docker"
 }
diff --git a/tests/src/test/scala/runtime/actionContainers/Nodejs8ConcurrentTests.scala b/tests/src/test/scala/runtime/actionContainers/Nodejs8ConcurrentTests.scala
index 2f1ffaa..565d20e 100644
--- a/tests/src/test/scala/runtime/actionContainers/Nodejs8ConcurrentTests.scala
+++ b/tests/src/test/scala/runtime/actionContainers/Nodejs8ConcurrentTests.scala
@@ -23,4 +23,5 @@ import org.scalatest.junit.JUnitRunner
 @RunWith(classOf[JUnitRunner])
 class Nodejs8ConcurrentTests extends NodeJsConcurrentTests {
   override lazy val nodejsContainerImageName = "action-nodejs-v8"
+  override lazy val nodejsTestDockerImageName = "nodejs8docker"
 }
diff --git a/tests/tests/dat b/tests/tests/dat
new file mode 120000
index 0000000..735ed3b
--- /dev/null
+++ b/tests/tests/dat
@@ -0,0 +1 @@
+../dat
\ No newline at end of file
diff --git a/tools/travis/build.sh b/tools/travis/build.sh
index 2d39fa9..a676794 100755
--- a/tools/travis/build.sh
+++ b/tools/travis/build.sh
@@ -38,8 +38,8 @@ TERM=dumb ./gradlew install
 echo "openwhisk.home=$WHISKDIR" > whisk.properties
 echo "vcap.services.file=" >> whisk.properties
 
-# Build runtime
+# Build runtime and dependencies
 cd $ROOTDIR
-TERM=dumb ./gradlew \
-:core:nodejs6Action:distDocker \
-:core:nodejs8Action:distDocker
+
+# Build runtime & test dependencies
+TERM=dumb ./gradlew distDocker


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services