acc: matrix the bundle suite over deployment-history recording

.github/workflows/push.yml

@@ -77,7 +77,7 @@ jobs:
 
     # Only run if the target is in the list of targets from testmask
     if: ${{ contains(fromJSON(needs.testmask.outputs.targets), 'test') }}
-    name: "task test (${{matrix.os.name}}, ${{matrix.deployment}})"
+    name: "task test (${{matrix.os.name}}, ${{matrix.deployment.name}})"
     runs-on: ${{ matrix.os.runner }}
 
     defaults:
@@ -111,9 +111,16 @@ jobs:
             runner:
               labels: macos-latest
 
+        # Each cell runs the suite once. envfilter picks the variants it covers: without the
+        # recording filter, the direct cell would run every bundle test twice, since
+        # bundle/test.toml matrixes deployment-history recording on top of the engine.
         deployment:
-          - "terraform"
-          - "direct"
+          - name: "terraform"
+            envfilter: "DATABRICKS_BUNDLE_ENGINE=terraform"
+          - name: "direct"
+            envfilter: "DATABRICKS_BUNDLE_ENGINE=direct,DMS="
+          - name: "directdms"
+            envfilter: "DATABRICKS_BUNDLE_ENGINE=direct,DMS=true"
 
         # Include "event_name" in the matrix so we can include/exclude based on it.
         event:
@@ -135,7 +142,7 @@ jobs:
       - name: Setup build environment
         uses: ./.github/actions/setup-build-environment
         with:
-          cache-key: test-${{ matrix.deployment }}
+          cache-key: test-${{ matrix.deployment.name }}
 
       # Make origin/main available with enough history for `git diff --merge-base
       # origin/main`, which the acceptance subset selector uses to always run the
@@ -154,7 +161,7 @@ jobs:
 
       - name: Run tests
         env:
-          ENVFILTER: DATABRICKS_BUNDLE_ENGINE=${{ matrix.deployment }}
+          ENVFILTER: ${{ matrix.deployment.envfilter }}
           # On pull requests, run only a subset of acceptance subtests on the slower
           # windows/macOS cells so neither lands on the critical path to merge. An
           # empty percentage disables subsetting, so Linux PRs and push-to-main run
@@ -171,7 +178,7 @@ jobs:
         if: ${{ always() }}
         uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
         with:
-          name: test-output-${{ matrix.os.name }}-${{ matrix.deployment }}
+          name: test-output-${{ matrix.os.name }}-${{ matrix.deployment.name }}
           path: test-output.json
           if-no-files-found: warn
           retention-days: 7

acceptance/acceptance_test.go

@@ -1063,6 +1063,13 @@ func checkEnvFilters(t *testing.T, testEnv, envFilters []string) {
 	}
 }
 
