feat: Add the ability to overlay configuration #698
+188
−6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
timraymond
requested review from
MikeZappa87,
xiaozhiche320,
anubhabMajumdar and
rbtr
|
I'm hearing... drop-ins? 🙂 |
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`.
timraymond
force-pushed
the
traymond/configmap-mgmt-init
branch
from
ec189e3
to
fcd8bdf
Compare
Both of these lints aren't particularly useful, so this just silences both of them.
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
GetConfigin 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 introducesinternaltypes for performing filtering of these configuration files.These are currently not used. A subsequent pull request will integrate this work.