From eebee27a08823023c3fd22efd9afcb5b6b29c7f4 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Wed, 2 Jun 2021 12:39:03 -0700 Subject: [PATCH] Base stamping decisions on --stamp configuration, not attributes on the rule 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 was already deprecated on container_push and container_bundle, it is now removed - the stamp attribute is removed from container_image, there is no replacement Fixes #1451 --- README.md | 70 +++++++++++++---------------------------- container/bundle.bzl | 17 ++-------- container/image.bzl | 43 +++---------------------- container/providers.bzl | 24 ++++++++++++-- container/push.bzl | 15 ++------- docker/BUILD | 17 ++++++++++ docker/context.bzl | 19 +++++++++++ testdata/BUILD | 20 +++++------- 8 files changed, 98 insertions(+), 127 deletions(-) create mode 100644 docker/context.bzl diff --git a/README.md b/README.md index ccc0ccf69..dd5c26ef8 100644 --- a/README.md +++ b/README.md @@ -250,14 +250,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 @@ -269,31 +273,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 `BuildContextInfo`, 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: +`bazel-out/stable-status.txt` and `bazel-out/volatile-status.txt`. -```shell -$ bazel build //docker/testdata:push_stamp -... +> Note that changes to the stable-status file +> cause a rebuild of the action, while volatile-status does not. -$ cat bazel-bin/docker/testdata/push_stamp.runfiles/io_bazel_rules_docker/stable-status.txt -BUILD_EMBED_LABEL -BUILD_HOST bazel -BUILD_USER mattmoor +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)"` -$ cat bazel-bin/docker/testdata/push_stamp.runfiles/io_bazel_rules_docker/volatile-status.txt -BUILD_TIMESTAMP 1498740967769 - -``` - -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 @@ -1546,20 +1548,6 @@ configuration. See [here](#container_push-custom-client-configuration) for detai

This field supports stamp variables.

- - stamp - -

Bool; optional

-

Deprecated: it is now automatically inferred.

-

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 the tag using Python format - syntax, e.g. - example.org/{BUILD_USER}/image:{BUILD_EMBED_LABEL}.

- - @@ -1794,26 +1782,12 @@ A rule that aliases and saves N images into a single `docker save` tarball.

A collection of the images to save into the tarball.

The keys are the tags with which to alias the image specified by the value. These tags may contain make variables ($FOO), - and if stamp is set to true, may also contain workspace + and may also contain workspace status variables ({BAR}).

The values may be the output of container_pull, container_image, or a docker save tarball.

- Toolchains - stamp - -

Bool; optional

-

Deprecated: it is now automatically inferred.

-

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 the tag using Python format - syntax, e.g. - example.org/{BUILD_USER}/image:{BUILD_EMBED_LABEL}.

- - diff --git a/container/bundle.bzl b/container/bundle.bzl index 6f2d9d600..785f38a8d 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", "BUILD_CONTEXT_ATTRS", "BuildContextInfo", "BundleInfo") load( "//container:layer_tools.bzl", _assemble_image = "assemble", @@ -38,18 +38,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.build_context_data[BuildContextInfo].stamp 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]) @@ -96,15 +89,11 @@ def _container_bundle_impl(ctx): ] container_bundle_ = rule( - attrs = dicts.add({ + attrs = dicts.add(BUILD_CONTEXT_ATTRS, { "image_target_strings": attr.string_list(), # Implicit dependencies. "image_targets": attr.label_list(allow_files = True), "images": attr.string_dict(), - "stamp": attr.bool( - default = False, - mandatory = False, - ), "tar_output": attr.output(), }, _layer_tools), executable = True, diff --git a/container/image.bzl b/container/image.bzl index b40c07620..dca2297af 100644 --- a/container/image.bzl +++ b/container/image.bzl @@ -26,6 +26,8 @@ load( ) load( "@io_bazel_rules_docker//container:providers.bzl", + "BUILD_CONTEXT_ATTRS", + "BuildContextInfo", "ImageInfo", "LayerInfo", ) @@ -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.build_context_data[BuildContextInfo].stamp 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, @@ -577,7 +552,7 @@ def _impl( ), ] -_attrs = dicts.add(_layer.attrs, { +_attrs = dicts.add(BUILD_CONTEXT_ATTRS, _layer.attrs, { "architecture": attr.string( doc = "The desired CPU architecture to be used as label in the container image.", default = "amd64", @@ -745,16 +720,6 @@ _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`.""", - ), "user": attr.string( doc = """The user that the image should run as. diff --git a/container/providers.bzl b/container/providers.bzl index c67dbedad..6873c701b 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,25 @@ FilterAspectInfo = provider( "depset": "a depset of struct(target=, target_deps=)", }, ) + +#Modelled after _GoContextData in rules_go/go/private/context.bzl +BuildContextInfo = provider( + doc = "Provides data about the build context, like config_setting's", + fields = { + "stamp": "If stamping is enabled", + }, +) + +BUILD_CONTEXT_ATTRS = { + "build_context_data": attr.label( + default = "@io_bazel_rules_docker//docker:build_context_data", + providers = [BuildContextInfo], + doc = """Provides info about the build context, such as stamping. + +By default it reads from the bazel command line, such as the `--stamp` argument. +Use this to override values for this target, such as enabling or disabling stamping. +You can use the `build_context_data` rule from `@io_bazel_rules_docker//docker:context.bzl` +to create a BuildContextInfo and override the --stamp config setting. +""", + ), +} diff --git a/container/push.bzl b/container/push.bzl index 032d1232d..62eb00419 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", "BUILD_CONTEXT_ATTRS", "BuildContextInfo", "PushInfo") 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.build_context_data[BuildContextInfo].stamp 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,14 +135,13 @@ def _impl(ctx): registry = registry, repository = repository, tag = tag, - stamp = stamp, stamp_inputs = stamp_inputs, digest = ctx.outputs.digest, ), ] _container_push = rule( - attrs = dicts.add({ + attrs = dicts.add(BUILD_CONTEXT_ATTRS, { "extension": attr.string( doc = "(optional) The file extension for the push script.", ), @@ -179,10 +174,6 @@ _container_push = rule( default = False, doc = "Only push images if the digest has changed, default to False", ), - "stamp": attr.bool( - default = False, - mandatory = False, - ), "tag": attr.string( default = "latest", doc = "(optional) The tag of the image, default to 'latest'.", diff --git a/docker/BUILD b/docker/BUILD index a7f15b237..c8d9abec8 100644 --- a/docker/BUILD +++ b/docker/BUILD @@ -13,6 +13,7 @@ # limitations under the License. load("@bazel_skylib//:bzl_library.bzl", "bzl_library") +load(":context.bzl", "build_context_data") package(default_visibility = ["//visibility:public"]) @@ -26,3 +27,19 @@ bzl_library( "@io_bazel_rules_docker//toolchains/docker", ], ) + +# Detect if the build is running under --stamp +config_setting( + name = "stamp", + values = {"stamp": "true"}, +) + +# Modelled after go_context_data from rules_go +# passes config values to starlark via a provider +build_context_data( + name = "build_context_data", + stamp = select({ + "@io_bazel_rules_docker//docker:stamp": True, + "//conditions:default": False, + }), +) diff --git a/docker/context.bzl b/docker/context.bzl new file mode 100644 index 000000000..d6b5d239e --- /dev/null +++ b/docker/context.bzl @@ -0,0 +1,19 @@ +"build_context_data rule" + +load("@io_bazel_rules_docker//container:providers.bzl", "BuildContextInfo") + +_DOC = """build_context_data gathers information about the build configuration. +It is a common dependency of all targets that are sensitive to configuration.""" + +def _impl(ctx): + return [BuildContextInfo(stamp = ctx.attr.stamp)] + +# Modelled after go_context_data in rules_go +# Works around github.com/bazelbuild/bazel/issues/1054 +build_context_data = rule( + implementation = _impl, + attrs = { + "stamp": attr.bool(mandatory = True), + }, + doc = _DOC, +) diff --git a/testdata/BUILD b/testdata/BUILD index 091cd5a33..0a29c03f4 100644 --- a/testdata/BUILD +++ b/testdata/BUILD @@ -39,6 +39,7 @@ load( container_image_with_tag = "container_image", ) load("//d:image.bzl", "d_image") +load("//docker:context.bzl", "build_context_data") load("//docker:docker.bzl", "docker_push") load("//go:image.bzl", "go_image") load("//groovy:image.bzl", "groovy_image") @@ -54,7 +55,6 @@ load("//python:image.bzl", "py_image", "py_layer") load("//python3:image.bzl", "py3_image") load("//scala:image.bzl", "scala_image") load("//testdata:utils.bzl", "generate_deb", "rule_with_symlinks") -load(":stamp_info.bzl", "stamp_info") package(default_visibility = ["//visibility:public"]) @@ -334,8 +334,8 @@ container_image( container_image( name = "with_stamped_creation_time", base = ":base_with_volume", + build_context_data = ":always_stamp", creation_time = "{BUILD_TIMESTAMP}", - stamp = True, ) container_image( @@ -346,7 +346,7 @@ container_image( container_image( name = "with_default_stamped_creation_time", base = ":base_with_volume", - stamp = True, + build_context_data = ":always_stamp", ) container_image( @@ -405,10 +405,10 @@ container_image( container_image( name = "with_stamp_label", base = ":base_with_volume", + build_context_data = ":always_stamp", labels = { "BUILDER": "{BUILD_USER}", }, - stamp = True, ) [genrule( @@ -992,8 +992,10 @@ generate_deb( metadata_compression_type = "xz", ) -# Make the Bazel stamp file available in tests. -stamp_info(name = "stamp_info_file") +build_context_data( + name = "always_stamp", + stamp = True, +) # Auto-generated by Go linter. go_binary( @@ -1096,12 +1098,6 @@ container_push( tags = ["manual"], ) -bzl_library( - name = "stamp_info_bzl", - srcs = ["stamp_info.bzl"], - visibility = ["//visibility:private"], -) - bzl_library( name = "utils_bzl", srcs = ["utils.bzl"],