Skip to content
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

feat: Add the ability to overlay configuration #698

Closed
wants to merge 9 commits into from

Conversation

timraymond
Copy link
Member

In certain deployments, it's desirable to have one set of configuration pre-set and another set of overrides to that configuration. In this overlay configuration, only certain fields should be overridable--additional provided fields should be ignored.

This modifies GetConfig in a backwards-compatible way to accept a variadic list of configuration overlays. These overlays take a filesystem path and a set of allowed fields that are permitted from those overlay configs. It also introduces internal types for performing filtering of these configuration files.

These are currently not used. A subsequent pull request will integrate this work.

@rbtr
Copy link
Collaborator

rbtr commented Sep 3, 2024

I'm hearing... drop-ins? 🙂

anubhabMajumdar
anubhabMajumdar previously approved these changes Sep 5, 2024
"testdata" is the well-known directory name for files and fixtures
related to a package's test. The toolchain recognizes specifically this
name and ignores it, so it is the proper one to use.
In order to support multiple configuration sources (some managed, some
unmanaged), Retina needs to be able to consider N configuration files
and apply them in an ordered fashion. This makes the GetConfig function
variadic, and uses Viper's MergeInConfig functionality to overlay the
configuration from each subsequent file from the first one.
Trying to figure out a better way to structure this, but since this is a
variadic list, there is now a possibility for zero files (which would be
equivalent to the previous behavior of "" for the file name).
We need to restrict the overlay configuration to a subset of fields, so
that certain configuration may only configure certain fields. This
introduces a FilteredYAML that removes any other fields that it finds so
that *only* those fields that should be allowed to be modified will be.
In order to make it possible for specific fields to be overridden by
user-controllable config files, the GetConfig function must be modified
to support an additional configuration file that can be filtered to
specific fields. This modifies GetConfig in a backwards-compatible way
to support a FilteredConfig type, specifiying additional overlays with
allowable fields from those config overlays.
It turns out that this type is useful outside of this package, which
makes it a good candidate for graduating out of `internal`.
Both of these lints aren't particularly useful, so this just silences
both of them.
anubhabMajumdar
anubhabMajumdar previously approved these changes Sep 5, 2024
In order to merge configurations, we need to be able to write out the
resultant configuration as YAML.
This can be used by variant implementations to decide whether or not TLS
management will be handled by retina or not.
Copy link

This PR will be closed in 7 days due to inactivity.

@github-actions github-actions bot added the meta/waiting-for-author Blocked and waiting on the author label Oct 12, 2024
@timraymond timraymond removed the meta/waiting-for-author Blocked and waiting on the author label Oct 15, 2024
Copy link

This PR will be closed in 7 days due to inactivity.

@github-actions github-actions bot added the meta/waiting-for-author Blocked and waiting on the author label Nov 15, 2024
Copy link

Pull request closed due to inactivity.

@github-actions github-actions bot closed this Nov 22, 2024
auto-merge was automatically disabled November 22, 2024 00:23

Pull request was closed

@github-actions github-actions bot deleted the traymond/configmap-mgmt-init branch November 22, 2024 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta/waiting-for-author Blocked and waiting on the author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants