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

OSAC-1550: rename ssh_key to ssh_public_key in BareMetalInstance API - #738

Merged
openshift-merge-bot[bot] merged 1 commit into
osac-project:mainfrom
mennyaboush:feat/OSAC-1550
Jun 23, 2026
Merged

openshift-merge-bot[bot] merged 1 commit into
osac-project:mainfrom
mennyaboush:feat/OSAC-1550

Conversation

@mennyaboush

@mennyaboush mennyaboush commented Jun 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Rename spec.ssh_key to spec.ssh_public_key in BareMetalInstance proto definitions (public and private) to align with naming convention used by Clusters and ComputeInstance
  • Update server validation, immutability checks, error messages, controller, CLI, and all related tests
  • Proto field number (= 2) is unchanged — wire-compatible for gRPC clients

Reviewer feedback: https://github.com/osac-project/enhancement-proposals/pull/57/changes#r3418648740

Changes

File Change
proto/public/.../baremetal_instance_type.proto ssh_key → ssh_public_key
proto/private/.../baremetal_instance_type.proto ssh_key → ssh_public_key
internal/api/... Regenerated with buf generate
internal/servers/private_baremetal_instances_server.go Field paths, error messages, validation
internal/servers/private_baremetal_instances_server_test.go Updated test assertions and struct fields
internal/controllers/baremetalinstance/baremetalinstance_reconciler_function.go HasSshKey()→HasSshPublicKey(), GetSshKey()→GetSshPublicKey()
internal/controllers/baremetalinstance/baremetalinstance_reconciler_function_test.go Updated test struct fields
internal/cmd/cli/create/baremetalinstance/create_bare_metal_instance_cmd.go spec.SshKey → spec.SshPublicKey

Notes

  • The AAP template parameter name (params["sshKey"]) is intentionally kept unchanged — it is an internal parameter passed to Ansible playbooks, not a proto field name
  • ComputeInstance's ssh_key field is NOT renamed in this PR — it is a separate concern

Test plan

  • buf lint passes
  • All 1036 server tests pass (including BareMetalInstance create, validate SSH key, immutability check)
  • No remaining ssh_key references in changed BareMetalInstance files

Assisted-by: Claude Code noreply@anthropic.com

Summary by CodeRabbit

  • Refactor
    • Renamed the Bare Metal Instance SSH field from ssh_key to ssh_public_key across the API, including CLI input mapping, server validation, and immutability enforcement after creation.
    • The CLI --ssh-key value is now stored in ssh_public_key, and update requests will reject changes to ssh_public_key.
  • Tests
    • Updated server and spec tests to use ssh_public_key, including validation and PATCH immutability expectations.

@openshift-ci-robot

openshift-ci-robot commented Jun 22, 2026 •

Copy link
Copy Markdown

@mennyaboush: This pull request references OSAC-1550 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

  • Rename spec.ssh_key to spec.ssh_public_key in BareMetalInstance proto definitions (public and private) to align with naming convention used by Clusters and ComputeInstance
  • Update server validation, immutability checks, error messages, controller, CLI, and all related tests
  • Proto field number (= 2) is unchanged — wire-compatible for gRPC clients

Reviewer feedback: https://github.com/osac-project/enhancement-proposals/pull/57/changes#r3418648740

Changes

File Change
proto/public/.../baremetal_instance_type.proto ssh_key → ssh_public_key
proto/private/.../baremetal_instance_type.proto ssh_key → ssh_public_key
internal/api/... Regenerated with buf generate
internal/servers/private_baremetal_instances_server.go Field paths, error messages, validation
internal/servers/private_baremetal_instances_server_test.go Updated test assertions and struct fields
internal/controllers/baremetalinstance/baremetalinstance_reconciler_function.go HasSshKey()→HasSshPublicKey(), GetSshKey()→GetSshPublicKey()
internal/controllers/baremetalinstance/baremetalinstance_reconciler_function_test.go Updated test struct fields
internal/cmd/cli/create/baremetalinstance/create_bare_metal_instance_cmd.go spec.SshKey → spec.SshPublicKey

Notes

  • The AAP template parameter name (params["sshKey"]) is intentionally kept unchanged — it is an internal parameter passed to Ansible playbooks, not a proto field name
  • ComputeInstance's ssh_key field is NOT renamed in this PR — it is a separate concern

Test plan

  • buf lint passes
  • All 1036 server tests pass (including BareMetalInstance create, validate SSH key, immutability check)
  • No remaining ssh_key references in changed BareMetalInstance files

Assisted-by: Claude Code noreply@anthropic.com

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 requested review from trewest and tzumainn June 22, 2026 10:06
@coderabbitai

coderabbitai Bot commented Jun 22, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@mennyaboush, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 24 minutes and 42 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 459c563a-b7f3-477b-88cb-63a2b0708ffc

📥 Commits

Reviewing files that changed from the base of the PR and between 93144f3 and ce846c9.

⛔ Files ignored due to path filters (4)
  • internal/api/osac/private/v1/baremetal_instance_type.pb.go is excluded by !**/*.pb.go
  • internal/api/osac/private/v1/baremetal_instance_type_protoopaque.pb.go is excluded by !**/*.pb.go
  • internal/api/osac/public/v1/baremetal_instance_type.pb.go is excluded by !**/*.pb.go
  • internal/api/osac/public/v1/baremetal_instance_type_protoopaque.pb.go is excluded by !**/*.pb.go
📒 Files selected for processing (7)
  • internal/cmd/cli/create/baremetalinstance/create_bare_metal_instance_cmd.go
  • internal/controllers/baremetalinstance/baremetalinstance_reconciler_function.go
  • internal/controllers/baremetalinstance/baremetalinstance_reconciler_function_test.go
  • internal/servers/private_baremetal_instances_server.go
  • internal/servers/private_baremetal_instances_server_test.go
  • proto/private/osac/private/v1/baremetal_instance_type.proto
  • proto/public/osac/public/v1/baremetal_instance_type.proto

Walkthrough

Renames the ssh_key field to ssh_public_key in BareMetalInstanceSpec across both public and private proto definitions (field number 2 preserved), then updates all downstream consumers: server-side validation and immutability enforcement, reconciler template parameter building, CLI flag wiring, and all associated tests.

Changes

ssh_key → ssh_public_key rename across BareMetalInstanceSpec

Layer / File(s) Summary
Proto field rename
proto/public/osac/public/v1/baremetal_instance_type.proto, proto/private/osac/private/v1/baremetal_instance_type.proto
Renames optional string ssh_key = 2 to optional string ssh_public_key = 2 in BareMetalInstanceSpec in both public and private proto files; field number and IMMUTABLE annotation unchanged.
Server validation and immutability enforcement
internal/servers/private_baremetal_instances_server.go, internal/servers/private_baremetal_instances_server_test.go
validateSpec switches to HasSshPublicKey/spec.ssh_public_key for create-time key validation; validateImmutability tracks spec.ssh_public_key in update-mask logic and compares GetSshPublicKey values with updated error messages. Tests updated to use SshPublicKey and assert against ssh_public_key in error strings.
Reconciler buildSpec and tests
internal/controllers/baremetalinstance/baremetalinstance_reconciler_function.go, internal/controllers/baremetalinstance/baremetalinstance_reconciler_function_test.go
buildSpec reads HasSshPublicKey/GetSshPublicKey instead of HasSshKey/GetSshKey when populating the sshKey template parameter. Test cases updated to supply SshPublicKey on the spec builder and rename the no-key test case.
CLI flag wiring
internal/cmd/cli/create/baremetalinstance/create_bare_metal_instance_cmd.go
Sets spec.SshPublicKey instead of spec.SshKey when the --ssh-key flag is provided.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • osac-project/fulfillment-service#683: Introduced private bare-metal server validation and spec.ssh_key immutability enforcement — the exact logic this PR renames to spec.ssh_public_key.
  • osac-project/fulfillment-service#703: Introduced the buildSpec reconciler function that sources SSH template parameters from spec.ssh_key, which this PR updates to spec.ssh_public_key.

