Skip to content

Commit

Permalink
Merge pull request #427 from jack-w-shaw/JUJU-5926_add_method_to_retr…
Browse files Browse the repository at this point in the history
…eive_raw_bundle

#427

Add this RawBundle 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

You can see a draft implementation for this change here:
juju/juju#17350

As a flyby, delete some incorrect comments

## QA Steps

Compile into juju form the following PR and run it's QA steps:
juju/juju#17350
  • Loading branch information
jujubot authored May 21, 2024
2 parents 9ea5816 + 3845e5d commit 8920fe1
Show file tree
Hide file tree
Showing 10 changed files with 124 additions and 70 deletions.
2 changes: 2 additions & 0 deletions bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
18 changes: 13 additions & 5 deletions bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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")
Expand Down Expand Up @@ -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 {
Expand Down
22 changes: 16 additions & 6 deletions bundlearchive.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ package charm
import (
"bytes"
"io"
"io/ioutil"

ziputil "github.com/juju/utils/v4/zip"
)

type BundleArchive struct {
zopen zipOpener

Path string
data *BundleData
readMe string
Path string
data *BundleData
bundleBytes []byte
readMe string

containsOverlays bool
}
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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
Expand Down
17 changes: 9 additions & 8 deletions bundlearchive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package charm_test

import (
"fmt"
"io/ioutil"
"os"
"path/filepath"

Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
14 changes: 7 additions & 7 deletions bundledata.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
52 changes: 33 additions & 19 deletions bundledatasrc.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package charm
import (
"bytes"
"io"
"io/ioutil"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}

Expand All @@ -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
}

Expand All @@ -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.
Expand All @@ -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:") {
Expand Down
Loading

0 comments on commit 8920fe1

Please sign in to comment.