Base stamping decisions on --stamp configuration, not attributes on t…

…he 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 bazelbuild#1451
alexeagle · Jun 4, 2021 · bbbe99d · bbbe99d
1 parent e005f17
commit bbbe99d
Show file tree

Hide file tree

Showing 8 changed files with 98 additions and 127 deletions.
diff --git a/README.md b/README.md
@@ -250,14 +250,19 @@ 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 a bazel is run with `--stamp`.
+This enables replacements in stamp-aware attributes.
+A python format placeholder (e.g. `{BUILD_USER}`)
+is replaced by the corresponding key from the `bazel-out/stable-status.txt` or
+`bazel-out/volatile-status.txt` file.
 
 ```python
 # A common pattern when users want to avoid trampling
@@ -269,31 +274,28 @@ 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
 	<p>This field supports stamp variables.</p>
       </td>
     </tr>
-    <tr>
-      <td><code>stamp</code></td>
-      <td>
-        <p><code>Bool; optional</code></p>
-        <p>Deprecated: it is now automatically inferred.</p>
-        <p>If true, enable use of workspace status variables
-        (e.g. <code>BUILD_USER</code>, <code>BUILD_EMBED_LABEL</code>,
-        and custom values set using <code>--workspace_status_command</code>)
-        in tags.</p>
-        <p>These fields are specified in the tag using Python format
-        syntax, e.g.
-        <code>example.org/{BUILD_USER}/image:{BUILD_EMBED_LABEL}</code>.</p>
-      </td>
-    </tr>
   </tbody>
 </table>
 
@@ -1794,26 +1782,12 @@ A rule that aliases and saves N images into a single `docker save` tarball.
         <p>A collection of the images to save into the tarball.</p>
         <p>The keys are the tags with which to alias the image specified by the
            value. These tags may contain make variables (<code>$FOO</code>),
-           and if <code>stamp</code> is set to true, may also contain workspace
+           and may also contain workspace
            status variables (<code>{BAR}</code>).</p>
         <p>The values may be the output of <code>container_pull</code>,
            <code>container_image</code>, or a <code>docker save</code> tarball.</p>
       </td>
     </tr>
-    <tr>Toolchains
-      <td><code>stamp</code></td>
-      <td>
-        <p><code>Bool; optional</code></p>
-        <p>Deprecated: it is now automatically inferred.</p>
-        <p>If true, enable use of workspace status variables
-        (e.g. <code>BUILD_USER</code>, <code>BUILD_EMBED_LABEL</code>,
-        and custom values set using <code>--workspace_status_command</code>)
-        in tags.</p>
-        <p>These fields are specified in the tag using Python format
-        syntax, e.g.
-        <code>example.org/{BUILD_USER}/image:{BUILD_EMBED_LABEL}</code>.</p>
-      </td>
-    </tr>
   </tbody>
 </table>
 

diff --git 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
@@ -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
@@ -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>, target_deps=<depset>)",
     },
 )
+
+#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
@@ -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
@@ -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
@@ -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,
+)