Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Development: Add support for ephemeral SSH keys for the authentication of build agents #8951

Merged
merged 28 commits into from
Aug 4, 2024

Conversation

bensofficial
Copy link
Member

@bensofficial bensofficial commented Jul 2, 2024

Checklist

General

Server

  • Important: I implemented the changes with a very good performance and prevented too many (unnecessary) database calls.
  • I strictly followed the server coding and design guidelines.
  • I added multiple integration tests (Spring) related to the features (with a high test coverage).

Changes affecting Programming Exercises

  • High priority: I tested the basic programming exercise setup on a test server configured with the integrated lifecycle setup (LocalVC and LocalCI).

Motivation and Context

Artemis already supports SSH for cloning repos as a user.
In this pull request, we implement that build agents can also use SSH for cloning the repositories.

Description

For build agents, there is the new BuildAgentSSHKeyService. This service generates the SSH key pair on application startup and writes the SSH private key to a file.

This file is used by the GitService later. This functionality already exists for other version control systems.

Additionally, the public key of the build agent is put into the build agent information map provided by Hazelcast.

A core node checks an incoming request by comparing the public key to the public keys of all build agents and may eventually authenticate the build agent. In this case, the build agent is allowed to perform clone operations.

Steps for Testing

Note

Please test the functionality in this PR only on TS1. This PR needs special SSH server configuration.

Prerequisites:

  • 1 Student
  1. Log in to Artemis
  2. Navigate to a programming exercise
  3. Submit any change
  4. Check that you receive the feedback and results from the test execution

Review Progress

Performance Review

  • I (as a reviewer) confirm that the server changes (in particular related to database calls) are implemented with a very good performance

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Summary by CodeRabbit

  • New Features

    • Implemented SSH key handling for build agent authentication, including key generation and storage.
    • Added public SSH key retrieval functionality in the build agent setup.
  • Enhancements

    • Updated authentication logic to support build agent public key validation.
    • Enhanced the BuildAgentInformation structure for future extensibility.
  • Tests

    • Introduced integration tests for SSH key writing and verification in Hazelcast.
  • Bug Fixes

    • Fixed role-based access control for build agents based on session attributes.

@github-actions github-actions bot added tests server Pull requests that update Java code. (Added Automatically!) labels Jul 2, 2024
@bensofficial bensofficial temporarily deployed to artemis-test1.artemis.cit.tum.de July 2, 2024 16:58 — with GitHub Actions Inactive
@bensofficial bensofficial changed the title Feature/localci/build agent ssh Development: Add support for ephemeral SSH keys for the authentication of build agents Jul 2, 2024
@bensofficial bensofficial marked this pull request as ready for review July 2, 2024 17:29
@bensofficial bensofficial requested a review from a team as a code owner July 2, 2024 17:29
Copy link

coderabbitai bot commented Jul 2, 2024

Walkthrough

This update enhances the SSH authentication framework for build agents by integrating the BuildAgentSSHKeyService for SSH key management, updating several existing services to utilize this functionality, and introducing new test configurations. The changes enable more robust public key authentication and extend the data captured by the BuildAgentInformation class while ensuring comprehensive integration through configuration and testing enhancements.

Changes

File(s) Change Summary
src/main/java/.../GitPublickeyAuthenticatorService.java Added an optional SharedQueueManagementService parameter to the constructor, updated the authenticate method for build agent authentication, and introduced the checkPublicKeyMatchesBuildAgentPublicKey method.
src/main/java/.../BuildAgentSSHKeyService.java Implemented a new class for generating and managing SSH key pairs, with methods for retrieving formatted SSH public keys, generating key pairs, and writing private keys.
src/main/java/.../SharedQueueProcessingService.java Added the buildAgentSSHKeyService field, adjusted the constructor, and modified the getUpdatedLocalBuildAgentInformation method to include public SSH key retrieval.
src/test/java/.../AbstractSpringIntegrationLocalCILocalVCTest.java Enhanced test setup by adding configuration properties for SSH key paths, SSH usage for build agents, contact email, and SSH clone URL templates.
src/main/java/.../SshGitLocationResolverService.java Introduced conditional checks based on session attributes for authorizing users, accommodating build agents and specific repository actions.
src/test/java/.../BuildAgentSshAuthenticationIntegrationTest.java Added integration tests for SSH authentication in a build agent context, including methods to verify key presence and integrity.
src/main/java/.../SharedQueueManagementService.java Modified getBuildAgentInformationWithoutRecentBuildJobs method to include an additional parameter in BuildAgentInformation constructor for future extensibility.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant GitPublickeyAuthenticatorService
    participant BuildAgentSSHKeyService
    participant SharedQueueManagementService
    participant SshGitLocationResolverService

    Client->>GitPublickeyAuthenticatorService: Send Public Key for Authentication
    GitPublickeyAuthenticatorService->>BuildAgentSSHKeyService: Request Public Key Verification
    BuildAgentSSHKeyService-->>GitPublickeyAuthenticatorService: Return Verification Result
    GitPublickeyAuthenticatorService-->>Client: Return Authentication Success/Failure

    activate Client
    Client->>SshGitLocationResolverService: Request Repository Access
    SshGitLocationResolverService->>GitPublickeyAuthenticatorService: Verify if Build Agent
    GitPublickeyAuthenticatorService-->>SshGitLocationResolverService: Return Verification Result
    SshGitLocationResolverService-->>Client: Grant/Deny Access
    deactivate Client
Loading

This sequence diagram illustrates the high-level flow of SSH key-based authentication for a build agent, detailing the interactions between the client, GitPublickeyAuthenticatorService, and BuildAgentSSHKeyService in the authentication process, as well as the authorization for repository access.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

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>.
    • Generate unit testing code for this file.
    • 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 generate unit testing code for this file.
    • @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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @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.

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 as 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 resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

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.

@magkue magkue temporarily deployed to artemis-test1.artemis.cit.tum.de July 15, 2024 08:17 — with GitHub Actions Inactive
Copy link
Contributor

@magkue magkue left a comment

Choose a reason for hiding this comment

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

Works as expected!

Copy link

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

@krusche krusche added feature component:Programming config-change Pull requests that change the config in a way that they require a deployment via Ansible. ready to merge component:integrated code lifecycle and removed stale labels Jul 28, 2024
@krusche
Copy link
Member

krusche commented Jul 28, 2024

We wait until the exam phase is finished to avoid any issues based on config changes. This PR will be merged into the milestone 7.5.0

@krusche krusche added this to the 7.5.0 milestone Jul 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:integrated code lifecycle component:Programming config-change Pull requests that change the config in a way that they require a deployment via Ansible. feature ready to merge server Pull requests that update Java code. (Added Automatically!) tests
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

9 participants