(feat) Add ssh key support to abs ssh transport #182
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Allows for an ABS_SSH_PRIVATE_KEY env variable to provide a path to a local private key for the inventory ssh config rather than a password credential. Prefers ABS_SSH_PRIVATE_KEY if present. This allows us to use ssh keys to reach abs hosts in CI.
MikaelSmith
approved these changes
Nov 9, 2021
|
@binford2k do you have anyone that should take a look at this? |
|
Hi @jpartlow, I had a look on the PR and it looks good, I'll merge it now, thank you! |
MikaelSmith
reviewed
Dec 14, 2021
| 'facts' => { 'provisioner' => 'abs', 'platform' => host['type'], 'job_id' => job_id } } | ||
| if !ENV['ABS_SSH_PRIVATE_KEY'].empty? |
There was a problem hiding this comment.
@jpartlow this throws an error when the environment variable is unset.
There was a problem hiding this comment.
This happens when merging goes fast forward. We need to come with a proper fix in order to avoid blocking errors.
There was a problem hiding this comment.
Should we revert or fix?
jpartlow
added a commit
to jpartlow/provision
that referenced
this pull request
Dec 15, 2021
The abs.rb is refactored into a class that is executed only if the task file is the executing program. This allows the spec to load the task and manipulate the class while mocking externals. The webmock gem is used to provide HTTP::Net mocking. The spec is not exhaustive, but hopefully covers enough to validate that the refactor didn't break the task, and that the bug I added in puppetlabs#182 is patched. It could be expanded, and could serve as a template for adding specs for other tasks in the module. It does look to expose an edge case bug of calling the provision task with an inventory directory specified that contains a pre-existing inventory file.
jpartlow
added a commit
to jpartlow/provision
that referenced
this pull request
Dec 15, 2021
The abs.rb is refactored into a class that is executed only if the task file is the executing program. This allows the spec to load the task and manipulate the class while mocking externals. The webmock gem is used to provide HTTP::Net mocking. The spec is not exhaustive, but hopefully covers enough to validate that the refactor didn't break the task, and that the bug I added in puppetlabs#182 is patched. It could be expanded, and could serve as a template for adding specs for other tasks in the module. It does look to expose an edge case bug of calling the provision task with an inventory directory specified that contains a pre-existing inventory file.
jpartlow
added a commit
to jpartlow/provision
that referenced
this pull request
Dec 16, 2021
The abs.rb is refactored into a class that is executed only if the task file is the executing program. This allows the spec to load the task and manipulate the class while mocking externals. The webmock gem is used to provide HTTP::Net mocking. The spec is not exhaustive, but hopefully covers enough to validate that the refactor didn't break the task, and that the bug I added in puppetlabs#182 is patched. It could be expanded, and could serve as a template for adding specs for other tasks in the module. It does look to expose an edge case bug of calling the provision task with an inventory directory specified that contains a pre-existing inventory file.
jpartlow
added a commit
to jpartlow/provision
that referenced
this pull request
Feb 11, 2022
The abs.rb is refactored into a class that is executed only if the task file is the executing program. This allows the spec to load the task and manipulate the class while mocking externals. The webmock gem is used to provide HTTP::Net mocking. The spec is not exhaustive, but hopefully covers enough to validate that the refactor didn't break the task, and that the bug I added in puppetlabs#182 is patched. It could be expanded, and could serve as a template for adding specs for other tasks in the module. It does look to expose an edge case bug of calling the provision task with an inventory directory specified that contains a pre-existing inventory file.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allows for an ABS_SSH_PRIVATE_KEY env variable to provide a path to a
local private key for the inventory ssh config rather than a password
credential. Prefers ABS_SSH_PRIVATE_KEY if present.
This allows us to use ssh keys to reach abs hosts in CI.
I've been testing with my branch here:
https://github.com/puppetlabs/kurl_test/pull/47/files#diff-e6ffa5dc854b843b3ee3c3c28f8eae2f436c2df2b1ca299cca1fa5982e390cf8R36