Suggested labels

approved, lgtm

Suggested reviewers

  • tzumainn
  • carbonin
  • trewest
  • adriengentil

Poem

A field once named ssh_key took flight,
reborn as ssh_public_key overnight. 🔑
Proto numbers stayed, behaviors too,
just a cleaner name shining through.
Rename complete — tests green, all right! ✅

🚥 Pre-merge checks | ✅ 11
✅ Passed checks (11 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: renaming the ssh_key field to ssh_public_key across the BareMetalInstance API.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
No-Hardcoded-Secrets ✅ Passed No hardcoded secrets detected. PR only contains SSH public key test data (safe) and legitimate Kubernetes Secret object references for external data storage.
No-Weak-Crypto ✅ Passed PR renames ssh_key to ssh_public_key with no weak crypto (MD5, SHA1, DES, RC4, etc.), custom crypto implementations, or non-constant-time secret comparisons introduced. SSH validation uses golang.o...
No-Injection-Vectors ✅ Passed PR introduces no injection vectors: field rename only, SSH key validated via golang.org/x/crypto/ssh.ParseAuthorizedKey, JSON-safe param passing, no SQL/shell/eval operations.
Container-Privileges ✅ Passed PR contains no K8s manifests or container configurations; changes are limited to Go code (.go) and proto definitions (.proto). Container-privileges check is not applicable.
No-Sensitive-Data-In-Logs ✅ Passed PR does not expose SSH keys in logs. Error messages reference field names without actual values (e.g., "ssh_public_key is immutable"), validation errors only show library text, and all logging uses...
Ai-Attribution ✅ Passed PR properly attributes AI tool assistance with "Assisted-by: Claude Code" trailer, following Red Hat standards; no improper Co-Authored-By used for AI.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@mennyaboush
mennyaboush requested a review from adriengentil June 22, 2026 10:07
Rename the spec.ssh_key field to spec.ssh_public_key in the
BareMetalInstance proto definitions (public and private), and update
all server validation, immutability checks, controller references,
CLI, and tests to match. This aligns BareMetalInstance with the
naming convention used by Clusters and ComputeInstance.

The proto field number (= 2) is unchanged, so this is wire-compatible
for gRPC clients. The JSON field name changes from sshKey to
sshPublicKey.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: MENNY ABOUSH <maboush@maboush-thinkpadt14gen5.raanaii.csb>

@adriengentil adriengentil left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@openshift-ci

openshift-ci Bot commented Jun 23, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adriengentil, mennyaboush

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-merge-bot
openshift-merge-bot Bot merged commit 1b30479 into osac-project:main Jun 23, 2026
21 of 22 checks passed
mennyaboush pushed a commit to mennyaboush/fulfillment-service that referenced this pull request Jun 24, 2026
Align the AAP template parameter key with the proto field rename
from ssh_key to ssh_public_key completed in PR osac-project#738.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: MENNY ABOUSH <maboush@maboush-thinkpadt14gen5.raanaii.csb>
mennyaboush pushed a commit to mennyaboush/fulfillment-service that referenced this pull request Jun 24, 2026
Align the AAP template parameter key with the proto field rename
from ssh_key to ssh_public_key completed in PR osac-project#738.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: MENNY ABOUSH <maboush@maboush-thinkpadt14gen5.raanaii.csb>
akshaynadkarni pushed a commit to rgolangh/fulfillment-service that referenced this pull request Jul 13, 2026
Align the AAP template parameter key with the proto field rename
from ssh_key to ssh_public_key completed in PR osac-project#738.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: MENNY ABOUSH <maboush@maboush-thinkpadt14gen5.raanaii.csb>
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.

3 participants