elastic / elastic/elastic-package

Review data structures and helpers to read source and built manifests

Open
#3,585 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
72
Forks
141
Avg merge
19h 42m
Merged PRs (30d)
55

Description

`elastic-package` has several helpers to read manifests, data streams, fields and so on. Some of them are specific to build or source packages, or to zipped packages. They sometimes make assumptions on their location without explicit checks or with different interfaces. And their assumptions on paths are sometimes even different. This is confusing and error prone. See for example https://github.com/elastic/elastic-package/pull/3576.

These methods have been added as they have been needed, without a clear architecture.

We should introduce a cleaner architecture that is orthogonal for zipped or not zipped packages, and for source and built packages. This new architecture should probably be based on `fs.FS`, and could maybe have different structs, or at least different methods to read built and source packages. And it should not make assumptions on the paths.

It could be based on a single object that is able to access all the relevant data from a package, something like this:
```go
type PackageKind int
const (
SourcePackage PackageKind = iota
BuiltPackage
)
type PackageRoot struct {
path string
kind PackageKind
fsys fs.FS
closer io.Closer
}
func OpenPackageRoot(path string, kind PackageKind) (*PackageRoot, error)
func OpenPackageRootFromZip(zipPath string) (*PackageRoot, error) // always BuiltPackage
func (r *PackageRoot) Kind() PackageKind
func (r *PackageRoot) Path() string
func (r *PackageRoot) FS() fs.FS
func (r *PackageRoot) Close() error
func (r *PackageRoot) Manifest() (*PackageManifest, error)
func (r *PackageRoot) AllDataStreamManifests() ([]DataStreamManifest, error)
func (r *PackageRoot) DataStreamManifest(name string) (*DataStreamManifest, error)
func (r *PackageRoot) Transforms() ([]Transform, error)
func (r *PackageRoot) Assets() ([]Asset, error)
```

Maybe this base functionality should be provided by the Package Spec. Related:
* https://github.com/elastic/package-spec/issues/322
* https://github.com/elastic/package-spec/issues/549

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.