From 6480ba3fac1b09502ac39fcbe14554920daa5066 Mon Sep 17 00:00:00 2001 From: Javier Maestro Date: Fri, 13 Sep 2024 19:13:13 +0100 Subject: [PATCH] refactor: lockfile, pkg, and lock v1-to-v2 auto-migration * move all of the "package logic" to pkg.bzl * The v2 lockfile format: * doesn't need the fast_package_lookup dict because it's already using a dict to store the packages. * has the dependencies sorted so the lockfile now has stable serialization and the diffs of the lock are actually usable and useful to compare with the changes to the manifest. * removes the package and dependency key from the lockfile, now it's done via an external function (make_deb_import_key in deb_import.bzl) * Remove add_package_dependency from the lockfile API. Now, the package dependencies are passed as an argument to add_package. This way, the lockfile functionality is fully contained in lockfile.bzl and e.g. we can remove the "consistency checks" that were only needed because users could forget to add the dependency as a package to the lockfile. * Ensure backwards-compatibility by internally converting lock v1 to v2. Also, when a lock is set and it's in v1 format, there's a reminder that encourages the users to run @repo//:lock to update the lockfile format. --- apt/extensions.bzl | 27 ++- apt/index.bzl | 6 +- apt/private/BUILD.bazel | 20 ++- apt/private/deb_import.bzl | 9 + apt/private/index.bzl | 68 ++++---- apt/private/lockfile.bzl | 161 +++++++++++------- apt/private/manifest.bzl | 6 +- apt/private/pkg.bzl | 42 +++++ examples/debian_snapshot_security/BUILD.bazel | 2 +- 9 files changed, 220 insertions(+), 121 deletions(-) create mode 100644 apt/private/pkg.bzl diff --git a/apt/extensions.bzl b/apt/extensions.bzl index ceb91d3..225893f 100644 --- a/apt/extensions.bzl +++ b/apt/extensions.bzl @@ -1,6 +1,6 @@ "apt extensions" -load("//apt/private:deb_import.bzl", "deb_import") +load("//apt/private:deb_import.bzl", "deb_import", "make_deb_import_key") load("//apt/private:index.bzl", "deb_package_index") load("//apt/private:lockfile.bzl", "lockfile") load("//apt/private:manifest.bzl", "manifest") @@ -21,23 +21,22 @@ def _distroless_extension(module_ctx): ) if not install.nolock: - # buildifier: disable=print - print("\nNo lockfile was given, please run `bazel run @%s//:lock` to create the lockfile." % install.name) + print( + "\nNo lockfile was given. To create one please run " + + "`bazel run @{}//:lock`".format(install.name), + ) else: lockf = lockfile.from_json(module_ctx, module_ctx.read(install.lock)) - for (package) in lockf.packages(): - package_key = lockfile.make_package_key( - package["name"], - package["version"], - package["arch"], - ) + for architectures in lockf.packages.values(): + for package in architectures.values(): + deb_import_key = make_deb_import_key(install.name, package) - deb_import( - name = "%s_%s" % (install.name, package_key), - urls = [package["url"]], - sha256 = package["sha256"], - ) + deb_import( + name = deb_import_key, + url = package.url, + sha256 = package.sha256, + ) deb_resolve( name = install.name + "_resolve", diff --git a/apt/index.bzl b/apt/index.bzl index b23c31b..e8fe772 100644 --- a/apt/index.bzl +++ b/apt/index.bzl @@ -92,8 +92,10 @@ def deb_index( ) if not lock and not nolock: - # buildifier: disable=print - print("\nNo lockfile was given, please run `bazel run @%s//:lock` to create the lockfile." % name) + print( + "\nNo lockfile was given. To create one please run " + + "`bazel run @{}//:lock`".format(name), + ) _deb_package_index( name = name, diff --git a/apt/private/BUILD.bazel b/apt/private/BUILD.bazel index 6c126cc..287c316 100644 --- a/apt/private/BUILD.bazel +++ b/apt/private/BUILD.bazel @@ -26,6 +26,7 @@ bzl_library( srcs = ["index.bzl"], visibility = ["//apt:__subpackages__"], deps = [ + ":deb_import", ":lockfile", ], ) @@ -34,7 +35,13 @@ bzl_library( name = "lockfile", srcs = ["lockfile.bzl"], visibility = ["//apt:__subpackages__"], - deps = [":util"], + deps = [":pkg"], +) + +bzl_library( + name = "pkg", + srcs = ["pkg.bzl"], + visibility = ["//apt:__subpackages__"], ) bzl_library( @@ -79,7 +86,16 @@ bzl_library( name = "deb_import", srcs = ["deb_import.bzl"], visibility = ["//apt:__subpackages__"], - deps = ["@bazel_tools//tools/build_defs/repo:http.bzl"], + deps = [ + ":util", + # NOTE: I had to add cache.bzl because bazel test //docs:update_1_test was failing with + # Stardoc documentation generation failed: + # File /bin/docs/apt_stardoc.runfiles/bazel_tools/tools/build_defs/repo/http.bzl imported ':cache.bzl', + # yet /bin/docs/apt_stardoc.runfiles/bazel_tools/tools/build_defs/repo/cache.bzl was not found. + "@bazel_tools//tools/build_defs/repo:cache.bzl", + "@bazel_tools//tools/build_defs/repo:http.bzl", + "@bazel_tools//tools/build_defs/repo:utils.bzl", + ], ) bzl_library( diff --git a/apt/private/deb_import.bzl b/apt/private/deb_import.bzl index 864d887..d05ba49 100644 --- a/apt/private/deb_import.bzl +++ b/apt/private/deb_import.bzl @@ -1,6 +1,7 @@ "deb_import" load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") +load(":util.bzl", "util") # BUILD.bazel template _DEB_IMPORT_BUILD_TMPL = ''' @@ -53,6 +54,14 @@ filegroup( ) ''' +def make_deb_import_key(repo_name, package): + return "{}_{}_{}_{}".format( + repo_name, + util.sanitize(package.name), + package.arch, + util.sanitize(package.version), + ) + def deb_import(**kwargs): http_archive( build_file_content = _DEB_IMPORT_BUILD_TMPL, diff --git a/apt/private/index.bzl b/apt/private/index.bzl index d2ebf69..18b1bf8 100644 --- a/apt/private/index.bzl +++ b/apt/private/index.bzl @@ -1,5 +1,6 @@ "apt-get" +load(":deb_import.bzl", "make_deb_import_key") load(":lockfile.bzl", "lockfile") # header template for packages.bzl file @@ -15,7 +16,7 @@ def {}_packages(): _DEB_IMPORT_TMPL = '''\ deb_import( name = "{name}", - urls = {urls}, + urls = ["{urls}"], sha256 = "{sha256}", ) ''' @@ -39,45 +40,42 @@ def _deb_package_index_impl(rctx): if not lock_content: package_defs = [_DEB_IMPORT_HEADER_TMPL.format(rctx.attr.name)] - if len(lockf.packages()) < 1: + if not lockf.packages: package_defs.append(" pass") - for (package) in lockf.packages(): - package_key = lockfile.make_package_key( - package["name"], - package["version"], - package["arch"], - ) - - if not lock_content: - package_defs.append( - _DEB_IMPORT_TMPL.format( - name = "%s_%s" % (rctx.attr.name, package_key), - package_name = package["name"], - urls = [package["url"]], - sha256 = package["sha256"], + for architectures in lockf.packages.values(): + for package in architectures.values(): + deb_import_key = make_deb_import_key(rctx.attr.name, package) + + if not lock_content: + package_defs.append( + _DEB_IMPORT_TMPL.format( + name = deb_import_key, + package_name = package.name, + urls = package.url, + sha256 = package.sha256, + ), + ) + + repo_name = "%s%s" % ("@" if lock_content else "", deb_import_key) + + rctx.file( + "%s/%s/BUILD.bazel" % (package.name, package.arch), + package_template.format( + target_name = package.arch, + src = '"@%s//:data"' % repo_name, + deps = ",\n ".join([ + '"//%s/%s"' % (dep.name, package.arch) + for dep in package.dependencies + ]), + urls = [package.url], + name = package.name, + arch = package.arch, + sha256 = package.sha256, + repo_name = "%s" % repo_name, ), ) - repo_name = "%s%s_%s" % ("@" if lock_content else "", rctx.attr.name, package_key) - - rctx.file( - "%s/%s/BUILD.bazel" % (package["name"], package["arch"]), - package_template.format( - target_name = package["arch"], - src = '"@%s//:data"' % repo_name, - deps = ",\n ".join([ - '"//%s/%s"' % (dep["name"], package["arch"]) - for dep in package["dependencies"] - ]), - urls = [package["url"]], - name = package["name"], - arch = package["arch"], - sha256 = package["sha256"], - repo_name = "%s" % repo_name, - ), - ) - rctx.file("packages.bzl", "\n".join(package_defs)) rctx.file("BUILD.bazel", _BUILD_TMPL.format(rctx.attr.name.split("~")[-1])) diff --git a/apt/private/lockfile.bzl b/apt/private/lockfile.bzl index 09e2fd9..53dbd13 100644 --- a/apt/private/lockfile.bzl +++ b/apt/private/lockfile.bzl @@ -1,82 +1,119 @@ "lock" -load(":util.bzl", "util") +load(":pkg.bzl", "dep", "pkg") -def _make_package_key(name, version, arch): - return "%s_%s_%s" % ( - util.sanitize(name), - util.sanitize(version), - arch, - ) +def __empty(): + return struct(version = 2, packages = {}) -def _package_key(package, arch): - return _make_package_key(package["Package"], package["Version"], arch) +def __has_package(lock, p): + return ( + p.name in lock.packages and + p.arch in lock.packages[p.name] and + p.version == lock.packages[p.name][p.arch].version + ) -def _add_package(lock, package, arch): - k = _package_key(package, arch) - if k in lock.fast_package_lookup: +def __add_package(lock, p, arch): + if __has_package(lock, p): return - lock.packages.append({ - "key": k, - "name": package["Package"], - "version": package["Version"], - "url": "%s/%s" % (package["Root"], package["Filename"]), - "sha256": package["SHA256"], - "arch": arch, - "dependencies": [], - }) - lock.fast_package_lookup[k] = len(lock.packages) - 1 - -def _add_package_dependency(lock, package, dependency, arch): - k = _package_key(package, arch) - if k not in lock.fast_package_lookup: - fail("Broken state: %s is not in the lockfile." % package["Package"]) - i = lock.fast_package_lookup[k] - lock.packages[i]["dependencies"].append(dict( - key = _package_key(dependency, arch), - name = dependency["Package"], - version = dependency["Version"], - )) - -def _has_package(lock, name, version, arch): - key = "%s_%s_%s" % (util.sanitize(name), util.sanitize(version), arch) - return key in lock.fast_package_lookup + + if p.name not in lock.packages: + lock.packages[p.name] = {} + + lock.packages[p.name][p.arch] = p + +def _add_package(lock, package, arch, dependencies_to_add = None): + dependencies_to_add = dependencies_to_add or [] + + p = pkg.from_index(package, arch) + + __add_package(lock, p, arch) + + # NOTE: sorting the dependencies makes the contents + # of the lock file stable and thus they diff better + dependencies_to_add = sorted( + dependencies_to_add, + key = lambda p: (p["Package"], p["Version"]), + ) + + for dependency in dependencies_to_add: + p_dep = pkg.from_index(dependency, arch) + + __add_package(lock, p_dep, arch) + + d = dep.from_index(dependency, arch) + lock.packages[p.name][p.arch].dependencies.append(d) + +def _as_json(lock): + return json.encode_indent( + struct( + version = lock.version, + packages = lock.packages, + ), + ) + +def _write(rctx, lock, out): + return rctx.file(out, _as_json(lock)) def _create(rctx, lock): return struct( - has_package = lambda *args, **kwargs: _has_package(lock, *args, **kwargs), - add_package = lambda *args, **kwargs: _add_package(lock, *args, **kwargs), - add_package_dependency = lambda *args, **kwargs: _add_package_dependency(lock, *args, **kwargs), - packages = lambda: lock.packages, - write = lambda out: rctx.file(out, json.encode_indent(struct(version = lock.version, packages = lock.packages))), - as_json = lambda: json.encode_indent(struct(version = lock.version, packages = lock.packages)), + packages = lock.packages, + add_package = lambda package, arch, dependencies_to_add: _add_package( + lock, + package, + arch, + dependencies_to_add, + ), + as_json = lambda: _as_json(lock), + write = lambda out: _write(rctx, lock, out), ) def _empty(rctx): - lock = struct( - version = 1, - packages = list(), - fast_package_lookup = dict(), - ) - return _create(rctx, lock) + return _create(rctx, __empty()) -def _from_json(rctx, content): - lock = json.decode(content) - if lock["version"] != 1: - fail("invalid lockfile version") +def _from_lock_v1(lock_content): + if lock_content["version"] != 1: + fail("Invalid lockfile version: %s" % lock_content["version"]) + + lockv2 = __empty() + + for package in lock_content["packages"]: + p = pkg.from_lock_v1(package) + __add_package(lockv2, p, p.arch) + + return lockv2 + +def _from_lock_v2(lock_content): + if lock_content["version"] != 2: + fail("Invalid lockfile version: %s" % lock_content["version"]) + + lockv2 = __empty() + + for archs in lock_content["packages"].values(): + for package in archs.values(): + p = pkg.from_lock(package) + __add_package(lockv2, p, p.arch) + + return lockv2 + +def _from_json(rctx, lock_content): + lock_content = json.decode(lock_content) + + if lock_content["version"] == 2: + lock = _from_lock_v2(lock_content) + elif lock_content["version"] == 1: + print( + "\n\nAuto-converting lockfile format from v1 to v2. " + + "To permanently convert an existing lockfile please run " + + "`bazel run @//:lock`\n\n", + ) + + lock = _from_lock_v1(lock_content) + else: + fail("Invalid lockfile version: %s" % lock_content["version"]) - lock = struct( - version = lock["version"], - packages = lock["packages"], - fast_package_lookup = dict(), - ) - for (i, package) in enumerate(lock.packages): - lock.packages[i] = package - lock.fast_package_lookup[package["key"]] = i return _create(rctx, lock) lockfile = struct( empty = _empty, from_json = _from_json, - make_package_key = _make_package_key, ) diff --git a/apt/private/manifest.bzl b/apt/private/manifest.bzl index c96d8cd..ceaf750 100644 --- a/apt/private/manifest.bzl +++ b/apt/private/manifest.bzl @@ -105,11 +105,7 @@ def _lock(rctx, manifest, include_transitive): if not package: fail("Unable to locate package `%s`" % package_name) - lockf.add_package(package, arch) - - for dep in dependencies: - lockf.add_package(dep, arch) - lockf.add_package_dependency(package, dep, arch) + lockf.add_package(package, arch, dependencies) return lockf diff --git a/apt/private/pkg.bzl b/apt/private/pkg.bzl new file mode 100644 index 0000000..68c6d7a --- /dev/null +++ b/apt/private/pkg.bzl @@ -0,0 +1,42 @@ +"pkg" + +def _pkg_from_index(package, arch): + return struct( + name = package["Package"], + version = package["Version"], + url = "%s/%s" % (package["Root"], package["Filename"]), + sha256 = package["SHA256"], + arch = arch, + dependencies = [], + ) + +def _dep_from_index(package, arch): + return struct( + name = package["Package"], + version = package["Version"], + ) + +def _pkg_from_lock(package): + package["dependencies"] = [struct(**d) for d in package["dependencies"]] + return struct(**package) + +def _pkg_from_lock_v1(package): + package["dependencies"] = [ + struct(**{"name": d["name"], "version": d["version"]}) + for d in package["dependencies"] + ] + sorted(package["dependencies"], key = lambda d: (d.name, d.version)) + + package.pop("key") + + return struct(**package) + +pkg = struct( + from_index = _pkg_from_index, + from_lock = _pkg_from_lock, + from_lock_v1 = _pkg_from_lock_v1, +) + +dep = struct( + from_index = _dep_from_index, +) diff --git a/examples/debian_snapshot_security/BUILD.bazel b/examples/debian_snapshot_security/BUILD.bazel index f20f5cb..30d0b4e 100644 --- a/examples/debian_snapshot_security/BUILD.bazel +++ b/examples/debian_snapshot_security/BUILD.bazel @@ -7,7 +7,7 @@ jq( "@apt_security_resolve//:lockfile", ], args = ["-rj"], - filter = '.packages | map(select(.name == "libuuid1")) | .[0].version', + filter = ".packages.libuuid1[] | .version", ) assert_contains(