Introduce mode-specific config files #828

JeSuisUnCaillou · 2025-07-31T14:16:36Z

⚠️ This needs to be viewed by the Ops team, especially @jordanguedj 👇

Override the VITE_DATAGOUV_BASE_URL and VITE_DATAGOUV_OAUTH_CLIENT_ID in your deployments scripts, the same way you already override the VITE_SITE_ID.
You can find the suitable values of VITE_DATAGOUV_OAUTH_CLIENT_ID for each site in their respective config (Except ecospheres which has 2 values for demo and prod in their respective branches).

Here is all the links to all the existing oauth_client_ids (before merging this PR) :

ecospheres demo
ecospheres prod
meteo-france
defis
hackathon
logistique
simplifions

JeSuisUnCaillou · 2025-07-31T15:34:11Z

I talked about this with @jordanguedj , he seemed ok with having to manage the base_url and oauth_token in the infra

streino

This isn't what I had in mind when discussing on Tchap.

My proposal is to have a CONFIG_PATH env var that will contain the path of the config.yaml you want to load. Then this CONFIG_PATH var can be set in a .env or .env.$env to be loaded automatically.

I see two problems with the approach of this PR:

config.yaml isn't the only source of truth anymore. You now have to look into two different locations to know about config. VITE_SITE_ID is an exception because it serves a similar purpose to CONFIG_PATH, ie we have to know where to look for the config. Note that if we were to separate code and config we wouldn't need VITE_SITE_ID (because there would be a single site on a given repo), and if we use per-env branches, we don't need CONFIG_PATH. Combine per-branch and per-site and you have a single config.yaml that you know where to find, and which remains the single source of truth.
If you start extracting parameters from config.yaml, there's no end to it. Next you'll want to extract the matomo id, then maybe the site notice, then...

In the future we may want to have a way to override config.yaml params with env-vars as this is a common pattern used eg in the docker world. But 1) I don't think we need it yet, and 2) this is different from extracting parameters one at a time, we'll want to have a generic mechanism for it.

JeSuisUnCaillou · 2025-08-01T12:37:40Z

Oh ok, yeah I didn't exactly get what you meant on Tchap.

Managing your env-dependent variables in env variables is quite standard, so I don't understand the resistance you oppose here.

Yes the matomo ID should be in an env variable too.

Yes the notice "you are on a demo version" should depend on if import.meta.env.MODE != 'production' (or maybe if import.meta.env.PROD if you prefer), and not be in some config that you keep on a few designated branches.

I want to be able to take the main branch, and push it on any vertical. I don't want to manage different configurations in different branches. Env vars are made for this. Let's use them.

streino · 2025-08-01T13:35:06Z

Yes the matomo ID should be in an env variable too.

Yes the notice "you are on a demo version" should depend on if import.meta.env.MODE != 'production' (or maybe if import.meta.env.PROD if you prefer), and not be in some config that you keep on a few designated branches.

Then where does it stop?

And what if different verticals want different banners? Or a banner in prod? (at some point we had one on ecologie). We could argue that for that specific banner config we don't need it, but you get the general idea: each thing you'll want to configure per-env will have to be extracted from the config.

I want to be able to take the main branch, and push it on any vertical. I don't want to manage different configurations in different branches. Env vars are made for this. Let's use them.

A couple reasons I'm not fond of envvar-only approach:

Not a single source of truth, as explained above. This is the biggest deal-breaker for me. You have to go look around to know what state you're in (and in our current deployment setup, you can't).
Config can be documented and therefore discoverable. You need a good doc with envvars, which is extra work. Also config will always be up-to-date, because it has to work.
Still about doc, verticals can document why they configure stuff in the config. How do you do that in envvars?
No versioning of envvars. If things go wrong, you need access to the deployed machine to investigate, and hope state didn't change. Also, no history of config changes, diffing is more complex, ... none of the nice stuff you get for free with VC. You could argue envvars is VC'ed at the ops level, but then that's two different places where you handle config (back to point one), and in our context it's two different teams in some cases.
No typing, no schema, no pre-run validation on envvars, whereas this is all easy to do on a config file. We're not doing any of it at this point, but if usage expands, it could be nice to have at some point.
More general things, like envvars is global state (booo), are limited to simple types (everything is a string), are hard to name/maintain consistently once they proliferate, ...
On our current deployment setup, vertical teams have no direct control of envvars, we have to go through the ops team for any change, vs you can do it yourself in the config.

This all is less true if you start overriding config with envvars, but still is better than envvar-only.

I'm not saying envvars shouldn't be used at all, but I strongly prefer to put as much as possible in config, and to put it in config first, then leave the possibility to override with envvar, for those contexts where it makes sense, like the one you explain.

