From d350818126e627a5b5de83dcbe90666dfdf9a5f8 Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Wed, 13 May 2020 13:16:09 -0400 Subject: [PATCH] internal/getproviders: fix panic with invalid path parts (#24940) * internal/getproviders: fix panic with invalid path parts If the search path is missing a directory, the provider installer would try to create an addrs.Provider with the wrong parts. For example if the hostname was missing (as in the test case), it would call addrs.NewProvider with (namespace, typename, version). This adds a validation step for each part before calling addrs.NewProvider to avoid the panic. --- .../filesystem_mirror_source_test.go | 10 ++++++++++ internal/getproviders/filesystem_search.go | 16 ++++++++++++++++ .../2.0.0/darwin_amd64/terraform-provider-null | 1 + .../2.0.0/linux_amd64/terraform-provider-null | 1 + .../hashicorp/null/invalid | 1 + ...terraform-provider-null_2.1.0_linux_amd64.zip | 5 +++++ .../null/terraform-provider-null_invalid.zip | 1 + ...orm-provider-null_invalid_invalid_invalid.zip | 1 + .../linux_amd64/terraform-provider-random-beta | 1 + .../1.2.0/linux_amd64/terraform-provider-random | 1 + 10 files changed, 38 insertions(+) create mode 100644 internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/2.0.0/darwin_amd64/terraform-provider-null create mode 100644 internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/2.0.0/linux_amd64/terraform-provider-null create mode 100644 internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/invalid create mode 100644 internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/terraform-provider-null_2.1.0_linux_amd64.zip create mode 100644 internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/terraform-provider-null_invalid.zip create mode 100644 internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/terraform-provider-null_invalid_invalid_invalid.zip create mode 100644 internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/random-beta/1.2.0/linux_amd64/terraform-provider-random-beta create mode 100644 internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/random/1.2.0/linux_amd64/terraform-provider-random diff --git a/internal/getproviders/filesystem_mirror_source_test.go b/internal/getproviders/filesystem_mirror_source_test.go index 6d66dd29b..24ecd595f 100644 --- a/internal/getproviders/filesystem_mirror_source_test.go +++ b/internal/getproviders/filesystem_mirror_source_test.go @@ -92,6 +92,16 @@ func TestFilesystemMirrorSourceAllAvailablePackages(t *testing.T) { } } +// In this test the directory layout is invalid (missing the hostname +// subdirectory). The provider installer should ignore the invalid directory. +func TestFilesystemMirrorSourceAllAvailablePackages_invalid(t *testing.T) { + source := NewFilesystemMirrorSource("testdata/filesystem-mirror-invalid") + _, err := source.AllAvailablePackages() + if err != nil { + t.Fatal(err) + } +} + func TestFilesystemMirrorSourceAvailableVersions(t *testing.T) { source := NewFilesystemMirrorSource("testdata/filesystem-mirror") got, err := source.AvailableVersions(nullProvider) diff --git a/internal/getproviders/filesystem_search.go b/internal/getproviders/filesystem_search.go index 23a7d3ce7..e3cd047b1 100644 --- a/internal/getproviders/filesystem_search.go +++ b/internal/getproviders/filesystem_search.go @@ -52,6 +52,22 @@ func SearchLocalDirectory(baseDir string) (map[addrs.Provider]PackageMetaList, e namespace := parts[1] typeName := parts[2] + // validate each part + // The legacy provider namespace is a special case. + if namespace != addrs.LegacyProviderNamespace { + _, err = addrs.ParseProviderPart(namespace) + if err != nil { + log.Printf("[WARN] local provider path %q contains invalid namespace %q; ignoring", fullPath, namespace) + return nil + } + } + + _, err = addrs.ParseProviderPart(typeName) + if err != nil { + log.Printf("[WARN] local provider path %q contains invalid type %q; ignoring", fullPath, typeName) + return nil + } + hostname, err := svchost.ForComparison(hostnameGiven) if err != nil { log.Printf("[WARN] local provider path %q contains invalid hostname %q; ignoring", fullPath, hostnameGiven) diff --git a/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/2.0.0/darwin_amd64/terraform-provider-null b/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/2.0.0/darwin_amd64/terraform-provider-null new file mode 100644 index 000000000..daa9e3509 --- /dev/null +++ b/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/2.0.0/darwin_amd64/terraform-provider-null @@ -0,0 +1 @@ +# This is just a placeholder file for discovery testing, not a real provider plugin. diff --git a/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/2.0.0/linux_amd64/terraform-provider-null b/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/2.0.0/linux_amd64/terraform-provider-null new file mode 100644 index 000000000..daa9e3509 --- /dev/null +++ b/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/2.0.0/linux_amd64/terraform-provider-null @@ -0,0 +1 @@ +# This is just a placeholder file for discovery testing, not a real provider plugin. diff --git a/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/invalid b/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/invalid new file mode 100644 index 000000000..289663a2a --- /dev/null +++ b/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/invalid @@ -0,0 +1 @@ +This should be ignored because it doesn't follow the provider package naming scheme. diff --git a/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/terraform-provider-null_2.1.0_linux_amd64.zip b/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/terraform-provider-null_2.1.0_linux_amd64.zip new file mode 100644 index 000000000..68a550271 --- /dev/null +++ b/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/terraform-provider-null_2.1.0_linux_amd64.zip @@ -0,0 +1,5 @@ +This is just a placeholder file for discovery testing, not a real provider package. + +This file is what we'd find for mirrors using the "packed" mirror layout, +where the mirror maintainer can just download the packages from upstream and +have Terraform unpack them automatically when installing. diff --git a/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/terraform-provider-null_invalid.zip b/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/terraform-provider-null_invalid.zip new file mode 100644 index 000000000..289663a2a --- /dev/null +++ b/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/terraform-provider-null_invalid.zip @@ -0,0 +1 @@ +This should be ignored because it doesn't follow the provider package naming scheme. diff --git a/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/terraform-provider-null_invalid_invalid_invalid.zip b/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/terraform-provider-null_invalid_invalid_invalid.zip new file mode 100644 index 000000000..289663a2a --- /dev/null +++ b/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/null/terraform-provider-null_invalid_invalid_invalid.zip @@ -0,0 +1 @@ +This should be ignored because it doesn't follow the provider package naming scheme. diff --git a/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/random-beta/1.2.0/linux_amd64/terraform-provider-random-beta b/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/random-beta/1.2.0/linux_amd64/terraform-provider-random-beta new file mode 100644 index 000000000..daa9e3509 --- /dev/null +++ b/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/random-beta/1.2.0/linux_amd64/terraform-provider-random-beta @@ -0,0 +1 @@ +# This is just a placeholder file for discovery testing, not a real provider plugin. diff --git a/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/random/1.2.0/linux_amd64/terraform-provider-random b/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/random/1.2.0/linux_amd64/terraform-provider-random new file mode 100644 index 000000000..daa9e3509 --- /dev/null +++ b/internal/getproviders/testdata/filesystem-mirror-invalid/hashicorp/random/1.2.0/linux_amd64/terraform-provider-random @@ -0,0 +1 @@ +# This is just a placeholder file for discovery testing, not a real provider plugin.