NO-ISSUE: Drop github.com/pkg/errors

[skitt]

Why

The module is deprecated and replaceable using stdlib constructs (with differences around stacktrace handling which shouldn't be significant here).

What

This was mostly done using

git grep -l errors.Wrapf | xargs sed -i -E 's/errors.Wrapf\(err, "([^"]*)"([^)]*)\)/fmt.Errorf("\1: %w"\2, err)/'
git grep -l errors.Errorf | xargs sed -i 's/errors.Errorf/fmt.Errorf/'
git grep -l errors.Wrap | xargs sed -i -E 's/errors.Wrap\(err, "([^"]*)"\)/fmt.Errorf("\1: %w", err)/'

Some fmt.Errorf calls could be replaced by errors.New but the impact is minimal (I can't find the reference right now, but the intention of the Go authors is that fmt.Errorf can be used by default even when formatting isn't needed; it means one less thing to think about when the need for formatting changes).

Verification

This is a refactor with no functional changes, so most existing tests don’t need adjusting and should be sufficient to verify correctness. There is one behaviour change, related to error-message capitalisation; the corresponding test is adjusted to suit.

[openshift-ci-robot]

@skitt: This pull request explicitly references no jira issue.

Details

In response to this:

The module is deprecated and replaceable using stdlib constructs (with differences around stacktrace handling which shouldn't be significant here).

Mostly done using

git grep -l errors.Wrapf | xargs sed -i -E 's/errors.Wrapf\(err, "([^"]*)"([^)]*)\)/fmt.Errorf("\1: %w"\2, err)/'
git grep -l errors.Errorf | xargs sed -i 's/errors.Errorf/fmt.Errorf/'
git grep -l errors.Wrap | xargs sed -i -E 's/errors.Wrap\(err, "([^"]*)"\)/fmt.Errorf("\1: %w", err)/'

Some fmt.Errorf calls could be replaced by errors.New but the impact is minimal (I can't find the reference right now, but the intention of the Go authors is that fmt.Errorf can be used by default even when formatting isn't needed; it means one less thing to think about when the need for formatting changes).

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]

Summary by CodeRabbit

• Refactor
  • Standardized error handling across the application using built-in mechanisms.
  • Preserved existing validation, control flow, successful operations, and troubleshooting details.
  • Improved propagation of underlying errors for clearer diagnostics.
  • Minor error-message capitalization updates were made without changing functionality.
  • No public interfaces or end-user workflows were changed.

Walkthrough

The pull request removes the direct github.com/pkg/errors dependency. It replaces legacy error constructors and wrappers with standard-library errors and fmt.Errorf, while preserving control flow and wrapped causes.

Changes

Standard-library error migration

Layer / File(s)	Summary
Dependency removal and core error construction go.mod, pkg/apply/..., pkg/controller/..., pkg/platform/..., pkg/render/..., pkg/util/...	The direct dependency is removed. Application, controller, platform, rendering, and utility errors now use standard-library constructors and %w wrapping.
Network validation and configuration errors pkg/network/cloud_network.go, pkg/network/cluster_config.go, pkg/network/mtu.go, pkg/network/ovn_kubernetes.go, pkg/network/additional_networks.go, pkg/network/kube_proxy.go, pkg/network/multus_admission_controller.go	Network validation and configuration errors now use fmt.Errorf. Underlying parse and generation errors remain wrapped with %w.
Network rendering and transition errors pkg/network/render.go, pkg/network/ovn_kubernetes.go, pkg/network/*	Network rendering, transition validation, manifest generation, and console-plugin errors now use standard-library formatting and wrapping.
Validation test expectation pkg/network/additional_networks_test.go	The raw CNI test expects lowercase "failed to Unmarshal RawCNIConfig".

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

Suggested reviewers: mattedallo, arghosh93

---

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

• Ignore

❌ Failed checks (1 error)

Check name	Status	Explanation	Resolution
E2e Tests For Feature Changes	❌ Error	The PR changes user-visible error capitalization, but the commit has no test/e2e changes and the description has only “Verification,” without required Testing/How to verify CI, platform, and pass d...	Add or modify test/e2e coverage and add a Testing or How to verify it section with CI lanes, platforms, and results; if E2E is infeasible, document why there and follow the stated override process.

✅ Passed checks (23 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 91.49% which is sufficient. The required threshold is 80.00%.
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.
Pr Quality	✅ Passed	The description includes Why, What, and Verification. The change is a scoped non-functional refactor; its non-vendor size is 351 lines, well below 7000.
Commit Message Quality	✅ Passed	The PR has one non-merge commit. Its subject is concise and descriptive, and its body explains the deprecation rationale. The diff is one coherent dependency-removal change.
Unit Tests For Go Changes	✅ Passed	The PR modifies 20 production Go files under pkg/ and also modifies pkg/network/additional_networks_test.go; no bindata YAML files changed.
Rbac Least Privilege	✅ Passed	The commit changes no YAML files under bindata/ or manifests/, so no RBAC rules were added or modified.
Docs For Feature And Behavior Changes	✅ Passed	The commit is a dependency-removal refactor: it replaces error construction, updates one test, and changes no CNO feature, configuration, architecture, or control flow; no docs update is required.
Stale Project Docs And Config	✅ Passed	The PR changes dependency/error handling and vendored files only; no docs or .coderabbit.yaml paths changed, and searches found no stale references to pkg/errors or renamed project entities.
Go And Test Code Quality	✅ Passed	The diff adds no prohibited logging, bare returns, duration literals, shadowing, IPv4-only logic, unsafe new shared access, or test misuse.
Ai-Generated Code Smell	✅ Passed	The diff contains mechanical error-constructor replacements, one proportional test assertion update, no AI references, no redundant declarations, and only a lint-rationale comment.
Stable And Deterministic Test Names	✅ Passed	The PR changes only an assertion string in a standard Go test; it adds no Ginkgo test titles or dynamic test-name expressions.
Test Structure And Quality	✅ Passed	The PR changes one standard Go test assertion only; it adds no Ginkgo It blocks, cluster operations, Eventually/Consistently calls, setup, or cleanup requiring this check.
Microshift Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added. The only test change updates an assertion in an existing Go unit test, so MicroShift compatibility checks do not apply.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The commit adds no Ginkgo e2e tests; it only changes one existing unit-test assertion in pkg/network/additional_networks_test.go.
Topology-Aware Scheduling Compatibility	✅ Passed	The changed files only replace github.com/pkg/errors calls and update one error-message test; the diff adds no scheduling constraints, replica settings, or topology-targeting logic.
Ote Binary Stdout Contract	✅ Passed	PR changes only error handling and dependency files; no OTE binary or suite setup changed. Network diagnostics are not OTE, and endpoint errors use os.Stderr.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added. The only test change is a standard Go TestValidateRaw assertion that lowercases an error message and uses no IP or external connectivity.
No-Weak-Crypto	✅ Passed	The patch only replaces pkg/errors calls; no weak algorithms, custom crypto, or secret comparisons are added, and existing crypto use is SHA-256.
Container-Privileges	✅ Passed	The PR changes Go error handling, go.mod, and vendor files only; no added manifest lines set privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, root, or allowPrivilegeEscalation.
No-Sensitive-Data-In-Logs	✅ Passed	The patch adds no logging calls or sensitive-data identifiers; it only replaces error constructors, and the raw CNI text in the validation error already existed unchanged apart from capitalization.
Title check	✅ Passed	The title uses the imperative verb "Drop", stays under 72 characters, and accurately describes the dependency removal.
Description check	✅ Passed	The description clearly explains the dependency removal, replacement approach, expected behavior, and verification performed.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[skitt]

@coderabbitai resume

[coderabbitai]

✅ Action performed

Reviews resumed.

[danwinship]

might as well replace the one errors.New() call with fmt.Errorf()

[danwinship]

these are still out-of-place. (there are other CNO imports below)

[skitt]

I’ve grouped them for now — I wasn’t able to determine from the rest of the project whether CNO imports are supposed to be in their own section; is that the case?

[danwinship]

/lgtm

[openshift-merge-bot]

Scheduling required tests:
/test e2e-aws-ovn-upgrade-ipsec

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-ovn-fdp-qe
/test e2e-aws-ovn-hypershift-conformance
/test e2e-aws-ovn-serial-1of2
/test e2e-aws-ovn-serial-2of2
/test e2e-aws-ovn-upgrade
/test e2e-aws-ovn-windows
/test e2e-azure-ovn-upgrade
/test e2e-gcp-ovn
/test e2e-gcp-ovn-upgrade
/test e2e-metal-ipi-ovn-dualstack-bgp
/test e2e-metal-ipi-ovn-dualstack-bgp-local-gw
/test e2e-metal-ipi-ovn-ipv6
/test e2e-metal-ipi-ovn-ipv6-ipsec
/test e2e-ovn-ipsec-step-registry
/test hypershift-e2e-aks

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danwinship, skitt

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 [danwinship]

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

[danwinship]

/retest

[skitt]

/verified by CI

[openshift-ci-robot]

@skitt: This PR has been marked as verified by CI.

Details

In response to this:

/verified by CI

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]

@skitt: 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.