Skip to content

Conversation

@carbonin
Copy link
Collaborator

@carbonin carbonin commented Jul 30, 2025

This PR adds an optional parameter to the cluster create tool for the ssh public key which adds it to both the created cluster and infraenv. Additionally a new tool is added to update the ssh public key for existing clusters. The update tool also updates all associated infraenvs.

Resolves https://issues.redhat.com/browse/MGMT-21215

Summary by CodeRabbit

  • New Features

    • Added the ability to specify an SSH public key when creating a cluster, enabling SSH access to cluster nodes.
    • Introduced a new tool to set or update the SSH public key for an existing cluster, allowing SSH access during and after installation.
  • Documentation

    • Updated user guides and examples to include SSH key management for clusters and usage instructions for the new tool.
  • Tests

    • Added tests to verify SSH key handling during cluster creation and updates, including error scenarios.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 30, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jul 30, 2025

@carbonin: This pull request references MGMT-21215 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 bug to target the "4.20.0" version, but no target version was set.

Details

In response to this:

This PR adds an optional parameter to the cluster create tool for the ssh public key which adds it to both the created cluster and infraenv. Additionally a new tool is added to update the ssh public key for existing clusters. The update tool also updates all associated infraenvs.

Resolves https://issues.redhat.com/browse/MGMT-21215

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.

@coderabbitai
Copy link

coderabbitai bot commented Jul 30, 2025

Walkthrough

This change introduces SSH key management capabilities for clusters. The create_cluster function now optionally accepts an SSH public key, and a new set_cluster_ssh_key tool enables updating the SSH key for existing clusters. Documentation and example prompts are updated, and new tests cover the added SSH key functionality.

Changes

Cohort / File(s) Change Summary
Cluster SSH Key Management Documentation
README.md, doc/example_prompts.md
Updated documentation to describe new and updated tools for SSH key management, including usage examples and prompt documentation for set_cluster_ssh_key.
Cluster SSH Key Management Implementation
server.py, service_client/assisted_service_api.py
Modified create_cluster to accept an optional SSH public key. Added set_cluster_ssh_key tool to update SSH keys for clusters and associated infra environments. Added method to update infra environments with new SSH keys.
Cluster SSH Key Management Testing
tests/test_server.py, tests/test_assisted_service_api.py
Added and extended tests to verify SSH key handling in cluster creation, SSH key updates, and error handling for infra environment updates.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Server
    participant InventoryClient
    participant AssistedServiceAPI

    User->>Server: create_cluster(name, ..., ssh_public_key)
    Server->>InventoryClient: create_cluster(..., ssh_public_key)
    InventoryClient->>AssistedServiceAPI: create_cluster API call
    Server->>InventoryClient: create_infra_env(..., ssh_authorized_key=ssh_public_key)
    InventoryClient->>AssistedServiceAPI: create_infra_env API call
    Server-->>User: Cluster created

    User->>Server: set_cluster_ssh_key(cluster_id, ssh_public_key)
    Server->>InventoryClient: update_cluster(cluster_id, ssh_public_key)
    InventoryClient->>AssistedServiceAPI: update_cluster API call
    Server->>InventoryClient: list_infra_envs(cluster_id)
    InventoryClient->>AssistedServiceAPI: list_infra_envs API call
    loop For each InfraEnv
        Server->>InventoryClient: update_infra_env(infra_env_id, ssh_authorized_key)
        InventoryClient->>AssistedServiceAPI: update_infra_env API call
    end
    Server-->>User: SSH key updated
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

size/L, lgtm

Suggested reviewers

  • eranco74

Poem

A rabbit with keys, both shiny and new,
Hopped through the clusters, updating a few.
With SSH magic, the nodes now unlock,
Secure little burrows, protected from shock.
Documentation in paw, and tests all in place—
This bunny ensures clusters are safe in their space! 🐇🔑

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f02ec69 and 438d7b3.

📒 Files selected for processing (6)
  • README.md (3 hunks)
  • doc/example_prompts.md (1 hunks)
  • server.py (4 hunks)
  • service_client/assisted_service_api.py (1 hunks)
  • tests/test_assisted_service_api.py (1 hunks)
  • tests/test_server.py (3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
service_client/assisted_service_api.py (1)
service_client/exceptions.py (1)
  • sanitize_exceptions (24-52)
server.py (3)
tests/test_assisted_service_api.py (1)
  • client (33-38)
service_client/assisted_service_api.py (6)
  • InventoryClient (22-518)
  • create_cluster (222-258)
  • create_infra_env (261-283)
  • update_cluster (310-344)
  • list_infra_envs (201-219)
  • update_infra_env (286-307)
service_client/metrics.py (1)
  • track_tool_usage (43-57)
tests/test_assisted_service_api.py (3)
service_client/assisted_service_api.py (1)
  • update_infra_env (286-307)
tests/test_utils.py (1)
  • create_test_infra_env (61-79)
service_client/exceptions.py (1)
  • AssistedServiceAPIError (15-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Red Hat Konflux / assisted-service-mcp-saas-main-on-pull-request
🔇 Additional comments (14)
service_client/assisted_service_api.py (1)

285-307: LGTM! Well-implemented method following established patterns.

The update_infra_env method implementation is excellent:

  • Consistent with other async methods in the class
  • Proper error handling via @sanitize_exceptions decorator
  • Flexible parameter handling with **update_params
  • Appropriate logging and return type casting
  • Comprehensive docstring following class conventions
doc/example_prompts.md (1)

64-69: LGTM! Documentation follows established format.

The new set_cluster_ssh_key section is well-documented:

  • Clear description of SSH key functionality
  • Follows the same format as other tool documentation
  • Realistic example prompt that demonstrates usage
  • Appropriately placed under "Cluster Creation and Configuration"
README.md (3)

91-91: LGTM! Clear parameter documentation.

The ssh_public_key parameter documentation is concise and informative, clearly explaining its optional nature and purpose.


125-129: LGTM! Comprehensive tool documentation.

The set_cluster_ssh_key tool documentation is excellent:

  • Clear description of functionality and timing considerations
  • Proper parameter documentation with type and requirement info
  • Follows the established documentation format

148-149: LGTM! Updated usage examples reflect new capabilities.

The usage examples are updated appropriately to demonstrate the new SSH key management functionality.

tests/test_assisted_service_api.py (2)

447-472: LGTM! Comprehensive success case test.

The test_update_infra_env_success test is well-implemented:

  • Uses appropriate test utility functions
  • Proper mocking of API calls
  • Verifies correct parameter passing to the underlying API
  • Asserts expected return value
  • Follows established testing patterns in the file

474-492: LGTM! Proper exception handling test.

The test_update_infra_env_api_exception test properly validates error handling:

  • Tests API exception scenario with appropriate status code
  • Uses proper exception matching with pytest.raises
  • Consistent with other exception handling tests in the file
server.py (3)

11-11: LGTM! Appropriate import addition.

The Optional import is correctly added to support the new optional SSH key parameter.


292-357: LGTM! Well-implemented SSH key support in cluster creation.

The create_cluster function enhancements are excellent:

  • Optional ssh_public_key parameter with clear documentation
  • Conditional parameter inclusion using dictionaries
  • SSH key propagated to both cluster and InfraEnv creation
  • Updated logging includes SSH key status
  • Maintains backward compatibility with existing functionality

567-620: LGTM! Comprehensive SSH key management tool.

The set_cluster_ssh_key function is well-implemented:

  • Proper MCP tool decoration and usage tracking
  • Comprehensive docstring explaining functionality and limitations
  • Updates both cluster and associated InfraEnvs
  • Robust error handling with partial failure reporting
  • Appropriate logging throughout the process
  • Clear return messages indicating success or partial failure
tests/test_server.py (4)

5-5: LGTM: Appropriate pylint disable for growing test file.

The too-many-lines disable is justified given the comprehensive test coverage being added.


566-614: Well-structured test for SSH key parameter in cluster creation.

The test properly verifies that the SSH public key is passed to both create_cluster and create_infra_env methods with the correct parameter names (ssh_public_key and ssh_authorized_key respectively).


835-872: Comprehensive test coverage for successful SSH key updates.

The test correctly verifies:

  • Cluster SSH key update
  • InfraEnv listing and updates
  • Proper return value handling
  • All expected method calls with correct parameters

The test structure follows the established patterns in the file.


873-911: Good error handling test for partial failures.

The test properly simulates InfraEnv update failures and verifies that:

  • The cluster SSH key is still updated successfully
  • Partial failure messages are returned appropriately
  • All expected API calls are made despite the failure

This ensures robust error handling in the SSH key update functionality.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@openshift-ci openshift-ci bot requested review from jhernand and omertuc July 30, 2025 16:03
@openshift-ci
Copy link

openshift-ci bot commented Jul 30, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: carbonin

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 added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 30, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jul 30, 2025

@carbonin: This pull request references MGMT-21215 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 bug to target the "4.20.0" version, but no target version was set.

Details

In response to this:

This PR adds an optional parameter to the cluster create tool for the ssh public key which adds it to both the created cluster and infraenv. Additionally a new tool is added to update the ssh public key for existing clusters. The update tool also updates all associated infraenvs.

Resolves https://issues.redhat.com/browse/MGMT-21215

Summary by CodeRabbit

  • New Features

  • Added the ability to specify an SSH public key when creating a cluster, enabling SSH access to cluster nodes.

  • Introduced a new tool to set or update the SSH public key for an existing cluster, allowing SSH access during and after installation.

  • Documentation

  • Updated user guides and examples to include SSH key management for clusters and usage instructions for the new tool.

  • Tests

  • Added tests to verify SSH key handling during cluster creation and updates, including error scenarios.

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.

@jhernand
Copy link
Collaborator

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 31, 2025
@openshift-merge-bot openshift-merge-bot bot merged commit 6d3c807 into openshift-assisted:master Jul 31, 2025
14 of 15 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants