shihanng / terraform-provider-installer

A Terraform provider to setup development environment machine.
https://registry.terraform.io/providers/shihanng/installer/latest/docs
MIT License
11 stars 3 forks source link

Support remote connection? #92

Open ScionOfDesign opened 1 year ago

ScionOfDesign commented 1 year ago

This terraform provider is exactly what I have been looking for, but the only problem is that it doesn't seem to support remote connections.

Could support for remote connections be added? I feel like it would dramatically increase the usefulness of this provider.

shihanng commented 1 year ago

Thank you for the suggestion. Remote connection seems to be a great additional feature. I am happy to accept PR for this feature.

ScionOfDesign commented 1 year ago

Thank you! I agree that it would be a great feature. Unfortunately, however, my experience with Go and the Terraform plugin modules is limited.

Is there a way that I could sponsor you or someone else to develop such a feature or, at the very least, could you point me in the right direction for developing it myself?

I know that resources like null_resource support a connection block like so:

resource "null_resource" "attach_disks" {
  connection {
    host = "myhostip"
    user = "root"
    private_key = file(var.private_key_path)
    agent = false
    timeout = "2m"
  }
}

Something similar for this provider would be amazing, but I don't really know where I'd even start.

ScionOfDesign commented 1 year ago

I took a deeper look into it.

It looks like the current SDK has data.ConnInfo() and data.SetConnInfo(connInfo) which can be used: https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/v2@v2.27.0/helper/schema#ResourceData.ConnInfo

However, in resourceAptCreate data.ConnInfo() returns an empty map, even when I have a connection block in the resource like above. So I'm not sure how to parse it.

I would like to use Terraform's communicator interface, but I'm not exactly quite sure how just yet. https://github.com/hashicorp/terraform/blob/919e62089b4886c0f090da757ee28ed9b8f3a2f0/internal/communicator/communicator.go#L25

The only example I found was from 4 years ago: https://github.com/partitio/terraform-provisioner-multi-remote-exec/blob/master/provisioner.go

ScionOfDesign commented 1 year ago

Ok, after setting up a debugger and learning a bit more about Go, I am fairly confident about what needs to be done to accomplish this task.

  1. Migrate to the Terraform Plugin Framework https://github.com/shihanng/terraform-provider-installer/issues/93
  2. Reorganize the existing providers and reduce duplicated code.
  3. Import Terraform's Communicator Interface.
  4. Update the schema of both the provider and the resources to support taking in a remote_connection map of data that behaves exactly the same as the connection map for provisioners.
  5. Update the resources to use the new remote connection.
  6. Add tests for the additional remote connection functionality.
shihanng commented 1 year ago

Migrate to the Terraform Plugin Framework https://github.com/shihanng/terraform-provider-installer/issues/93

Is migrating to the Terraform Plugin Framework necessary for implementing this feature?

Import Terraform's Communicator Interface.

Maybe I am not sure how to use the Communicator interface. From a quick look at the source code, it is hidden inside the internal package, and this means that it is not exportable.

ScionOfDesign commented 1 year ago

Is migrating to the Terraform Plugin Framework necessary for implementing this feature?

It is not, but I think it would be good to do it regardless. It seems to make several design improvements. I definitely like the way the new Framework is setup, and it would make me more familiar with your codebase.

Maybe I am not sure how to use the Communicator interface. From a quick look at the source code, it is hidden inside the internal package, and this means that it is not exportable.

I was able to just copy over the source files directly, along with its dependencies. It does not seem to rely on too many other files. I think it should be fine to just re-use the code that Hashicorp already wrote. We could maybe encourage them to export it as a public package.

shihanng commented 1 year ago

I read through the documentation about null_resource. It seems that the "connection" is part of the provisioner's API. I am not sure how much we can reuse it.

Stealing the idea from provisioner, I can see that we have the following in the provider block:

provider "installer" {
  remote {
    // Supporting arguments in https://developer.hashicorp.com/terraform/language/resources/provisioners/connection
    host = "myhostip"
    user = "root"
    private_key = file(var.private_key_path)
    agent = false
    timeout = "2m"
  }
}
ScionOfDesign commented 1 year ago

Yes, exactly. Either a nested block variable or map variable can work.

ScionOfDesign commented 1 year ago

Hey, just an update on this: https://github.com/StudioWhyNot/terraform-provider-installer/tree/feature/add-remote-connection Comparison: https://github.com/shihanng/terraform-provider-installer/compare/trunk...StudioWhyNot:terraform-provider-installer:feature/add-remote-connection I've been continuing to work on my PR, and have successfully been able to run remote apt and script commands remotely using the Communicator class from Terraform/OpenTofu.

Currently, it uses the same remote connection from the provider across all resources, though I plan to add resource-specific connections soon.

I kind of went all out and the PR as a whole is a major overhaul of pretty much everything to reduce code duplication and make extending the provider easier.

brew and asdf still need to be added, but I am not as familiar with those package managers yet. I also need to think about how to re-add unit tests.

Unfortunately, it looks like there will be a potentially breaking change with regard to the FindExecutablePath method. I can't make use of the os.Executable method remotely. However, not all apt packages actually do provide an executable (some are documentation), and many provide multiple executables, so not finding one shouldn't really be an error I don't think.

Anyway, feel free to try it out or provide feedback if you'd like. Also, since this is kind of a complete overhaul, let me know if you would still be comfortable pulling it in as a new major version or if I should just publish my own provider.

shihanng commented 1 year ago

Thanks for the efforts! This has the potential to be another provider. I can provide assistance to do so if necessary.