Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
pkg/metadata: bundle metadata utils and types #60
Conversation
… in a directory and its children
|
Relevant to this PR: because we're adding this function to the api lib, can we consider adding the bundle spec itself to this lib, specifically labels and some convenient access methods for them? |
| // readAnnotations reads annotations from file(s) in bundleRoot and returns them as a map. | ||
| func readAnnotations(fs afero.Fs, annotationsPath string) (map[string]string, error) { | ||
| // The annotations file is well-defined. | ||
| b, err := afero.ReadFile(fs, annotationsPath) |
kevinrizza
Aug 12, 2020
Member
Why not just use ioutil instead of adding an external dependency?
Why not just use ioutil instead of adding an external dependency?
estroz
Aug 12, 2020
Author
Member
It's much easier to test with afero since we don't have to write testdata to disk for each new case. Plus there's a Go design draft for a std file system interface, which we could replace this with in the future.
It's much easier to test with afero since we don't have to write testdata to disk for each new case. Plus there's a Go design draft for a std file system interface, which we could replace this with in the future.
|
|
||
| // Use the arbitrarily-indexed representation of the annotations file for forwards and backwards compatibility. | ||
| annotations := struct { | ||
| Annotations map[string]string `json:"annotations"` |
kevinrizza
Aug 12, 2020
Member
Why don't we use a more versioned structured type here? Just so that additional arbitrary annotations can be added? It seems like either way we need to validate that it has the set of required annotations to make the bundle work, so maybe that should be included here?
Why don't we use a more versioned structured type here? Just so that additional arbitrary annotations can be added? It seems like either way we need to validate that it has the set of required annotations to make the bundle work, so maybe that should be included here?
estroz
Aug 12, 2020
Author
Member
Exactly, so arbitrary annotations can be added. There may be a way to load both versioned and arbitrary annotations into some struct type, which I can experiment with in this PR, but I wasn't sure about doing that initially (see #60 (comment)).
Exactly, so arbitrary annotations can be added. There may be a way to load both versioned and arbitrary annotations into some struct type, which I can experiment with in this PR, but I wasn't sure about doing that initially (see #60 (comment)).
estroz
Aug 12, 2020
Author
Member
Done
Done
and return the full AnnotationsFile from FindAnnotations() pkg/manifests: move metadata types to pkg/metadata
| "strings" | ||
|
|
||
| log "github.com/sirupsen/logrus" | ||
| "github.com/spf13/afero" |
njhale
Aug 12, 2020
Member
nice find!
nice find!
|
/lgtm |
|
/lgtm |
There should be a canonical set of utilities and types that can find and parse an
annotations.yamlin a bundle directory instead of re-implementing that logic in various ways in various operator-framework repos. This repo is a good home for them because it is the source of truth for many OF APIs./cc @kevinrizza @dinhxuanvu
/kind feature