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

[JENKINS-74836] Allow using a file based ssh credential via system property #1003

Merged

Conversation

mikecirioli
Copy link
Contributor

@mikecirioli mikecirioli commented Nov 7, 2024

JENKINS-74836

For some use cases it may not be desirable to create a Jenkins ssh credential to use for connecting to agents, as this credential may be visible to users in the ux, or you may find it easier to manage this private key as a mounted k8s secret.

To enable this functionality simply define a new System.property as follows:
SSH_KEY_PAIR_PRIVATE_KEY_FILE=/path/to/plaintext/private_key.pem

If this system property is defined then any credential configured in the cloud config page will be ignored.

configuration validation has also been updated to support the new feature:

file based key, no jenkins credentials - form validation
image

`file based key, no jenkins credentials - test connection
image

file based key, jenkins credentials selected - form validation
image

file based key, jenkins credentials selected - test connection
image

Testing done

  • Manually tested and works as expected in my k8s test cluster.
  • fixup 1 failing unit test
  • unit test for new functionality

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@mikecirioli mikecirioli marked this pull request as draft November 7, 2024 20:06
@mikecirioli mikecirioli changed the title allow using a file based ssh credential via system property to avoid having to create a Jenkins ssh credential allow using a file based ssh credential via system property Nov 7, 2024
@mikecirioli
Copy link
Contributor Author

feedback addressed @jglick , i am fixing up the testing now

Co-authored-by: Jesse Glick <jglick@cloudbees.com>
mikecirioli and others added 2 commits November 8, 2024 04:34
pr feedback

Co-authored-by: Antonio Muniz <amuniz@users.noreply.github.com>
@mikecirioli mikecirioli force-pushed the allow_file_based_private_key branch from 329e698 to 8c68a51 Compare November 8, 2024 09:35
@mikecirioli mikecirioli marked this pull request as ready for review November 8, 2024 09:36
@mikecirioli
Copy link
Contributor Author

@fcojfernandez i believe this is ready for your review

Copy link
Member

@amuniz amuniz left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@fcojfernandez fcojfernandez left a comment

Choose a reason for hiding this comment

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

would it be possible to have more tests? Something to test what prevails if the user includes the credentials but the system property is overriding the value

README.md Outdated Show resolved Hide resolved
src/main/java/hudson/plugins/ec2/EC2Cloud.java Outdated Show resolved Hide resolved
src/main/java/hudson/plugins/ec2/EC2Cloud.java Outdated Show resolved Hide resolved
src/main/java/hudson/plugins/ec2/EC2Cloud.java Outdated Show resolved Hide resolved
src/test/java/hudson/plugins/ec2/EC2CloudTest.java Outdated Show resolved Hide resolved
mikecirioli and others added 4 commits November 8, 2024 08:42
Co-authored-by: Francisco Javier Fernandez <31063239+fcojfernandez@users.noreply.github.com>
@mikecirioli
Copy link
Contributor Author

added screenshot and validated manually again as awell @fcojfernandez

@fcojfernandez fcojfernandez changed the title allow using a file based ssh credential via system property Allow using a file based ssh credential via system property Nov 11, 2024
@fcojfernandez fcojfernandez added the enhancement Feature additions or enhancements label Nov 11, 2024
@mikecirioli mikecirioli changed the title Allow using a file based ssh credential via system property [JENKINS-74836] Allow using a file based ssh credential via system property Nov 11, 2024
@fcojfernandez fcojfernandez merged commit ff2f5a2 into jenkinsci:master Nov 11, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature additions or enhancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants