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

Allow connections via SSH agent #31

Merged
merged 5 commits into from
Feb 3, 2022
Merged

Conversation

phaer
Copy link
Contributor

@phaer phaer commented Jan 25, 2022

Hi & thanks a lot for this project!

Here's a minimal implementation which allows connecting via a local SSH agent, which can be useful if the private key is either password protected or not available because it's stored on a hardware token (as is the case with my yubikey).
Is this something you would merge?

Additionally, would you welcome a breaking change in another PR which synchronizes the parameters of this providers conn blocks with modern terraforms connection blocks?

phaer added a commit to phaer/terraform-hetzner-kube that referenced this pull request Jan 26, 2022
docs/data-sources/file.md Outdated Show resolved Hide resolved
Copy link
Owner

@tenstad tenstad left a comment

Choose a reason for hiding this comment

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

Thank you for your PR! This is definitely something we could merge. Would prefer to have it tested first tho.

@tenstad
Copy link
Owner

tenstad commented Jan 26, 2022

synchronizes the parameters of this providers conn blocks with modern terraforms connection blocks

I'm up for a breaking change, what would you change?
I suppose this is the target: https://www.terraform.io/language/resources/provisioners/connection

@phaer
Copy link
Contributor Author

phaer commented Jan 26, 2022

I'm up for a breaking change, what would you change?
Yes, that's the right target. On second reading, it seems to be just about the block naming: conn -> connection

@tenstad
Copy link
Owner

tenstad commented Jan 26, 2022

Yes, that's the right target. On second reading, it seems to be just about the block naming: conn -> connection

If I remember correctly, terraform does not allow the block name connection. Please try to rename it and see if anything has changed, as I tested it quite some time ago.

Copy link
Owner

@tenstad tenstad left a comment

Choose a reason for hiding this comment

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

I got the tests working! If you fix the final comment, we can merge 😄

internal/provider/connection.go Outdated Show resolved Hide resolved
@tenstad
Copy link
Owner

tenstad commented Feb 2, 2022

@phaer could we get this one merged?

Co-authored-by: Amund Tenstad <github@amund.io>
@phaer
Copy link
Contributor Author

phaer commented Feb 3, 2022

@tenstad Sorry for the wait, the project I am using this provider for is on ice for the moment and I had quite a few other things going on. But looks goot to me, thanks for all the fixes and the provider in general! :)

Co-authored-by: Amund Tenstad <github@amund.io>
@tenstad tenstad merged commit cfe3abb into tenstad:main Feb 3, 2022
@tenstad
Copy link
Owner

tenstad commented Feb 3, 2022

Thank you for adding this @phaer ! 😄
I have now created a test for it #40, although it only works locally (#36).

phaer added a commit to phaer/terraform-hetzner-kube that referenced this pull request Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants