-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
internal/config: include remote configuration files #96
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,7 +59,7 @@ var ( | |
|
||
// Config represents a Lava configuration. | ||
type Config struct { | ||
// Include is the list of the configuration files included. | ||
// Includes is the list of included configuration files. | ||
Includes []string `yaml:"include"` | ||
|
||
// LavaVersion is the minimum required version of Lava. | ||
|
@@ -86,12 +86,33 @@ type Config struct { | |
var reEnv = regexp.MustCompile(`\$\{[a-zA-Z_][a-zA-Z_0-9]*\}`) | ||
|
||
// Parse returns a parsed Lava configuration given an [io.Reader]. | ||
func Parse(r io.Reader) (Config, error) { | ||
func Parse(r io.Reader, path string) (Config, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't like this signature at all. If we always pass a file path, and given that this is an internal API, let's have only func Parse(path string) (Config, error) {
...
} It also removes the problem of having an empty path when generating the DAG. We can even avoid the first decoding and let |
||
b, err := io.ReadAll(r) | ||
if err != nil { | ||
return Config{}, fmt.Errorf("read config: %w", err) | ||
} | ||
|
||
cfg, err := Decode(b) | ||
if err != nil { | ||
return Config{}, fmt.Errorf("decode config: %w", err) | ||
} | ||
dag, err := NewDAG(cfg, path) | ||
if err != nil { | ||
return Config{}, fmt.Errorf("build dag: %w", err) | ||
} | ||
if err = dag.Resolve(); err != nil { | ||
return Config{}, fmt.Errorf("resolve dag: %w", err) | ||
} | ||
|
||
if err = cfg.validate(); err != nil { | ||
return Config{}, fmt.Errorf("validate config: %w", err) | ||
} | ||
|
||
return cfg, nil | ||
} | ||
|
||
// Decode decodes from a slice of bytes to a [Config] structure. | ||
func Decode(b []byte) (Config, error) { | ||
s := reEnv.ReplaceAllStringFunc(string(b), func(match string) string { | ||
return os.Getenv(match[2 : len(match)-1]) | ||
}) | ||
|
@@ -106,23 +127,18 @@ func Parse(r io.Reader) (Config, error) { | |
if err := dec.Decode(&cfg); err != nil { | ||
return Config{}, fmt.Errorf("decode config: %w", err) | ||
} | ||
|
||
return cfg, nil | ||
} | ||
|
||
// ParseFile returns a parsed Lava configuration given a path to a | ||
// file. | ||
func ParseFile(path string) (Config, error) { | ||
r := newResolver(path) | ||
cfg, err := r.resolve() | ||
f, err := os.Open(path) | ||
if err != nil { | ||
return Config{}, fmt.Errorf("process include: %w", err) | ||
return Config{}, fmt.Errorf("open config file: %w", err) | ||
} | ||
|
||
if err := cfg.validate(); err != nil { | ||
return Config{}, fmt.Errorf("validate config: %w", err) | ||
} | ||
return cfg, nil | ||
defer f.Close() | ||
return Parse(f, path) | ||
} | ||
|
||
// validate validates the Lava configuration. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you change this? IIRC this setting allowed to get inline error messages in pull requests while avoiding duplicates because of runs on push.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of this warning message: https://github.com/adevinta/lava/actions/runs/12827986995/job/35771096163