feat: add smart warning if old excludes config is provided (#256)

* feat: add smart warning if old excludes config is provided

* chore: add comment for readability

* test: additional test case for no warning with new exclusion format

* feat: update warning with env variable instructions

gdk/build_system/Zip.py

@@ -44,6 +44,7 @@ def build(self, **kwargs):
 
             unwanted_paths = self.generate_ignore_list_from_globs(root_directory_path,
                                                                   self.get_ignored_file_patterns(project_config))
+            self.smart_excludes_warning(project_config)
 
             def ignore_with_glob_support(dir, names):
                 ignore_set = set()
@@ -126,3 +127,33 @@ def generate_ignore_list_from_globs(self, root_directory, globs):
             glob_pattern_whole = f"{root_directory}{os.path.sep}{pattern}"
             ignored_pathnames = ignored_pathnames | set(glob.glob(glob_pattern_whole, recursive=True))
         return ignored_pathnames
+
+    def smart_excludes_warning(self, project_config: ComponentBuildConfiguration):
+        """
+        Smart warning to warn user of excludes behavior change, if it is detected that a custom excludes is provided
+        and none of the patterns attempt to match any directories. This warning can be ignored by setting the
+        environment variable GDK_EXCLUDES_WARN_IGNORE to true. This is added in the 1.5.0 release and can be removed
+        at some point in the future.
+        """
+        warning_string = ("Build option \'excludes\' found in config and none of the provided patterns " +
+                          "include directory matching. In GDK version 1.5.0, patterns for exclusions in zip builds " +
+                          "were changed to use the glob format, so patterns with no specified directory pattern will " +
+                          "only match at the root level of the project. If this is intentional, you can ignore this " +
+                          "warning, but to achieve the old behavior excluding from each subdirectory, append \'**/\' " +
+                          "to each pattern as such: ")
+        warning_string_2 = (". To ignore this warning in the future, set the GDK_EXCLUDES_WARN_IGNORE environment " +
+                            "variable to true.")
+        GDK_EXCLUDES_ENV_KEY = "GDK_EXCLUDES_WARN_IGNORE"
+        build_options = project_config.build_options
+        excludes_list = build_options.get("excludes", [])
+        if not excludes_list or os.environ.get(GDK_EXCLUDES_ENV_KEY, "False").lower() == "true":
+            return
+        elif all(pattern.find("/") == -1 for pattern in excludes_list):
+            # We check if we issue the warning by seeing if there are any slashes in any items in the provided excludes
+            # configuration. If there are any slashes, we assume that the project is created using GDK 1.5.0+ and does
+            # not need the warning.
+            suggestion_list = [f"**/{old_pattern}" for old_pattern in excludes_list]
+            suggestion_list_str = str(suggestion_list).replace("'", '"')
+            logging.warning(f"{warning_string}{suggestion_list_str}{warning_string_2}")
+        else:
+            return

integration_tests/gdk/components/test_integ_BuildCommand.py

@@ -1,5 +1,6 @@
 from unittest import TestCase
 import pytest
+import logging
 from pathlib import Path
 import os
 import shutil
@@ -21,6 +22,10 @@ def __inject_fixtures(self, mocker, tmpdir):
         yield
         os.chdir(self.c_dir)
 
+    @pytest.fixture(autouse=True)
+    def caplog(self, caplog):
+        self.caplog = caplog
+
     def test_GIVEN_zip_build_system_WHEN_build_THEN_build_zip_artifacts(self):
         self.zip_test_data()
         bc = BuildCommand({})
@@ -46,6 +51,36 @@ def test_GIVEN_zip_build_system_WHEN_build_THEN_build_zip_artifacts(self):
         with open(build_recipe_file, "r") as f:
             assert f"s3://BUCKET_NAME/COMPONENT_NAME/COMPONENT_VERSION/{self.tmpdir.name}.zip" in f.read()
 
+    def test_GIVEN_zip_build_system_WHEN_excludes_provided_with_old_patterns_THEN_warn_in_logs(self):
+        self.caplog.set_level(logging.WARNING)
+        self.zip_old_excludes_test_data()
+        bc = BuildCommand({})
+        bc.run()
+
+        logs = self.caplog.text
+        assert "In GDK version 1.5.0, patterns for exclusions in zip builds" in logs
+        assert "[\"**/node_modules\", \"**/test*\", \"**/*.txt\"]" in logs
+
+    def test_GIVEN_zip_build_system_WHEN_excludes_provided_with_old_patterns_and_env_var_set_THEN_no_warn_in_logs(self):
+        self.caplog.set_level(logging.WARNING)
+        self.mocker.patch.dict(os.environ, {"GDK_EXCLUDES_WARN_IGNORE": "true"})
+        self.zip_old_excludes_test_data()
+        bc = BuildCommand({})
+        bc.run()
+
+        logs = self.caplog.text
+        assert "In GDK version 1.5.0, patterns for exclusions in zip builds" not in logs
+        assert "[\"**/node_modules\", \"**/test*\", \"**/*.txt\"]" not in logs
+
+    def test_GIVEN_zip_build_system_WHEN_excludes_provided_with_new_patterns_THEN_no_warn_in_logs(self):
+        self.caplog.set_level(logging.WARNING)
+        self.zip_new_excludes_test_data()
+        bc = BuildCommand({})
+        bc.run()
+
+        logs = self.caplog.text
+        assert "In GDK version 1.5.0, patterns for exclusions in zip builds" not in logs
+
     def test_GIVEN_zip_build_system_WHEN_build_and_artifacts_not_on_s3_THEN_build_raises_exception(self):
         self.zip_test_data()
         content = CaseInsensitiveRecipeFile().read(self.tmpdir.joinpath("recipe.yaml"))
@@ -177,6 +212,44 @@ def zip_test_data(self):
         self.tmpdir.joinpath("src", "node_modules").mkdir()
         self.tmpdir.joinpath("src", "node_modules", "excluded_file.txt").touch()
 
+    def zip_old_excludes_test_data(self):
+        old_excludes_config_path = "integration_tests/test_data/config/config_old_excludes.json"
+        shutil.copy(
+            self.c_dir.joinpath(old_excludes_config_path), self.tmpdir.joinpath("gdk-config.json")
+        )
+
+        shutil.copy(
+            self.c_dir.joinpath("integration_tests/test_data/recipes/hello_world_recipe.yaml"),
+            self.tmpdir.joinpath("recipe.yaml"),
+        )
+        with open(self.tmpdir.joinpath("recipe.yaml"), "r") as f:
+            recipe = f.read()
+            recipe = recipe.replace("$GG_ARTIFACT", self.tmpdir.name + ".zip")
+
+        with open(self.tmpdir.joinpath("recipe.yaml"), "w") as f:
+            f.write(recipe)
+
+        self.tmpdir.joinpath("hello_world.py").touch()
+
+    def zip_new_excludes_test_data(self):
+        new_excludes_config_path = "integration_tests/test_data/config/config_new_excludes.json"
+        shutil.copy(
+            self.c_dir.joinpath(new_excludes_config_path), self.tmpdir.joinpath("gdk-config.json")
+        )
+
+        shutil.copy(
+            self.c_dir.joinpath("integration_tests/test_data/recipes/hello_world_recipe.yaml"),
+            self.tmpdir.joinpath("recipe.yaml"),
+        )
+        with open(self.tmpdir.joinpath("recipe.yaml"), "r") as f:
+            recipe = f.read()
+            recipe = recipe.replace("$GG_ARTIFACT", self.tmpdir.name + ".zip")
+
+        with open(self.tmpdir.joinpath("recipe.yaml"), "w") as f:
+            f.write(recipe)
+
+        self.tmpdir.joinpath("hello_world.py").touch()
+
     def zip_test_data_oversized_recipe(self):
         shutil.copy(
             self.c_dir.joinpath("integration_tests/test_data/config/config.json"), self.tmpdir.joinpath("gdk-config.json")

integration_tests/test_data/config/config_new_excludes.json

@@ -0,0 +1,29 @@
+{
+  "component": {
+    "abc": {
+      "author": "abc",
+      "version": "NEXT_PATCH",
+      "build": {
+        "build_system": "zip",
+        "options": {
+          "excludes": [
+            "**/node_modules",
+            "test*",
+            "**/*.txt"
+          ]
+        }
+      },
+      "publish": {
+        "bucket": "default",
+        "region": "us-east-1"
+      }
+    }
+  },
+  "test-e2e": {
+    "otf_version": "1.2.0",
+    "otf_options": {
+      "tags": "testtags"
+    }
+  },
+  "gdk_version": "1.0.0"
+}

integration_tests/test_data/config/config_old_excludes.json

@@ -0,0 +1,29 @@
+{
+  "component": {
+    "abc": {
+      "author": "abc",
+      "version": "NEXT_PATCH",
+      "build": {
+        "build_system": "zip",
+        "options": {
+          "excludes": [
+            "node_modules",
+            "test*",
+            "*.txt"
+          ]
+        }
+      },
+      "publish": {
+        "bucket": "default",
+        "region": "us-east-1"
+      }
+    }
+  },
+  "test-e2e": {
+    "otf_version": "1.2.0",
+    "otf_options": {
+      "tags": "testtags"
+    }
+  },
+  "gdk_version": "1.0.0"
+}