From 2b35b2dd56f0be6cc6b8df957332a31435f6b3ce Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Fri, 26 Nov 2021 11:42:56 -0700 Subject: [PATCH] By defaults, stamp only under --stamp configuration. (#1878) This follows the example of rules_go and rules_nodejs. It means we don't produce cache-busting outputs unless the user explicitly requests non-deteriminism in the build with --stamp. BREAKING CHANGE: - To get stamped outputs, users must now call bazel with `--stamp` - the stamp attribute is removed from some providers - the stamp attribute is now trinary Fixes #1451 --- README.md | 40 ++++++++++++++++++++------------------- container/bundle.bzl | 16 +++------------- container/image.bzl | 42 ++++------------------------------------- container/providers.bzl | 29 ++++++++++++++++++++++++++-- container/push.bzl | 14 +++----------- docs/BUILD | 2 +- docs/container.md | 4 ++-- stamp/BUILD.bazel | 30 +++++++++++++++++++++++++++++ stamp/stamp.bzl | 18 ++++++++++++++++++ testdata/BUILD | 10 +++++++--- 10 files changed, 116 insertions(+), 89 deletions(-) create mode 100644 stamp/BUILD.bazel create mode 100644 stamp/stamp.bzl diff --git a/README.md b/README.md index e3a63a64b..cf5f85df8 100644 --- a/README.md +++ b/README.md @@ -253,14 +253,18 @@ to use `container_push` with custom docker authentication credentials. ## Varying image names -A common request from folks using `container_push` or `container_bundle` is to +A common request from folks using +`container_push`, `container_bundle`, or `container_image` is to be able to vary the tag that is pushed or embedded. There are two options at present for doing this. ### Stamping -The first option is to use stamping. Stamping is enabled when a supported -attribute contains a python format placeholder (e.g. `{BUILD_USER}`). +The first option is to use stamping. +Stamping is enabled when bazel is run with `--stamp`. +This enables replacements in stamp-aware attributes. +A python format placeholder (e.g. `{BUILD_USER}`) +is replaced by the value of the corresponding workspace-status variable. ```python # A common pattern when users want to avoid trampling @@ -272,31 +276,29 @@ container_push( # Any of these components may have variables. registry = "gcr.io", repository = "my-project/my-image", + # This will be replaced with the current user when built with --stamp tag = "{BUILD_USER}", ) ``` +> Rules that are sensitive to stamping can also be forced to stamp or non-stamp mode +> irrespective of the `--stamp` flag to Bazel. Use the `build_context_data` rule +> to make a target that provides `StampSettingInfo`, and pass this to the +> `build_context_data` attribute. + The next natural question is: "Well what variables can I use?" This option consumes the workspace-status variables Bazel defines in -`stable-status.txt` and `volatile-status.txt`. These files will appear -in the target's runfiles: - -```shell -$ bazel build //docker/testdata:push_stamp -... - -$ cat bazel-bin/docker/testdata/push_stamp.runfiles/io_bazel_rules_docker/stable-status.txt -BUILD_EMBED_LABEL -BUILD_HOST bazel -BUILD_USER mattmoor +`bazel-out/stable-status.txt` and `bazel-out/volatile-status.txt`. -$ cat bazel-bin/docker/testdata/push_stamp.runfiles/io_bazel_rules_docker/volatile-status.txt -BUILD_TIMESTAMP 1498740967769 +> Note that changes to the stable-status file +> cause a rebuild of the action, while volatile-status does not. -``` +You can add more stamp variables via `--workspace_status_command`, +see the [bazel docs](https://docs.bazel.build/versions/master/user-manual.html#workspace_status). +A common example is to provide the current git SHA, with +`--workspace_status_command="echo STABLE_GIT_SHA $(git rev-parse HEAD)"` -You can augment these variables via `--workspace_status_command`, -including through the use of [`.bazelrc`](https://github.com/kubernetes/kubernetes/blob/81ce94ae1d8f5d04058eeb214e9af498afe78ff2/build/root/.bazelrc#L6). +That flag is typically passed in the `.bazelrc` file, see for example [`.bazelrc` in kubernetes](https://github.com/kubernetes/kubernetes/blob/81ce94ae1d8f5d04058eeb214e9af498afe78ff2/build/root/.bazelrc#L6). ### Make variables diff --git a/container/bundle.bzl b/container/bundle.bzl index 43c2e74e2..9ce24dc07 100644 --- a/container/bundle.bzl +++ b/container/bundle.bzl @@ -14,7 +14,7 @@ """Rule for bundling Container images into a tarball.""" load("@bazel_skylib//lib:dicts.bzl", "dicts") -load("@io_bazel_rules_docker//container:providers.bzl", "BundleInfo") +load("@io_bazel_rules_docker//container:providers.bzl", "BundleInfo", "STAMP_ATTR", "StampSettingInfo") load( "//container:layer_tools.bzl", _assemble_image = "assemble", @@ -40,18 +40,11 @@ def _container_bundle_impl(ctx): images = {} runfiles = [] - if ctx.attr.stamp: - print("Attr 'stamp' is deprecated; it is now automatically inferred. Please remove it from %s" % ctx.label) - stamp = False + stamp = ctx.attr.stamp[StampSettingInfo].value for unresolved_tag in ctx.attr.images: # Allow users to put make variables into the tag name. tag = ctx.expand_make_variables("images", unresolved_tag, {}) - # If any tag contains python format syntax (which is how users - # configure stamping), we enable stamping. - if "{" in tag: - stamp = True - target = ctx.attr.images[unresolved_tag] layer = _get_layers(ctx, ctx.label.name, image_target_dict[target]) @@ -101,10 +94,7 @@ container_bundle_ = rule( # Implicit dependencies. "image_targets": attr.label_list(allow_files = True), "images": attr.string_dict(), - "stamp": attr.bool( - default = False, - mandatory = False, - ), + "stamp": STAMP_ATTR, "tar_output": attr.output(), "experimental_tarball_format": attr.string( values = [ diff --git a/container/image.bzl b/container/image.bzl index b40c07620..b9cf0de3e 100644 --- a/container/image.bzl +++ b/container/image.bzl @@ -28,6 +28,8 @@ load( "@io_bazel_rules_docker//container:providers.bzl", "ImageInfo", "LayerInfo", + "STAMP_ATTR", + "StampSettingInfo", ) load( "//container:layer.bzl", @@ -105,14 +107,7 @@ def _add_create_image_config_args( args.add_all(ctx.attr.ports, before_each = "-ports") args.add_all(ctx.attr.volumes, before_each = "-volumes") - stamp = None - - # If base image is having enabled stamping then it is propagated - # to child images. - if ctx.attr.stamp == True: - stamp = ctx.attr.stamp - elif ctx.attr.base and ImageInfo in ctx.attr.base: - stamp = ctx.attr.base[ImageInfo].stamp + stamp = ctx.attr.stamp[StampSettingInfo].value if creation_time: args.add("-creationTime", creation_time) @@ -540,31 +535,11 @@ def _impl( ([container_parts["legacy"]] if container_parts["legacy"] else []), ) - # Stamp attribute needs to be propagated between definitions to enhance actions - # with ability to determine properly whether root image has activated stamping. - # - # This covers the following example case: - # container_image( - # name = “base_image”, - # base = “@base//image”, - # stamp = True, - # ) - # - # lang_image( - # base = “:base_image”, - # ) - stamp = None - if ctx.attr.stamp: - stamp = ctx.attr.stamp - elif ctx.attr.base and ImageInfo in ctx.attr.base: - stamp = ctx.attr.base[ImageInfo].stamp - return [ ImageInfo( container_parts = container_parts, legacy_run_behavior = ctx.attr.legacy_run_behavior, docker_run_flags = docker_run_flags, - stamp = stamp, ), DefaultInfo( executable = build_executable, @@ -745,16 +720,7 @@ _attrs = dicts.add(_layer.attrs, { Setting this attribute to `gcr.io/dummy` would set the default tag to `gcr.io/dummy/package_name:target`.""", ), - "stamp": attr.bool( - default = False, - doc = """If true, enable use of workspace status variables - (e.g. `BUILD_USER`, `BUILD_EMBED_LABEL`, - and custom values set using `--workspace_status_command`) - in tags. - - These fields are specified in attributes using Python format - syntax, e.g. `foo{BUILD_USER}bar`.""", - ), + "stamp": STAMP_ATTR, "user": attr.string( doc = """The user that the image should run as. diff --git a/container/providers.bzl b/container/providers.bzl index c67dbedad..09e19c4db 100644 --- a/container/providers.bzl +++ b/container/providers.bzl @@ -27,7 +27,6 @@ ImageInfo = provider(fields = [ "container_parts", "legacy_run_behavior", "docker_run_flags", - "stamp", ]) # A provider containing information exposed by container_import rules @@ -54,7 +53,6 @@ PushInfo = provider(fields = [ "registry", "repository", "tag", - "stamp", "stamp_inputs", "digest", ]) @@ -73,3 +71,30 @@ FilterAspectInfo = provider( "depset": "a depset of struct(target=, target_deps=)", }, ) + +#Modelled after _GoContextData in rules_go/go/private/context.bzl +StampSettingInfo = provider( + fields = { + "value": "Whether stamping is enabled", + }, +) + +STAMP_ATTR = attr.label( + default = "@io_bazel_rules_docker//stamp:use_stamp_flag", + providers = [StampSettingInfo], + doc = """Whether to encode build information into the output. Possible values: + + - `@io_bazel_rules_docker//stamp:always`: + Always stamp the build information into the output, even in [--nostamp][stamp] builds. + This setting should be avoided, since it potentially causes cache misses remote caching for + any downstream actions that depend on it. + + - `@io_bazel_rules_docker//stamp:never`: + Always replace build information by constant values. This gives good build result caching. + + - `@io_bazel_rules_docker//stamp:use_stamp_flag`: + Embedding of build information is controlled by the [--[no]stamp][stamp] flag. + Stamped binaries are not rebuilt unless their dependencies change. + + [stamp]: https://docs.bazel.build/versions/main/user-manual.html#flag--stamp""", +) diff --git a/container/push.bzl b/container/push.bzl index 966b5866b..8979e06f6 100644 --- a/container/push.bzl +++ b/container/push.bzl @@ -17,7 +17,7 @@ Bazel rule for publishing images. """ load("@bazel_skylib//lib:dicts.bzl", "dicts") -load("@io_bazel_rules_docker//container:providers.bzl", "PushInfo") +load("@io_bazel_rules_docker//container:providers.bzl", "PushInfo", "STAMP_ATTR", "StampSettingInfo") load( "//container:layer_tools.bzl", _gen_img_args = "generate_args_for_image", @@ -61,11 +61,7 @@ def _impl(ctx): tag = "$(cat {})".format(_get_runfile_path(ctx, ctx.file.tag_file)) pusher_input.append(ctx.file.tag_file) - # If any stampable attr contains python format syntax (which is how users - # configure stamping), we enable stamping. - if ctx.attr.stamp: - print("Attr 'stamp' is deprecated; it is now automatically inferred. Please remove it from %s" % ctx.label) - stamp = "{" in tag or "{" in registry or "{" in repository + stamp = ctx.attr.stamp[StampSettingInfo].value stamp_inputs = [ctx.info_file, ctx.version_file] if stamp else [] for f in stamp_inputs: pusher_args += ["-stamp-info-file", "%s" % _get_runfile_path(ctx, f)] @@ -139,7 +135,6 @@ def _impl(ctx): registry = registry, repository = repository, tag = tag, - stamp = stamp, stamp_inputs = stamp_inputs, digest = ctx.outputs.digest, ), @@ -179,10 +174,7 @@ container_push_ = rule( default = False, doc = "Only push images if the digest has changed, default to False", ), - "stamp": attr.bool( - default = False, - mandatory = False, - ), + "stamp": STAMP_ATTR, "tag": attr.string( default = "latest", doc = "The tag of the image.", diff --git a/docs/BUILD b/docs/BUILD index 2a237c68e..f0141d63c 100644 --- a/docs/BUILD +++ b/docs/BUILD @@ -1,5 +1,5 @@ -load("@bazel_skylib//rules:write_file.bzl", "write_file") load("@bazel_skylib//rules:diff_test.bzl", "diff_test") +load("@bazel_skylib//rules:write_file.bzl", "write_file") load("@io_bazel_stardoc//stardoc:stardoc.bzl", "stardoc") # Workaround https://github.com/bazelbuild/stardoc/issues/25 diff --git a/docs/container.md b/docs/container.md index 528500a2f..f4930a32b 100644 --- a/docs/container.md +++ b/docs/container.md @@ -26,7 +26,7 @@ A rule that aliases and saves N images into a single `docker save` tarball. | image_targets | - | List of labels | optional | [] | | images | - | Dictionary: String -> String | optional | {} | | incremental_load_template | - | Label | optional | //container:incremental_load_template | -| stamp | - | Boolean | optional | False | +| stamp | Whether to encode build information into the output. Possible values:

- @io_bazel_rules_docker//stamp:always: Always stamp the build information into the output, even in [--nostamp][stamp] builds. This setting should be avoided, since it potentially causes cache misses remote caching for any downstream actions that depend on it.

- @io_bazel_rules_docker//stamp:never: Always replace build information by constant values. This gives good build result caching.

- @io_bazel_rules_docker//stamp:use_stamp_flag: Embedding of build information is controlled by the [--[no]stamp][stamp] flag. Stamped binaries are not rebuilt unless their dependencies change.

[stamp]: https://docs.bazel.build/versions/main/user-manual.html#flag--stamp | Label | optional | @io_bazel_rules_docker//stamp:use_stamp_flag | | tar_output | - | Label | optional | | @@ -231,7 +231,7 @@ container_push(name, repository | The name of the image. | String | required | | | repository_file | The label of the file with repository value. Overrides 'repository'. | Label | optional | None | | skip_unchanged_digest | Only push images if the digest has changed, default to False | Boolean | optional | False | -| stamp | - | Boolean | optional | False | +| stamp | Whether to encode build information into the output. Possible values:

- @io_bazel_rules_docker//stamp:always: Always stamp the build information into the output, even in [--nostamp][stamp] builds. This setting should be avoided, since it potentially causes cache misses remote caching for any downstream actions that depend on it.

- @io_bazel_rules_docker//stamp:never: Always replace build information by constant values. This gives good build result caching.

- @io_bazel_rules_docker//stamp:use_stamp_flag: Embedding of build information is controlled by the [--[no]stamp][stamp] flag. Stamped binaries are not rebuilt unless their dependencies change.

[stamp]: https://docs.bazel.build/versions/main/user-manual.html#flag--stamp | Label | optional | @io_bazel_rules_docker//stamp:use_stamp_flag | | tag | The tag of the image. | String | optional | "latest" | | tag_file | The label of the file with tag value. Overrides 'tag'. | Label | optional | None | | tag_tpl | The script template to use. | Label | required | | diff --git a/stamp/BUILD.bazel b/stamp/BUILD.bazel new file mode 100644 index 000000000..7b46e1337 --- /dev/null +++ b/stamp/BUILD.bazel @@ -0,0 +1,30 @@ +"Helpers to determine when to stamp build outputs" + +load(":stamp.bzl", "stamp_setting") + +package(default_visibility = ["//visibility:public"]) + +# Detect if the build is running under --stamp +config_setting( + name = "stamp", + values = {"stamp": "true"}, +) + +# Enable stamping based on the --stamp flag +stamp_setting( + name = "use_stamp_flag", + stamp = select({ + "@io_bazel_rules_docker//stamp:stamp": True, + "//conditions:default": False, + }), +) + +stamp_setting( + name = "always", + stamp = True, +) + +stamp_setting( + name = "never", + stamp = False, +) diff --git a/stamp/stamp.bzl b/stamp/stamp.bzl new file mode 100644 index 000000000..742af6891 --- /dev/null +++ b/stamp/stamp.bzl @@ -0,0 +1,18 @@ +"Helper for determining when to stamp build outputs" + +load("@io_bazel_rules_docker//container:providers.bzl", "StampSettingInfo") + +def _impl(ctx): + return [StampSettingInfo(value = ctx.attr.stamp)] + +# Modelled after go_context_data in rules_go +# Works around github.com/bazelbuild/bazel/issues/1054 +stamp_setting = rule( + implementation = _impl, + attrs = { + "stamp": attr.bool(mandatory = True), + }, + doc = """Determines whether build outputs should be stamped with version control info. + + Stamping causes outputs to be non-deterministic, resulting in cache misses.""", +) diff --git a/testdata/BUILD b/testdata/BUILD index 091cd5a33..9099f625d 100644 --- a/testdata/BUILD +++ b/testdata/BUILD @@ -335,18 +335,19 @@ container_image( name = "with_stamped_creation_time", base = ":base_with_volume", creation_time = "{BUILD_TIMESTAMP}", - stamp = True, + stamp = "//stamp:always", ) container_image( name = "with_base_stamped_image", base = ":with_stamped_creation_time", + stamp = "//stamp:always", ) container_image( name = "with_default_stamped_creation_time", base = ":base_with_volume", - stamp = True, + stamp = "//stamp:always", ) container_image( @@ -408,7 +409,7 @@ container_image( labels = { "BUILDER": "{BUILD_USER}", }, - stamp = True, + stamp = "//stamp:always", ) [genrule( @@ -466,6 +467,7 @@ docker_push( image = ":link_with_files_base", registry = "gcr.io", repository = "convoy-adapter/bazel-oci-test", + stamp = "//stamp:always", tag = "{BUILD_USER}", ) @@ -474,6 +476,7 @@ oci_push( image = ":link_with_files_base", registry = "gcr.io", repository = "convoy-adapter/bazel-oci-test", + stamp = "//stamp:always", tag = "{BUILD_USER}", ) @@ -516,6 +519,7 @@ container_bundle( # Pad username so that it is long enough. "example.com/aaaaa{BUILD_USER}:stamped": ":with_user", }, + stamp = "//stamp:always", ) # Generate a dummy debian package with a test/ directory