Skip to content

Warn if there are configuration keys in the configuration file that are not loaded by the configuration store#9575

Merged
mitchellhenke merged 3 commits intomainfrom
mitchellhenke/warn-if-unused-configuration-keys
Nov 13, 2023
Merged

Warn if there are configuration keys in the configuration file that are not loaded by the configuration store#9575
mitchellhenke merged 3 commits intomainfrom
mitchellhenke/warn-if-unused-configuration-keys

Conversation

@mitchellhenke
Copy link
Contributor

🛠 Summary of changes

A small follow-up to #9566 to print out one class of unused configuration keys. This is specifically aimed at keys that are not defined in the configuration management, but are still defined in the configuration file. It is very likely this will find unused keys in deployed environments.

It would be better to log this to the log file, but logging is not set up at this point and in the interest of having a partial solution, I think this is an alright start.

Example output:

$ rails s
WARNING: There are defined configuration keys that are not loaded by the application. They are: [:disable_csp_unsafe_inline]
=> Booting Puma
=> Rails 7.1.1 application starting in development

@mitchellhenke mitchellhenke requested a review from a team November 9, 2023 21:36
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a way we can log this in the prod-deployed boxes? Like Rails.logger.warn as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much to my disappointment, Rails.logger doesn't exist at this point unfortunately :/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will log, but only in provisioning logs, not any of the Rails logs (or if you start a console).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

somehow we have config.active_job.logger and Identity::Hostdata.logger
what if we store the unused keys back on IdentityConfig.unused_keys or something and then add an initializer that logs them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a great idea, I'll give it a shot

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in f62fbfb, much bigger fan of that approach

…re not loaded by the configuration store

changelog: Internal, Configuration, Warn if there are configuration keys in the configuration file that are not loaded by the configuration store
@mitchellhenke mitchellhenke force-pushed the mitchellhenke/warn-if-unused-configuration-keys branch from 40d88e8 to f62fbfb Compare November 9, 2023 22:35
Mitchell Henke and others added 2 commits November 9, 2023 16:50
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
@mitchellhenke mitchellhenke merged commit 3d10133 into main Nov 13, 2023
@mitchellhenke mitchellhenke deleted the mitchellhenke/warn-if-unused-configuration-keys branch November 13, 2023 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants