From 01ea12b4fb1db1b59b0fb183ce0ff1b82abbc9a4 Mon Sep 17 00:00:00 2001 From: Sean Sullivan Date: Wed, 14 Feb 2018 14:52:33 -0800 Subject: [PATCH] Checkpoint including new Loader class and minor fixes. --- pkg/kinflate/commands/inflate.go | 4 +- pkg/kinflate/tree/builder.go | 71 +++++++++++++++-------------- pkg/kinflate/tree/builder_test.go | 14 ++++-- pkg/kinflate/util/manifestloader.go | 10 ++-- 4 files changed, 54 insertions(+), 45 deletions(-) diff --git a/pkg/kinflate/commands/inflate.go b/pkg/kinflate/commands/inflate.go index 3fb482526..e50bcf4d5 100644 --- a/pkg/kinflate/commands/inflate.go +++ b/pkg/kinflate/commands/inflate.go @@ -83,8 +83,8 @@ func (o *inflateOptions) Complete(cmd *cobra.Command, args []string) error { // RunKinflate runs inflate command (do real work). func (o *inflateOptions) RunKinflate(out, errOut io.Writer) error { // Build a tree of ManifestData. - loader := tree.Loader{FS: fs.MakeRealFS(), Path: o.manifestPath} - root, err := loader.LoadManifestDataFromPath(o.manifestPath) + loader := tree.Loader{FS: fs.MakeRealFS(), InitialPath: o.manifestPath} + root, err := loader.LoadManifestDataFromPath() if err != nil { return err } diff --git a/pkg/kinflate/tree/builder.go b/pkg/kinflate/tree/builder.go index 970433be1..88ec80918 100644 --- a/pkg/kinflate/tree/builder.go +++ b/pkg/kinflate/tree/builder.go @@ -29,25 +29,43 @@ import ( ) type Loader struct { - FS fs.FileSystem - Path string + // Allows unit tests with fake filesystem. + FS fs.FileSystem + // Unexpanded manifest directory or manifest filename. + // Examples: "." or "sean-manifest.yaml" + InitialPath string + // Full expanded manifest file path. + // Examples: "/usr/local/Kube-manifest.yaml" or "/home/seans/project/sean-manifest.yaml" + FullFilePath string } // LoadManifestDataFromPath takes a path to a Kube-manifest.yaml or a dir that has a Kube-manifest.yaml. // It returns a tree of ManifestData. -func (l *Loader) LoadManifestDataFromPath(path string) (*ManifestData, error) { - return l.loadManifestDataFromPath(path) -} - -// loadManifestDataFromPath make a ManifestData from path -func (l *Loader) loadManifestDataFromPath(path string) (*ManifestData, error) { - m, err := l.loadManifestFileFromPath(path) +func (l *Loader) LoadManifestDataFromPath() (*ManifestData, error) { + m, err := l.loadManifestFileFromPath() if err != nil { return nil, err } return l.manifestToManifestData(m) } +// loadManifestFileFromPath loads a manifest object from file. +func (l *Loader) loadManifestFileFromPath() (*manifest.Manifest, error) { + mLoader := kutil.ManifestLoader{FS: l.FS} + // Expand the initial directory or file path into the full manifest file path. + fullFilepath, err := mLoader.MakeValidManifestPath(l.InitialPath) + if err != nil { + return nil, err + } + l.FullFilePath = fullFilepath + m, err := mLoader.Read(fullFilepath) + if err != nil { + return nil, err + } + mLoader.Validate(m) + return m, err +} + // manifestToManifestData make a ManifestData given an Manifest object func (l *Loader) manifestToManifestData(m *manifest.Manifest) (*ManifestData, error) { mdata, err := l.loadManifestDataFromManifestFileAndResources(m) @@ -57,7 +75,8 @@ func (l *Loader) manifestToManifestData(m *manifest.Manifest) (*ManifestData, er pkgs := []*ManifestData{} for _, pkg := range m.Packages { - pkgNode, err := l.LoadManifestDataFromPath(pkg) + loader := &Loader{FS: l.FS, InitialPath: pkg} + pkgNode, err := loader.LoadManifestDataFromPath() if err != nil { return nil, err } @@ -67,22 +86,6 @@ func (l *Loader) manifestToManifestData(m *manifest.Manifest) (*ManifestData, er return mdata, nil } -// loadManifestFileFromPath loads a manifest object from file. -func (l *Loader) loadManifestFileFromPath(filename string) (*manifest.Manifest, error) { - var loader = kutil.ManifestLoader{FS: l.FS} - fullFilename, err := loader.MakeValidManifestPath(filename) - if err != nil { - return nil, err - } - l.Path = fullFilename - m, err := loader.Read(fullFilename) - if err != nil { - return nil, err - } - loader.Validate(m) - return m, err -} - func (l *Loader) loadManifestDataFromManifestFileAndResources(m *manifest.Manifest) (*ManifestData, error) { mdata := &ManifestData{} var err error @@ -91,15 +94,15 @@ func (l *Loader) loadManifestDataFromManifestFileAndResources(m *manifest.Manife mdata.ObjectLabels = m.ObjectLabels mdata.ObjectAnnotations = m.ObjectAnnotations - res, err := l.LoadKObjectFromPaths(m.Resources) + res, err := l.loadKObjectFromPaths(m.Resources) if err != nil { errorMsg := fmt.Sprintf("Resource from Manifest (%s) couldn't be loaded properly.\n%v\n"+ - "Please check the Resource subsection in (%s).", l.Path, err, l.Path) + "Please check the Resource subsection in (%s).", l.FullFilePath, err, l.FullFilePath) return nil, fmt.Errorf(errorMsg) } mdata.Resources = ResourcesType(res) - pat, err := l.LoadKObjectFromPaths(m.Patches) + pat, err := l.loadKObjectFromPaths(m.Patches) if err != nil { return nil, err } @@ -125,10 +128,10 @@ func (l *Loader) loadManifestDataFromManifestFileAndResources(m *manifest.Manife return mdata, nil } -func (l *Loader) LoadKObjectFromPaths(paths []string) (types.KObject, error) { +func (l *Loader) loadKObjectFromPaths(paths []string) (types.KObject, error) { res := types.KObject{} for _, path := range paths { - err := l.LoadKObjectFromPath(path, res) + err := l.loadKObjectFromPath(path, res) if err != nil { return nil, err } @@ -136,7 +139,7 @@ func (l *Loader) LoadKObjectFromPaths(paths []string) (types.KObject, error) { return res, nil } -func (l *Loader) LoadKObjectFromPath(path string, into types.KObject) error { +func (l *Loader) loadKObjectFromPath(path string, into types.KObject) error { _, err := l.FS.Stat(path) if err != nil { return err @@ -156,13 +159,13 @@ func (l *Loader) LoadKObjectFromPath(path string, into types.KObject) error { return nil } - err = l.LoadKObjectFromFile(filepath, into) + err = l.loadKObjectFromFile(filepath, into) return nil }) return e } -func (l *Loader) LoadKObjectFromFile(filename string, into types.KObject) error { +func (l *Loader) loadKObjectFromFile(filename string, into types.KObject) error { f, err := l.FS.Stat(filename) if err != nil { return err diff --git a/pkg/kinflate/tree/builder_test.go b/pkg/kinflate/tree/builder_test.go index f61ef5b81..c84e9d06d 100644 --- a/pkg/kinflate/tree/builder_test.go +++ b/pkg/kinflate/tree/builder_test.go @@ -130,11 +130,12 @@ func TestFileToMap(t *testing.T) { }, } + // TODO: Convert to a fake filesystem instead of using test files. loader := Loader{FS: fs.MakeRealFS()} for _, tc := range testcases { actual := types.KObject{} - err := loader.LoadKObjectFromFile(tc.filename, actual) + err := loader.loadKObjectFromFile(tc.filename, actual) if err == nil { if tc.expectErr { t.Errorf("filename: %q, expect an error containing %q, but didn't get an error", tc.filename, tc.errorStr) @@ -182,11 +183,12 @@ func TestPathToMap(t *testing.T) { }, } + // TODO: Convert to a fake filesystem instead of using test files. loader := Loader{FS: fs.MakeRealFS()} for _, tc := range testcases { actual := types.KObject{} - err := loader.LoadKObjectFromPath(tc.filename, actual) + err := loader.loadKObjectFromPath(tc.filename, actual) if err == nil { if tc.expectErr { t.Errorf("filename: %q, expect an error containing %q, but didn't get an error", tc.filename, tc.errorStr) @@ -245,10 +247,11 @@ func TestPathsToMap(t *testing.T) { }, } + // TODO: Convert to a fake filesystem instead of using test files. loader := Loader{FS: fs.MakeRealFS()} for _, tc := range testcases { - actual, err := loader.LoadKObjectFromPaths(tc.filenames) + actual, err := loader.loadKObjectFromPaths(tc.filenames) if err == nil { if tc.expectErr { t.Errorf("filenames: %q, expect an error containing %q, but didn't get an error", tc.filenames, tc.errorStr) @@ -308,6 +311,7 @@ func TestManifestToManifestData(t *testing.T) { Secrets: SecretsType(types.KObject{}), } + // TODO: Convert to a fake filesystem instead of using test files. loader := Loader{FS: fs.MakeRealFS()} actual, err := loader.loadManifestDataFromManifestFileAndResources(m) if err != nil { @@ -329,9 +333,9 @@ func TestLoadManifestDataFromPath(t *testing.T) { parent1.Packages = []*ManifestData{child1} parent2.Packages = []*ManifestData{child2} - loader := Loader{FS: fs.MakeRealFS()} + loader := Loader{FS: fs.MakeRealFS(), InitialPath: "testdata/hierarchy"} expected := grandparent - actual, err := loader.loadManifestDataFromPath("testdata/hierarchy") + actual, err := loader.LoadManifestDataFromPath() if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/pkg/kinflate/util/manifestloader.go b/pkg/kinflate/util/manifestloader.go index c2ba51873..d4bcaf2ee 100644 --- a/pkg/kinflate/util/manifestloader.go +++ b/pkg/kinflate/util/manifestloader.go @@ -33,9 +33,11 @@ type ManifestLoader struct { FS fs.FileSystem } -//This is a placeholder for the ManifestErrors type -//TODO: Think about this struct and make is better -type ManifestErrors []string +// First pass to encapsulate fields for more informative error messages. +type ManifestErrors struct { + filepath string + errorMsg string +} func (m *ManifestLoader) fs() fs.FileSystem { if m.FS == nil { @@ -99,7 +101,7 @@ func (m *ManifestLoader) Write(filename string, manifest *manifest.Manifest) err } // Read must have already been called and we have a loaded manifest -func (m *ManifestLoader) Validate(manifest *manifest.Manifest) ManifestErrors { +func (m *ManifestLoader) Validate(manifest *manifest.Manifest) []ManifestErrors { //TODO: implement this function //// validate Packages //merrors := m.validatePackages(manifest.Packages)