Change the way we check the required version of the dependency (#428)

There are still some cases for false negative where the direct constrant is ok and other indirect dependency has a problem. Although the issue is not completely fixed, this logic change allows more legitimate cases which has been failing with the previous code. It will alleviate #426
GoogleCloudPlatform · Mar 28, 2018 · a5a4dd5 · a5a4dd5
1 parent f6de70a
commit a5a4dd5
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 7 deletions.
diff --git a/builder/gen-dockerfile/src/ValidateGoogleCloud.php b/builder/gen-dockerfile/src/ValidateGoogleCloud.php
@@ -91,14 +91,19 @@ public static function doCheck($workspace)
                     "no available matching version of $package"
                 );
             }
+            $found = false;
             foreach ($filtered as $version) {
-                if (Comparator::lessThan($version, $minimumVersionMap[$package])) {
-                    throw new GoogleCloudVersionException(
-                        "stackdriver integration needs $package "
-                        . $minimumVersionMap[$package] . ' or higher'
-                    );
+                if (Comparator::greaterThanOrEqualTo($version, $minimumVersionMap[$package])) {
+                    $found = true;
+                    break;
                 }
             }
+            if ($found === false) {
+                throw new GoogleCloudVersionException(
+                    "stackdriver integration needs $package "
+                    . $minimumVersionMap[$package] . ' or higher'
+                );
+            }
         }
 
         if (array_key_exists('google/cloud', $constraintsMap)) {

diff --git a/builder/gen-dockerfile/tests/GenFilesCommandTest.php b/builder/gen-dockerfile/tests/GenFilesCommandTest.php
@@ -202,6 +202,21 @@ public function dataProvider()
                  "enable_stackdriver_integration.sh"
                 ]
             ],
+            [
+                // stackdriver wildcard dep
+                __DIR__ . '/test_data/stackdriver_wildcard',
+                null,
+                '',
+                '/app/web',
+                'added by the php runtime builder',
+                'gcr.io/google-appengine/php72:latest',
+                ["COMPOSER_FLAGS='--no-dev --prefer-dist' \\\n",
+                 "FRONT_CONTROLLER_FILE='index.php' \\\n",
+                 "DETECTED_PHP_VERSION='7.2' \\\n",
+                 "IS_BATCH_DAEMON_RUNNING='true' \n",
+                 "enable_stackdriver_integration.sh"
+                ]
+            ],
             [
                 // stackdriver individual packages
                 __DIR__ . '/test_data/stackdriver_individual',
@@ -224,7 +239,7 @@ public function dataProvider()
                 '',
                 '/app/web',
                 'added by the php runtime builder',
-                'gcr.io/google-appengine/php71:latest',
+                'gcr.io/google-appengine/php72:latest',
                 [],
                 '\\Google\\Cloud\\Runtimes\\Builder\\Exception\\GoogleCloudVersionException'
             ],

diff --git a/builder/gen-dockerfile/tests/test_data/stackdriver_old_logging/composer.json b/builder/gen-dockerfile/tests/test_data/stackdriver_old_logging/composer.json
@@ -1,6 +1,6 @@
 {
     "require": {
-        "google/cloud-logging": "^1.2.0",
+        "google/cloud-logging": "<=1.2.0",
         "google/cloud-error-reporting": "^0.4.1"
     }
 }
diff --git a/builder/gen-dockerfile/tests/test_data/stackdriver_wildcard/app.yaml b/builder/gen-dockerfile/tests/test_data/stackdriver_wildcard/app.yaml
@@ -0,0 +1,6 @@
+env: flex
+runtime: php
+
+runtime_config:
+  enable_stackdriver_integration: true
+  document_root: web
diff --git a/builder/gen-dockerfile/tests/test_data/stackdriver_wildcard/composer.json b/builder/gen-dockerfile/tests/test_data/stackdriver_wildcard/composer.json
@@ -0,0 +1,5 @@
+{
+    "require": {
+        "google/cloud": "*"
+    }
+}