So I'm open to a PR that would allow overriding config settings with envvars in a somewhat generic way (even a first PR that does is sort of adhoc but with the intent of going the generic way). But I'm really not in favor of extracting config bit by bit and have it become less consistent and less maintainable (from the POV of a vertical devops).

JeSuisUnCaillou · 2025-08-05T14:15:51Z

I can see your point.

I'll propose a solution that :

doesn't use more env vars (or just one more, but it might not even be required)
doesn't duplicate the entire config for each env (that would be annoying to maintain)
enables us to have the exact same code on all deployment branches (which is my main original concern)

I'll get cooking and will let you know.

streino · 2025-08-05T14:37:38Z

I still fail to grasp why branches don't work for you. Agreed it's not "the same (machine) code" in the sense that you build each branch separately and so it may not be identical to the bit. But it still can be "the same (source) code" when you sync the branches (cf what we do on ecospheres/ecologie.data.gouv). This seems like a good-enough approximation of "the same code", at least until we deal with #809 and can have several config-repo branches pull the same code-repo lib.

But if this doesn't work for you, no problem to look at other proposals. In the mean time I'll flag this PR as "needs changes" to make its status easier to figure out.

JeSuisUnCaillou · 2025-08-05T15:31:58Z

I'm very uncomfortable with having config bits isolated in specific branches for specific environments because :

Not a single source of truth
Config should be discoverable

Hehe 🙈

I'm trolling but not really trolling. I was really confused when I arrived on the repo. I couldn't understand how the base_url was adapted to each environment. It was impossible to understand how it works from the main branch's code only.

If it was in an env variable, I would have assumed this variable is properly set on the production machine, like most of the projects I've encountered so far. But I get your point, we can and should keep most of the configuration in the codebase for all the reasons you cited.

I think I'll have time to work on something tomorrow.

streino · 2025-08-06T09:33:49Z

I was really confused when I arrived on the repo. I couldn't understand how the base_url was adapted to each environment. It was impossible to understand how it works from the main branch's code only.

We definitely should fix that part in #778. But I'm not sure why you'd figure envvars would be set somewhere else, but not config files? Would it have helped to leave some values blank (or fill them with dummy data) in the config to make it more explicit? Basically, do you have specific suggestions about how to make it more understandable for a newcomer? I repeat myself, but I also think having separate config repos will help.

Aside from the doc, you seem more used to envvars, but deploying from branches is also a very common pattern (we could name the branches better for discoverability). In the end, having config in envvars or files doesn't change the fact that you have to have different versions of (at least some of) the config for different deployments ; versions which eventually end up in files anyway, in an ops repo (Ansible in our case, which we don't control), in deployments notes (can get out of sync), or whatever means you have to store config.

So basically you have a single branch with a bunch of files with different versions of a config (.env files count), or multiple branches with a specific version each. Short of having a way to override your config, it seems to me branches will let you easily handle different versions of a full config and keep them in sync with minimal effort (that's what they're for after all). Each of us is normally dealing with a single vertical, so we're generally talking about two extra branches, or even one if you consider main as your demo/preprod. This is the pattern we've had on ecologie (and I believe at least meteo as well) in prod for 2 years and while not perfect, it's clearly not the most "itching" part of deployment.

That said, having finer-grained control of how you split/override config to avoid redundancy would be nice indeed, and it's great if you can propose a way to do it without adding too much complexity.

streino

As discussed above

JeSuisUnCaillou · 2025-08-06T12:38:39Z

Here is my new proposition ☝️

One default config file in the usual /config/<site_id>/config.yaml
Optional mode-specific config files in /config/<site_id>/config.<mode>.yaml
Mode-specific config files are deep-merged into the default one, so they only need to contain the overriden keys (check my /config/simplifions/config.production.yaml for an example)

(I don't want to define the whole config object, but maybe I should)

JeSuisUnCaillou · 2025-08-06T12:48:37Z

Me waiting for the reviews :

streino · 2025-08-06T13:38:08Z

Will review asap.
In the mean time, putting @jordanguedj back in the loop. WDYT? Also cc @abulte if you have an opinion.

abulte · 2025-08-06T13:54:19Z

I don't think we (and any other site?) will use this. We want to keep a branch in sync with what is deployed on a given env (code + config), so we'll stick with the current system (a config file customized by env on a branch). The PR workflow is also quite cool for deployment.

It will also make the maintenance of the config file (which is a pain) a bit more arduous, since we'll have more files to check for migrations if any.

If it's really a blocker, why not I guess, but I'm not sure this is really needed.

JeSuisUnCaillou · 2025-08-06T14:12:11Z

I don't think we (and any other site?) will use this. We want to keep a branch in sync with what is deployed on a given env (code + config)

You still can. All branches are going to be in sync with main which contains all configs. So any specific deployment branch will be in sync with what is deployed on a given env (code + config).

And you can give the option --mode to your run command if you want to simulate any mode in local (production for example).

It will also make the maintenance of the config file (which is a pain) a bit more arduous

This will make it easier to maintain. Any change in the config can directly be applied to all the config files in the PR responsible of the change. No more conflicts when merging main to the ecospheres-prod branch.

And there is only like 3 values in a mode-specific config, thanks to the deep merge. So not that much to maintain.

streino · 2025-08-06T16:30:21Z

I don't think we (and any other site?) will use this. We want to keep a branch in sync with what is deployed on a given env (code + config), so we'll stick with the current system (a config file customized by env on a branch). The PR workflow is also quite cool for deployment.

I agree we (ecologie) will probably stick to branch-based deployment for a bunch of reasons¹. But if other verticals want to live in main (and later organize their own config repo the way they want) I don't see why not. I think the change is contained enough that it won't affect complexity much and IMO this isn't something the front-kit needs to enforce one way or another.

And you can give the option --mode to your run command if you want to simulate any mode in local (production for example).

Famous last words ;) But whoever wants to use this, why not.

It will also make the maintenance of the config file (which is a pain) a bit more arduous

This will make it easier to maintain. Any change in the config can directly be applied to all the config files in the PR responsible of the change. No more conflicts when merging main to the ecospheres-prod branch.

I think it's fair to say that whoever uses per-env config overrides is in charge of keeping their overrides in sync. We can't request core devs or people from other verticals to patch more than the base config in (other) verticals. This will change anyway if/when we extract config into separate repos since each vertical will have to maintain their own configs (but on fixed library releases, so it won't be as chaotic as it is now).

@abulte I'll take a look at the code, but can you as well ? 🙏

- PRs as buffer between changes landing in main and what gets deployed to prod.
- All kinds of git/github niceties that come from free with a branch model.
- Automatic deploys on deployment branches if we want.
- Easier (IMO) to revert back to previous state if needed, do hotfixes, etc.
↩

JeSuisUnCaillou · 2025-08-06T16:48:41Z

I agree we (ecologie) will probably stick to branch-based deployment for a bunch of reasons

I repeat : this is not challenging the branch-based deployment. I don't understand why you both say that.

streino · 2025-08-06T16:41:10Z

vite.config.mts

              routeFile.includes(`custom/${env.VITE_SITE_ID}/routes.ts`)
            )
          }
+          // Include only the current mode's config file in the bundle


Might be completely off, but IIUC this is a build-time action and we then deep-merge at run-time? If so, why not merge at build-time and only have a single config object to manipulate at run-time?

I couldn't manage to expose the merged config by merging it at build time. If you know how to do, I'll be glad to get your advice.

I'm the front-end noob here, so this is really a question.
If someone else has a suggestion (maybe Claude ?) then cool, otherwise nevermind.

Something like that seems to work #846.

Added a comment to that PR, but TBD here if it gets merged.

streino · 2025-08-06T16:45:09Z

src/config.ts

+import merge from 'deepmerge'

-export default config
+const mode = import.meta.env.MODE


Name is a bit too generic. Maybe something like DEPLOYMENT_ENV? I've seen ENV in other contexts but I don't know if this'll conflict in the present case?

I'm not sure I understand how .env.* varations are loaded, but can't we piggyback on that mechanism to get the value we need, instead of redefining it in a new envvar?

Also, if we end up introducing an envvar, will need doc.

This is not a new env var. "MODE" is defined by vite : typically "development" with npm run dev and production with npm run build; npm run preview.

We could use the NODE_ENV if you prefer, this is the same in most cases.

Ah ok I thought this was a new setting. I can't say which one it preferable, so maybe @abulte?

Vite might be slightly "better".

Vite might be slightly "better".

What do you mean ? Are you talking about "MODE" ?

I was thinking about this in my bed yesterday (yes), and I think this will require a minor change in the infra to pass the --mode demo (or preproduction) at build time for the demo (or preproduction) environments. Otherwise, they will be treated as production, and thus will use the production config.

streino · 2025-08-06T16:53:49Z

I agree we (ecologie) will probably stick to branch-based deployment for a bunch of reasons

I repeat : this is not challenging the branch-based deployment. I don't understand why you both say that.

I'm not saying it's challenging another model (and I don't think @abulte is either). Just that this adds (limited) complexity, and another way to do things ; meaning possibly more room for ambiguity and/or maintenance. And for a gain we have a hard time evaluating at this point (ie we don't who other than you will use it), because we recommend going with branch-based deployments and so far it's worked out well enough.

IMO your latest proposal is okay, and we're not saying no. But while each PR doesn't add much by itself, in the end it's code we'll have to maintain, and we're a small team with limited time to dedicate to the front-kit. So we try to keep things simple (at least by some definition of simple 😬) and try to stick to a single way to do things. It's necessarily a balancing act.

* chore: merge config at build time * comment * Use ViteRestart to detect config changes (#865) --------- Co-authored-by: streino <[email protected]>

JeSuisUnCaillou force-pushed the base_url_per_environments branch from 414ede1 to b72dcfc Compare July 31, 2025 14:50

JeSuisUnCaillou requested review from abulte and streino July 31, 2025 15:28

JeSuisUnCaillou force-pushed the base_url_per_environments branch from c2c6364 to 2416bff Compare July 31, 2025 15:30

JeSuisUnCaillou requested a review from geoffreyaldebert July 31, 2025 15:43

streino reviewed Aug 1, 2025

View reviewed changes

streino requested changes Aug 6, 2025

View reviewed changes

JeSuisUnCaillou added 4 commits August 6, 2025 14:28

dirty but functional way to import and merge mode-specific configs

63d3ffb

use deepmerge to merge configForMode into the default config

84c29e3

comment default and production config files

6222028

don't import all modes configs, just the required one

2f6a489

JeSuisUnCaillou force-pushed the base_url_per_environments branch from 308e882 to 2f6a489 Compare August 6, 2025 12:36

JeSuisUnCaillou requested a review from maudetes as a code owner August 6, 2025 12:36

JeSuisUnCaillou requested a review from streino August 6, 2025 12:36

ignore "any" lint warning in config loading

dfea007

(I don't want to define the whole config object, but maybe I should)

JeSuisUnCaillou changed the title ~~use .env.[mode] files for environment variables like datagouvfr.base_url~~ Introduce mode-specific config files Aug 6, 2025

streino reviewed Aug 6, 2025

View reviewed changes

JeSuisUnCaillou mentioned this pull request Aug 13, 2025

Add datagouv components next #851

Merged

4 tasks

JeSuisUnCaillou mentioned this pull request Sep 8, 2025

Use ViteRestart to detect config changes #865

Merged

chore: merge config at build time (#846)

ce1e2d2

* chore: merge config at build time * comment * Use ViteRestart to detect config changes (#865) --------- Co-authored-by: streino <[email protected]>

JeSuisUnCaillou marked this pull request as draft October 2, 2025 12:56

Introduce mode-specific config files #828

Are you sure you want to change the base?

Introduce mode-specific config files #828

Uh oh!

Conversation

JeSuisUnCaillou commented Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JeSuisUnCaillou commented Jul 31, 2025

Uh oh!

streino left a comment

Choose a reason for hiding this comment

Uh oh!

JeSuisUnCaillou commented Aug 1, 2025

Uh oh!

streino commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JeSuisUnCaillou commented Aug 5, 2025

Uh oh!

streino commented Aug 5, 2025

Uh oh!

JeSuisUnCaillou commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

streino commented Aug 6, 2025

Uh oh!

streino left a comment

Choose a reason for hiding this comment

Uh oh!

JeSuisUnCaillou commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JeSuisUnCaillou commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

streino commented Aug 6, 2025

Uh oh!

abulte commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JeSuisUnCaillou commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

streino commented Aug 6, 2025

Footnotes

Uh oh!

JeSuisUnCaillou commented Aug 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JeSuisUnCaillou Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JeSuisUnCaillou Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JeSuisUnCaillou Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

JeSuisUnCaillou commented Jul 31, 2025 •

edited

Loading

streino commented Aug 1, 2025 •

edited

Loading

JeSuisUnCaillou commented Aug 5, 2025 •

edited

Loading

JeSuisUnCaillou commented Aug 6, 2025 •

edited

Loading

JeSuisUnCaillou commented Aug 6, 2025 •

edited

Loading

abulte commented Aug 6, 2025 •

edited

Loading

JeSuisUnCaillou commented Aug 6, 2025 •

edited

Loading

JeSuisUnCaillou Aug 6, 2025 •

edited

Loading

JeSuisUnCaillou Aug 6, 2025 •

edited

Loading

JeSuisUnCaillou Aug 8, 2025 •

edited

Loading

streino commented Aug 6, 2025 •

edited

Loading