Add dynamic disk CLI support: disks --dynamic and delete-disk --dynamic

[Alphasite]

Summary

Adds CLI support for BOSH Director dynamic disks, scoped to listing and deletion:

• bosh disks --dynamic — list all dynamic disks (name, CID, size, disk pool,
  deployment, instance, AZ, CPI)
• bosh delete-disk CID-OR-NAME --dynamic — delete a dynamic disk by name
  (with confirmation)

Provide/create/attach/detach are intentionally not included in this PR and
will follow once their Director endpoints land.

Changes

• director/interfaces.go — added DynamicDisk interface + DynamicDisks() and
  DeleteDynamicDisk() Director methods
• director/dynamic_disks.go — HTTP client implementation for the list + delete
  endpoints (delete validates a non-empty disk name)
• director/dynamic_disks_test.go — request-path/response-parsing/error specs
• director/directorfakes/ — regenerated FakeDirector + new FakeDynamicDisk
• cmd/opts/opts.go — --dynamic flag on DisksOpts and DeleteDiskOpts
  (positional arg relabeled CID-OR-NAME)
• cmd/disks.go — --dynamic listing; rejects --orphaned --dynamic together
• cmd/delete_disk.go — --dynamic deletes by name

Test plan

• go build ./... — clean build
• go test ./cmd/... ./director/... — all passing
• go generate ./director/... — fakes regenerated cleanly
• gofmt -l — clean

Related: TNZ-99509, TNZ-109499

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e01b737d-6b88-4ffa-90f4-76dfa9713379

📥 Commits

Reviewing files that changed from the base of the PR and between 062a1a8 and 001aa75.

📒 Files selected for processing (2)

• cmd/opts/opts_test.go
• director/dynamic_disks.go

💤 Files with no reviewable changes (1)

• director/dynamic_disks.go

---

Walkthrough

Adds dynamic disk lifecycle contracts and HTTP client support, including listing and deletion operations. The disks command gains --dynamic listing with a dynamic disk table, while delete-disk gains --dynamic deletion by name. Generated director and dynamic disk fakes support the new interfaces. Tests cover option tags, table rendering, error propagation, deletion behavior, and confirmation rejection.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: dynamic disk CLI support for disks and delete-disk commands.
Description check	✅ Passed	The description matches the changeset and accurately outlines the CLI, Director, tests, and fake updates.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/dynamic-disk-cli-commands

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

Adds new BOSH CLI support for managing “dynamic disks” via the Director API, including create/provide/attach/detach/delete operations and a bosh disks --dynamic listing mode.

Changes:

• Extended the Director interface with dynamic disk methods and added a DynamicDisk model interface.
• Implemented Director HTTP client calls for dynamic disk endpoints and regenerated Counterfeiter fakes.
• Added new CLI commands + options and updated bosh disks to support --dynamic, with command-level tests.

Reviewed changes

Copilot reviewed 16 out of 18 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
director/interfaces.go	Adds DynamicDisk interface and new Director methods for dynamic disk operations.
director/dynamic_disks.go	Implements dynamic disk HTTP API calls and DirectorImpl delegations.
director/directorfakes/fake_dynamic_disk.go	New Counterfeiter fake for DynamicDisk.
director/directorfakes/fake_director.go	Regenerated fake to include new dynamic disk methods.
cmd/opts/opts.go	Adds new command option structs and --dynamic flag for bosh disks.
cmd/cmd.go	Wires new command opts to their command handlers.
cmd/provide_disk.go	Implements bosh provide-disk.
cmd/provide_disk_test.go	Adds tests for ProvideDiskCmd (needs gofmt).
cmd/create_dynamic_disk.go	Implements bosh create-dynamic-disk.
cmd/create_dynamic_disk_test.go	Adds tests for CreateDynamicDiskCmd.
cmd/attach_dynamic_disk.go	Implements bosh attach-dynamic-disk.
cmd/attach_dynamic_disk_test.go	Adds tests for AttachDynamicDiskCmd (needs gofmt).
cmd/detach_dynamic_disk.go	Implements bosh detach-dynamic-disk with confirmation.
cmd/detach_dynamic_disk_test.go	Adds tests for DetachDynamicDiskCmd.
cmd/delete_dynamic_disk.go	Implements bosh delete-dynamic-disk with confirmation.
cmd/delete_dynamic_disk_test.go	Adds tests for DeleteDynamicDiskCmd.
cmd/disks.go	Extends bosh disks to support --dynamic listing.
cmd/disks_test.go	Adds tests for bosh disks --dynamic and updated error message.

Files not reviewed (2)

• director/directorfakes/fake_director.go: Generated file
• director/directorfakes/fake_dynamic_disk.go: Generated file

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmd/attach_dynamic_disk_test.go`:
- Around line 33-40: The test setup block containing the slug initialization and
attachOpts assignment has incorrect indentation that is causing goimports/gofmt
linting failures. Fix the indentation by aligning the closing brace of the
AttachDynamicDiskOpts struct initialization so that it matches the indentation
level of the opening line where attachOpts is assigned. Ensure the closing brace
for the outer struct and the closing brace for the nested Args field are
properly aligned according to Go formatting standards.

In `@cmd/opts/opts.go`:
- Around line 126-132: The cmd/opts/opts.go file is failing goimports linting
checks. Run the goimports formatter on this file using the command goimports -w
cmd/opts/opts.go to automatically fix the import formatting issues, then commit
the formatted result. This will resolve the lint failures on both macOS and
Ubuntu CI jobs.

In `@cmd/provide_disk_test.go`:
- Around line 33-42: The ProvideDiskOpts struct initialization in the test has
improper indentation alignment. The DiskPool and Size fields within the
ProvideDiskOpts initialization are incorrectly indented and should be aligned at
the same level as the Args field. Fix the indentation of these nested fields so
they align properly according to Go's gofmt standards, ensuring the opening and
closing braces match their corresponding field declarations.

In `@director/dynamic_disks.go`:
- Around line 94-101: The function currently returns a nil error with an empty
DiskCID when the response body is empty or when the disk_cid field is missing
from the JSON response, which appears as a false success. After the
json.Unmarshal call completes (or when resultBytes is empty), add validation to
check that result.DiskCID is not empty and return an appropriate error if the
DiskCID is missing, ensuring the function fails fast on invalid responses before
returning.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2bbbe253-7862-4302-86e1-a11acba294cb

📥 Commits

Reviewing files that changed from the base of the PR and between 38ac211 and e093a56.

📒 Files selected for processing (18)

• cmd/attach_dynamic_disk.go
• cmd/attach_dynamic_disk_test.go
• cmd/cmd.go
• cmd/create_dynamic_disk.go
• cmd/create_dynamic_disk_test.go
• cmd/delete_dynamic_disk.go
• cmd/delete_dynamic_disk_test.go
• cmd/detach_dynamic_disk.go
• cmd/detach_dynamic_disk_test.go
• cmd/disks.go
• cmd/disks_test.go
• cmd/opts/opts.go
• cmd/provide_disk.go
• cmd/provide_disk_test.go
• director/directorfakes/fake_director.go
• director/directorfakes/fake_dynamic_disk.go
• director/dynamic_disks.go
• director/interfaces.go

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

cmd/disks.go (1)

22-27: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reject conflicting disk-list selectors.

With the new early opts.Dynamic branch, bosh disks --orphaned --dynamic silently ignores --orphaned and returns only dynamic disks. Validate the combination before dispatching so users cannot receive a different resource set from the flags they supplied.

Suggested fix

 func (c DisksCmd) Run(opts DisksOpts) error {
+	if opts.Orphaned && opts.Dynamic {
+		return errors.New("Only one of --orphaned or --dynamic may be used")
+	}
 	if opts.Dynamic {
 		return c.runDynamic()
 	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/disks.go` around lines 22 - 27, Validate the --orphaned and --dynamic
