Skip to content
This repository was archived by the owner on Sep 9, 2026. It is now read-only.

NO-ISSUE: Move hub kubeconfig and namespace fields into nested spec - #516

Closed
jhernand wants to merge 1 commit into
osac-project:mainfrom
jhernand:move_hub_fields_to_spec
Closed

jhernand wants to merge 1 commit into
osac-project:mainfrom
jhernand:move_hub_fields_to_spec

Conversation

@jhernand

@jhernand jhernand commented May 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Moves the kubeconfig and namespace fields of the Hub protobuf type into a nested
    HubSpec message, and adds an empty HubStatus message for future use.
  • Adds database migration 37 that restructures the stored JSON data accordingly, including
    an empty status object.
  • Introduces a migration testing mechanism (DescribeMigration helper and test suite) so
    that non-trivial data migrations can be verified with unit tests.
  • Fixes a bug in the Migrate method where the version parameter was shadowed by a local
    variable, causing it to always run all migrations regardless of the target version.

Test plan

  • All existing unit tests pass (ginkgo run -r internal)
  • Migration tests verify the JSON restructuring for all cases (no fields, kubeconfig
    only, namespace only, both fields)
  • Migration coverage test ensures new migrations have corresponding test files

Summary by CodeRabbit

Release Notes

  • New Features

    • Restructured hub data model to separate specification (kubeconfig, namespace) and status fields, enabling better organization and future extensibility.
  • Tests

    • Enhanced database migration testing infrastructure with comprehensive coverage validation and individual migration test suites.

Review Change Stack

@openshift-ci-robot

Copy link
Copy Markdown

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

Details

In response to this:

Summary

  • Moves the kubeconfig and namespace fields of the Hub protobuf type into a nested
    HubSpec message, and adds an empty HubStatus message for future use.
  • Adds database migration 37 that restructures the stored JSON data accordingly, including
    an empty status object.
  • Introduces a migration testing mechanism (DescribeMigration helper and test suite) so
    that non-trivial data migrations can be verified with unit tests.
  • Fixes a bug in the Migrate method where the version parameter was shadowed by a local
    variable, causing it to always run all migrations regardless of the target version.

Test plan

  • All existing unit tests pass (ginkgo run -r internal)
  • Migration tests verify the JSON restructuring for all cases (no fields, kubeconfig
    only, namespace only, both fields)
  • Migration coverage test ensures new migrations have corresponding test files

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

openshift-ci Bot commented May 9, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jhernand

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:

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

@openshift-ci
openshift-ci Bot requested review from akshaynadkarni and larsks May 9, 2026 18:04
@openshift-ci openshift-ci Bot added the approved label May 9, 2026
@coderabbitai

coderabbitai Bot commented May 9, 2026 •

Copy link
Copy Markdown

Warning

Rate limit exceeded

@jhernand has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 50 minutes and 14 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 499d0ae3-bfd7-4a39-a57d-54ad97b4f6d8

📥 Commits

Reviewing files that changed from the base of the PR and between c6305f1 and 3e6b9c7.

⛔ Files ignored due to path filters (2)
  • internal/api/osac/private/v1/hub_type.pb.go is excluded by !**/*.pb.go
  • internal/api/osac/private/v1/hub_type_protoopaque.pb.go is excluded by !**/*.pb.go
📒 Files selected for processing (16)
  • internal/cmd/cli/create/hub/create_hub_cmd.go
  • internal/cmd/service/start/grpcserver/start_grpc_server_cmd.go
  • internal/controllers/hub_cache.go
  • internal/database/database_tool.go
  • internal/database/migrations/38_move_hub_fields_to_spec.up.sql
  • internal/database/migrations/38_move_hub_fields_to_spec_test.go
  • internal/database/migrations/migrations_coverage_test.go
  • internal/database/migrations/migrations_suite_test.go
  • internal/rendering/tables/osac.private.v1.Hub.yaml
  • internal/servers/clusters_server.go
  • internal/servers/console_server.go
  • internal/servers/console_server_test.go
  • internal/servers/generic_server.go
  • internal/servers/private_hubs_server_test.go
  • it/it_tool.go
  • proto/private/osac/private/v1/hub_type.proto

Walkthrough

This pull request implements a comprehensive data structure refactoring of the Hub message from a flat schema with top-level kubeconfig and namespace fields to a structured design with nested spec and status fields. The change includes a protobuf schema update, a database migration that transforms existing hub records, a new versioned migration tooling infrastructure, updates to all code accessing hub fields, comprehensive test updates, and rendering layer adjustments. The refactoring maintains backward compatibility through the database migration while establishing a cleaner, more extensible schema structure.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

approved, lgtm

Suggested reviewers

  • adriengentil
  • SiddarthR56
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main structural change: moving hub kubeconfig and namespace fields into a nested spec object, which aligns with the core refactoring across all modified files.
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.

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


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

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

@jhernand
jhernand force-pushed the move_hub_fields_to_spec branch from c6305f1 to bf7703f Compare May 15, 2026 11:04
…pec`

The `Hub` type now uses a nested `HubSpec` message for the `kubeconfig`
and `namespace` fields, and an empty `HubStatus` message for future use.
All code that accessed these fields directly on the `Hub` message has
been updated to go through the `spec` sub-message.

This migration isn't the typical one that adds tables or indexes: it
needed to do non-trivial changes in the stored JSON data, moving fields
into a nested object and adding a new empty object. To be confident that
this will work correctly we added a mechanism to create tests for
database migrations. The new `DescribeMigration` helper in the
`migrations` test package sets up a database at the previous schema
version, allowing tests to insert data, run the migration, and verify
the result.

The `Migrate` method of the database `Tool` now accepts a target version
parameter, which was needed by the migration tests.

Signed-off-by: Juan Hernandez <juan.hernandez@redhat.com>
@jhernand
jhernand force-pushed the move_hub_fields_to_spec branch from bf7703f to 3e6b9c7 Compare May 17, 2026 16:42
@openshift-ci

openshift-ci Bot commented May 17, 2026

Copy link
Copy Markdown

@jhernand: The following tests 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-vmaas 3e6b9c7 link true /test e2e-vmaas
ci/prow/images 3e6b9c7 link true /test images

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.

@jhernand

Copy link
Copy Markdown
Contributor Author

Included in #557.

@jhernand jhernand closed this May 19, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants