refactor: avoid panic if commit len < 7

[xcoulon]

ensure that the commit length is greater or equal to 7 characters before
initializing the short commit variable

also, fix a few lint issues

Signed-off-by: Xavier Coulon xcoulon@redhat.com

Summary by CodeRabbit

Release Notes

• Refactor

  • Improved context handling in authentication operations for better reliability
  • Enhanced metrics initialization for version tracking

• Tests

  • Updated and expanded test coverage for metrics validation

[coderabbitai]

Walkthrough

Updates to context handling in a goroutine, linter suppression comments in tests, and refactoring of metrics initialization logic to derive and set version-related gauge metrics based on commit length. Test updates added to validate the new metrics derivation behavior with varying commit string lengths.

Changes

Cohort / File(s)	Summary
Test Linter Annotations controllers/spacerequest/spacerequest_controller_test.go	Added inline // nolint:gosec comments to three test assertions verifying HasNamespaceAccess(...) expectations without altering the underlying test data or assertions.
Context Propagation controllers/usersignup/usersignup_controller.go	Modified annotateCaptchaAssessment goroutine to accept and use the reconciliation context instead of context.Background() for reCAPTCHA client initialization and assessment annotation calls.
Metrics Initialization & Logic pkg/metrics/metrics.go, pkg/metrics/metrics_test.go	Moved host-operator version gauge initialization from RegisterCustomMetrics() to initMetrics() and added conditional logic to derive shortCommit by truncating version.Commit to 7 characters when length permits. Updated tests to use version.Commit values and added subtests for commits of varying lengths (long, short, empty).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately reflects the main objective: adding a length check to prevent panics when commit is less than 7 characters, which is the core change across the metrics-related files.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

controllers/usersignup/usersignup_controller.go (1)

833-852: ⚠️ Potential issue | 🟠 Major

Context cancellation risk in goroutine.

The reconciliation context ctx passed to the goroutine will likely be cancelled when the Reconcile function returns, which happens before this async goroutine completes. This could cause recaptcha.NewClient(ctx) or rclient.AnnotateAssessment(ctx, ...) to fail immediately with a context cancellation error.

The previous approach using context.Background() inside the goroutine was actually correct for this "fire-and-forget" pattern. Consider reverting to context.Background() or using a detached context with a timeout:

Suggested fix

-	go func(ctx context.Context) {
-		rclient, err := recaptcha.NewClient(ctx)
+	go func() {
+		// Use a detached context with timeout since the reconcile context 
+		// will be cancelled when Reconcile returns
+		ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+		defer cancel()
+		rclient, err := recaptcha.NewClient(ctx)

-	}(ctx)
+	}()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@controllers/usersignup/usersignup_controller.go` around lines 833 - 852, The
goroutine currently uses the Reconcile `ctx` which may be cancelled before the
async work completes; change the goroutine to create a detached context (e.g.,
use context.Background() or a context.WithTimeout/WithCancel derived from
context.Background()) before calling recaptcha.NewClient and
rclient.AnnotateAssessment so the fire-and-forget annotate logic in the
anonymous goroutine does not immediately fail due to Reconcile cancellation;
update the goroutine closure to use that new context when calling
recaptcha.NewClient(ctx) and rclient.AnnotateAssessment(ctx, ...), and ensure
you still defer rclient.Close() and log errors/results as before (refer to the
anonymous goroutine surrounding recaptcha.NewClient, AnnotateAssessment, and the
logger calls).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@controllers/usersignup/usersignup_controller.go`:
- Around line 833-852: The goroutine currently uses the Reconcile `ctx` which
may be cancelled before the async work completes; change the goroutine to create
a detached context (e.g., use context.Background() or a
context.WithTimeout/WithCancel derived from context.Background()) before calling
recaptcha.NewClient and rclient.AnnotateAssessment so the fire-and-forget
annotate logic in the anonymous goroutine does not immediately fail due to
Reconcile cancellation; update the goroutine closure to use that new context
when calling recaptcha.NewClient(ctx) and rclient.AnnotateAssessment(ctx, ...),
and ensure you still defer rclient.Close() and log errors/results as before
(refer to the anonymous goroutine surrounding recaptcha.NewClient,
AnnotateAssessment, and the logger calls).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 848af4d4-ad33-448c-9ab7-015476196073

📥 Commits

Reviewing files that changed from the base of the PR and between 7377936 and e6a12a4.

📒 Files selected for processing (4)

• controllers/spacerequest/spacerequest_controller_test.go
• controllers/usersignup/usersignup_controller.go
• pkg/metrics/metrics.go
• pkg/metrics/metrics_test.go

[sonarqubecloud]

Quality Gate failed

Failed conditions
62.8% Duplication on New Code (required ≤ 30%)

See analysis details on SonarQube Cloud

[rsoaresd]

🚀

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexeykazakov, MatousJobanek, rsoaresd, xcoulon

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 [MatousJobanek,alexeykazakov,rsoaresd,xcoulon]

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

[rsoaresd]

/retest

flaky test

[rsoaresd]

Going to dig

[openshift-ci]

@xcoulon: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e	6fb5013	link	true	/test e2e

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.

[xcoulon]

Going to dig

thanks @rsoaresd :)