[sebrandon1]

Summary

• Add unit tests for pkg/controller/certs (previously zero coverage): GenerateCA, CreateSignedServingPair, VerifyCert, Active, ToPEM, PEMToCert, PEMSHA256, and the CertGeneratorFunc adapter.
• Guard CreateSignedServingPair against an empty hosts slice: CommonName: hosts[0] indexed without a length check and panicked with index out of range; it now returns an error.

Motivation

pkg/controller/certs backs OLM's serving certificate rotation and the CA bundles for its webhooks, but had no tests. Every function in the package is pure crypto, so it can be covered without cluster fixtures.

The empty-hosts guard follows the panic-to-error conversions in #3904 and #3899: the sole caller (pkg/controller/install/certresources.go:30) consumes it through the certs.CertGenerator interface and already handles errors, so a bad input now surfaces as a reconciliation error instead of a crash.

Testing

• go test ./pkg/controller/certs/... -race -count=1 — 16 tests pass
• go vet ./pkg/controller/certs/..., gofmt -l pkg/controller/certs/ — clean

No behavior change for any input that previously succeeded; the only new path is the empty-hosts error return.

Summary by CodeRabbit

• Bug Fixes

  • Certificate generation now reports an error when no hostnames are provided, preventing creation of invalid serving certificates.

• Tests

  • Expanded certificate utility coverage, including certificate generation, signing and verification, validity checks, hostname validation, PEM handling, hashing, malformed inputs, and generator output.

[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 joelanford 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:

• OWNERS

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

[openshift-ci]

Hi @sebrandon1. Thanks for your PR.

I'm waiting for a operator-framework member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Enterprise
• Run ID: cac57e7e-9892-4ac0-a14c-cdfd860ae05d

📥 Commits

Reviewing files that changed from the base of the PR and between 99851e9 and a224987.

📒 Files selected for processing: 1

• pkg/controller/certs/certs_test.go

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.

📝 Walkthrough

CreateSignedServingPair now returns an error when the host list is empty. New tests cover certificate generation, verification, validity, PEM handling, hashing, and CertGenerator delegation.

Changes: Certificate utilities

Layer / File(s)	Summary
Certificate generation and validation pkg/controller/certs/certs.go, pkg/controller/certs/certs_test.go	CreateSignedServingPair rejects an empty host list. Tests cover CA generation, signed serving certificates, issuer and hostname verification, and validity windows.
Certificate encoding and adapter tests pkg/controller/certs/certs_test.go	Tests cover PEM serialization, PEM parsing, SHA-256 hashing, and CertGenerator delegation and error propagation.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to a2249

No actionable issue is established for the certificate tests or empty-host handling. The PR is mergeable after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes both main changes: adding unit tests and rejecting empty hosts in CreateSignedServingPair.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 golangci-lint (2.14.0)

golangci-lint (2.14.0) skipped for .golangci.yaml: configuration requires version: "2". Migrate older configurations using golangci-lint migrate and review the result; see https://golangci-lint.run/docs/product/migration-guide/.

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