diff --git a/samcli/commands/init/init_templates.py b/samcli/commands/init/init_templates.py index 84af20d57d..369f780e42 100644 --- a/samcli/commands/init/init_templates.py +++ b/samcli/commands/init/init_templates.py @@ -12,7 +12,13 @@ from samcli.cli.global_config import GlobalConfig from samcli.commands.exceptions import UserException, AppTemplateUpdateException -from samcli.lib.utils.git_repo import GitRepo, CloneRepoException, CloneRepoUnstableStateException +from samcli.lib.utils import configuration +from samcli.lib.utils.git_repo import ( + GitRepo, + CloneRepoException, + CloneRepoUnstableStateException, + ManifestNotFoundException, +) from samcli.lib.utils.packagetype import IMAGE from samcli.local.common.runtime_template import ( RUNTIME_DEP_TEMPLATE_MAPPING, @@ -23,7 +29,10 @@ ) LOG = logging.getLogger(__name__) -MANIFEST_URL = "https://raw.githubusercontent.com/aws/aws-sam-cli-app-templates/master/manifest-v2.json" +APP_TEMPLATES_REPO_COMMIT = configuration.get_app_template_repo_commit() +MANIFEST_URL = ( + f"https://raw.githubusercontent.com/aws/aws-sam-cli-app-templates/{APP_TEMPLATES_REPO_COMMIT}/manifest-v2.json" +) APP_TEMPLATES_REPO_URL = "https://github.com/aws/aws-sam-cli-app-templates" APP_TEMPLATES_REPO_NAME = "aws-sam-cli-app-templates" @@ -66,7 +75,12 @@ def clone_templates_repo(self): if not self._git_repo.clone_attempted: shared_dir: Path = GlobalConfig().config_dir try: - self._git_repo.clone(clone_dir=shared_dir, clone_name=APP_TEMPLATES_REPO_NAME, replace_existing=True) + self._git_repo.clone( + clone_dir=shared_dir, + clone_name=APP_TEMPLATES_REPO_NAME, + replace_existing=True, + commit=APP_TEMPLATES_REPO_COMMIT, + ) except CloneRepoUnstableStateException as ex: raise AppTemplateUpdateException(str(ex)) from ex except (OSError, CloneRepoException): @@ -215,7 +229,16 @@ def _get_manifest(self): try: response = requests.get(MANIFEST_URL, timeout=10) body = response.text - except (requests.Timeout, requests.ConnectionError): + # if the commit is not exist then MANIFEST_URL will be invalid, fall back to use manifest in latest commit + if response.status_code == 404: + LOG.warning( + "Request to MANIFEST_URL: %s failed, the commit hash in this url maybe invalid, " + "Using manifest.json in the latest commit instead.", + MANIFEST_URL, + ) + raise ManifestNotFoundException() + + except (requests.Timeout, requests.ConnectionError, ManifestNotFoundException): LOG.debug("Request to get Manifest failed, attempting to clone the repository") self.clone_templates_repo() manifest_path = self.get_manifest_path() diff --git a/samcli/lib/utils/configuration.py b/samcli/lib/utils/configuration.py new file mode 100644 index 0000000000..f9ad731c20 --- /dev/null +++ b/samcli/lib/utils/configuration.py @@ -0,0 +1,27 @@ +""" Read info from runtime_config.json file""" + +import json +from pathlib import Path +import logging + +from click import ClickException + +LOG = logging.getLogger(__name__) + +CONFIG_FILE = Path(Path(__file__).resolve().parents[2], "runtime_config.json") +config = json.loads(CONFIG_FILE.read_text()) + + +def get_app_template_repo_commit(): + """ + Returns + ------- + the value of app_template_repo_commit + + """ + commit_hash = config.get("app_template_repo_commit", None) + if not commit_hash: + raise ClickException( + message="Error when retrieving app_template_repo_commit, runtime_config.json file maybe invalid" + ) + return commit_hash diff --git a/samcli/lib/utils/git_repo.py b/samcli/lib/utils/git_repo.py index 341979f8aa..bef3f2f2d2 100644 --- a/samcli/lib/utils/git_repo.py +++ b/samcli/lib/utils/git_repo.py @@ -30,6 +30,12 @@ class CloneRepoUnstableStateException(CloneRepoException): """ +class ManifestNotFoundException(Exception): + """ + Exception class when request Manifest file return 404. + """ + + class GitRepo: """ Class for managing a Git repo, currently it has a clone functionality only @@ -81,7 +87,7 @@ def _git_executable() -> str: raise OSError("Cannot find git, was looking at executables: {}".format(executables)) - def clone(self, clone_dir: Path, clone_name: str, replace_existing: bool = False) -> Path: + def clone(self, clone_dir: Path, clone_name: str, replace_existing: bool = False, commit: str = "") -> Path: """ creates a local clone of this Git repository. This method is different from the standard Git clone in the following: @@ -98,7 +104,8 @@ def clone(self, clone_dir: Path, clone_name: str, replace_existing: bool = False The dirname of the local clone replace_existing: bool Whether to replace the current local clone directory if already exists or not - + commit: str + if a commit is provided, it will checkout out the commit in the clone repo Returns ------- The path of the created local clone @@ -127,6 +134,12 @@ def clone(self, clone_dir: Path, clone_name: str, replace_existing: bool = False cwd=tempdir, stderr=subprocess.STDOUT, ) + + # bind a certain sam cli release to a specific commit of the aws-sam-cli-app-templates's repo, avoiding + # regression + if commit: + self._checkout_commit(temp_path, commit) + self.local_path = self._persist_local_repo(temp_path, clone_dir, clone_name, replace_existing) return self.local_path except OSError as ex: @@ -162,3 +175,18 @@ def _persist_local_repo(temp_path: str, dest_dir: Path, dest_name: str, replace_ f"Check that you have permissions to create/delete files in {dest_dir} directory " "or file an issue at https://github.com/aws/aws-sam-cli/issues" ) from ex + + @staticmethod + def _checkout_commit(repo_dir: str, commit: str): + try: + # if the checkout commit failed, it will use the latest commit instead + git_executable = GitRepo._git_executable() + check_output( + [git_executable, "checkout", commit], + cwd=repo_dir, + stderr=subprocess.STDOUT, + ) + except subprocess.CalledProcessError as checkout_error: + output = checkout_error.output.decode("utf-8") + if "fatal" in output.lower() or "error" in output.lower(): + LOG.warning("WARN: Commit not exist: %s, using the latest one", commit, exc_info=checkout_error) diff --git a/samcli/runtime_config.json b/samcli/runtime_config.json new file mode 100644 index 0000000000..be4e7a1645 --- /dev/null +++ b/samcli/runtime_config.json @@ -0,0 +1,3 @@ +{ + "app_template_repo_commit" : "09b7de41c32ee5f50ec8ceeec4a304534767844b" +} \ No newline at end of file diff --git a/tests/integration/init/test_init_command.py b/tests/integration/init/test_init_command.py index 7458166172..69add26779 100644 --- a/tests/integration/init/test_init_command.py +++ b/tests/integration/init/test_init_command.py @@ -12,6 +12,8 @@ TIMEOUT = 300 +COMMIT_ERROR = "WARN: Commit not exist:" + class TestBasicInitCommand(TestCase): def test_init_command_passes_and_dir_created(self): @@ -33,16 +35,20 @@ def test_init_command_passes_and_dir_created(self): "--no-interactive", "-o", temp, - ] + ], + stdout=PIPE, + stderr=PIPE, ) try: - process.communicate(timeout=TIMEOUT) + stdout_data, stderr_data = process.communicate(timeout=TIMEOUT) + stderr = stderr_data.decode("utf-8") except TimeoutExpired: process.kill() raise self.assertEqual(process.returncode, 0) self.assertTrue(Path(temp, "sam-app").is_dir()) + self.assertNotIn(COMMIT_ERROR, stderr) def test_init_command_passes_and_dir_created_image(self): with tempfile.TemporaryDirectory() as temp: @@ -89,16 +95,20 @@ def test_init_new_app_template(self): "--no-interactive", "-o", temp, - ] + ], + stdout=PIPE, + stderr=PIPE, ) try: - process.communicate(timeout=TIMEOUT) + stdout_data, stderr_data = process.communicate(timeout=TIMEOUT) + stderr = stderr_data.decode("utf-8") except TimeoutExpired: process.kill() raise self.assertEqual(process.returncode, 0) self.assertTrue(Path(temp, "qs-scratch").is_dir()) + self.assertNotIn(COMMIT_ERROR, stderr) def test_init_command_java_maven(self): with tempfile.TemporaryDirectory() as temp: @@ -117,16 +127,20 @@ def test_init_command_java_maven(self): "--no-interactive", "-o", temp, - ] + ], + stdout=PIPE, + stderr=PIPE, ) try: - process.communicate(timeout=TIMEOUT) + stdout_data, stderr_data = process.communicate(timeout=TIMEOUT) + stderr = stderr_data.decode("utf-8") except TimeoutExpired: process.kill() raise self.assertEqual(process.returncode, 0) self.assertTrue(Path(temp, "sam-app-maven").is_dir()) + self.assertNotIn(COMMIT_ERROR, stderr) def test_init_command_java_gradle(self): with tempfile.TemporaryDirectory() as temp: @@ -145,16 +159,20 @@ def test_init_command_java_gradle(self): "--no-interactive", "-o", temp, - ] + ], + stdout=PIPE, + stderr=PIPE, ) try: - process.communicate(timeout=TIMEOUT) + stdout_data, stderr_data = process.communicate(timeout=TIMEOUT) + stderr = stderr_data.decode("utf-8") except TimeoutExpired: process.kill() raise self.assertEqual(process.returncode, 0) self.assertTrue(Path(temp, "sam-app-gradle").is_dir()) + self.assertNotIn(COMMIT_ERROR, stderr) def test_init_command_with_extra_context_parameter(self): with tempfile.TemporaryDirectory() as temp: @@ -175,16 +193,20 @@ def test_init_command_with_extra_context_parameter(self): '{"schema_name": "codedeploy", "schema_type": "aws"}', "-o", temp, - ] + ], + stdout=PIPE, + stderr=PIPE, ) try: - process.communicate(timeout=TIMEOUT) + stdout_data, stderr_data = process.communicate(timeout=TIMEOUT) + stderr = stderr_data.decode("utf-8") except TimeoutExpired: process.kill() raise self.assertEqual(process.returncode, 0) self.assertTrue(Path(temp, "sam-app-maven").is_dir()) + self.assertNotIn(COMMIT_ERROR, stderr) def test_init_command_passes_with_arm_architecture(self): with tempfile.TemporaryDirectory() as temp: @@ -205,16 +227,20 @@ def test_init_command_passes_with_arm_architecture(self): temp, "--architecture", "arm64", - ] + ], + stdout=PIPE, + stderr=PIPE, ) try: - process.communicate(timeout=TIMEOUT) + stdout_data, stderr_data = process.communicate(timeout=TIMEOUT) + stderr = stderr_data.decode("utf-8") except TimeoutExpired: process.kill() raise self.assertEqual(process.returncode, 0) self.assertTrue(Path(temp, "sam-app").is_dir()) + self.assertNotIn(COMMIT_ERROR, stderr) def test_init_command_passes_with_x86_64_architecture(self): with tempfile.TemporaryDirectory() as temp: @@ -235,16 +261,20 @@ def test_init_command_passes_with_x86_64_architecture(self): temp, "--architecture", "x86_64", - ] + ], + stdout=PIPE, + stderr=PIPE, ) try: - process.communicate(timeout=TIMEOUT) + stdout_data, stderr_data = process.communicate(timeout=TIMEOUT) + stderr = stderr_data.decode("utf-8") except TimeoutExpired: process.kill() raise self.assertEqual(process.returncode, 0) self.assertTrue(Path(temp, "sam-app").is_dir()) + self.assertNotIn(COMMIT_ERROR, stderr) def test_init_command_passes_with_unknown_architecture(self): with tempfile.TemporaryDirectory() as temp: diff --git a/tests/unit/lib/utils/test_configuration.py b/tests/unit/lib/utils/test_configuration.py new file mode 100644 index 0000000000..3f54d5b8d9 --- /dev/null +++ b/tests/unit/lib/utils/test_configuration.py @@ -0,0 +1,7 @@ +from unittest.case import TestCase +from samcli.lib.utils import configuration + + +class TestConfiguration(TestCase): + def test_return_correct_value(self): + self.assertEqual(configuration.get_app_template_repo_commit(), "09b7de41c32ee5f50ec8ceeec4a304534767844b") diff --git a/tests/unit/lib/utils/test_git_repo.py b/tests/unit/lib/utils/test_git_repo.py index 5030ffaec1..c3ce27b6df 100644 --- a/tests/unit/lib/utils/test_git_repo.py +++ b/tests/unit/lib/utils/test_git_repo.py @@ -9,6 +9,7 @@ REPO_NAME = "REPO NAME" CLONE_DIR = os.path.normpath("/tmp/local/clone/dir") EXPECTED_DEFAULT_CLONE_PATH = os.path.normpath(os.path.join(CLONE_DIR, REPO_NAME)) +COMMIT = "123" class TestGitRepo(TestCase): @@ -186,3 +187,37 @@ def test_clone_when_failed_to_move_cloned_repo_from_temp_to_final_destination( shutil_mock.rmtree.assert_called_once_with(EXPECTED_DEFAULT_CLONE_PATH, onerror=rmtree_callback) shutil_mock.copytree.assert_called_once_with(ANY, EXPECTED_DEFAULT_CLONE_PATH, ignore=ANY) shutil_mock.ignore_patterns.assert_called_once_with("*.git") + + @patch("samcli.lib.utils.git_repo.LOG") + @patch("samcli.lib.utils.git_repo.check_output") + def test_checkout_commit_when_commit_not_exist(self, check_output_mock, log_mock): + check_output_mock.side_effect = subprocess.CalledProcessError( + "fail", "fail", "fatal: reference is not a tree".encode("utf-8") + ) + try: + with self.assertRaises(CloneRepoException): + self.repo._checkout_commit(repo_dir="test", commit="1234") + except Exception: + pass + log_mock.warning.assert_called() + + @patch("samcli.lib.utils.git_repo.Path.exists") + @patch("samcli.lib.utils.git_repo.shutil") + @patch("samcli.lib.utils.git_repo.check_output") + @patch("samcli.lib.utils.git_repo.subprocess.Popen") + @patch("samcli.lib.utils.git_repo.platform.system") + def test_clone_with_commit(self, platform_mock, popen_mock, check_output_mock, shutil_mock, path_exist_mock): + path_exist_mock.return_value = False + self.repo.clone(clone_dir=self.local_clone_dir, clone_name=REPO_NAME, commit=COMMIT) + self.local_clone_dir.mkdir.assert_called_once_with(mode=0o700, parents=True, exist_ok=True) + popen_mock.assert_has_calls( + [call(["git"], stdout=subprocess.PIPE, stderr=subprocess.PIPE)], + [call(["git"], stdout=subprocess.PIPE, stderr=subprocess.PIPE)], + ) + check_output_mock.assert_has_calls( + [call(["git", "clone", self.repo.url, REPO_NAME], cwd=ANY, stderr=subprocess.STDOUT)], + [call(["git", "checkout", COMMIT], cwd=ANY, stderr=subprocess.STDOUT)], + ) + shutil_mock.rmtree.assert_not_called() + shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH, ignore=ANY) + shutil_mock.ignore_patterns.assert_called_with("*.git")