+// envAliases are short EnvMatrix keys, expanded here into the variable the CLI reads. Every
+// matrix key ends up in the variant's test name, so a long one makes every name that carries
+// it hard to read. Tests may still name the variable itself; the alias is only shorter.
+var envAliases = map[string]string{
+	"DMS": "DATABRICKS_BUNDLE_RECORD_DEPLOYMENT_HISTORY",
+}
+
 // buildTestEnv builds the test environment from config.Env and customEnv.
 // customEnv (from EnvMatrix) takes precedence over config.Env.
 func buildTestEnv(configEnv map[string]string, customEnv []string) []string {
@@ -1079,6 +1086,16 @@ func buildTestEnv(configEnv map[string]string, customEnv []string) []string {
 	// Add customEnv second (takes precedence)
 	env = append(env, customEnv...)
 
+	// An alias sets the variable it stands for, unless the test set that itself.
+	for _, kv := range customEnv {
+		key, value, _ := strings.Cut(kv, "=")
+		full, ok := envAliases[key]
+		if !ok || hasKey(env, full) {
+			continue
+		}
+		env = append(env, full+"="+value)
+	}
+
 	return env
 }
 

acceptance/bin/nostamp

@@ -0,0 +1,41 @@
+#!/usr/bin/env bash
+
+# Reads JSON on stdin, writes it back with the deployment stamp removed.
+#
+# Deployment history recording adds deployment_id and version_id to every job and
+# pipeline. Acceptance tests compare output byte for byte, so those two extra fields
+# would fail every test in the DATABRICKS_BUNDLE_RECORD_DEPLOYMENT_HISTORY=true run
+# (see bundle/test.toml).
+# Pipe a plan, a state dump, or a resource payload through this and the test asserts one
+# golden file either way. Tests under bundle/dms assert the stamp itself and must not.
+#
+# Three passes, because the stamp shows up in three shapes:
+#
+#   1. Nested in a deployment block, as printed by `jobs get` / `pipelines get`:
+#        "deployment": {"deployment_id": "87..", "kind": "BUNDLE",
+#                       "metadata_file_path": "/x", "version_id": "1"}
+#      -> both keys dropped, "kind" and "metadata_file_path" kept.
+#
+#   2. Flat in a plan's "changes", keyed by field path:
+#        "changes": {"deployment.version_id": {"action": "skip", ...}, "name": {...}}
+#      -> the stamp entries dropped, real changes kept.
+#
+#   3. A "changes" object that pass 2 emptied to {} - it existed only because of
+#      recording, so the key goes too.
+#
+# Two things it deliberately keeps:
+#
+#   - A deployment_id anywhere else. Pass 1 requires "kind" and "metadata_file_path" as
+#     neighbours, a pair unique to the deployment block, so an unrelated field of the
+#     same name is untouched.
+#   - "version_id": "". A terraform state dump carries that for a job it never stamped,
+#     and it is the test's own expected output, so `.value == ""` keeps it.
+#
+# Arguments are passed to jq, for callers whose input is not formatted the way jq
+# formats by default: a state dump is printed verbatim from disk, so it needs
+# --indent 1 to come back out unchanged.
+jq "$@" '((.. | objects | select(has("kind") and has("metadata_file_path")))
+     |= with_entries(select((.key | IN("deployment_id", "version_id")) == false or .value == "")))
+    | ((.. | objects | .changes? | objects)
+       |= with_entries(select(.key | IN("deployment.deployment_id", "deployment.version_id") | not)))
+    | del(.. | objects | select(.changes == {}) | .changes)'

acceptance/bin/print_requests.py

@@ -90,6 +90,15 @@
 ADD_PREFIX = "/"
 NEGATE_PREFIX = "^/"
 
+# What --nostamp deletes. A job create sends the deployment block at the top level, an
+# update nests it under new_settings.
+STAMP_FIELDS = [
+    "deployment.deployment_id",
+    "deployment.version_id",
+    "new_settings.deployment.deployment_id",
+    "new_settings.deployment.version_id",
+]
+
 
 def read_json_many(s):
     result = []
@@ -123,7 +132,13 @@ def read_json_many(s):
 assert result == [{"method": "GET"}, {"method": "POST"}], result
 
 
-def filter_requests(requests, path_filters, include_get, should_sort, unique=False, method_filter=None):
+# DMS_PATH is the deployment metadata service's prefix; see filter_requests.
+DMS_PATH = "/api/2.0/bundle"
+
+
+def filter_requests(
+    requests, path_filters, include_get, should_sort, unique=False, method_filter=None, include_dms=False
+):
     """Filter requests based on method and path filters."""
     positive_filters = []
     negative_filters = []
