feat: add strict validation option to reject invalid flag configs#1939
feat: add strict validation option to reject invalid flag configs#1939buildingisfun23 wants to merge 3 commits into
Conversation
✅ Deploy Preview for polite-licorice-3db33c ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a strict schema validation option for the JSON evaluator. It adds a WithStrictValidation option that, when enabled, causes flag configurations failing schema validation to return an error instead of just logging a warning. The changes include updates to the JSON struct, the validation logic, and comprehensive unit tests to verify the new behavior and ensure backward compatibility. Feedback suggests using the %w verb for error wrapping in the new error return path to follow Go best practices.
buildingisfun23
force-pushed
the
feat/1487-reject-invalid-flags
branch
from
2791d98 to
5fe79e8
Compare
dosubot
Bot
added
the
size:S
|
@buildingisfun23 - I'm not opposed - I wonder if we should connect this directly to a flagd startup arg as well? WDYT? |
dosubot
Bot
added
size:M
|
Good call @toddbaert! I've added |
buildingisfun23
force-pushed
the
feat/1487-reject-invalid-flags
branch
from
a5d2bb6 to
3bf7ff7
Compare
|
Hey @buildingisfun23 - thanks again. I've been thinking about this and I think it made sense to make one more change... I made--strict-validation fail-fast: invalid initial configs exit the process, and readiness now requires every configured source to have produced a valid config at least once. Help text got a cascading-failure warning so SREs know what they're opting into. This is opt in, and it matches similar behavior in things like envoy/nginx. See changes. WDYT? Does this make sense? Should we divide it into 2 options? I kept thinking it would be weird to have no flags if an initial sync fails, and it led be down this path. |
…en-feature#1487) Add WithStrictValidation() option for the JSON evaluator that causes flag configurations failing schema validation to be rejected with an error instead of accepted with a warning. When enabled, the flag store retains its previous state on validation failure. This is an opt-in option to avoid breaking existing behavior. Signed-off-by: buildingisfun23 <buildingisfun23@users.noreply.github.com>
Wire the existing WithStrictValidation() evaluator option through to a new --strict-validation CLI flag (and FLAGD_STRICT_VALIDATION env var) so operators can enable strict schema validation at startup without needing to configure it programmatically. Signed-off-by: buildingisfun23 <buildingisfun23@users.noreply.github.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
toddbaert
force-pushed
the
feat/1487-reject-invalid-flags
branch
from
f03059e to
0858427
Compare
|
2 participants
Summary
WithStrictValidation()option to the JSON evaluatorFixes #1487