Skip to content

RUN-3734: Retrieve git password from key storage #35

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.

Sign up for GitHub

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

Jump to bottom
Open
ltamaster wants to merge 9 commits into
base: master
Choose a base branch
from
Open

RUN-3734: Retrieve git password from key storage #35

ltamaster wants to merge 9 commits into from

Conversation

@ltamaster
Copy link
Contributor

@ltamaster ltamaster commented Apr 2, 2025 •
edited by fdevans
Loading

Summary

This PR adds support for retrieving Git passwords and SSH keys from Rundeck's Key Storage system, providing a secure alternative to storing credentials in plain text or filesystem paths. This work is based on PR #29 and has been enhanced with comprehensive improvements.

Key Features

🔐 Key Storage Support for Passwords

  • Retrieve Git passwords securely from Rundeck Key Storage
  • New gitPasswordPathStorage property for Key Storage paths
  • Maintains backwards compatibility with gitPasswordPath for plain text
  • Key Storage credentials take precedence over plain text for security

🔑 Key Storage Support for SSH Keys

  • Retrieve SSH private keys from Rundeck Key Storage (new feature)
  • New gitKeyPathStorage property for SSH Key Storage paths
  • Maintains backwards compatibility with gitKeyPath for filesystem paths
  • Key Storage keys take precedence over filesystem paths for security
  • Feature parity with Workflow Steps - Resource Model now has same capabilities

📚 Comprehensive Documentation

  • Expanded README from 175 to 344 lines (+96%)
  • Step-by-step Key Storage setup instructions
  • Authentication examples for different Git URL types (HTTPS, SSH, tokens)
  • Troubleshooting section for common issues
  • Security best practices
  • Quick Reference with common configuration scenarios
  • Property reference table

Changes Made

Code Quality Improvements

  • Refactored duplicate code in GitPluginUtil into shared readResourceMetaAsString() helper method
  • Improved maintainability and DRY principle
  • Better error messages and logging

New Tests

  • Added test for password authentication via Key Storage
  • Added test for SSH key authentication via Key Storage
  • All tests passing ✅

Technical Details

Files Modified

  • GitResourceModel.groovy - Added Key Storage support for passwords and SSH keys
  • GitResourceModelFactory.groovy - Added new properties and UI configuration
  • GitPluginUtil.groovy - Refactored code, improved error handling
  • GitResourceModelSpec.groovy - Added comprehensive test coverage
  • README.md - Extensive documentation improvements
  • gradle/libs.versions.toml - Updated JGit SSH module version to match core version
  • PluginSshSessionFactory.groovy - Fixed package imports for JGit 6.x compatibility

JGit Dependency Updates

  • Fixed version mismatch: Updated jgitSsh from 5.13.3 to 6.6.1 to match core jgit version
  • API compatibility: Updated SSH class imports for JGit 6.x package structure changes
    • JschConfigSessionFactory and OpenSshConfig moved to org.eclipse.jgit.transport.ssh.jsch package
  • Resolves potential SSH authentication issues caused by the version mismatch

Backwards Compatibility

✅ No breaking changes - all existing property names maintained:

  • gitPasswordPath - Still works for plain text passwords
  • gitKeyPath - Still works for filesystem SSH key paths
  • gitPasswordPathStorage - New property for Key Storage passwords
  • gitKeyPathStorage - New property for Key Storage SSH keys

Requirements

  • Rundeck 5.16.0 or later - Required for Key Storage support
  • Earlier versions can continue using plain text/filesystem authentication

Testing

  • ✅ All unit tests passing
  • ✅ Clean build successful
  • ✅ No new linter errors
  • ✅ Backwards compatibility verified

Related

Based on PR #29. Thank you @blink38

Security

Improvements

  • Secure credential storage in Rundeck Key Storage
  • Key Storage credentials take precedence over plain text
  • Comprehensive error handling prevents credential exposure in logs
  • Security best practices documented

Best Practices

Users are encouraged to:

  1. Use Key Storage instead of plain text credentials
  2. Use project-specific keys when possible
  3. Use deployment keys with minimal permissions
  4. Enable strict host key checking for SSH in production
  5. Rotate credentials regularly

Migration Guide

No migration required! Existing configurations will continue to work without any changes.

To adopt Key Storage (recommended):

  1. Store credentials in Rundeck Key Storage (System Menu → Key Storage)
  2. Update plugin configuration to use new gitPasswordPathStorage or gitKeyPathStorage properties
  3. Remove plain text credentials after verifying Key Storage works

Screenshots

Resource Model Configuration

The plugin now offers clear options for authentication:

  • Git Password (Plain Text) - For plain text passwords (backwards compatible)
  • Git Password Storage Path - For Key Storage passwords (recommended)
  • SSH Key Path (Filesystem) - For filesystem SSH keys (backwards compatible)
  • SSH Key Storage Path - For Key Storage SSH keys (recommended)

Checklist

  • Code follows project style guidelines
  • All tests passing
  • Documentation updated
  • Backwards compatibility maintained
  • No breaking changes
  • Copilot review comments addressed
  • Feature parity with Workflow Steps achieved

@ltamaster ltamaster changed the title Retrieve git password from key storageNodes with key storage Retrieve git password from key storage Apr 2, 2025
@ltamaster ltamaster force-pushed the nodes-with-key-storage branch from 03ce7bc to c0cf954 Compare April 8, 2025 20:22
@fdevans fdevans requested a review from Copilot December 2, 2025 18:23

This comment was marked as outdated.

Sign in to view

@fdevans
Merge branch 'master' into nodes-with-key-storage
b9215c5
@fdevans
fix: address Copilot PR review comments
c3eff80 
- Fix UI labels to distinguish plain text vs Key Storage password fields
- Reverse precedence: Key Storage password now takes precedence over plain text
- Add null safety check before setting password from Key Storage
- Fix NullPointerException by reusing storageTree variable
- Add error handling with try-catch and logging for Key Storage access
- Add JavaDoc documentation to getFromKeyStorage method
- Use explicit UTF-8 encoding for password strings
- Update README with Key Storage authentication documentation
- Clean up test file whitespace

All changes maintain backwards compatibility - no breaking changes to property names.
@fdevans
feat: add SSH Key Storage support and refactor code duplication
1ff647d 
- Refactor GitPluginUtil: Extract duplicate code into readResourceMetaAsString() helper method
- Add SSH Key Storage support for Resource Model (feature parity with Workflow Steps)
- Add GIT_KEY_STORAGE_PATH property for SSH keys from Rundeck Key Storage
- Update GitResourceModel to retrieve SSH keys from Key Storage with precedence over filesystem
- Add UI field label clarification (Filesystem vs Storage Path)
- Add comprehensive test for SSH Key Storage authentication
- Update README with SSH Key Storage documentation

Maintains backwards compatibility - filesystem SSH key paths still work.
@fdevans
docs: comprehensive README improvements for authentication and Key St…
1b6fa02 
…orage

- Added detailed step-by-step setup instructions for Key Storage
- Added example Key Storage paths for different scenarios
- Created authentication examples table by Git URL type
- Added comprehensive troubleshooting section for common issues
- Added security best practices section
- Added Quick Reference section with:
  - Key Storage setup instructions
  - Common configuration scenarios (GitHub, GitLab, HTTPS, SSH)
  - Property reference table
  - Version requirements
- Improved workflow steps authentication documentation
- Organized content with clear sections and examples

README expanded from 175 to 344 lines with actionable guidance.
@fdevans fdevans requested a review from Copilot December 2, 2025 20:03
@fdevans fdevans changed the title Retrieve git password from key storage RUN-3734: Retrieve git password from key storage Dec 2, 2025

This comment was marked as outdated.

Sign in to view

@fdevans
Fix JGit SSH module version mismatch and update imports for JGit 6.x …
b1740a3 
…compatibility

- Updated jgitSsh from 5.13.3 to 6.6.1 to match core jgit version
- Fixed package imports in PluginSshSessionFactory for JGit 6.x
  - JschConfigSessionFactory moved to org.eclipse.jgit.transport.ssh.jsch
  - OpenSshConfig moved to org.eclipse.jgit.transport.ssh.jsch
- Resolves version mismatch that could cause SSH authentication issues
@fdevans fdevans mentioned this pull request Dec 2, 2025
Closed
@fdevans
Address Copilot feedback: code quality improvements
d6d8de2 
- Renamed misleading constant names for clarity (no breaking changes to property values)
  - GIT_KEY_STORAGE -> GIT_KEY_PATH (filesystem paths)
  - GIT_PASSWORD_STORAGE -> GIT_PASSWORD_PATH (plain text passwords)
- Fixed potential NullPointerException in GitPluginUtil
  - Added null checks before calling logger.log()
- Fixed misleading error messages to use 'credential' instead of 'password'
  - Messages now accurate for both passwords and SSH keys
- Added comprehensive documentation to getFromKeyStorage methods
  - Documented parameters, return values, and exception behavior
- Fixed spelling errors in README: 'commiting' -> 'committing'
- All tests passing
@fdevans fdevans requested a review from Copilot December 2, 2025 20:37

This comment was marked as resolved.

Sign in to view

@fdevans
Address additional Copilot feedback: refactoring and style improvements
3e9375f 
- Refactored ExecutionContext creation to avoid duplication
  - Create context once and reuse for both password and SSH key retrieval
- Changed method parameter from ExecutionContextImpl to ExecutionContext interface
  - Improves flexibility and follows dependency inversion principle
- Removed unnecessary semicolon from import statement
- Fixed inconsistent spacing in test mock interactions (1* -> 1 *)
- Fixed @OverRide annotation formatting for consistency
- All tests passing
@fdevans fdevans requested a review from a team December 2, 2025 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ltamaster @blink38 @fdevans