tools/terraform-bundle: refactor to use new provider installer and provider directory layouts#24629
Merged
mildwonkey merged 12 commits intomasterfrom Apr 21, 2020
Merged
tools/terraform-bundle: refactor to use new provider installer and provider directory layouts#24629mildwonkey merged 12 commits intomasterfrom
mildwonkey merged 12 commits intomasterfrom
Conversation
Codecov Report
|
alisdair
approved these changes
Apr 15, 2020
Contributor
alisdair
left a comment
alisdair
left a comment
There was a problem hiding this comment.
This is great! 👏 Being able to reuse the new provider installer code with multiple sources is really cool. I left a few minor notes inline. Note that I haven't tested this branch locally (yet), this review is just based on the diff.
|
|
||
| ``` | ||
| ./plugins/example.com/myorg/customplugin/0.1/linux_amd64/ | ||
| ``` |
Contributor
There was a problem hiding this comment.
🙌 This is example is very helpful!
| }, | ||
| FetchPackageBegin: func(provider addrs.Provider, version getproviders.Version, location getproviders.PackageLocation) { | ||
| c.ui.Info(fmt.Sprintf("- Installing %s v%s...", provider.ForDisplay(), version)) | ||
| }, |
Contributor
There was a problem hiding this comment.
We may want to add the FetchPackageSuccess callback here after #24617 is merged, if only for consistency.
alisdair
reviewed
Apr 15, 2020
Contributor
|
I was unable to convince Here's the config: terraform {
version = "0.12.0"
}
providers {
aws = {
versions = ["~> 1.0"]
}
google = {
versions = ["~> 1.0", "~> 2.0"]
}
customplugin = {
versions = ["0.1"]
source = "example.com/myorg/customplugin"
}
}Local filesystem layout: ls -lRLog output: TF_LOG=trace terraform-bundle package bundle.tf |
mildwonkey
force-pushed
the
mildwonkey/ps-tf-bundle
branch
from
d12932b to
e7e3139
Compare
Contributor
Author
|
@apparentlymart this is the trace output when I've attempted to exclude locally-added providers from the registry search: in case you missed my super professional debugging statement in there 😉 : |
mildwonkey
force-pushed
the
mildwonkey/ps-tf-bundle
branch
3 times, most recently
from
d047fb0 to
c22bcd4
Compare
alisdair
approved these changes
Apr 20, 2020
Contributor
alisdair
left a comment
alisdair
left a comment
There was a problem hiding this comment.
This looks great to me! Verified working locally 👍
| # a name of the form: terraform-provider-*, and must be build with the operating | ||
| # system and architecture that terraform enterprise is running, e.g. linux and amd64 | ||
| customplugin = ["0.1"] | ||
| # plugins directory and package it with the bundle archive. Plugin must have |
Contributor
There was a problem hiding this comment.
Suggested change
| # plugins directory and package it with the bundle archive. Plugin must have | |
| # ./.plugins directory and package it with the bundle archive. Plugin must have |
I'm less sure about the helpfulness of this one…
| subdirectory under "./plugins". The directory must have the following layout: | ||
|
|
||
| ``` | ||
| ./plugins/$SOURCEHOST/$SOURCENAMESPACE/$NAME/$VERSION/$OS_$ARCH/ |
Contributor
There was a problem hiding this comment.
Suggested change
| ./plugins/$SOURCEHOST/$SOURCENAMESPACE/$NAME/$VERSION/$OS_$ARCH/ | |
| ./.plugins/$SOURCEHOST/$SOURCENAMESPACE/$NAME/$VERSION/$OS_$ARCH/ |
| foundLocally := map[addrs.Provider]struct{}{} | ||
|
|
||
| if absPluginDir, err := filepath.Abs(pluginDir); err == nil { | ||
| if _, err := os.Stat(absPluginDir); err == nil { |
Contributor
There was a problem hiding this comment.
Some logging around here might be helpful. Examples of things that would've helped me find my own mistake today
No ./.plugins directory found, skipping local provider discovery.Found ./.plugins directory, discovering local providers.Local provider found: "example.com/myorg/customplugin"No local providers found.
Contributor
Author
There was a problem hiding this comment.
love it, added all of them
Contributor
Author
There was a problem hiding this comment.
I hope you like that commit message 😝
provider directory layouts terraform-bundle now supports a "source" attribute for providers, uses the new provider installer, and the archive it creates preserves the new (required) directory hierarchy for providers, under a "plugins" directory. This is a breaking change in many ways: source is required for any non-HashiCorp provider, locally-installed providers must be given a source (can be arbitrary, see docs) and placed in the expected directory hierarchy, and the unzipped archive is no longer flat; there is a new "plugins" directory created with providers in the new directory layout. This PR also extends the existing test to check the contents of the zip file.
…uld have saved a lot of trouble.
mildwonkey
force-pushed
the
mildwonkey/ps-tf-bundle
branch
from
87abaff to
30cb109
Compare
mildwonkey
changed the title
apparentlymart
approved these changes
Apr 21, 2020
|
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
... to use new provider installer and provider directory layouts
UPDATE
This supersedes the "NOTE" below (saved for history).
I updated this branch to include the hard-coded requirement for terraform v0.13 and temporarily disabled the e2e tests, which will fail until there is a v0.13 on releases.hashicorp.com (or we decide it's worth the effort to do a bit of hacking for test purposes). We've had at least one user (understandably) try to build terraform-bundle from master and find that it does not work with terraform v0.12 so it seems better to merge now so other users will see the expected error (and updated documentation).
NOTE (obsolete, saved for history)
This PR is in an awkward state: it needs one more commit which will limit its use to Terraform v0.13 (see the
zeroThirteenvariable inconfig.go, currently and un-intuitively set to">= 0.12.0"), but setting that variable properly causes tests to fail because there is no terraform v0.13 in releases. I would like a review on this PR, but it will not be merged before we have a 0.13 release (alpha or beta).terraform-bundlenow supports a "source" attribute for providers,uses the new provider installer, and the archive it creates preserves
the new (required) directory hierarchy for providers, under a "plugins"
directory.
This is a breaking change in many ways: source is required for any
non-HashiCorp provider, locally-installed providers must be given a
source (can be arbitrary, see docs) and placed in the expected directory
hierarchy, and the unzipped archive is no longer flat; there is a new
"plugins" directory created with providers in the new directory layout.
I've open an internal task with the Terraform Enterprise team letting them know about this new directory layout; my hope is that we can use an environment variable (taking advantage of the provider installer's support for XDG basedir spec) to point TFE to the new
pluginsdirectory (which will be in the same directory as the terraform binary)This PR also extends the existing test to check the contents of the zip file and adds the terraform-bundle e2e test to CircleCi.
I've also updated the documentation, though it definitely needs some editing/help!