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

Introducing CompositeConfig #1674

Open
cmwylie19 opened this issue Jan 16, 2025 · 1 comment
Open

Introducing CompositeConfig #1674

cmwylie19 opened this issue Jan 16, 2025 · 1 comment
Labels
enhancement New feature or request ON-HOLD Needs investigation

Comments

@cmwylie19
Copy link
Contributor

cmwylie19 commented Jan 16, 2025

Is your feature request related to a problem? Please describe.

We need a less ad-hoc / chaotic way to utilize Pepr configuration.

Currently Pepr accepts configuration values via (at least) package.json / pepr build-time flags / pepr deploy-time flags / system environment variables / Helm chart values & overrides / etc. and each of those sources is being individually read-and-respected (or not!) on a just-in-time, as-needed basis... mostly. HOWEVER, Issues like this one demonstrate that config propagation / override is far from a "solved problem" for Pepr and hint at future problems (since Pepr is still growing / as config sources become more numerous over time).

In addition to that, some select few config values (the choice of which being mostly a series of accidents of history) are being propagated throughout controller code via independent function parameters, and -- as we're already bumping up against the linter's max-params limits -- continuing to add/propagate config values in this manner is unsustainable.

Describe the solution you'd like

  1. Introduce a new config abstraction: the CompositeConfig. This should represent a singular, unified view of Pepr-known configuration values -- created by loading values from all available config sources, validating/sanitizing inputs, ordering/overriding conflicting values using a well-understood & clearly defined order of precedence, and then presenting a single, resolved view of Pepr config to downstream callers.
  2. Update existing build / controller code to create/populate an instance of CompositeConfig on startup.
  3. Refactor existing build / controller code to propagate a single CompositeConfig instance -- or an accessor method that returns an instance (depending on how dynamic the implementation needs to be) -- in place of multiple ad-hoc config values to reduce max-params pressure & future-proof controller code function parameter lists.

Additional context

As an implementation suggestion, the attached graphic recommends some reasonable configuration starting targets to abstract (the fully saturated yellow boxes) and others some to consider for the future (the de-emphasized boxes).

Image

@cmwylie19 cmwylie19 added the enhancement New feature or request label Jan 16, 2025
@btlghrants btlghrants changed the title PeprConfigCR to read in config at Deploy Introducing CompositeConfig Jan 22, 2025
@cmwylie19 cmwylie19 modified the milestones: v0.44.0, v0.45.0 Jan 22, 2025
@cmwylie19
Copy link
Contributor Author

Going to put this on hold right now to focus on immediate business value and stability

@cmwylie19 cmwylie19 added the ON-HOLD Needs investigation label Feb 3, 2025
@cmwylie19 cmwylie19 removed this from the v0.45.0 milestone Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ON-HOLD Needs investigation
Projects
Status: 🆕 New
Development

No branches or pull requests

1 participant