Skip to content

HYPERFLEET-732 - fix: remove ksuid and related unused functions to avoid confusion with KSUID usage#88

Open
86254860 wants to merge 1 commit intoopenshift-hyperfleet:mainfrom
86254860:HYPERFLEET-732-replace-id-with-uuid
Open

HYPERFLEET-732 - fix: remove ksuid and related unused functions to avoid confusion with KSUID usage#88
86254860 wants to merge 1 commit intoopenshift-hyperfleet:mainfrom
86254860:HYPERFLEET-732-replace-id-with-uuid

Conversation

@86254860
Copy link
Copy Markdown
Contributor

@86254860 86254860 commented Mar 31, 2026

Summary

Remove ksuid and related unused functions to avoid confusion with KSUID usage

  • HYPERFLEET-732

Test Plan

  • Unit tests added/updated
  • make test-all passes
  • make lint passes
  • Helm chart changes validated with make test-helm (if applicable)
  • Deployed to a development cluster and verified (if Helm/config changes)
  • E2E tests passed (if cross-component or major changes)

Summary by CodeRabbit

  • Chores
    • Stopped automatic generation and propagation of operation IDs; logs/traces may no longer include per-request operation identifiers.
    • Removed an unused third-party dependency from project configuration.

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Mar 31, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Mar 31, 2026

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

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

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 31, 2026

Walkthrough

The pull request removes the direct github.com/segmentio/ksuid requirement from go.mod and deletes two functions from pkg/logger/operationid_middleware.go: WithOpID and GetOperationID. The middleware logic that generated and stored an operation ID in context under OpIDKey and the API to retrieve it are no longer present. No other exported/public APIs were changed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ⚠️ Warning The PR title mentions removing ksuid and related functions, which aligns with the actual changes (removed ksuid dependency and WithOpID/GetOperationID functions), but contradicts the PR objectives which focus on replacing IDs with RFC4122 UUID v7. The title should align with the stated PR objective of replacing existing IDs with RFC4122 UUID v7, not just removing ksuid. Consider updating the title to reflect the primary objective.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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.

@86254860 86254860 marked this pull request as ready for review April 1, 2026 07:58
@openshift-ci openshift-ci bot requested review from jsell-rh and rafabene April 1, 2026 07:58
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/logger/operationid_middleware.go`:
- Line 33: Replace the panic-prone uuid.Must(uuid.NewV7()) call by calling
uuid.NewV7() and handling its error in the operation ID middleware: if NewV7
returns a uuid and nil error, use its String(); if it returns an error, do not
panic—log or record the error and fall back to a safe alternative (e.g.,
generate a uuid.NewV4() or compose a deterministic fallback using time+counter)
and assign that to opID; ensure the variable opID is still set and
returned/injected into the request context so downstream code uses a valid ID
even when NewV7 fails.
🪄 Autofix (Beta)

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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9e8aff44-d8f4-48fb-923d-0ec5931e2f69

📥 Commits

Reviewing files that changed from the base of the PR and between 16b49fb and 5bedf23.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (2)
  • go.mod
  • pkg/logger/operationid_middleware.go
💤 Files with no reviewable changes (1)
  • go.mod

@86254860 86254860 force-pushed the HYPERFLEET-732-replace-id-with-uuid branch from 5bedf23 to 60aa28b Compare April 1, 2026 12:33
@86254860 86254860 changed the title HYPERFLEET-732 - feat: replace existing IDs with RFC4122 UUID v7 for both Cluster and NodePool IDs HYPERFLEET-732 - fix: remove ksuid and related unused functions to avoid confusion with KSUID usage Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant