feat: add secrets management #17648
feat: add secrets management #17648
Conversation
There was a problem hiding this comment.
This is good! Super minimal change surface.
I'd actually iterate over this PR and make this ready to be merged once common change is merged. We could double check all tests etc. 💪🏽
There are definitely some tests to fix, but it might be as trivial as adding (f *Field) Equals(other Field) bool so comparisons work correctly (they don't depend on state):
| }, | ||
| func(error) { | ||
| logger.Info("Stopping scrape discovery manager...") | ||
| cancelScrape() |
There was a problem hiding this comment.
It probably needs its own context.
| var ( | ||
| secretsManager *secrets.Manager | ||
| ) | ||
| // needs logger: logger.With("component", "secrets manager") | ||
| secretsManager, err = secrets.NewManager(context.Background(), prometheus.DefaultRegisterer, secrets.Providers, cfgFile) |
There was a problem hiding this comment.
| var ( | |
| secretsManager *secrets.Manager | |
| ) | |
| // needs logger: logger.With("component", "secrets manager") | |
| secretsManager, err = secrets.NewManager(context.Background(), prometheus.DefaultRegisterer, secrets.Providers, cfgFile) | |
| // TODO: Pass logger.With("component", "secrets manager") | |
| secretsManager, err := secrets.NewManager(context.Background(), prometheus.DefaultRegisterer, secrets.Providers, cfgFile) |
| select { | ||
| case <-hup: | ||
| if err := reloadConfig(cfg.configFile, cfg.tsdb.EnableExemplarStorage, logger, noStepSubqueryInterval, callback, reloaders...); err != nil { | ||
| if err := reloadConfig(cfg.configFile, cfg.tsdb.EnableExemplarStorage, logger, noStepSubqueryInterval, secretsManager, callback, reloaders...); err != nil { |
There was a problem hiding this comment.
We could probably DRY this g.Add with something like
// before loop
reload = func()error {
return reloadConfig(cfg.configFile, cfg.tsdb.EnableExemplarStorage, logger, noStepSubqueryInterval, callback, reloaders...),
}
// ...
if err := reload(); err != nil {
| } | ||
| } | ||
|
|
||
| if err := secretsManager.HydrateConfig(nil, conf); err != nil { |
There was a problem hiding this comment.
Maybe we could work on this name. Reminds me to grab a cup of water though! (:
Maybe:
| if err := secretsManager.HydrateConfig(nil, conf); err != nil { | |
| if err := secretsManager.ReloadConfig(context.TODO(), conf); err != nil { |
| NamespaceDiscovery NamespaceDiscovery `yaml:"namespaces,omitempty"` | ||
| Selectors []SelectorConfig `yaml:"selectors,omitempty"` | ||
| AttachMetadata AttachMetadataConfig `yaml:"attach_metadata,omitempty"` | ||
| ExampleSecret secrets.Field `yaml:"example_secret"` |
There was a problem hiding this comment.
Nice, easy use 👍🏽
Let's remove and iterate over this PR to be effectively testable and potentially mergable once common change is done, looks promising!
There was a problem hiding this comment.
Although your other examples use *secrets.Field (pointer)?
|
For release log, keep in mind this is for user facing changes (e.g. user of Prometheus YAML). I would mention what user can expect (generic field and removal of ref). I'd recommend: |
|
Exciting feature! We even plan to mention it on KubeCon EU in March if that's ok (: cc @hsmatulis Hopefully it's done until then 💪🏽 |
2 participants
secrets: Add remote secrets providers
See the proposal here. This PR is currently a work in progress, demonstrating how user's would interact with the secrets management API
Which issue(s) does the PR fix:
Does this PR introduce a user-facing change?