Skip to content

Use ref.Name consistently in FirewallDriver.Update error formatting#64

Merged
intel352 merged 3 commits intomainfrom
copilot/use-ref-name-consistently
May 6, 2026
Merged

Use ref.Name consistently in FirewallDriver.Update error formatting#64
intel352 merged 3 commits intomainfrom
copilot/use-ref-name-consistently

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 5, 2026

FirewallDriver.Update called validateFirewallTargets(spec.Name, req) while every other error in the same method used ref.Name. If ref.Name and spec.Name diverge (or spec.Name is empty), the no-targets error references the wrong firewall.

Changes

  • internal/drivers/firewall.go — Pass ref.Name to validateFirewallTargets instead of spec.Name, making error formatting consistent across the entire Update method.

  • internal/drivers/firewall_test.go — Add TestFirewallDriver_Update_NoTargets_RefNameInError: regression test with ref.Name = "ref-name" and spec.Name = "spec-name" asserting the error string contains ref.Name, not spec.Name.

// Before
validateFirewallTargets(spec.Name, req)

// After
validateFirewallTargets(ref.Name, req)

Copilot AI requested review from Copilot and removed request for Copilot May 5, 2026 07:48
Copilot AI linked an issue May 5, 2026 that may be closed by this pull request
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>
Copilot AI requested review from Copilot and removed request for Copilot May 5, 2026 07:58
Copilot AI changed the title [WIP] Update FirewallDriver.Update error formatting to use ref.Name Use ref.Name consistently in FirewallDriver.Update error formatting May 5, 2026
Copilot AI requested a review from intel352 May 5, 2026 07:58
@intel352 intel352 marked this pull request as ready for review May 5, 2026 08:13
Copilot AI review requested due to automatic review settings May 5, 2026 08:13
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (not spec.Name) into validateFirewallTargets within FirewallDriver.Update.
  • Add a regression test asserting the no-targets error uses ref.Name when ref.Name and spec.Name differ.

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.

@intel352
Copy link
Copy Markdown
Contributor

intel352 commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 5, 2026

@copilot failing https://github.com/GoCodeAlone/workflow-plugin-digitalocean/actions/runs/25364660655/job/74374514742?pr=64

Investigated the failure — it's in the codemod-report job at the "Comment summary on PR" step, which gets a 403 Resource not accessible by integration when posting a comment via the GitHub API.

Root cause: the workflow's GITHUB_TOKEN has issues: write but not pull-requests: write, so it can't comment on PRs. The actual iac-codemod refactor-apply (dry-run) step succeeded with 0 errors.

This is a pre-existing CI workflow permissions issue, not related to the code changes in this PR.

@intel352
Copy link
Copy Markdown
Contributor

intel352 commented May 5, 2026

@copilot failing https://github.com/GoCodeAlone/workflow-plugin-digitalocean/actions/runs/25364660655/job/74374514742?pr=64

Investigated the failure — it's in the codemod-report job at the "Comment summary on PR" step, which gets a 403 Resource not accessible by integration when posting a comment via the GitHub API.

Root cause: the workflow's GITHUB_TOKEN has issues: write but not pull-requests: write, so it can't comment on PRs. The actual iac-codemod refactor-apply (dry-run) step succeeded with 0 errors.

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: RELEASES_TOKEN

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 5, 2026

@copilot failing https://github.com/GoCodeAlone/workflow-plugin-digitalocean/actions/runs/25364660655/job/74374514742?pr=64

Investigated the failure — it's in the `codem...

Done in 88a6b34 — added github-token: ${{ secrets.RELEASES_TOKEN }} to the github-script step in .github/workflows/codemod-report.yml so the PR comment is posted with that token instead of the default GITHUB_TOKEN.

@intel352
Copy link
Copy Markdown
Contributor

intel352 commented May 5, 2026

iac-codemod refactor-apply report

Mode: dry-run
Sites: 1
Errors: 0

Skipped (// wfctl:skip-iac-codemod)

  • internal/provider.go:238 DOProvider.Apply skipped

Full report (90-day retention) attached as workflow artifact.

@intel352 intel352 merged commit a761a42 into main May 6, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use ref.Name consistently for FirewallDriver.Update error formatting

3 participants