OSAC-2361: add deletion protection for default networking resources

[ori-amizur]

Resources labeled osac.openshift.io/default: "true" (VirtualNetwork, Subnet, SecurityGroup, NATGateway, ExternalIP) are system-managed and cannot be deleted via the API. The Delete handler for each resource type now fetches the object and checks for the default label before proceeding, returning FailedPrecondition if present.

Summary by CodeRabbit

• Bug Fixes
  • Prevented deletion of default, system-managed networking resources.
  • Added protection for default external IPs, NAT gateways, security groups, subnets, and virtual networks.
  • Deletion attempts for protected resources now return a clear precondition error instead of proceeding.

[openshift-ci-robot]

@ori-amizur: This pull request references OSAC-2361 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Resources labeled osac.openshift.io/default: "true" (VirtualNetwork, Subnet, SecurityGroup, NATGateway, ExternalIP) are system-managed and cannot be deleted via the API. The Delete handler for each resource type now fetches the object and checks for the default label before proceeding, returning FailedPrecondition if present.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ori-amizur

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [ori-amizur]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Walkthrough

Deletion now rejects default-labeled ExternalIPs, NATGateways, SecurityGroups, Subnets, and VirtualNetworks with a gRPC FailedPrecondition error. Tests cover each protected resource type.

Changes

Default resource deletion protection

Layer / File(s)	Summary
Default-resource validation contract internal/servers/default_networking_provisioner.go	Adds shared validation that rejects resources labeled osac.openshift.io/default: "true" with a FailedPrecondition error.
Deletion guards across networking resources internal/servers/private_*_server.go	Delete handlers validate resource metadata before continuing with existing deletion operations.
Default deletion rejection tests internal/servers/*_test.go	Tests verify default-labeled resources cannot be deleted and return the expected gRPC status and message.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• osac-project/fulfillment-service#964: Adds default-networking provisioning in the same area and relates to system-managed default resources.

Suggested reviewers: danmanor, jhernand

🚥 Pre-merge checks | ✅ 10 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Ai-Attribution	⚠️ Warning	HEAD uses Co-Authored-By: Claude Opus 4.6, and no Assisted-by/Generated-by Red Hat trailer is present.	Replace the AI co-author line with an Assisted-by or Generated-by trailer per Red Hat policy; keep Co-Authored-By only for human coauthors.

✅ Passed checks (10 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: deletion protection for default networking resources.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
No-Hardcoded-Secrets	✅ Passed	Patch adds only labels/status checks and test assertions; no api_key/secret/token/password vars, base64 blobs, embedded creds, or private keys were found.
No-Weak-Crypto	✅ Passed	Touched files only add delete-label validation and gRPC status handling; no weak crypto, custom crypto, or secret/token comparisons were introduced.
No-Injection-Vectors	✅ Passed	No SQL/shell/eval/yaml/pickle/innerHTML patterns were added; the PR only adds label checks and gRPC precondition errors in delete paths.
Container-Privileges	✅ Passed	No container/K8s manifests changed; diff only touches Go server logic/tests, and no privileged/hostNetwork/allowPrivilegeEscalation fields were found.
No-Sensitive-Data-In-Logs	✅ Passed	No new secret/PII logging found; changes only add delete guards and existing log calls use resource IDs/tenant names, not passwords or tokens.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

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 `@internal/servers/security_groups_server_test.go`:
- Around line 469-493: Add this default-labeled deletion case to the
PrivateSecurityGroupsServer test suite, using privatev1 create and delete
requests so it exercises PrivateSecurityGroupsServer.Delete. Preserve the
existing assertions for FailedPrecondition and the “default” and
“system-managed” error details, and avoid relying on the public
SecurityGroupsServer test for coverage.

🪄 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: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 549470d9-a36d-4a3a-aaaf-81e321bf47c2

📥 Commits

Reviewing files that changed from the base of the PR and between a15b93c and f593e29.

📒 Files selected for processing (11)

• internal/servers/default_networking_provisioner.go
• internal/servers/private_external_ips_server.go
• internal/servers/private_external_ips_server_test.go
• internal/servers/private_nat_gateways_server.go
• internal/servers/private_nat_gateways_server_test.go
• internal/servers/private_security_groups_server.go
• internal/servers/private_subnets_server.go
• internal/servers/private_subnets_server_test.go
• internal/servers/private_virtual_networks_server.go
• internal/servers/private_virtual_networks_server_test.go
• internal/servers/security_groups_server_test.go

[sk-ilya]

Per the EP (and jira), default resources should be protected from deletion while resources depend on them. but here it's blocked unconditionally based just on the default label