@@ -136,6 +151,13 @@ def filter_requests(requests, path_filters, include_get, should_sort, unique=Fal
         else:
             sys.exit(f"Unrecognized filter: {f!r}")
 
+    # Deployment-history requests carry the resource key in the path
+    # (.../operations/jobs.foo), so a filter like //jobs matches them too and every test
+    # that records one resource type would pick these up in the recording run. Excluded
+    # unless --dms asks for them.
+    if not include_dms:
+        negative_filters.append(DMS_PATH)
+
     filtered_requests = []
     for req in requests:
         if method_filter:
@@ -178,11 +200,27 @@ def filter_requests(requests, path_filters, include_get, should_sort, unique=Fal
     return filtered_requests
 
 
+def del_path(body, field):
+    """Delete field from body. A dotted field descends into nested objects, e.g.
+    deployment.version_id removes only that key from the deployment block."""
+    *parents, leaf = field.split(".")
+    for name in parents:
+        body = body.get(name)
+        if not isinstance(body, dict):
+            return
+    body.pop(leaf, None)
+
+
 def main():
     parser = argparse.ArgumentParser()
     parser.add_argument("path_filters", nargs="*", help="Path substring filters")
     parser.add_argument("-v", "--verbose", action="store_true", help="Enable diagnostic messages")
     parser.add_argument("--get", action="store_true", help="Include GET requests (excluded by default)")
+    parser.add_argument(
+        "--dms",
+        action="store_true",
+        help="Include deployment-history requests (excluded by default; see filter_requests)",
+    )
     parser.add_argument("--keep", action="store_true", help="Keep out.requests.json file after processing")
     parser.add_argument("--sort", action="store_true", help="Sort requests before output")
     parser.add_argument(
@@ -210,10 +248,19 @@ def main():
         "--del-body, which edits the parsed JSON body, this drops a field of the request "
         "record itself, e.g. raw_body for a binary upload payload.",
     )
+    parser.add_argument(
+        "--nostamp",
+        action="store_true",
+        help="Drop the deployment stamp (deployment_id, version_id) from job and pipeline "
+        "bodies, so a test asserts the same requests whether or not deployment history "
+        "recording is on. Shorthand for the --del-body fields it implies.",
+    )
     parser.add_argument("--fname", default="out.requests.txt")
     args = parser.parse_args()
 
     del_body_fields = [field for group in args.del_body for field in group.split(",")]
+    if args.nostamp:
+        del_body_fields += STAMP_FIELDS
     del_fields = [field for group in args.del_field for field in group.split(",")]
 
     test_tmp_dir = os.environ.get("TEST_TMP_DIR")
@@ -232,13 +279,15 @@ def main():
         return
 
     requests = read_json_many(data)
-    filtered_requests = filter_requests(requests, args.path_filters, args.get, args.sort, args.unique, args.method)
+    filtered_requests = filter_requests(
+        requests, args.path_filters, args.get, args.sort, args.unique, args.method, args.dms
+    )
 
     for req in filtered_requests:
         body = req.get("body")
         if isinstance(body, dict):
             for field in del_body_fields:
-                body.pop(field, None)
+                del_path(body, field)
         for field in del_fields:
             req.pop(field, None)
     if args.verbose:

acceptance/bundle/ai_runtime_task/local_code_source/output.txt

@@ -21,7 +21,7 @@ src/train.py
 
 === both code_source_paths point into the bundle .air_snapshots, command_paths rewritten, deps on environments spec
 
->>> print_requests.py --sort --del-field raw_body //.air_snapshots/ //jobs/create
+>>> print_requests.py --nostamp --sort --del-field raw_body //.air_snapshots/ //jobs/create
 {
   "method": "POST",
   "path": "/api/2.0/workspace-files/import-file/Workspace/Users/[USERNAME]/.bundle/ai-runtime-test/default/files/.air_snapshots/[SNAPSHOT].tar.gz",
@@ -114,7 +114,7 @@ Updated jobs.train
 Files: 4 uploaded, 1 deleted
 Resources: 0 created, 1 changed, 0 deleted, 0 unchanged
 
->>> print_requests.py --sort --del-field raw_body //.air_snapshots/
+>>> print_requests.py --nostamp --sort --del-field raw_body //.air_snapshots/
 {
   "method": "POST",
   "path": "/api/2.0/workspace-files/import-file/Workspace/Users/[USERNAME]/.bundle/ai-runtime-test/default/files/.air_snapshots/[SNAPSHOT].tar.gz",

acceptance/bundle/ai_runtime_task/local_code_source/script

@@ -13,16 +13,16 @@ trace list_code_snapshot.py
 title "both code_source_paths point into the bundle .air_snapshots, command_paths rewritten, deps on environments spec\n"
 # --del-field raw_body drops the binary tarball upload payload (kept readable). Filters
 # use a leading // so Git Bash on Windows does not path-convert them. --keep is not
-# passed, so print_requests.py consumes out.requests.txt.
-trace print_requests.py --sort --del-field raw_body '//.air_snapshots/' '//jobs/create'
+# passed, so print_requests.py --nostamp consumes out.requests.txt.
+trace print_requests.py --nostamp --sort --del-field raw_body '//.air_snapshots/' '//jobs/create'
 
 title "re-planning unchanged code is a no-op (no changes)\n"
 trace $CLI bundle plan
 
 title "editing a file changes the snapshot hash (content-addressed name changes)\n"
 update_file.py src/train.py 'print("training")' 'print("training v2")'
 trace $CLI bundle deploy
-trace print_requests.py --sort --del-field raw_body '//.air_snapshots/'
+trace print_requests.py --nostamp --sort --del-field raw_body '//.air_snapshots/'
 
 title "destroy removes the deployed bundle (including the synced snapshots)\n"
 trace $CLI bundle destroy --auto-approve