update(ngrok): switch to envvars provisioner for ngrok 3.2.1 and higher

[arunsathiya]

Fixes #216 and fixes #274

Description

This PR updates the ngrok Shell Plugin to use environment variables to provision credentials when the ngrok CLI is 3.2.1 or higher. We'd retain the config file-based provisioner for older versions.

Testing instructions

• Make sure there isn't a ngrok config file on your computer at the path depending on your OS. (This is important because as of Be aware of already existing Ngrok config file  #194, we detect such configuration files and merge with 1Password-generated configuration file)
• Check ngrok version by running ngrok --version. If it's 3.2.1 or higher, any ngrok api command (example ngrok api tunnels list) should succeed. During this run, the credentials are provisioned using the NGROK_AUTH_TOKEN and NGROK_API_KEY environment variables. (Note that only NGROK_API_KEY is new as of 3.2.1, NGROK_AUTH_TOKEN was already supported in previous versions)
• If the version is 3.1.1 or older, ngrok api tunnels list should still succeed, but this time the provisioning happens with configuration files.

Another way to test

• Checkout this branch and switch to it.
• In the file plugins/ngrok/provisioner.go, change return fileProvisioner{} to return provision.EnvVars(defaultEnvVarMapping) (line 64)
• With ngrok 3.1.1, run ngrok api tunnels list. It should fail with ERROR: API key is missing; either use '--api-key' flag or set it as 'api_key' in the configuration file because at this point the provisioning happens with the env vars NGROK_AUTH_TOKEN and NGROK_API_KEY of which NGROK_API_KEY is not supported on 3.1.1

Download ngrok 3.1.1

To download ngrok 3.1.1, download the binary from one of these two links depending on your system architecture:

https://github.com/ngrok/homebrew-ngrok/blob/668bb053898e0f767916083f3fdeaf4a0f6eac84/Casks/ngrok.rb#L4-L10

And then run sudo unzip ~/Downloads/ngrok-v3-3.1.1-darwin-arm64.zip -d /usr/local/bin

Ensure you are running the ngrok binary that you downloaded now by running which ngrok. You don't want to be testing with another binary, say Homebrew's version if you had it installed previously. which ngrok should read /usr/local/bin/ngrok

[arunsathiya]

The CI test is failing with 2023/03/27 08:33:22 exec: "ngrok": executable file not found in $PATH but that's expected I think because ngrok binary isn't available on the machine that runs the test. Testing locally should work as expected.

[AndyTitu]

Thanks for this great addition @arunsathiya ! I really like the logic of using the env var provisioning from versions equal or higher than 3.2.1, and still use the file provisioner based on versions lower than that, so that the shell plugin works for ngrok users regardless of their installed version.

The CI test is failing with 2023/03/27 08:33:22 exec: "ngrok": executable file not found in $PATH but that's expected I think because ngrok binary isn't available on the machine that runs the test.

For this reason, I think an improvement we could make on the OP CLI side is to also provision the tool's version via the ProvisionInput struct. It should not be the plugin's responsibility to detect the tool on the user's machine. Even more so since the CLI handles tool detection already, for now just to detect and error if a user is running a shell-plugin for a tool without having the actual tool in their PATH.
What do you think @SimonBarendse @florisvdg ?

[florisvdg]

@AndyTitu That fits nicely into a plan I had a while back to make the Provisioner metadata more intelligent, which you can find on this branch.

[SimonBarendse]

I think it would be great to pull out the logic for version comparison to make it easier to re-use. Worth exploring if we can go even further than the code on that branch Floris pushed and also take care of parsing the version, and maybe even the comparison func, so all the user needs to do is pass the version number.

In order to support non-standard versioning schemes (i.e. not semver) or with non-standard version command output that requires custom parsing logic, we can make it possible to pass funcs to overwrite the default behavior. but in the most common scenarios the user/plugin should require as little as possible work/code.

I believe making the code re-usable is an additive improvement, and doesn't need to block the improvement in this PR to go out. Can we already release this work from @arunsathiya then we (1Password) can take on the internal changes to add the version meta support in 1Password CLI to make it re-usable for other plugins?

[arunsathiya]

Thanks for the review, all of you! This should be ready for one more look, and hopefully good to merge. 🤞🏼

[hculea]

I have one concern related to this MR - I think it ingrains too deeply the version comparison functionality in the ngrok provisioner.

The pattern where different versions of a tool would default to different provisioners should not be isolated only to ngrok - but rather should be a core feature of the plugin ecosystem.

For example, I can imagine something along the lines of:

DefaultProvisioner: provision.ExecDependent(
  provision.ForVersion("<=2.3.5", provision.EnvVars(envVarMapping),
  provision.ForVersion(">2.3.5", provision.TempFile(...)
)

together with a validation function that would assert that all versions are covered.

I'd love to hear what y'all think!

---

An additional question: are there any known limitations to the current temp file approach, when it comes to ngrok?

[florisvdg]

A helper function to switch provisioners based on certain conditions could be useful indeed, but I agree with Simon that it doesn't have to block this PR.

[arunsathiya]

An additional question: are there any known limitations to the current temp file approach, when it comes to ngrok?

As far as I know/remember, the only concern is that the temp file approach requires a "Version" field, without which the ngrok binary doesn't work. At this point, "version: 2" is a valid field, but when in the future ngrok moves to a different version as the required/default version, we'd have to make that change on the Shell Plugins side too. Environment variables-based approach doesn't require that field.

[hculea]

Very eager to bring versioning to all plugin provisioners!

Before then, I agree, let's not hold this back. 😄

[AndyTitu]

lgtm. I have created an internal issue for tackling the versioning logic discussed here.

[AndyTitu]

This generally looks good! I have left a few minor comments.

It would be good to test these changes with the Terraform plugin to make sure using ngrok Credentials with Terraform no longer returns the not supported error.

[AndyTitu]

This generally looks good! I have left a few minor comments.

It would be good to test these changes with the Terraform plugin to make sure using ngrok Credentials with Terraform no longer returns the not supported error.

[AndyTitu]

This generally looks good! I have left a few minor comments.

It would be good to test these changes with the Terraform plugin to make sure using ngrok Credentials with Terraform no longer returns the not supported error.

[arunsathiya]

Thanks for the guidance, @AndyTitu. I have tested with the Terraform plugin and it works as expected.

My config file is as below:

terraform {
  required_providers {
    ngrok = {
      source = "ngrok/ngrok"
      version = "0.1.4"
    }
  }
}

# Configure the ngrok provider
provider "ngrok" {}

resource "ngrok_api_key" "example" {
  description = "ad-hoc dev testing"
  metadata = "{\"environment\":\"dev\"}"
}

The resource block is from here:

https://github.com/ngrok/terraform-provider-ngrok/tree/main/examples/resources

[arunsathiya]

Fixes #274

[arunsathiya]

Testing instructions:

• Install ngrok 3.2.1, a higher version, and a lower version (using the original testing instructions here) and see that provisioning credentials work as expected for all of these. 3.2.1 and higher should use environment variables, while smaller ones, like 3.1.1 should use file provisioner.
• Create a Terraform config file main.tf with the content below.
• Run terraform init
• Run op plugin init terraform and choose ngrok credentials.
• Run terraform plan followed by terraform apply.
• See that the application goes through as expected.

Terraform config file:

terraform {
  required_providers {
    ngrok = {
      source = "ngrok/ngrok"
      version = "0.1.4"
    }
  }
}

# Configure the ngrok provider
provider "ngrok" {}

resource "ngrok_api_key" "example" {
  description = "ad-hoc dev testing"
  metadata = "{\"environment\":\"dev\"}"
}

[AndyTitu]

Re-approving to mark that I'm okay with the latest changes.