Remove default-true feature flags #6802
Assignees
Milestone
Comments
jsha
pushed a commit
that referenced
this issue
Nov 6, 2023
The RequireCommonName feature flag was our only "inverted" feature flag, which defaulted to true and had to be explicitly set to false. This inversion can lead to confusion, especially to readers who expect all Go default values to be zero values. We plan to remove the ability for our feature flag system to support default-true flags, which the existence of this flag blocked. Since this flag has not been set in any real configs, inverting it is easy. Part of #6802
pgporada
pushed a commit
that referenced
this issue
Dec 12, 2023
Replace the current three-piece setup (enum of feature variables, map of feature vars to default values, and autogenerated bidirectional maps of feature variables to and from strings) with a much simpler one-piece setup: a single struct with one boolean-typed field per feature. This preserves the overall structure of the package -- a single global feature set protected by a mutex, and Set, Reset, and Enabled methods -- although the exact function signatures have all changed somewhat. The executable config format remains the same, so no deployment changes are necessary. This change does deprecate the AllowUnrecognizedFeatures feature, as we cannot tell the json config parser to ignore unknown field names, but that flag is set to False in all of our deployment environments already. Fixes #6802 Fixes #5229
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently, Boulder feature flags must always be booleans, but they can have a default value of "true", and the presence of the feature flag in a config can cause them to be set to "false".
This has been considered useful, because it allows us to give feature flags names that minimize double negatives. For example, instead of checking
if !features.Enabled(features.DisableThingy), we could simply checkif features.Enabled(features.Thingy).However, this rubs up against a different go-ism, that the default value of a thing should be its zero value. Since feature flags are all booleans, it often feels like their default value should be "false", even when it isn't. This leads to confusion when both writing and reading feature-related code.
Therefore, as discussed in person, we've decided to remove the ability for feature flags to have a customized default value. This may also allow us additional benefits, such as:
The text was updated successfully, but these errors were encountered: