Skip to content

feat(config): add firewall-group cross-resource references#15

Open
devantler wants to merge 3 commits into
mainfrom
claude/firewall-group-refs
Open

feat(config): add firewall-group cross-resource references#15
devantler wants to merge 3 commits into
mainfrom
claude/firewall-group-refs

Conversation

@devantler

Copy link
Copy Markdown
Contributor

🤖 Generated by the Daily AI Assistant

Why

Firewall resources reference other UniFi objects (groups, networks, zones) by raw ID strings a user
can only know after those objects reconcile — the same wiring pain that cross-resource references
already solved for traffic routes. The firewall group is the densest cluster of these references and
the natural next place to remove that friction.

What

Adds Crossplane cross-resource references to firewall_rule and firewall_policy so a rule/policy
can point at a FirewallGroup, Network, or FirewallZone by name (via the generated *Ref/*Selector
companions) instead of a hard-coded post-create id. The raw id fields stay settable directly, so this
is purely additive — no behaviour change for existing manifests.

First reviewable increment of the cross-resource-references theme; sibling resources follow as
separate children.

Part of #10
Fixes #11

Add Crossplane cross-resource references for unifi_firewall_rule and
unifi_firewall_policy so a rule/policy can point at a FirewallGroup, Network,
or FirewallZone by name via the generated *Ref/*Selector companions instead of
a hard-coded post-reconcile UniFi id. The raw id fields stay settable, so the
change is purely additive.

- firewall_rule: src/dst_firewall_group_ids (list) -> Group,
  src/dst_network_id -> Network
- firewall_policy: source/destination.network_ids (list) -> Network,
  source/destination.zone_id -> FirewallZone (nested under the single-nested
  source/destination blocks, not top-level as the issue table assumed)

Generated on Linux (upjet make generate); deltas are firewall-scoped only.
Adds an example wiring a Rule to a Group and Network by *Ref.

Part of #10
Fixes #11
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds Crossplane reference and selector support for firewall Policy and Rule resources in both cluster-scoped and namespaced APIs. Generated types gain new *Ref/*Refs and *Selector fields, deepcopy methods and resolver methods are extended for those fields, the Terraform generator config is updated to map firewall fields to group, network, and zone resources, and the generated CRDs and example manifest are updated to use the new reference-based fields.

Related issues: #11

Suggested labels: generated, api-change, firewall

Suggested reviewers: devantler

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR adds the requested rule and policy references, preserves raw fields, includes generated resolvers/CRDs, and adds an example.
Out of Scope Changes check ✅ Passed The added files and config wiring all support firewall cross-resource references; no clearly unrelated code change stands out.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title is concise and correctly points to the firewall cross-resource reference addition, though it narrows the scope to firewall groups.
Description check ✅ Passed The description matches the changeset and clearly explains the added Crossplane references for firewall rules and policies.

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

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

⚠️MegaLinter analysis: Success with warnings

⚠️ MARKDOWN / markdownlint - 3 errors
.github/PULL_REQUEST_TEMPLATE.md:9 error MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "### Description of your change..."]
CLAUDE.md:1 error MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "@AGENTS.md"]
CODE_OF_CONDUCT.md:1 error MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "## Code of Conduct"]

✅ Linters with no issues

actionlint, bash-exec, checkov, cspell, git_diff, hadolint, jscpd, jsonlint, lychee, markdown-table-formatter, prettier, prettier, revive, shellcheck, shfmt, syft, trivy-sbom, trufflehog, v8r, v8r, yamllint

Notices

📣 MegaLinter 9.5.0 is out! Discover the new features and security recommendations in the release announcement. (Skip this info by defining SECURITY_SUGGESTIONS: false)

See detailed reports in MegaLinter artifacts

MegaLinter is graciously provided by OX Security
Show us your support by starring ⭐ the repository

goconst flags unifi_network (now 5 occurrences across shortGroups keys and the
new firewall references). Pull the referenced Terraform resource names out as
constants so each literal appears once. Reference values are unchanged, so the
generated output is identical (no regeneration).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@config/unifi/config.go`:
- Around line 74-83: The new resource-name constants are only used in the
cross-resource references, while shortGroups in config/unifi/config.go still
repeats the same string literals. Update the shortGroups map entries to use
resFirewallGroup, resFirewallZone, resNetwork, and resVPNClient so all
references to these resource names share the same constants and the duplicate
literals are removed consistently.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 45e26328-14d8-45ec-aa56-4113869dd701

📥 Commits

Reviewing files that changed from the base of the PR and between 450a1cb and 74fc8f2.

📒 Files selected for processing (1)
  • config/unifi/config.go
📜 Review details
⏰ Context from checks skipped due to timeout. (14)
  • GitHub Check: 🧪 Test
  • GitHub Check: 🔍 Dead Code Analysis
  • GitHub Check: 📊 Code Coverage
  • GitHub Check: 🏗️ Build
  • GitHub Check: 🧹 Lint - mega-linter
  • GitHub Check: 📦 Tidy
  • GitHub Check: 🧹 Lint - golangci-lint
  • GitHub Check: 🛡️ Vulnerability Scan
  • GitHub Check: check-diff
  • GitHub Check: unit-tests
  • GitHub Check: lint
  • GitHub Check: local-deploy
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (1)
config/unifi/config.go

📄 CodeRabbit inference engine (AGENTS.md)

Use config/unifi/config.go to define resource grouping (ShortGroups) and Kind disambiguation.

Files:

  • config/unifi/config.go
🔇 Additional comments (4)
config/unifi/config.go (4)

74-83: LGTM on the constant extraction.

Clean deduplication of repeated Terraform resource-name literals for goconst compliance.


97-102: LGTM!


104-122: Reference wiring for unifi_firewall_rule matches the issue requirements.

src_firewall_group_ids/dst_firewall_group_idsunifi_firewall_group, and src_network_id/dst_network_idunifi_network, aligning with issue #11's spec.


124-142: Nested reference paths for unifi_firewall_policy look correct.

Keying by source.network_ids/source.zone_id and destination.network_ids/destination.zone_id matches the documented upjet pattern for referencing fields nested in single-nested blocks (e.g., the zone_mappings.vswitch_id example used elsewhere in the upjet ecosystem), and correctly targets unifi_network/unifi_firewall_zone.

Comment thread config/unifi/config.go
Switch the shortGroups map keys for firewall_group, firewall_zone,
network, and vpn_client to the resFirewallGroup/resFirewallZone/
resNetwork/resVPNClient constants so each resource name has a single
source of truth instead of a repeated string literal.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Daily AI Assistant

Good catch — applied in d2be335. The shortGroups keys for firewall_group, firewall_zone, network, and vpn_client now use the resFirewallGroup/resFirewallZone/resNetwork/resVPNClient constants, so those resource names have a single source of truth. Map values (the API groups) are unchanged, so generation output is byte-identical (check-diff stays green).

@devantler devantler marked this pull request as ready for review July 8, 2026 06:20
@devantler devantler requested a review from a team as a code owner July 8, 2026 06:20
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.

feat(config): add firewall-group cross-resource references (firewall_rule/policy → group/network/zone)

1 participant