From 3845e5d0c42132f124060318f83028ceb01227c3 Mon Sep 17 00:00:00 2001 From: Jack Shaw Date: Thu, 9 May 2024 11:46:37 +0100 Subject: [PATCH] Add methods to retrieve raw bundle bytes Add this BundleBytes method to Bundle interface and BundleDataSource In Juju, we have come across a need to run some extra parsing + verification which we do not want to do in juju/charm. We want to keep the data model clean in juju/charm. As such, we need to way to keep the raw bundle data around for later parsing Add this to BundleDataSource, as this is what we use in Juju However, we also implement our own BundleDataSource in Juju. built with the Bundle interface here. So we need to add this method to Bundle as well --- bundle.go | 2 ++ bundle_test.go | 18 ++++++++++----- bundlearchive.go | 22 +++++++++++++----- bundlearchive_test.go | 17 +++++++------- bundledata.go | 14 ++++++------ bundledatasrc.go | 52 +++++++++++++++++++++++++++---------------- bundledatasrc_test.go | 39 ++++++++++++++++++++------------ bundledir.go | 21 ++++++++++------- bundledir_test.go | 2 +- overlay_test.go | 7 ++++-- 10 files changed, 124 insertions(+), 70 deletions(-) diff --git a/bundle.go b/bundle.go index 91fb36ff..9f43c335 100644 --- a/bundle.go +++ b/bundle.go @@ -15,6 +15,8 @@ import ( type Bundle interface { // Data returns the contents of the bundle's bundle.yaml file. Data() *BundleData + // BundleBytes returns the raw bytes content of a bundle + BundleBytes() []byte // ReadMe returns the contents of the bundle's README.md file. ReadMe() string // ContainsOverlays returns true if the bundle contains any overlays. diff --git a/bundle_test.go b/bundle_test.go index 0180b959..e3d35dab 100644 --- a/bundle_test.go +++ b/bundle_test.go @@ -4,6 +4,9 @@ package charm_test import ( + "os" + "path/filepath" + "github.com/juju/testing" jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" @@ -23,7 +26,7 @@ func (*BundleSuite) TestReadBundleDir(c *gc.C) { c.Assert(err, gc.IsNil) c.Assert(b.ContainsOverlays(), jc.IsFalse) c.Assert(b, gc.FitsTypeOf, (*charm.BundleDir)(nil)) - checkWordpressBundle(c, b, path) + checkWordpressBundle(c, b, path, "wordpress-simple") } func (*BundleSuite) TestReadMultiDocBundleDir(c *gc.C) { @@ -32,7 +35,7 @@ func (*BundleSuite) TestReadMultiDocBundleDir(c *gc.C) { c.Assert(err, gc.IsNil) c.Assert(b.ContainsOverlays(), jc.IsTrue) c.Assert(b, gc.FitsTypeOf, (*charm.BundleDir)(nil)) - checkWordpressBundle(c, b, path) + checkWordpressBundle(c, b, path, "wordpress-simple-multidoc") } func (*BundleSuite) TestReadBundleArchive(c *gc.C) { @@ -41,7 +44,7 @@ func (*BundleSuite) TestReadBundleArchive(c *gc.C) { c.Assert(err, gc.IsNil) c.Assert(b.ContainsOverlays(), jc.IsFalse) c.Assert(b, gc.FitsTypeOf, (*charm.BundleDir)(nil)) - checkWordpressBundle(c, b, path) + checkWordpressBundle(c, b, path, "wordpress-simple") } func (*BundleSuite) TestReadMultiDocBundleArchive(c *gc.C) { @@ -50,10 +53,10 @@ func (*BundleSuite) TestReadMultiDocBundleArchive(c *gc.C) { c.Assert(err, gc.IsNil) c.Assert(b.ContainsOverlays(), jc.IsTrue) c.Assert(b, gc.FitsTypeOf, (*charm.BundleDir)(nil)) - checkWordpressBundle(c, b, path) + checkWordpressBundle(c, b, path, "wordpress-simple-multidoc") } -func checkWordpressBundle(c *gc.C, b charm.Bundle, path string) { +func checkWordpressBundle(c *gc.C, b charm.Bundle, path string, bundleName string) { // Load the charms required by the bundle. wordpressCharm := readCharmDir(c, "wordpress") mysqlCharm := readCharmDir(c, "mysql") @@ -87,6 +90,11 @@ func checkWordpressBundle(c *gc.C, b charm.Bundle, path string) { case *charm.BundleDir: c.Assert(b.Path, gc.Equals, path) } + + bundlePath := filepath.Join("internal/test-charm-repo/bundle", bundleName, "bundle.yaml") + raw, err := os.ReadFile(bundlePath) + c.Assert(err, jc.ErrorIsNil) + c.Assert(string(b.BundleBytes()), gc.Equals, string(raw)) } func verifyOk(string) error { diff --git a/bundlearchive.go b/bundlearchive.go index 4bb2eea9..4a2feda7 100644 --- a/bundlearchive.go +++ b/bundlearchive.go @@ -6,7 +6,6 @@ package charm import ( "bytes" "io" - "io/ioutil" ziputil "github.com/juju/utils/v4/zip" ) @@ -14,9 +13,10 @@ import ( type BundleArchive struct { zopen zipOpener - Path string - data *BundleData - readMe string + Path string + data *BundleData + bundleBytes []byte + readMe string containsOverlays bool } @@ -61,7 +61,12 @@ func readBundleArchive(zopen zipOpener) (*BundleArchive, error) { if err != nil { return nil, err } - a.data, a.containsOverlays, err = readBaseFromMultidocBundle(reader) + b, err := io.ReadAll(reader) + if err != nil { + return nil, err + } + a.bundleBytes = b + a.data, a.containsOverlays, err = readBaseFromMultidocBundle(b) reader.Close() if err != nil { return nil, err @@ -70,7 +75,7 @@ func readBundleArchive(zopen zipOpener) (*BundleArchive, error) { if err != nil { return nil, err } - readMe, err := ioutil.ReadAll(reader) + readMe, err := io.ReadAll(reader) if err != nil { return nil, err } @@ -83,6 +88,11 @@ func (a *BundleArchive) Data() *BundleData { return a.data } +// BundleBytes implements Bundle.BundleBytes. +func (a *BundleArchive) BundleBytes() []byte { + return a.bundleBytes +} + // ReadMe implements Bundle.ReadMe. func (a *BundleArchive) ReadMe() string { return a.readMe diff --git a/bundlearchive_test.go b/bundlearchive_test.go index e6782769..f6d63e23 100644 --- a/bundlearchive_test.go +++ b/bundlearchive_test.go @@ -5,7 +5,6 @@ package charm_test import ( "fmt" - "io/ioutil" "os" "path/filepath" @@ -21,35 +20,37 @@ type BundleArchiveSuite struct { archivePath string } +const bundleName = "wordpress-simple" + func (s *BundleArchiveSuite) SetUpSuite(c *gc.C) { - s.archivePath = archivePath(c, readBundleDir(c, "wordpress-simple")) + s.archivePath = archivePath(c, readBundleDir(c, bundleName)) } func (s *BundleArchiveSuite) TestReadBundleArchive(c *gc.C) { archive, err := charm.ReadBundleArchive(s.archivePath) c.Assert(err, gc.IsNil) - checkWordpressBundle(c, archive, s.archivePath) + checkWordpressBundle(c, archive, s.archivePath, bundleName) } func (s *BundleArchiveSuite) TestReadBundleArchiveBytes(c *gc.C) { - data, err := ioutil.ReadFile(s.archivePath) + data, err := os.ReadFile(s.archivePath) c.Assert(err, gc.IsNil) archive, err := charm.ReadBundleArchiveBytes(data) c.Assert(err, gc.IsNil) c.Assert(archive.ContainsOverlays(), jc.IsFalse) - checkWordpressBundle(c, archive, "") + checkWordpressBundle(c, archive, "", bundleName) } func (s *BundleArchiveSuite) TestReadMultiDocBundleArchiveBytes(c *gc.C) { path := archivePath(c, readBundleDir(c, "wordpress-simple-multidoc")) - data, err := ioutil.ReadFile(path) + data, err := os.ReadFile(path) c.Assert(err, gc.IsNil) archive, err := charm.ReadBundleArchiveBytes(data) c.Assert(err, gc.IsNil) c.Assert(archive.ContainsOverlays(), jc.IsTrue) - checkWordpressBundle(c, archive, "") + checkWordpressBundle(c, archive, "", "wordpress-simple-multidoc") } func (s *BundleArchiveSuite) TestReadBundleArchiveFromReader(c *gc.C) { @@ -61,7 +62,7 @@ func (s *BundleArchiveSuite) TestReadBundleArchiveFromReader(c *gc.C) { archive, err := charm.ReadBundleArchiveFromReader(f, info.Size()) c.Assert(err, gc.IsNil) - checkWordpressBundle(c, archive, "") + checkWordpressBundle(c, archive, "", bundleName) } func (s *BundleArchiveSuite) TestReadBundleArchiveWithoutBundleYAML(c *gc.C) { diff --git a/bundledata.go b/bundledata.go index ebf3cb4a..89d28a3b 100644 --- a/bundledata.go +++ b/bundledata.go @@ -49,12 +49,10 @@ type BundleData struct { // Series holds the default series to use when // the bundle deploys applications. A series defined for an application // takes precedence. - // Series and Base cannot be mixed. Series string `bson:",omitempty" json:",omitempty" yaml:",omitempty"` // Base holds the default base to use when the bundle deploys // applications. A base defined for an application takes precedence. - // Series and Base cannot be mixed. DefaultBase string `bson:"default-base,omitempty" json:"default-base,omitempty" yaml:"default-base,omitempty"` // Relations holds a slice of 2-element slices, @@ -104,11 +102,9 @@ type ApplicationSpec struct { Revision *int `bson:"revision,omitempty" yaml:"revision,omitempty" json:"revision,omitempty"` // Series is the series to use when deploying the application. - // Series and Base cannot be mixed. Series string `bson:",omitempty" yaml:",omitempty" json:",omitempty"` // Base is the base to use when deploying the application. - // Series and Base cannot be mixed. Base string `bson:",omitempty" yaml:",omitempty" json:",omitempty"` // Resources is the set of resource revisions to deploy for the @@ -355,7 +351,11 @@ type OfferSpec struct { // The returned data is not verified - call Verify to ensure // that it is OK. func ReadBundleData(r io.Reader) (*BundleData, error) { - bd, _, err := readBaseFromMultidocBundle(r) + b, err := io.ReadAll(r) + if err != nil { + return nil, err + } + bd, _, err := readBaseFromMultidocBundle(b) if err != nil { return nil, err } @@ -370,8 +370,8 @@ func ReadBundleData(r io.Reader) (*BundleData, error) { // // Clients that are interested in reading multi-doc bundle data should use the // new helpers: LocalBundleDataSource and StreamBundleDataSource. -func readBaseFromMultidocBundle(r io.Reader) (*BundleData, bool, error) { - parts, err := parseBundleParts(r) +func readBaseFromMultidocBundle(b []byte) (*BundleData, bool, error) { + parts, err := parseBundleParts(b) if err != nil { return nil, false, err } diff --git a/bundledatasrc.go b/bundledatasrc.go index a7b97026..2440613b 100644 --- a/bundledatasrc.go +++ b/bundledatasrc.go @@ -6,7 +6,6 @@ package charm import ( "bytes" "io" - "io/ioutil" "os" "path/filepath" "strings" @@ -55,19 +54,25 @@ type BundleDataPart struct { // list of composable parts. type BundleDataSource interface { Parts() []*BundleDataPart + BundleBytes() []byte BasePath() string ResolveInclude(path string) ([]byte, error) } type resolvedBundleDataSource struct { - basePath string - parts []*BundleDataPart + basePath string + bundleBytes []byte + parts []*BundleDataPart } func (s *resolvedBundleDataSource) Parts() []*BundleDataPart { return s.parts } +func (s *resolvedBundleDataSource) BundleBytes() []byte { + return s.bundleBytes +} + func (s *resolvedBundleDataSource) BasePath() string { return s.basePath } @@ -95,7 +100,7 @@ func (s *resolvedBundleDataSource) ResolveInclude(path string) ([]byte, error) { return nil, errors.Errorf("include path %q resolves to a folder", absPath) } - data, err := ioutil.ReadFile(absPath) + data, err := os.ReadFile(absPath) if err != nil { return nil, errors.Annotatef(err, "reading include file at %q", absPath) } @@ -131,15 +136,20 @@ func LocalBundleDataSource(path string) (BundleDataSource, error) { } defer func() { _ = f.Close() }() - parts, pErr := parseBundleParts(f) + b, err := io.ReadAll(f) + if err != nil { + return nil, err + } + parts, pErr := parseBundleParts(b) if pErr == nil { absPath, err := filepath.Abs(path) if err != nil { return nil, errors.Annotatef(err, "resolve absolute path to %s", path) } return &resolvedBundleDataSource{ - basePath: filepath.Dir(absPath), - parts: parts, + basePath: filepath.Dir(absPath), + parts: parts, + bundleBytes: b, }, nil } @@ -159,10 +169,15 @@ func LocalBundleDataSource(path string) (BundleDataSource, error) { } defer func() { _ = r.Close() }() - if parts, pErr = parseBundleParts(r); pErr == nil { + b, err = io.ReadAll(r) + if err != nil { + return nil, err + } + if parts, pErr = parseBundleParts(b); pErr == nil { return &resolvedBundleDataSource{ - basePath: "", // use empty base path for archives - parts: parts, + basePath: "", // use empty base path for archives + parts: parts, + bundleBytes: b, }, nil } @@ -185,20 +200,19 @@ func isNotExistsError(err error) bool { // StreamBundleDataSource reads a (potentially multi-part) bundle from r and // returns a BundleDataSource for it. func StreamBundleDataSource(r io.Reader, basePath string) (BundleDataSource, error) { - parts, err := parseBundleParts(r) + b, err := io.ReadAll(r) + if err != nil { + return nil, err + } + parts, err := parseBundleParts(b) if err != nil { return nil, errors.NotValidf("cannot unmarshal bundle contents: %v", err) } - return &resolvedBundleDataSource{parts: parts, basePath: basePath}, nil + return &resolvedBundleDataSource{parts: parts, bundleBytes: b, basePath: basePath}, nil } -func parseBundleParts(r io.Reader) ([]*BundleDataPart, error) { - b, err := ioutil.ReadAll(r) - if err != nil { - return nil, err - } - +func parseBundleParts(b []byte) ([]*BundleDataPart, error) { var ( // Ideally, we would be using a single reader and we would // rewind it to read each block in structured and raw mode. @@ -216,7 +230,7 @@ func parseBundleParts(r io.Reader) ([]*BundleDataPart, error) { for docIdx := 0; ; docIdx++ { var part BundleDataPart - err = structDec.Decode(&part.Data) + err := structDec.Decode(&part.Data) if err == io.EOF { break } else if err != nil && !strings.HasPrefix(err.Error(), "yaml: unmarshal errors:") { diff --git a/bundledatasrc_test.go b/bundledatasrc_test.go index f72f8cbb..5f887b87 100644 --- a/bundledatasrc_test.go +++ b/bundledatasrc_test.go @@ -6,7 +6,6 @@ package charm import ( "archive/zip" "fmt" - "io/ioutil" "os" "path/filepath" "strings" @@ -22,29 +21,40 @@ type BundleDataSourceSuite struct { var _ = gc.Suite(&BundleDataSourceSuite{}) +var bundlePath = "internal/test-charm-repo/bundle/wordpress-multidoc/bundle.yaml" + func (s *BundleDataSourceSuite) TestReadBundleFromLocalFile(c *gc.C) { path := bundleDirPath(c, "wordpress-multidoc") src, err := LocalBundleDataSource(filepath.Join(path, "bundle.yaml")) c.Assert(err, gc.IsNil) - assertBundleSourceProcessed(c, src) + + raw, err := os.ReadFile(bundlePath) + c.Assert(err, jc.ErrorIsNil) + assertBundleSourceProcessed(c, src, string(raw)) } func (s *BundleDataSourceSuite) TestReadBundleFromExplodedArchiveFolder(c *gc.C) { path := bundleDirPath(c, "wordpress-multidoc") src, err := LocalBundleDataSource(path) c.Assert(err, gc.IsNil) - assertBundleSourceProcessed(c, src) + + raw, err := os.ReadFile(bundlePath) + c.Assert(err, jc.ErrorIsNil) + assertBundleSourceProcessed(c, src, string(raw)) } func (s *BundleDataSourceSuite) TestReadBundleFromArchive(c *gc.C) { path := archiveBundleDirPath(c, "wordpress-multidoc") src, err := LocalBundleDataSource(path) c.Assert(err, gc.IsNil) - assertBundleSourceProcessed(c, src) + + raw, err := os.ReadFile(bundlePath) + c.Assert(err, jc.ErrorIsNil) + assertBundleSourceProcessed(c, src, string(raw)) } func (s *BundleDataSourceSuite) TestReadBundleFromStream(c *gc.C) { - r := strings.NewReader(` + bundle := ` applications: wordpress: charm: wordpress @@ -68,18 +78,19 @@ applications: acl: admin: "admin" foo: "consume" -`) +` - src, err := StreamBundleDataSource(r, "https://example.com") + src, err := StreamBundleDataSource(strings.NewReader(bundle), "https://example.com") c.Assert(err, gc.IsNil) - assertBundleSourceProcessed(c, src) + assertBundleSourceProcessed(c, src, bundle) } -func assertBundleSourceProcessed(c *gc.C, src BundleDataSource) { +func assertBundleSourceProcessed(c *gc.C, src BundleDataSource, bundle string) { parts := src.Parts() c.Assert(parts, gc.HasLen, 3) assertFieldPresent(c, parts[1], "applications.wordpress.offers.offer1.endpoints") assertFieldPresent(c, parts[2], "applications.wordpress.offers.offer1.acl.admin") + c.Assert(string(src.BundleBytes()), gc.Equals, bundle) } func assertFieldPresent(c *gc.C, part *BundleDataPart, path string) { @@ -101,7 +112,7 @@ func assertFieldPresent(c *gc.C, part *BundleDataPart, path string) { } func (s *BundleDataSourceSuite) TestParseBundlePartsStrict(c *gc.C) { - r := strings.NewReader(` + b := []byte(` applications: wordpress: charm: wordpress @@ -128,7 +139,7 @@ applications: foo: "consume" `) - parts, err := parseBundleParts(r) + parts, err := parseBundleParts(b) c.Assert(err, gc.IsNil) c.Assert(parts, gc.HasLen, 3) c.Assert(parts[0].UnmarshallError, gc.NotNil) @@ -148,7 +159,7 @@ func (s *BundleDataSourceSuite) TestResolveAbsoluteFileInclude(c *gc.C) { c.Assert(err, gc.IsNil) expContent := "example content\n" - c.Assert(ioutil.WriteFile(target, []byte(expContent), os.ModePerm), gc.IsNil) + c.Assert(os.WriteFile(target, []byte(expContent), os.ModePerm), gc.IsNil) ds := new(resolvedBundleDataSource) @@ -163,7 +174,7 @@ func (s *BundleDataSourceSuite) TestResolveRelativeFileInclude(c *gc.C) { c.Assert(err, gc.IsNil) expContent := "example content\n" - c.Assert(ioutil.WriteFile(target, []byte(expContent), os.ModePerm), gc.IsNil) + c.Assert(os.WriteFile(target, []byte(expContent), os.ModePerm), gc.IsNil) ds := &resolvedBundleDataSource{ basePath: relTo, @@ -226,7 +237,7 @@ func assertIsDir(c *gc.C, path string) { func archiveBundleDirPath(c *gc.C, name string) string { src := filepath.Join("internal/test-charm-repo/bundle", name, "bundle.yaml") - srcYaml, err := ioutil.ReadFile(src) + srcYaml, err := os.ReadFile(src) c.Assert(err, gc.IsNil) dstPath := filepath.Join(c.MkDir(), "bundle.zip") diff --git a/bundledir.go b/bundledir.go index 10c59724..41d2f9b9 100644 --- a/bundledir.go +++ b/bundledir.go @@ -6,16 +6,16 @@ package charm import ( "fmt" "io" - "io/ioutil" "os" "path/filepath" ) // BundleDir defines a bundle from a given directory. type BundleDir struct { - Path string - data *BundleData - readMe string + Path string + data *BundleData + bundleBytes []byte + readMe string containsOverlays bool } @@ -27,16 +27,16 @@ var _ Bundle = (*BundleDir)(nil) // bundle directory. It does not verify the bundle data. func ReadBundleDir(path string) (dir *BundleDir, err error) { dir = &BundleDir{Path: path} - file, err := os.Open(dir.join("bundle.yaml")) + b, err := os.ReadFile(dir.join("bundle.yaml")) if err != nil { return nil, err } - dir.data, dir.containsOverlays, err = readBaseFromMultidocBundle(file) - file.Close() + dir.bundleBytes = b + dir.data, dir.containsOverlays, err = readBaseFromMultidocBundle(b) if err != nil { return nil, err } - readMe, err := ioutil.ReadFile(dir.join("README.md")) + readMe, err := os.ReadFile(dir.join("README.md")) if err != nil { return nil, fmt.Errorf("cannot read README file: %v", err) } @@ -49,6 +49,11 @@ func (dir *BundleDir) Data() *BundleData { return dir.data } +// BundleBytes implements Bundle.BundleBytes +func (dir *BundleDir) BundleBytes() []byte { + return dir.bundleBytes +} + // ReadMe returns the contents of the bundle's README.md file. func (dir *BundleDir) ReadMe() string { return dir.readMe diff --git a/bundledir_test.go b/bundledir_test.go index 6bb4d70d..882eca4b 100644 --- a/bundledir_test.go +++ b/bundledir_test.go @@ -23,7 +23,7 @@ func (s *BundleDirSuite) TestReadBundleDir(c *gc.C) { path := bundleDirPath(c, "wordpress-simple") dir, err := charm.ReadBundleDir(path) c.Assert(err, gc.IsNil) - checkWordpressBundle(c, dir, path) + checkWordpressBundle(c, dir, path, "wordpress-simple") } func (s *BundleDirSuite) TestReadBundleDirWithoutREADME(c *gc.C) { diff --git a/overlay_test.go b/overlay_test.go index fb607cfb..67d079a8 100644 --- a/overlay_test.go +++ b/overlay_test.go @@ -4,7 +4,6 @@ package charm_test import ( - "io/ioutil" "os" "path/filepath" "sort" @@ -954,6 +953,10 @@ func (s srcWithFakeIncludeResolver) Parts() []*charm.BundleDataPart { return s.src.Parts() } +func (s srcWithFakeIncludeResolver) BundleBytes() []byte { + return s.src.BundleBytes() +} + func (s srcWithFakeIncludeResolver) BasePath() string { return s.src.BasePath() } @@ -987,7 +990,7 @@ func testBundleMergeResult(c *gc.C, src, exp string) { } func mustWriteFile(c *gc.C, path, content string) { - err := ioutil.WriteFile(path, []byte(content), os.ModePerm) + err := os.WriteFile(path, []byte(content), os.ModePerm) c.Assert(err, gc.IsNil) }