Give warnings/errors for breaking changes in 5.0 config

[mrjones-plip]

What feature do you want to improve?
There's some config changes that are going to change with CHT 5.0. Specifically the declarative config will be mandatory and app_url are required when token login is enabled.

There's no way to know if you need to change any of these if you're using CHT Conf.

Describe the improvement you'd like
We should either warn or error out by detecting any 5.0 settings changes. This might look like detecting a 4.x version and warning that the features will soon be deprecated or erroring out if a 5.x instance is detected and an incompatible config is being pushed.

Describe alternatives you've considered
Do nothing and hope folks read the docs.

Additional context
This came up on the Stewardship call today as a good idea that we should add. I might be capturing some of the ideas here wrong - feel free to amend!

[dianabarsan]

I think config schema validation would be super useful. We've "talked" about it before so many times.
We even have this ancient issue.

I think the idea still stands, I think it's not cht-conf that should do the validation, it should be cht-core, and conf should react to the validation errors.

[mrjones-plip]

@dianabarsan - do you think you could update this ticket to be a "Good First Issue"?

[jkuester]

I think it's not cht-conf that should do the validation, it should be cht-core, and conf should react to the validation errors.

So, I have been thinking about this and doing some more reading of the older issues/PR. I think I agree that the ideal "correct" workflow would be to have cht-core be the source of truth for what is valid config (and /api/v1/settings will reject a PUT with invalid config). However, I think what we have learned from the lack of progress on this for the last 5 years is that adding the validation to cht-core is high cost/risk with medium-to-low rewards. Some problems doing the validation in cht-core include:

• If we get something wrong, we risk breaking deployments after someone takes an upgrade. We could add some kind of "skip-validation" parameter, but validation would remain broken on that instance until the next time they upgrade...
  • On the other hand, if we break something in a release of cht-conf, it is not good. But it is easily fixed and is unlikely to cause long-running issues for anyone. (App devs can always downgrade their cht-conf while waiting for the fix).
• It is hard to iterate quickly and get new validations out to folks. Consumers must be running the new version of cht-core to take advantage. This presents a challenge when wanting to warn people about something that is deprecated and will be removed in the future. For example, in 5.0.0, the app_url is required when you have token login enabled. We could be warning people, now, that they can update their settings (but not if it would require a cht-core upgrade to see the warning).

I considered proposing some kind of hybrid approach here that would involve cht-core and cht-conf sharing different responsibilities for various validations. But, after more consideration, I think for now we should move forwards with validation in cht-conf. Lets not make the perfect the enemy of the good. Keep things as simple as possible and actually ship features that are useful (instead of letting ourselves get tied up in questions of what is the perfect architectural approach and separation of responsibilities).

We already have the form validations as a working example of cht-conf config validation. It is not perfect, but it is massively useful. IMHO we can add a similar framework for non-trivial app_settings validations.

do you think you could update this ticket to be a "Good First Issue"?

I don't think this is a good first issue. But, we should spin off separate sub-issues for each of the validations we want to do. Those could be Good First Issues (once we get the basic validation framework code put together). Pending @dianabarsan's thoughts on my proposal here, I can try to get a "Help Wanted" issue logged for creating the first basic validation along with the framework code for running it.

[mrjones-plip]

Thanks @jkuester ! Following along with the progress here - differing to you experts 😅

[dianabarsan]

Sweet writedown @jkuester . I agree that the perfect architectural approach would be for cht-core to be the source of truth for configuration validation.
However, we generally refrain from making massive config updates, to avoid disruptions and errors, so the changes are usually small and well documented and frequently accompanied by a migration.

I agree that having something, even if unperfect, is much better than nothing. Let's go for it!

I don't think it's a good first issue though!