Forward workload identity to volume drivers via mount config

[Viktor-Velkov]

Summary

Rep already knows the authoritative workload identity (process GUID for LRPs, task GUID for Tasks) at the moment it calls volumeManager.Mount, but does not forward it to the volume driver. Volume drivers that validate a mount against the workload's service bindings therefore have to reconstruct that identity out-of-band (e.g. a cfdot actual-lrps --cell-id cell-wide scan followed by a DesiredLRP fetch), which is slow (~1–2s typical, seconds under load) and requires sudo + shell tooling on the cell.

This PR enriches the mount config map with the workload identity Rep already holds, before calling Mount:

• _workload_guid — process_guid (LRP) or task_guid (Task)
• _workload_type — "lrp" or "task"

No protocol change is required — the keys ride the existing config map, which the Docker plugin's Create call already forwards to the driver. A binding-validating driver can then read the identity directly instead of reconstructing it.

Backward compatibility

Breaking change? No. The change is additive:

• the original config map is not mutated (a copy is enriched);
• containers without a lifecycle tag are returned unchanged;
• volume drivers that do not read the keys ignore them;
• the underscore prefix distinguishes these injected keys from broker-supplied config.

Tests

• Unit tests cover LRP, Task, missing-lifecycle and nil-config cases.
• Existing containerstore mount tests continue to pass.
• Validated end-to-end against a binding-validating volume driver.

Related

Design discussion (cross-component contract, alternatives considered): #1192

[geofffranks]

Thanks! Can you fix a couple things with this?

2. storenode.go:312 / containerstore_test.go:824 — Existing tests pass only because mock containers omit tags entirely. No integration test verifies that containerStore.Create actually passes the enriched map to volumeManager.Mount for tagged LRP/Task containers. — minor
2. storenode.go:337-353 — Hardcoded string literals ("lifecycle", "lrp", "task", "process-guid") duplicate constants already defined in rep (rep.LifecycleTag, etc.). Share the constants to make the coupling explicit. — minor
3. workload_identity_test.go:70-85 — Missing test case for an unrecognized lifecycle tag value (e.g., lifecycle: "other"). — minor

[Viktor-Velkov]

Thanks for the review! Pushed a commit addressing points 1 and 3:

• (1) Added integration coverage in containerstore_test.go — two contexts under "when there are volume mounts configured" now set a real lifecycle tag on the container and assert volumeManager.Mount receives the enriched config (_workload_guid / _workload_type) for both LRP and Task. This exercises the enrichment end-to-end through containerStore.Create; the previous specs passed only because their mock containers omitted tags.
• (3) Added a workload_identity_test.go case for an unrecognized lifecycle value ("other"): no identity keys injected, existing config preserved, original map not mutated.

On (2) — sharing the constants — I hit a snag and wanted your call before implementing. The tag-key constants (LifecycleTag, ProcessGuidTag, LRPLifecycle, TaskLifecycle) live in rep, but rep already imports code.cloudfoundry.org/executor, so having executor/depot/containerstore import rep would create an import cycle (rep → executor → rep). Two options:

• (a) Move those tag-key constants down into the executor package (the layer rep already depends on) and have rep reference them from there — single source of truth, but it touches rep.
• (b) Keep named local constants in containerstore, documented as mirroring rep's tag keys — smaller, no cross-package coupling, but not a true shared definition.

Do you have a preference? I'm inclined toward (a) for the explicit coupling you're after, but it's a slightly broader change so wanted to check first.

[geofffranks]

Yeah let's go with option A.

I ran a followup review thoguh and it had these new findings:

Correctness:
1. storenode.go:344 — When config is nil and lifecycle is unrecognized (e.g. "other"), the function returns a non-nil empty map instead of nil — inconsistent with the no-lifecycle-tag path. — minor
2. containerstore_test.go:872-891 — Task integration test only asserts MountArgsForCall(0), skips MountArgsForCall(1) and doesn't verify base config keys (some-config) are preserved. — minor
3. workload_identity_test.go:109-123 — Nil-config block only covers LRP; missing Task + nil-tags cases. — minor

Cleanup:
1. storenode.go:349-353 — Injected keys (_workload_guid, _workload_type, "lrp", "task") also hardcoded across test files; should be named constants. — minor
2. storenode.go:336-356 — A single switch info.Tags["lifecycle"] with default: return config + empty-GUID guard would fix C1, K1, and the nil-config inconsistency in one pass. — minor

[Viktor-Velkov]

Thanks! Went with option A and addressed the follow-up review — pushed as a new commit (b18065a).

Shared constants (option A)

Defined the tag keys and lifecycle values (LifecycleTag, ProcessGuidTag, LRPLifecycle, TaskLifecycle) plus the injected keys (WorkloadGuidKey, WorkloadTypeKey) in the executor package — the layer rep already depends on — and aliased rep's existing constants to them. Single source of truth, no import cycle (rep imports executor, not the reverse). All literals in the production and test code now use these constants.

Correctness

• C1 / nil-consistency: Restructured injectWorkloadIdentity around a single switch with default: return config. An unrecognized lifecycle (and the nil-config case) now returns the original map (which may be nil), matching the no-lifecycle-tag path, instead of allocating an empty map.
• Empty-GUID guard: an LRP whose process-guid is empty now returns the config untouched rather than injecting _workload_guid: "".
• C2: the Task integration test now asserts both MountArgsForCall(0) and (1), and that base config keys (some-config) are preserved.
• C3: the nil-config block now covers Task and no-lifecycle / unrecognized (→ nil) in addition to LRP.

Cleanup

• K1 is folded into the constants change above.
• K2's single-switch-with-default suggestion is exactly the shape I used, which collapses C1, K1, and the nil inconsistency into one pass.

Verification

Full rep suite is green (the constant aliasing is behavior-preserving), and containerstore is at 268/269.

The one red spec — containerstore_test.go › increments the ContainerExitedOnTimeoutCount and ContainerCompletedCount metric — is a pre-existing flake unrelated to this change: an Eventually-then-bare-Expect race on the deferred ContainerCompletedCount counter in the run goroutine. It fails ~4/5 runs on develop without my changes too.