[KauzClay]

What is this change about?

When a deploy removes a provided link, the link stays in records.json and in the .bosh/links.json file of the job. It goes away only on the next deploy. Until then, the stale link group keeps the BOSH DNS link alias for the removed link alive.

The next deploy then renders a links.json without the link. The templates change, so the job restarts with no manifest change. For example, cloud_controller_ng restarted on an Apply Changes after a release removed a link.

The cause is the order of the deploy steps:

• The links parser gives each intent in the manifest the current links_serial_id.
• The update stage renders the DNS links and links.json from Planner#link_provider_intents. That method did not filter by serial_id.
• The deploy deletes the unused providers in the cleanup step, after the update stage.

The consumer path already filters by serial_id in LinksManager. The provider path got the unfiltered list in 5297d5c, when links.json and the link groups in records.json were added.

The fix filters the intents by links_serial_id only for a deploy action. A deploy action always runs the links parser, so each current intent has the current serial.

Other actions, such as cck and recreate, do not run the parser. A deploy can fail after it increments the serial and before it parses the links. Then no intent has the new serial. A filter on those actions would give empty DNS links and an empty links.json.

Alternatives ruled out:

• Call remove_unused_links before the update stage. That method deletes each consumer link that has no InstancesLink at the current serial. The update stage sets those serials. An early call deletes nearly all consumer links, and a failed deploy cannot restore them.
• Filter for all actions. This fails for cck and recreate after a failed deploy, as described above.

Please provide contextual information.

• Regression from 5297d5c ([bosh-dns] add metadata about link based dns names). Related: 3974876, 4675288.

What tests have you run against this PR?

• started a test pipeline here: https://bosh.ci.cloudfoundry.org/teams/main/pipelines/test-delete-when-finished-removed-link-provider
• planner_spec.rb: 34 examples, 0 failures. Without the fix, the new deploy-action example fails.
• Related unit specs (update_deployment_spec, deployment_plan/, links/, errand/, cloudcheck_helper_spec): 1775 examples, 0 failures.
• Integration spec spec/integration/links/removed_link_provider_spec.rb, with 2 examples. One example removes custom_provider_definitions from the manifest. The other uploads a release version that no longer provides the link. After the removal deploy, each example deploys again with no change. The output must not contain Updating instance mysql.
  • With the fix: 2 examples, 0 failures.
  • Without the fix: 2 examples, 2 failures. .bosh/links.json still has the link: expected ["my-custom-link", "db"] not to include ....

How should this change be described in bosh release notes?

When a deploy removes a provided link, the Director now removes the link from the BOSH DNS records and from .bosh/links.json in the same deploy. The next deploy no longer restarts the job.

Does this PR introduce a breaking change?

No.

Tag your pair, your PM, and/or team!

AI Review Feedback

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 62bdc671-0c9c-495f-8f90-f680e26ce2ab

📥 Commits

Reviewing files that changed from the base of the PR and between b40d089 and 84cc1af.

📒 Files selected for processing (1)

• src/spec/integration/links/removed_link_provider_spec.rb

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

For deploy actions, Planner#link_provider_intents now returns intents that match the deployment model’s links serial ID. For non-deploy actions, it returns all provider intents. Unit and integration specs cover these results and verify that removing a link provider removes its link and DNS group ID after redeployment.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 84cc1

Deploys filter removed provider links while preserving existing link data for non-deploy actions. The reviewed change has no identified material regression and appears ready to merge after normal checks.

Security architecture risk: 🔵 Low · up to 84cc1

The change narrows deploy-time output to current link providers without adding a public endpoint or increasing privileges. Existing recovery behavior is preserved. Risk remains low because interrupted deployments can leave mixed persisted and rendered state, and recovery scenarios are not demonstrated by the added tests.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The directly changed data is provider metadata owned by the deployment being planned, with downstream effects on job link files and DNS discovery for dependent consumers. The change narrows deploy-time output; it does not grant additional control over other deployments or introduce a new production invocation path.

Security Findings and Attack Paths

• inferred — No introduced or materially worsened attack path was established in the inspected change. The public-entrypoint routing corresponds to test code, and the production branch removes older intent entries rather than accepting new input or increasing sink authority.

Trust Boundaries and Controls

• observed — The deployment operation encloses serial advancement, parsing, and updates in a deployment-named lock. Serial matching acts as a freshness control over deployment-owned metadata, not an authentication or authorization mechanism.

Hardening Proposals

• proposed — Exercise interruption after serial advancement and after a partial instance update, followed by non-deploy repair and redeploy. Check persisted intents, rendered link files, DNS groups, and metadata aliases together to document recovery and prevent discovery-state drift. This is additional assurance, not a verified vulnerability.

Pre-merge checks · 4 · 1

Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title clearly and concisely describes the main change: filtering stale link provider intents during deploys.
Description check		The description covers the change, rationale, context, alternatives, tests, release notes, and breaking-change status. The team-tag section is empty, but this is non-critical and the description is ot…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.}

[KauzClay]

testing here: https://bosh.ci.cloudfoundry.org/teams/main/pipelines/test-delete-when-finished-removed-link-provider

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

🟢 Approval recommended

The narrow filter matches the deploy lifecycle, preserves other actions, and includes focused regression coverage with no unresolved findings.

0 open findings

🧠 Review effort: Balanced

[a-hassanin]

Thank you for your contribution!