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

Initial suggestions for plugin release #4

Open
misraved opened this issue Sep 1, 2023 · 2 comments
Open

Initial suggestions for plugin release #4

misraved opened this issue Sep 1, 2023 · 2 comments

Comments

@misraved
Copy link

misraved commented Sep 1, 2023

@jreyesr we want to express our appreciation for the outstanding work you've done on this new plugin 🎉!

While the fundamental structure is impressive, our usage of the plugin has led us to formulate a few suggestions in line with our best practices:

  • LICENSE
    • Is there any specific reason as to why you used the Mozilla Public License 2.0 license instead of Apache License 2.0?
  • config/tfbridge.spc
    • In general, we prefer to comment out the plugin config arguments in the tfbridge.spc file, that way we implore the users to set the value they need for plugin initialization. Would be more interested to know if you have a different stand on this 👍.
    • Do the config arguments support environment variables?
    • Suggested config argument:
      connection "tfbridge" {
      plugin = "jreyesr/tfbridge"
      # Write the name of a Terraform provider, in the same way that you'd write it
      # in the required_providers block in the Terraform main file
      # Examples: "hashicorp/aws", "TimDurward/slack", "hashicorp/random"
      # If using a private Terraform registry, also include the hostname: "registry.acme.com/acme/supercloud"
      # provider = "integrations/github"
      
      # Write a single version for a Terraform provider, in the same way that you'd write it
      # in the required_providers block in the Terraform main file
      # Note that, unlike in the Terraform file, you can't use a version constraint, such as "~> 1.0" or ">= 1.2.0, < 2.0.0",
      # only explicit versions are allowed
      # version = "5.33.0"
      
      # If the Terraform provider would require some configuration in its provider {...} block,
      # copy it here directly (just the _contents_ of the provider {} block, not the entire block!)
      # provider_config = <<EOT
      #  token = "github_pat_9fu38f0amil9FVOKmI0_0F8PmI0m0FNm"
      # EOT
      }  
      
  • tfbridge/config.go
    • Does the plugin support CLI commands like the AWS provider etc.?
  • docs/index.md

Thank you once again for the new plugin @jreyesr, please let me know if you have questions on the above comments. Thanks!!

Looking forward to getting this published on the hub 👍 🚀 !!!

@jreyesr
Copy link
Owner

jreyesr commented Sep 2, 2023

Hello @misraved, and thanks for taking the time!

Is there any specific reason as to why you used the Mozilla Public License 2.0 license?

The plugin embeds a bunch of code taken directly from Terraform (just before it switched from MPL 2.0 to BSL). There's a bit more info at the end of the README. This is used to interface with the actual Terraform providers, since that is quite convoluted and not documented anywhere. Since I'm not a lawyer, and open source license compatibility is what it is, I opted for keeping the entire plugin under MPL. Now, I have read that "if the author keeps the MPL’d code in separate files, they can combine that code with closed-source code to create an aggregate work. The new code files can be kept proprietary or released under a different license", but I haven't found a clear statement that "you can use MPL files in an Apache-licensed project, as long as you do so-and-so".

Do you have any experiences mixing MPL and Apache code? Can it be done safely, and the whole thing licensed under Apache?

In general, we prefer to comment out the plugin config arguments in the tfbridge.spc file, that way we implore the users to set the value they need for plugin initialization.

Ah, gotcha. That's evil smart :) I guess I didn't comment them to have "a working configuration" or something, I can't really remember. Maybe I didn't even decide to keep params uncommented as opposed to commenting them. I'll get them commented.

Do the config arguments support environment variables?

Not that I know. A question: from where would those envvars come from? The classical example is AWS's plugin, but there the envvars are very clear: those that the AWS CLI can read should also be respected by the plugin. However, on this plugin there's no preexisting CLI that does the same (okay, there's Terraform itself, but with Terraform you don't operate a provider in isolation, and you don't set the provider, version or config by using envvars). Is it common practice to "invent" envvars just to give users another way of setting config params instead of writing them in the HCL file, or is that only done to reflect an already-existing CLI? In such a case, this plugin wouldn't support them

Does the plugin support CLI commands like the AWS provider?

I think I don't quite understand. For example, which CLI commands does the AWS provider support? Do you mean the Terraform AWS provider or the Steampipe AWS plugin? (yes, I've been fighting with that confusion since I started this project)

The og_description seems to be incorrect

Oopsie. I'll get that fixed

Are all the config arguments required parameters?

Yes, for now. Should that be explicitly indicated somewhere in the docs, or maybe in the HCL comments?

What happens if I don't pass in the version param? Will the plugin automatically retrieve the default/latest value depending on the provider?

As of now, you have to provide it. There's #1 for adding a latest version, but Terraform Registry's API does not (AFAIK) let you query for the "latest" version, it just gives you a list of all versions, so I'd have to see how Terraform does it, exactly. Also, I'm worried about unclear dependencies causing problems (the latest version may change over time, and a Steampipe plugin that worked could suddenly break, if the Terraform provider's author pushed a new version with breaking changes). Terraform proper doesn't have that problem because they have a lock file, but I don't and I download the provider on every restart of Steampipe.

In other words, if the plugin defaults to the latest version if the user doesn't provide one, this could happen:

  • Steampipe user runs steampipe plugin install tfbridge
  • Steampipe user edits the HCL file, sets a provider name and config, and leaves the version unset
  • Steampipe user queries the plugin
  • TFBridge plugin boots, downloads the latest version of the provider (let's say 1.0.0), runs queries and returns
  • Terraform maintainer of the provider publishes version 2.0.0, which changes the data sources published
  • Steampipe is rebooted
  • Steampipe user queries the plugin with the same query as before
  • TFBridge plugin boots, downloads the latest version of the provider (2.0.0), runs queries and returns
  • Queries that were previously successful now fail

This could be especially bad if the queries aren't run on-demand by a user, but as part of another automatic system where they could be failing for a while and generating bad or empty data.

It would be cool if the plugin could somehow detect that it's being launched without a version, find the latest one, and then "freeze" that version forever by editing the HCL file itself (so any further launches would then use that version, which is supposed to work), but that's not possible. And it may be confusing too, having the HCL file change without warning

Could you please add the link to the community slack channel?

Of course, sorry! I remember deleting it, I don't remember why

jreyesr added a commit that referenced this issue Sep 2, 2023
@e-gineer
Copy link

e-gineer commented Sep 8, 2023

This is a really interesting plugin!

I agree if copying MPL code it's simplest to keep the code under the MPL. This can be through: whole repo as MPL, specific files/dir in repo under MPL or create a separate repo for MPL components to depend upon. For our work we prefer the separate repo approach, but I think for a specific plugin any option is fine.

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

No branches or pull requests

3 participants