From 283d88551a71767fc8679b8019fcb1c390a53e15 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Sat, 21 Oct 2017 11:56:53 -0400 Subject: [PATCH] Add a new module storage manifest To add registry support, a workaround in the local module storage was added to record the subdirectory containing the module source from within the archive file. Here we replace that temporary implementation with the full manifest needed to record the necessary module metadata for module loading. In order to support versioned modules, the actual stored version needs to be recorded. This can't be derived from the configuration, because the configuration only contains the constraints, and at load time we need to be able to enumerate the stored modules and all versions in order to resolve them. While the local storage key will be derived from the source and version, that information is lost once it's hashed. While the entire storage layer could be replaced to encode the needed data in the path itself, this provides a minimal change to work with the existing storage code. --- config/module/storage.go | 134 +++++++++++++++++++++++++++++++++++++ config/module/tree.go | 68 +------------------ config/module/tree_test.go | 20 +++--- 3 files changed, 146 insertions(+), 76 deletions(-) create mode 100644 config/module/storage.go diff --git a/config/module/storage.go b/config/module/storage.go new file mode 100644 index 000000000..24c6079c4 --- /dev/null +++ b/config/module/storage.go @@ -0,0 +1,134 @@ +package module + +import ( + "encoding/json" + "io/ioutil" + "log" + "os" + "path/filepath" +) + +// moduleManifest is the serialization structure used to record the stored +// module's metadata. +type moduleManifest struct { + Modules []moduleRecord +} + +// moduleRecords represents the stored module's metadata. +// This is compared for equality using '==', so all fields needs to remain +// comparable. +type moduleRecord struct { + // Source is the module source string, minus any subdirectory. + // If it is sourced from a registry, it will include the hostname if it is + // supplied in configuration. + Source string + + // Version is the exact version string that is stored in this Key. + Version string + + // Dir is the directory name returned by the FileStorage. This is what + // allows us to correlate a particular module version with the location on + // disk. + Dir string + + // Root is the root directory containing the module. If the module is + // unpacked from an archive, and not located in the root directory, this is + // used to direct the loader to the correct subdirectory. This is + // independent from any subdirectory in the original source string, which + // may traverse further into the module tree. + Root string +} + +// Return the path to the manifest in parent of the storage directory dir. +func moduleManifestPath(dir string) string { + const filename = "modules.json" + // Get the parent directory. + // The current FolderStorage implementation needed to be able to create + // this directory, so we can be reasonably certain we can use it. + parent := filepath.Dir(filepath.Clean(dir)) + return filepath.Join(parent, filename) +} + +// loadManifest returns the moduleManifest file from the parent directory. +func loadManifest(dir string) (moduleManifest, error) { + manifest := moduleManifest{} + + manifestPath := moduleManifestPath(dir) + data, err := ioutil.ReadFile(manifestPath) + if err != nil && !os.IsNotExist(err) { + return manifest, err + } + + if len(data) == 0 { + return manifest, nil + } + + if err := json.Unmarshal(data, &manifest); err != nil { + return manifest, err + } + return manifest, nil +} + +// Store the location of the module, along with the version used and the module +// root directory. The storage method loads the entire file and rewrites it +// each time. This is only done a few times during init, so efficiency is +// not a concern. +func recordModule(dir string, m moduleRecord) error { + manifest, err := loadManifest(dir) + if err != nil { + // if there was a problem with the file, we will attempt to write a new + // one. Any non-data related error should surface there. + log.Printf("[WARN] error reading module manifest from %q: %s", dir, err) + } + + // do nothing if we already have the exact module + for i, stored := range manifest.Modules { + if m == stored { + return nil + } + + // they are not equal, but if the storage path is the same we need to + // remove this record to be replaced. + if m.Dir == stored.Dir { + manifest.Modules[i] = manifest.Modules[len(manifest.Modules)-1] + manifest.Modules = manifest.Modules[:len(manifest.Modules)-1] + break + } + } + + manifest.Modules = append(manifest.Modules, m) + + js, err := json.Marshal(manifest) + if err != nil { + panic(err) + } + + manifestPath := moduleManifestPath(dir) + return ioutil.WriteFile(manifestPath, js, 0644) +} + +// return only the root directory of the module stored in dir. +func getModuleRoot(dir string) (string, error) { + manifest, err := loadManifest(dir) + if err != nil { + return "", err + } + + for _, m := range manifest.Modules { + if m.Dir == dir { + return m.Root, nil + } + } + return "", nil +} + +// record only the Root directory for the module stored at dir. +// TODO: remove this compatibility function to store the full moduleRecord. +func recordModuleRoot(dir, root string) error { + m := moduleRecord{ + Dir: dir, + Root: root, + } + + return recordModule(dir, m) +} diff --git a/config/module/tree.go b/config/module/tree.go index 03c2a1621..aed75cb8f 100644 --- a/config/module/tree.go +++ b/config/module/tree.go @@ -3,11 +3,8 @@ package module import ( "bufio" "bytes" - "encoding/json" "fmt" - "io/ioutil" "log" - "os" "path/filepath" "strings" "sync" @@ -188,7 +185,7 @@ func (t *Tree) Load(s getter.Storage, mode GetMode) error { // The key is the string that will be hashed to uniquely id the Source. // The leading digit can be incremented to force re-fetch all existing // modules. - key := fmt.Sprintf("0.root.%s-%s", strings.Join(path, "."), m.Source) + key := fmt.Sprintf("1.root.%s-%s", strings.Join(path, "."), m.Source) log.Printf("[TRACE] module source: %q", m.Source) // Split out the subdir if we have one. @@ -209,7 +206,7 @@ func (t *Tree) Load(s getter.Storage, mode GetMode) error { // In order to load the Tree we need to find out if there was another // subDir stored from discovery. if found && mode != GetModeUpdate { - subDir, err := t.getSubdir(dir) + subDir, err := getModuleRoot(dir) if err != nil { // If there's a problem with the subdir record, we'll let the // recordSubdir method fix it up. Any other errors filesystem @@ -271,7 +268,7 @@ func (t *Tree) Load(s getter.Storage, mode GetMode) error { subDir = fullDir[len(dir)+1:] - if err := t.recordSubdir(dir, subDir); err != nil { + if err := recordModuleRoot(dir, subDir); err != nil { return err } dir = fullDir @@ -435,65 +432,6 @@ func (t *Tree) inheritProviderConfigs(stack []*Tree) { } -func subdirRecordsPath(dir string) string { - const filename = "module-subdir.json" - // Get the parent directory. - // The current FolderStorage implementation needed to be able to create - // this directory, so we can be reasonably certain we can use it. - parent := filepath.Dir(filepath.Clean(dir)) - return filepath.Join(parent, filename) -} - -// unmarshal the records file in the parent directory. Always returns a valid map. -func loadSubdirRecords(dir string) (map[string]string, error) { - records := map[string]string{} - - recordsPath := subdirRecordsPath(dir) - data, err := ioutil.ReadFile(recordsPath) - if err != nil && !os.IsNotExist(err) { - return records, err - } - - if len(data) == 0 { - return records, nil - } - - if err := json.Unmarshal(data, &records); err != nil { - return records, err - } - return records, nil -} - -func (t *Tree) getSubdir(dir string) (string, error) { - records, err := loadSubdirRecords(dir) - if err != nil { - return "", err - } - - return records[dir], nil -} - -// Mark the location of a detected subdir in a top-level file so we -// can skip detection when not updating the module. -func (t *Tree) recordSubdir(dir, subdir string) error { - records, err := loadSubdirRecords(dir) - if err != nil { - // if there was a problem with the file, we will attempt to write a new - // one. Any non-data related error should surface there. - log.Printf("[WARN] error reading subdir records: %s", err) - } - - records[dir] = subdir - - js, err := json.Marshal(records) - if err != nil { - return err - } - - recordsPath := subdirRecordsPath(dir) - return ioutil.WriteFile(recordsPath, js, 0644) -} - // Path is the full path to this tree. func (t *Tree) Path() []string { return t.path diff --git a/config/module/tree_test.go b/config/module/tree_test.go index ae95dbcbb..de7b23335 100644 --- a/config/module/tree_test.go +++ b/config/module/tree_test.go @@ -264,7 +264,7 @@ func TestTreeLoad_subdir(t *testing.T) { } } -func TestTree_recordSubDir(t *testing.T) { +func TestTree_recordManifest(t *testing.T) { td, err := ioutil.TempDir("", "tf-module") if err != nil { t.Fatal(err) @@ -275,13 +275,11 @@ func TestTree_recordSubDir(t *testing.T) { subDir := "subDirName" - tree := Tree{} - // record and read the subdir path - if err := tree.recordSubdir(dir, subDir); err != nil { + if err := recordModuleRoot(dir, subDir); err != nil { t.Fatal(err) } - actual, err := tree.getSubdir(dir) + actual, err := getModuleRoot(dir) if err != nil { t.Fatal(err) } @@ -292,10 +290,10 @@ func TestTree_recordSubDir(t *testing.T) { // overwrite the path, and nmake sure we get the new one subDir = "newSubDir" - if err := tree.recordSubdir(dir, subDir); err != nil { + if err := recordModuleRoot(dir, subDir); err != nil { t.Fatal(err) } - actual, err = tree.getSubdir(dir) + actual, err = getModuleRoot(dir) if err != nil { t.Fatal(err) } @@ -305,21 +303,21 @@ func TestTree_recordSubDir(t *testing.T) { } // create a fake entry - if err := ioutil.WriteFile(subdirRecordsPath(dir), []byte("BAD DATA"), 0644); err != nil { + if err := ioutil.WriteFile(moduleManifestPath(dir), []byte("BAD DATA"), 0644); err != nil { t.Fatal(err) } // this should fail because there aare now 2 entries - actual, err = tree.getSubdir(dir) + actual, err = getModuleRoot(dir) if err == nil { t.Fatal("expected multiple subdir entries") } // writing the subdir entry should remove the incorrect value - if err := tree.recordSubdir(dir, subDir); err != nil { + if err := recordModuleRoot(dir, subDir); err != nil { t.Fatal(err) } - actual, err = tree.getSubdir(dir) + actual, err = getModuleRoot(dir) if err != nil { t.Fatal(err) }