Feat: support HTTPS URLs in the -c/--config flag - #950
Conversation
Allow Bento to fetch its configuration from an HTTPS URL at startup in addition to a local file path. Adds an optional --config-header flag (repeatable) for authenticated endpoints. - URL detection in config loading layer (reader.go, lint.go) - --watcher + HTTPS URL exits with a clear fatal error - env var interpolation works after fetch - bento lint supports HTTPS URLs as positional args - Unit tests for fetchRemoteConfig and ReadYAMLFileLinted Closes warpstreamlabs#939
jem-davies
left a comment
jem-davies
left a comment
There was a problem hiding this comment.
Would need to document the new feature by changing the documentation strings for the -c / --config flag as well
| var lints []docs.Lint | ||
| var err error | ||
|
|
||
| if strings.HasPrefix(path, "https://") { |
There was a problem hiding this comment.
| if strings.HasPrefix(path, "https://") { | |
| parsedPath, err := url.Parse(path) | |
| if err != nil { | |
| ... | |
| } | |
| if parsedPath.Scheme == "https" || parsedPath.Scheme == "http" { | |
| ... | |
| } |
Could use url.Parse here - if the provided path is a relative filepath ./config.yaml scheme should be empty string, you could also have a scheme file too - file:///Users/.../config.yaml
| } | ||
|
|
||
| func fetchRemoteConfig(url string, headers []string) ([]byte, error) { | ||
| req, err := http.NewRequest(http.MethodGet, url, nil) |
There was a problem hiding this comment.
could use a http.NewRequestWithContext() and pass a context with a short deadline.
| req.Header.Add(strings.TrimSpace(name), strings.TrimSpace(value)) | ||
| } | ||
|
|
||
| client := &http.Client{ |
There was a problem hiding this comment.
I think we might want to expose some more options for the client - such it could work with TLS certificates that are signed with private CA's etc.
There was a problem hiding this comment.
Hello @jem-davies
Thanks for the feedback, i'm just wondering should this reuse existing CLI flags or
env vars, or would you prefer dedicated flags like --config-tls-ca-file ?
2 participants
Summary
Implements #939, allows
-c/--configto accept anhttps://URLin addition to a local file path.
Changes
internal/config/reader.go— URL detection, fetch logic, watcher guardinternal/config/lint.go— URL support forReadYAMLFileLintedinternal/cli/run.go—--config-headerflag definitioninternal/cli/common/reader.go— watcher fatal error, headers wired intoNewReaderinternal/config/reader_test.go— added unit tests forfetchRemoteConfigand remote reader behaviourinternal/config/lint_test.go— new file, unit tests forReadYAMLFileLintedwith remote URLsBehaviour
https://prefix → remote fetch before config parsing--watcher+ URL → exits with fatal error immediately${FOO:default}) still works after fetch--config-header "Name: Value"(repeatable) for auth headersTesting
Known limitations
Only
https://URLs are supported. plainhttp://falls back tofile path behaviour.
bento lint <url>works for public endpoints but does not support--config-header.ReadYAMLFileLintedhas no access to CLI flagsat that call site. Can be addressed in a follow-up.
Open question
As noted in #939. happy to move to a dedicated
--config-urlflag if that is preferred over extending-c.