combination before the early opts.Dynamic dispatch in the disk-list command.
Reject cases where both selectors are enabled, then preserve the existing
runDynamic and unsupported-selector behavior for valid flag combinations.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmd/opts/opts.go`:
- Around line 752-754: The DeleteDiskOpts positional argument is misleadingly
labeled CID when DeleteDiskCmd.Run forwards it to DeleteDynamicDisk as a name
under --dynamic. Update the positional argument’s label/field definition to a
mode-neutral name such as DISK-CID-OR-NAME, preserving its use for both regular
and dynamic deletion.

---

Outside diff comments:
In `@cmd/disks.go`:
- Around line 22-27: Validate the --orphaned and --dynamic combination before
the early opts.Dynamic dispatch in the disk-list command. Reject cases where
both selectors are enabled, then preserve the existing runDynamic and
unsupported-selector behavior for valid flag combinations.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1aa9dd29-d514-453b-8937-90a2c9e1f231

📥 Commits

Reviewing files that changed from the base of the PR and between e093a56 and 0541a50.

📒 Files selected for processing (9)

• cmd/delete_disk.go
• cmd/delete_disk_test.go
• cmd/disks.go
• cmd/disks_test.go
• cmd/opts/opts.go
• cmd/opts/opts_test.go
• director/directorfakes/fake_dynamic_disk.go
• director/dynamic_disks.go
• director/interfaces.go

[Copilot]

Pull request overview

Copilot reviewed 8 out of 10 changed files in this pull request and generated 7 comments.

Files not reviewed (2)

• director/directorfakes/fake_director.go: Generated file
• director/directorfakes/fake_dynamic_disk.go: Generated file

Comments suppressed due to low confidence (4)

director/dynamic_disks.go:93

• JSON POST requests here pass nil header setters (e.g. ProvideDynamicDisk/CreateDynamicDisk/AttachDynamicDisk). Most other Director client methods explicitly set Content-Type: application/json for JSON payloads; without it these endpoints may fail against Directors that require the header.

	resultBytes, err := c.taskClientRequest.PostResult("/dynamic_disks/provide", bodyBytes, nil)
	if err != nil {

director/dynamic_disks.go:76

• There are no director-package HTTP client specs covering the new dynamic disk endpoints (provide/detach/delete/list/create/attach). The director package has extensive request/response tests for other endpoints; adding similar specs would help prevent request-path/body/tag regressions.

func (c Client) ProvideDynamicDisk(instanceID, diskName, diskPool string, sizeInMB int, metadata map[string]interface{}) (string, error) {

cmd/disks.go:28

• DisksCmd.Run treats --dynamic as higher priority than --orphaned. If a user supplies both flags, the command will silently ignore --orphaned, which is ambiguous; consider rejecting the combination with a clear error message.

func (c DisksCmd) Run(opts DisksOpts) error {
	if opts.Dynamic {
		return c.runDynamic()
	}

	if !opts.Orphaned {
		return errors.New("Only --orphaned or --dynamic is supported") //nolint:staticcheck
	}

cmd/opts/opts.go:760

• DeleteDiskOpts --dynamic reuses DeleteDiskArgs.CID with positional-arg-name:"CID", but the flag description says it deletes a dynamic disk by name. This will likely show confusing help output (CID vs disk name) and blurs the meaning of the positional argument.

type DeleteDiskOpts struct {
	Args    DeleteDiskArgs `positional-args:"true" required:"true"`
	Dynamic bool           `long:"dynamic" description:"Delete a dynamic disk by name"`
	cmd
}

type DeleteDiskArgs struct {
	CID string `positional-arg-name:"CID"`
}

[Copilot]

Pull request overview

Copilot reviewed 9 out of 11 changed files in this pull request and generated 1 comment.

Files not reviewed (2)

• director/directorfakes/fake_director.go: Generated file
• director/directorfakes/fake_dynamic_disk.go: Generated file

Comments suppressed due to low confidence (1)

cmd/opts/opts.go:749

• DisksOpts adds a new --dynamic flag, but the opts-tag test suite only asserts the Orphaned tag for this struct. This makes it easy to accidentally break the CLI flag wiring for bosh disks --dynamic without a unit test catching it. Consider adding a DisksOpts.Dynamic struct-tag assertion in cmd/opts/opts_test.go alongside the existing DisksOpts.Orphaned test.

type DisksOpts struct {
	Orphaned bool `long:"orphaned" short:"o" description:"List orphaned disks"`
	Dynamic  bool `long:"dynamic" description:"List dynamic disks"`
	cmd