refactor: replace go:linkname with oidctrusttest helper package

[pabloszel11]

Motivation

Several test packages used //go:linkname to access the private newOIDCTrustModuleWithRepo constructor in modules/oidctrust. This required importing unsafe in both the exporting and consuming files, which triggers false positives in security scanners (e.g. Checkmarx) and is generally fragile: go:linkname is a linker-level hack with no type-safety guarantees.

Approach

oidctrust.NewModule is promoted to a proper exported constructor (the types it works with, TrustRepository and TrustModule, were already public, so this doesn't widen the surface meaningfully). Test packages now import oidctrust and call oidctrust.NewModule directly.

The export.go file and its //nolint:unused suppression are removed entirely.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 4c82ca51-e1d5-4699-9e0b-19c15d5d3ea5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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

[alienvspredator]

It just doubles oidctrust.NewModule function. Do we need this package and why can't we use the oidctrust.NewModule directly?

[pabloszel11]

It was added to follow httptest convention and signal test-only usage. Obviously, since TrustRepository and TrustModule are public types, promoting NewModule to the public API of oidctrust is all right in my eyes. I proposed the approach with oidctrusttest package to propose a compromise between unsafe approach and simply exporting, but if we find it more confusing than helpful, then I'd simply export it. Please let me know how we prefer it, curious to hear @cb80 take on this as well

[alienvspredator]

httptest does a a bit more than just calls functions from http package as they are. It wraps and modifies the input parameters, defines an implementation of some interfaces such as http.ResponseWriter, and defines a type httptest.Server which listens on in-memory network, unlike http.Server which listens on a loopback network. It makes sense in that context.

In the current implementation having oidctrusttest package does not help to incapsulate oidctrust.NewModule function. It is already publicly exported, and used as is. In my opinion, having an extra noop package as it's currently present makes the readability of the API more difficult. I think this approach would be worth it if oidctrusttest package helped to hide oidctrust.NewModule function from public API, and therefore protect it from wrong usage as the intended usage of a module is through the application lifecycle. The public oidctrust.NewModule function is a compromise and may be accepted though.

[pabloszel11]

Fair enough. Made the changes