Make sure server flags are configured consistently

[lukemassa]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request. Searching for pre-existing feature requests helps us consolidate datapoints for identical requirements into a single place, thank you!
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
• If you are interested in working on this issue or have submitted a pull request, please leave a comment.

---

• I'd be willing to implement this feature (contributing guide)

Describe the user story

When flags are added to the server, four places are supposed to be updated:

1. a const in cmd/server.go
2. an entry in one of the flag maps (stringFlags, boolFlags, etc.) in cmd/server.go
3. An entry in testFlags in cmd/server_test.go
4. An entry in userConfig in server/server.go

Sometimes users forget to add to all four places. Additionally, when flags are removed, they are sometimes not cleaned up in all four places.

The compiler and unit tests help somewhat. For example, if the const is removed from cmd/server.go, the build will fail until the entry in testFlags is also removed. Also there's an explicit test to make sure all the flags specified in testFlags are present in userConfig, which catches some mistakes.

I'd like to identify which of the remaining potential misconfigurations are in fact a problem, then add unit tests to prevent them in the future.

Has FlagName	Flag in stringFlags etc	In testFlags	In userConfig	Detected Issue	description
true	true	true	true	✅	Configured properly
true	true	true	false	✅	Should set the defaults for all unspecified flags.
true	true	false	true	❌	Untested config 1
true	true	false	false	✅	"Should set the defaults for all unspecified flags." test failed
true	false	true	true	❌	Flag without configuration 2
true	false	true	false	✅	"Should set the defaults for all unspecified flags." test failed
true	false	false	true	❌	Untested config 1
true	false	false	false	❌	Flag without configuration 2
false	true	true	true	✅	Build failed due to lack of flag variable
false	true	true	false	✅	Build failed due to lack of flag variable
false	true	false	true	✅	Build failed due to lack of flag variable
false	true	false	false	✅	Build failed due to lack of flag variable
false	false	true	true	✅	Build failed due to lack of flag variable
false	false	true	false	✅	Build failed due to lack of flag variable
false	false	false	true	❌	Extra config 3
false	false	false	false	✅	No config, not an issue

1. A flag is setup but missing from server_test.go. No reason not to add it to that default list, so we should fail here
2. This corresponds to a flag that is set but not used in the stringFlags, boolFlags, etc. Harder to detect, but I think we should fail here as well if possible
3. This one I'm actually not if it's an "error" or not. Because settings can be specified via yaml from --config, not all flags need to be present. Indeed, webhooks which is of type []Webhook may be impossible to specify except in yaml.

Describe the solution you'd like

1 I think I can add a unit test for pretty simply. 2 I'm less sure about without some tricky reflection and/or static analysis, so we may have to live with that one. 3 I'd like to understand people's thoughts.

An additional dimension I did not capture above is whether or not the flag is referenced in the docs. I think this is also a similar problem, but should be addressed by a separate issue (presumably, once this issue is addressed, it'll be straightforward to add that to CI).

Describe the drawbacks of your solution

A bit pedantic, and the error messages might be confusing, we just have to be super clear of how to address each of the situations as they arise.

Describe alternatives you've considered
Leaving it as is? It just accumulates cruft.

[nitrocode]

See this comment for one suggestion that might help eliminate having to keep multiple structures in sync

#4068 (comment)