ssh: accept private key contents instead of path #3846
Conversation
|
LGTM |
|
Isn't it kinda weird to have an argument that ends in |
|
@apparentlymart yeah agreed it's kinda weird I guess we could make:
|
|
@phinze As we discussed earlier we should probably make the |
We've been moving away from config fields expecting file paths that Terraform will load, instead prefering fields that expect file contents, leaning on `file()` to do loading from a path. This helps with consistency and also flexibility - since this makes it easier to shift sensitive files into environment variables. Here we add a little helper package to manage the transitional period for these fields where we support both behaviors. Also included is the first of several fields being shifted over - SSH private keys in provisioner connection config. We're moving to new field names so the behavior is more intuitive, so instead of `key_file` it's `private_key` now. Additional field shifts will be included in follow up PRs so they can be reviewed and discussed individually.
phinze
force-pushed
the
phinze/pathorcontents
branch
from
8a11451
to
7ffa66d
Compare
|
Okay! This got to be a rather large change, so I decided to chunk it up. This PR will serve as the base, and I'll follow on with individual PRs for the other fields that are changing, since they each have potential semantic discussion around the new field names. |
phinze
changed the title
|
LGTM. |
|
The expansion code in the new helper package should help solve #3856 as well when it comes around to that PR. |
ssh: accept private key contents instead of path
Builds on the work of #3846, shifting the Chef provisioner's configuration options from `secret_key_path` and `validation_key_path` over to `secret_key` and `validation_key`.
Building on the work in #3846, shifting the Azure provider's configuration option from `settings_file` to `publish_settings`.
Builds on the work of #3846, shifting the Chef provisioner's configuration options from `secret_key_path` and `validation_key_path` over to `secret_key` and `validation_key`.
Building on the work in #3846, shifting the Azure provider's configuration option from `settings_file` to `publish_settings`.
Building on the work in #3846, shifting the Google provider's configuration option from `account_file` to `credentials`.
Building on the work of #3846, deprecate `filename` in favor of a `template` attribute that accepts file contents instead of a path. Required a bit of work in the interpolation code to prevent Terraform from assuming that template interpolations were resource variables that needed to be resolved. Leaving them as "Unknown Variables" prevents interpolation from happening early and lets the `template_file` resource do its thing.
Building on the work of #3846, deprecate `filename` in favor of a `template` attribute that accepts file contents instead of a path. Required a bit of work in the interpolation code to prevent Terraform from assuming that template interpolations were resource variables that needed to be resolved. Leaving them as "Unknown Variables" prevents interpolation from happening early and lets the `template_file` resource do its thing.
Building on the work in #3846, shifting the Google provider's configuration option from `account_file` to `credentials`.
Building on the work in #3846, shifting the Google provider's configuration option from `account_file` to `credentials`.
Building on the work in #3846, shifting the Azure provider's configuration option from `settings_file` to `publish_settings`.
|
@phinze, In the context of SSH keys, I am a bit surprised to see this change, and the deprecation of |
|
@ketzacoatl I definitely understand the concern, however Terraform was already reading that file prior to this change. We use Go standard libraries for SSH communication, so it has to be in memory with respect to Terraform. |
|
ah, ok, connecting over SSH thru Go makes sense - sorry for the noise! |
…isterner-link-to-cert-resource docs/resource/aws_lb_listener: Add link to aws_lb_listener_certificate resource
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
We've been moving in the direction of pushing away from config fields
expecting file paths that Terraform will load, instead prefering fields
that expect file contents, leaning on
file()to do loading from apath.
This helps with consistency and also flexibility - since this makes it
easier to shift sensitive files into environment variables.
Here we add a little helper package to manage the transitional period
for these fields where we support both behaviors.
These fields already had this behavior, just got to move to a shared
implementation:
These fields move to the new contents-preferrred
path-supported-but-deprecated behavior: