Use ref.Name consistently in FirewallDriver.Update error formatting#64
Use ref.Name consistently in FirewallDriver.Update error formatting#64
Conversation
Pass ref.Name (not spec.Name) to validateFirewallTargets so the no-targets error always references the authoritative ref identity, even when ref.Name and spec.Name diverge or spec.Name is empty. Add regression test TestFirewallDriver_Update_NoTargets_RefNameInError that explicitly sets a different ref.Name and spec.Name to assert the error uses ref.Name. Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-plugin-digitalocean/sessions/8294bc4d-8194-4369-b026-d9e51cc8d951 Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes inconsistent error formatting in FirewallDriver.Update by ensuring the “no targets” validation error references the resource’s authoritative identifier (ref.Name) instead of spec.Name. This prevents confusing errors when ref.Name and spec.Name diverge (or when spec.Name is empty) and aligns the validation error with the rest of Update’s error messages.
Changes:
- Pass
ref.Name(notspec.Name) intovalidateFirewallTargetswithinFirewallDriver.Update. - Add a regression test asserting the no-targets error uses
ref.Namewhenref.Nameandspec.Namediffer.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/drivers/firewall.go | Make Update’s no-targets validation error use ref.Name for consistent, correct error identity. |
| internal/drivers/firewall_test.go | Add regression coverage for divergent ref.Name vs spec.Name ensuring the error string uses ref.Name. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Investigated the failure — it's in the Root cause: the workflow's This is a pre-existing CI workflow permissions issue, not related to the code changes in this PR. |
@copilot Switch to this token, I believe it has the access you'll need: |
Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-plugin-digitalocean/sessions/64500282-714c-45fe-ac51-3080b615ac0a Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Done in 88a6b34 — added |
iac-codemod refactor-apply reportMode: dry-run Skipped (// wfctl:skip-iac-codemod)
Full report (90-day retention) attached as workflow artifact. |
FirewallDriver.UpdatecalledvalidateFirewallTargets(spec.Name, req)while every other error in the same method usedref.Name. Ifref.Nameandspec.Namediverge (orspec.Nameis empty), the no-targets error references the wrong firewall.Changes
internal/drivers/firewall.go— Passref.NametovalidateFirewallTargetsinstead ofspec.Name, making error formatting consistent across the entireUpdatemethod.internal/drivers/firewall_test.go— AddTestFirewallDriver_Update_NoTargets_RefNameInError: regression test withref.Name = "ref-name"andspec.Name = "spec-name"asserting the error string containsref.Name, notspec.Name.