OCPBUGS-94042: Return proper 404 when deleting a non-existent project

[tchap]

The project delete handler wraps NotFound errors from the underlying namespace GET with fmt.Errorf, turning the *StatusError into a *fmt.wrapError. The apiserver framework's ErrorToAPIStatus uses a direct type assertion (not errors.As) to extract the status, so the wrapped error falls through to the default case and produces HTTP 500 with no Reason. This makes --ignore-not-found ineffective because errors.IsNotFound checks for Reason=NotFound or Code=404.

Return NotFound errors unwrapped from getProjectForDeletion and treat them as terminal in the retry loop so the original StatusError reaches ErrorToAPIStatus intact, producing a proper 404 response.

Summary by CodeRabbit

• Bug Fixes
  • Project deletion now stops immediately when the requested project cannot be found, avoiding unnecessary retry attempts.
  • Other project retrieval errors continue to follow the existing retry behavior.
  • Error handling now more clearly distinguishes missing projects from temporary or unexpected retrieval failures, providing more predictable deletion outcomes.

[openshift-ci-robot]

@tchap: This pull request references Jira Issue OCPBUGS-94042, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (5.0.0) matches configured target version for branch (5.0.0)
• bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

The project delete handler wraps NotFound errors from the underlying namespace GET with fmt.Errorf, turning the *StatusError into a *fmt.wrapError. The apiserver framework's ErrorToAPIStatus uses a direct type assertion (not errors.As) to extract the status, so the wrapped error falls through to the default case and produces HTTP 500 with no Reason. This makes --ignore-not-found ineffective because errors.IsNotFound checks for Reason=NotFound or Code=404.

Return NotFound errors unwrapped from getProjectForDeletion and treat them as terminal in the retry loop so the original StatusError reaches ErrorToAPIStatus intact, producing a proper 404 response.

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 NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign deads2k for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• pkg/project/OWNERS

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

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 58e35f30-fdba-4bc2-9361-aa38d807ca52

📥 Commits

Reviewing files that changed from the base of the PR and between 58298ec and 1cb13a9.

📒 Files selected for processing (2)

• pkg/project/apiserver/registry/project/proxy/proxy.go
• pkg/project/apiserver/registry/project/proxy/proxy_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• pkg/project/apiserver/registry/project/proxy/proxy.go
• pkg/project/apiserver/registry/project/proxy/proxy_test.go

---

Walkthrough

Project deletion now returns project not-found errors immediately. Other lookup errors retain their retry and contextual wrapping behavior. Tests verify error classification, request count, and validation calls.

Changes

Project deletion error handling

Layer / File(s)	Summary
Deletion lookup handling pkg/project/apiserver/registry/project/proxy/proxy.go, pkg/project/apiserver/registry/project/proxy/proxy_test.go	Deletion lookup returns not-found errors without retries or contextual wrapping. Other errors keep existing behavior. Tests verify one GET attempt, zero validation calls, and error classification.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: derekwaynecarr, deads2k

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: returning HTTP 404 when deleting a non-existent project.
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.
Stable And Deterministic Test Names	✅ Passed	The changed test adds a static t.Run title; the file uses Go table tests, not Ginkgo, and no title contains runtime data, timestamps, IDs, nodes, namespaces, or IPs.
Test Structure And Quality	✅ Passed	The changed tests are standard Go unit tests using fake clients, not Ginkgo; they create no cluster resources, use bounded retry behavior, and include assertions for error, status, validation, and...
Microshift Test Compatibility	✅ Passed	The pull request adds only standard Go unit-test cases in proxy_test.go; it adds no Ginkgo e2e tests, so the MicroShift API compatibility check does not apply.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The changes add standard Go unit-test cases in TestDeleteProject, not new Ginkgo e2e tests. No SNO multi-node assumptions apply.
Topology-Aware Scheduling Compatibility	✅ Passed	The pull request changes project deletion error handling and tests only; it adds no manifests, operators, controllers, replicas, affinities, topology constraints, node selectors, or tolerations.
Ote Binary Stdout Contract	✅ Passed	The PR changes only project deletion logic and unit-test assertions; it adds no process-level OTE stdout writes or suite setup.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The PR adds standard Go unit-test cases in TestDeleteProject, not Ginkgo e2e tests; the added lines contain no IPv4 assumptions or external connectivity.
No-Weak-Crypto	✅ Passed	The PR changes only NotFound handling and tests in project deletion; no MD5, SHA1, DES, RC4, Blowfish, ECB, custom crypto, or secret comparisons were added.
Container-Privileges	✅ Passed	The pull request changes only Go source and test files; it adds no container or Kubernetes manifest settings covered by this check.
No-Sensitive-Data-In-Logs	✅ Passed	The PR only changes error classification and tests; it adds no logging calls or sensitive-data output in the affected files.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[openshift-ci-robot]

@tchap: This pull request references Jira Issue OCPBUGS-94042, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (5.0.0) matches configured target version for branch (5.0.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Details

In response to this:

The project delete handler wraps NotFound errors from the underlying namespace GET with fmt.Errorf, turning the *StatusError into a *fmt.wrapError. The apiserver framework's ErrorToAPIStatus uses a direct type assertion (not errors.As) to extract the status, so the wrapped error falls through to the default case and produces HTTP 500 with no Reason. This makes --ignore-not-found ineffective because errors.IsNotFound checks for Reason=NotFound or Code=404.

Return NotFound errors unwrapped from getProjectForDeletion and treat them as terminal in the retry loop so the original StatusError reaches ErrorToAPIStatus intact, producing a proper 404 response.

Summary by CodeRabbit

• Bug Fixes
• Project deletion now stops immediately when the requested project cannot be found.
• Other project retrieval errors continue to follow the existing retry behavior.

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.

[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 `@pkg/project/apiserver/registry/project/proxy/proxy_test.go`:
- Around line 248-268: Update the test case’s expected error assertion to verify
the not-found classification with kerrors.IsNotFound(err), rather than requiring
an unwrapped StatusError or matching only its message. Preserve the existing
no-retry behavior and expectedValidationCalls assertion.

🪄 Autofix

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: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ffdc39f4-496d-45c3-82e4-2943dae90ac6

📥 Commits

Reviewing files that changed from the base of the PR and between 58298ec and 3d9fafc.

📒 Files selected for processing (2)

• pkg/project/apiserver/registry/project/proxy/proxy.go
• pkg/project/apiserver/registry/project/proxy/proxy_test.go

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[openshift-ci]

@tchap: all tests passed!

Full PR test history. Your PR dashboard.

Details

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

[tchap]

Verified on a ClusterBot cluster that oc delete project --ignore-not-found unknown works now.