From 3c224453c825d5c9b247ab5c7c8c8cd502708806 Mon Sep 17 00:00:00 2001 From: Andrew Hlynskyi Date: Sat, 13 Jun 2020 22:51:36 +0300 Subject: [PATCH 1/9] package.py - Log correctly exceptions aborting zip archiving --- package.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/package.py b/package.py index 7cc8c13e..9b1b4e0d 100644 --- a/package.py +++ b/package.py @@ -291,7 +291,11 @@ def __enter__(self): return self.open() def __exit__(self, exc_type, exc_val, exc_tb): - self.close(failed=exc_type is not None) + if exc_type is not None: + self._log.exception("Error during zip archive creation") + self.close(failed=True) + raise SystemExit(1) + self.close() def _ensure_open(self): if self._zip is not None: From 80bde357f7163dafce1e9708764ddb33832fb8e8 Mon Sep 17 00:00:00 2001 From: Andrew Hlynskyi Date: Sat, 13 Jun 2020 23:39:54 +0300 Subject: [PATCH 2/9] package.py - Improved logging, added special DUMP_ENV level --- README.md | 2 +- package.py | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 51c5243c..0439d970 100644 --- a/README.md +++ b/README.md @@ -312,7 +312,7 @@ Hash of zip-archive created with the same content of the files is always identic When calling this module multiple times in one execution to create packages with the same `source_path`, zip-archives will be corrupted due to concurrent writes into the same file. There are two solutions - set different values for `hash_extra` to create different archives, or create package once outside (using this module) and then pass `local_existing_package` argument to create other Lambda resources. -Building and packaging has been historically hard to debug (especially with Terraform), so we made an effort to make it easier for user to see debug info. There are 3 different debug levels: `DEBUG` - to see only what is happening during planning phase, `DEBUG2` - to see all logging values, `DEBUG3` - to see all logging values and env variables (be careful sharing your env variables as they may contain secrets!). +Building and packaging has been historically hard to debug (especially with Terraform), so we made an effort to make it easier for user to see debug info. There are 3 different debug levels: `DEBUG` - to see only what is happening during planning phase and how a zip file content filtering in case of applied patterns, `DEBUG2` - to see more logging output, `DEBUG3` - to see all logging values, `DUMP_ENV` - to see all logging values and env variables (be careful sharing your env variables as they may contain secrets!). User can specify debug level like this: diff --git a/package.py b/package.py index 9b1b4e0d..bab7ace3 100644 --- a/package.py +++ b/package.py @@ -32,6 +32,7 @@ DEBUG2 = 9 DEBUG3 = 8 +DUMP_ENV = 1 log_handler = None log = logging.getLogger() @@ -43,6 +44,7 @@ def configure_logging(use_tf_stderr=False): logging.addLevelName(DEBUG2, 'DEBUG2') logging.addLevelName(DEBUG3, 'DEBUG3') + logging.addLevelName(DUMP_ENV, 'DUMP_ENV') class LogFormatter(logging.Formatter): default_format = '%(message)s' @@ -896,10 +898,15 @@ def prepare_command(args): # Load the query. query_data = json.load(sys.stdin) - if log.isEnabledFor(DEBUG3): + if log.isEnabledFor(DUMP_ENV): log.debug('ENV: %s', json.dumps(dict(os.environ), indent=2)) if log.isEnabledFor(DEBUG2): - log.debug('QUERY: %s', json.dumps(query_data, indent=2)) + if log.isEnabledFor(DEBUG3): + log.debug('QUERY: %s', json.dumps(query_data, indent=2)) + else: + log_excludes = ('source_path', 'hash_extra_paths', 'paths') + qd = {k: v for k, v in query_data.items() if k not in log_excludes} + log.debug('QUERY (excerpt): %s', json.dumps(qd, indent=2)) query = datatree('prepare_query', **query_data) @@ -1054,7 +1061,7 @@ def zip_cmd(args): subprocess.call([zipinfo, args.zipfile]) log.debug('-' * 80) log.debug('Source code hash: %s', - source_code_hash(open(args.zipfile, 'rb').read())) + source_code_hash(open(args.zipfile, 'rb').read())) p = hidden_parser('zip', help='Zip folder with provided files timestamp') p.set_defaults(command=zip_cmd) From 80cbec3ec21be6ec57e623051966a4600a4c2e76 Mon Sep 17 00:00:00 2001 From: Andrew Hlynskyi Date: Sat, 13 Jun 2020 23:41:30 +0300 Subject: [PATCH 3/9] package.py - Don't pass source_path in *.plan.json files --- package.py | 1 - 1 file changed, 1 deletion(-) diff --git a/package.py b/package.py index bab7ace3..756d842b 100644 --- a/package.py +++ b/package.py @@ -955,7 +955,6 @@ def prepare_command(args): build_data = { 'filename': filename, 'runtime': runtime, - 'source_path': source_path, 'artifacts_dir': artifacts_dir, 'build_plan': build_plan, } From 8a3a10dc835b73587186d232582afa740f608437 Mon Sep 17 00:00:00 2001 From: Andrew Hlynskyi Date: Sun, 14 Jun 2020 00:53:38 +0300 Subject: [PATCH 4/9] package.py - Added path patterns filtering for pip_requirements --- package.py | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/package.py b/package.py index 756d842b..0eb39434 100644 --- a/package.py +++ b/package.py @@ -717,13 +717,8 @@ def execute(self, build_plan, zip_stream, query): if sh_work_dir: if source_path != sh_work_dir: source_path = sh_work_dir - if pf: - for path in pf.filter(source_path, prefix): - if os.path.isdir(source_path): - arcname = os.path.relpath(path, source_path) - else: - arcname = os.path.basename(path) - zs.write_file(path, prefix, arcname) + if pf: + self._zip_write_with_filter(zs, pf, source_path, prefix) else: if os.path.isdir(source_path): zs.write_dirs(source_path, prefix=prefix) @@ -731,11 +726,13 @@ def execute(self, build_plan, zip_stream, query): zs.write_file(source_path, prefix=prefix) elif cmd == 'pip': runtime, pip_requirements, prefix = action[1:] - with install_pip_requirements(query, zs, - pip_requirements) as rd: + with install_pip_requirements(query, pip_requirements) as rd: if rd: - # XXX: timestamp=0 - what actually do with it? - zs.write_dirs(rd, prefix=prefix, timestamp=0) + if pf: + self._zip_write_with_filter(zs, pf, rd, prefix) + else: + # XXX: timestamp=0 - what actually do with it? + zs.write_dirs(rd, prefix=prefix, timestamp=0) elif cmd == 'sh': r, w = os.pipe() side_ch = os.fdopen(r) @@ -756,9 +753,21 @@ def execute(self, build_plan, zip_stream, query): elif cmd == 'clear:filter': pf = None + @staticmethod + def _zip_write_with_filter(zip_stream, path_filter, source_path, prefix): + for path in path_filter.filter(source_path, prefix): + if os.path.isdir(source_path): + arcname = os.path.relpath(path, source_path) + else: + arcname = os.path.basename(path) + zip_stream.write_file(path, prefix, arcname) + @contextmanager -def install_pip_requirements(query, zip_stream, requirements_file): +def install_pip_requirements(query, requirements_file): + # TODO: + # 1. Emit files instead of temp_dir + if not os.path.exists(requirements_file): yield return From 625dd3e8b804bcc7b3f019b1a19d8da0bf86bc0a Mon Sep 17 00:00:00 2001 From: Andrew Hlynskyi Date: Sun, 14 Jun 2020 01:55:09 +0300 Subject: [PATCH 5/9] package.py - Applied 0 ts for a :zip packed content --- package.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/package.py b/package.py index 0eb39434..3caf175a 100644 --- a/package.py +++ b/package.py @@ -650,10 +650,10 @@ def commands_step(path, commands): _, _path, prefix = c prefix = prefix.strip() _path = os.path.normpath(os.path.join(path, _path)) - step('zip', _path, prefix) + step('zip:embedded', _path, prefix) elif len(c) == 1: prefix = None - step('zip', path, prefix) + step('zip:embedded', path, prefix) else: raise ValueError( ':zip command can have zero ' @@ -712,24 +712,27 @@ def execute(self, build_plan, zip_stream, query): for action in build_plan: cmd = action[0] - if cmd == 'zip': + if cmd.startswith('zip'): + ts = 0 if cmd == 'zip:embedded' else None source_path, prefix = action[1:] if sh_work_dir: if source_path != sh_work_dir: source_path = sh_work_dir if pf: - self._zip_write_with_filter(zs, pf, source_path, prefix) + self._zip_write_with_filter(zs, pf, source_path, prefix, + timestamp=ts) else: if os.path.isdir(source_path): - zs.write_dirs(source_path, prefix=prefix) + zs.write_dirs(source_path, prefix=prefix, timestamp=ts) else: - zs.write_file(source_path, prefix=prefix) + zs.write_file(source_path, prefix=prefix, timestamp=ts) elif cmd == 'pip': runtime, pip_requirements, prefix = action[1:] with install_pip_requirements(query, pip_requirements) as rd: if rd: if pf: - self._zip_write_with_filter(zs, pf, rd, prefix) + self._zip_write_with_filter(zs, pf, rd, prefix, + timestamp=0) else: # XXX: timestamp=0 - what actually do with it? zs.write_dirs(rd, prefix=prefix, timestamp=0) @@ -754,13 +757,14 @@ def execute(self, build_plan, zip_stream, query): pf = None @staticmethod - def _zip_write_with_filter(zip_stream, path_filter, source_path, prefix): + def _zip_write_with_filter(zip_stream, path_filter, source_path, prefix, + timestamp=None): for path in path_filter.filter(source_path, prefix): if os.path.isdir(source_path): arcname = os.path.relpath(path, source_path) else: arcname = os.path.basename(path) - zip_stream.write_file(path, prefix, arcname) + zip_stream.write_file(path, prefix, arcname, timestamp=timestamp) @contextmanager From 94289378a1d285c55c7614bb85be4f4017872286 Mon Sep 17 00:00:00 2001 From: Andrew Hlynskyi Date: Sun, 14 Jun 2020 02:05:53 +0300 Subject: [PATCH 6/9] package.py - Support :zip args all optional --- package.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/package.py b/package.py index 3caf175a..fd19530a 100644 --- a/package.py +++ b/package.py @@ -651,13 +651,17 @@ def commands_step(path, commands): prefix = prefix.strip() _path = os.path.normpath(os.path.join(path, _path)) step('zip:embedded', _path, prefix) + elif len(c) == 2: + prefix = None + _, _path = c + step('zip:embedded', _path, prefix) elif len(c) == 1: prefix = None step('zip:embedded', path, prefix) else: raise ValueError( - ':zip command can have zero ' - 'or 2 arguments: {}'.format(c)) + ":zip invalid call signature, use: " + "':zip [path [prefix_in_zip]]'") hash(path) else: batch.append(c) From 0beec32f67e8344f3590e486b587bb9afe0d452b Mon Sep 17 00:00:00 2001 From: Andrew Hlynskyi Date: Sun, 14 Jun 2020 02:21:36 +0300 Subject: [PATCH 7/9] package.py - Made a path optional for commands --- package.py | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/package.py b/package.py index fd19530a..a41887b3 100644 --- a/package.py +++ b/package.py @@ -141,28 +141,29 @@ def list_files(top_path, log=None): return results -def dataclass(name, **fields): - typ = type(name, (object,), { - '__slots__': fields.keys(), - '__getattr__': lambda *_: None, +def dataclass(name): + typ = type(name, (dict,), { + '__getattr__': lambda self, x: self.get(x), + '__init__': lambda self, **k: self.update(k), }) - for k, v in fields.items(): - setattr(typ, k, v) return typ def datatree(name, **fields): - def decode_json(v): + def decode_json(k, v): if v and isinstance(v, str) and v[0] in '"[{': try: - return json.loads(v) + o = json.loads(v) + if isinstance(o, dict): + return dataclass(k)(**o) + return o except json.JSONDecodeError: pass return v - return dataclass(name, **dict((( - k, datatree(k, **v) if isinstance(v, dict) else decode_json(v)) - for k, v in fields.items())))() + return dataclass(name)(**dict((( + k, datatree(k, **v) if isinstance(v, dict) else decode_json(k, v)) + for k, v in fields.items()))) def timestamp_now_ns(): @@ -637,11 +638,19 @@ def pip_requirements_step(path, prefix=None, required=False): hash(requirements) def commands_step(path, commands): - path = os.path.normpath(path) + if path: + path = os.path.normpath(path) batch = [] for c in commands: if isinstance(c, str): if c.startswith(':zip'): + if path: + hash(path) + else: + # If path doesn't defined for a block with + # commands it will be set to Terraform's + # current working directory + path = query.paths.cwd if batch: step('sh', path, '\n'.join(batch)) batch.clear() @@ -662,7 +671,6 @@ def commands_step(path, commands): raise ValueError( ":zip invalid call signature, use: " "':zip [path [prefix_in_zip]]'") - hash(path) else: batch.append(c) From 91f048d2acc10054cd2b6f4689bb9e8ec5fc2331 Mon Sep 17 00:00:00 2001 From: Andrew Hlynskyi Date: Sun, 14 Jun 2020 05:33:25 +0300 Subject: [PATCH 8/9] package.py - Fixed an issue with py37 and inverted strict_timestamp checks --- package.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.py b/package.py index a41887b3..76508fcc 100644 --- a/package.py +++ b/package.py @@ -445,9 +445,9 @@ def _zinfo_from_file(filename, arcname=None, *, strict_timestamps=True): isdir = stat.S_ISDIR(st.st_mode) mtime = time.localtime(st.st_mtime) date_time = mtime[0:6] - if not strict_timestamps and date_time[0] < 1980: + if strict_timestamps and date_time[0] < 1980: date_time = (1980, 1, 1, 0, 0, 0) - elif not strict_timestamps and date_time[0] > 2107: + elif strict_timestamps and date_time[0] > 2107: date_time = (2107, 12, 31, 23, 59, 59) # Create ZipInfo instance to store file information if arcname is None: From 8c3cceb47613e9c108ff7c89bb4fc282d87ce7f8 Mon Sep 17 00:00:00 2001 From: Andrew Hlynskyi Date: Sun, 14 Jun 2020 05:41:29 +0300 Subject: [PATCH 9/9] package.py - Downgrade required python version to 3.6 --- package.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/package.py b/package.py index 76508fcc..c7590b83 100644 --- a/package.py +++ b/package.py @@ -2,8 +2,8 @@ import sys -if sys.version_info < (3, 7): - raise RuntimeError("A python version 3.7 or newer is required") +if sys.version_info < (3, 6): + raise RuntimeError("A python version 3.6 or newer is required") import os import re @@ -26,6 +26,8 @@ import logging PY38 = sys.version_info >= (3, 8) +PY37 = sys.version_info >= (3, 7) +PY36 = sys.version_info >= (3, 6) ################################################################################ # Logging @@ -387,10 +389,11 @@ def _write_zinfo(self, zinfo, filename, else: zinfo.compress_type = self._compress_type - if compresslevel is not None: - zinfo._compresslevel = compresslevel - else: - zinfo._compresslevel = self._compresslevel + if PY37: + if compresslevel is not None: + zinfo._compresslevel = compresslevel + else: + zinfo._compresslevel = self._compresslevel if zinfo.is_dir(): with zip._lock: