feat: Ignore top-level keys started with x-#5026
feat: Ignore top-level keys started with x-#5026ilia-rassadin-rn wants to merge 2 commits intothomaspoignant:mainfrom
x-#5026Conversation
✅ Deploy Preview for go-feature-flag-doc-preview canceled.
|
ilia-rassadin-rn
force-pushed
the
ir-top-level-anchors
branch
2 times, most recently
from
1dd63a0 to
961b95e
Compare
ilia-rassadin-rn
changed the title
x-x-
![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 refactors the readConfigFile function to filter out configuration keys prefixed with "x-" after parsing TOML, JSON, or YAML files. Additionally, it introduces YAML anchors for reusable targeting configurations in testdata/flag-config.yaml. The review feedback points out a typo in the newly added YAML anchor x-reusable-tartgetting, suggesting it should be x-reusable-targeting, and requests updating both the anchor definition and its alias reference accordingly.
Currently, go-feature-flag doesn't support reusable targeting conditions out of the box. So we cannot define set of rules
```yaml
my-custom-group:
- query: role eq "admin"
- query: group eq "QA"
- query: email eq "custom@example.com"
...
my-flag:
targeting:
- query: my-custom-group eq true
variation: enabled
```
But something similar can be achieved using YAML anchors. The issue with this approach is go-feature-flag treats every top-level key as a flag. So this is invalid configuration because my-custom-group is invalid flag:
```yaml
x-custom-group: &x-custom-group
- query: role eq "admin"
variation: enabled
- query: group eq "QA"
variation: enabled
- query: email eq "custom@example.com"
variation: enabled
...
my-flag:
variations:
enabled: true
disabled: false
targeting:
- *x-custom-group
defaultRule:
variation: disabled
```
It is possible to define anchors as part of the flag and then reuse it, but it lowers readability of the config file.
Usually, YAML consumers ignores keys started with `x-`. E.g. docker compose does this since 2017, openapi allows `x-` keys for extending schema since it was swagger, etc.
So this commit brings this approach into `go-feature-flag`. Now we can define top-level anchors started with `x-` and reuse these snippets to define multiple flags.
I've decided to ignore `x-` keys for JSON and TOML too. E.g. for JSON it might be used as a comment, so there is still value provided. And it is nice to be able to transparently convert flags definition between formats so they remain valid.
ilia-rassadin-rn
force-pushed
the
ir-top-level-anchors
branch
from
961b95e to
2e2ea94
Compare
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5026 +/- ##
==========================================
- Coverage 85.79% 85.76% -0.03%
==========================================
Files 153 153
Lines 6541 6542 +1
==========================================
- Hits 5612 5611 -1
- Misses 698 699 +1
- Partials 231 232 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Sorry for the delay answering this. I am wondering if the prefix should not be configurable instead of being default to What do you think @ilia-rassadin-rn ? |
2 participants
Currently, go-feature-flag doesn't support reusable targeting conditions out of the box. So we cannot define set of rules
But something similar can be achieved using YAML anchors. The issue with this approach is go-feature-flag treats every top-level key as a flag. So this is invalid configuration because my-custom-group is invalid flag:
It is possible to define anchors as part of the flag and then reuse it, but it lowers readability of the config file.
Usually, YAML consumers ignores keys started with
x-. E.g. docker compose does this since 2017, openapi allowsx-keys for extending schema since it was swagger, etc.So this commit brings this approach into
go-feature-flag. Now we can define top-level anchors started withx-and reuse these snippets to define multiple flags.I've decided to ignore
x-keys for JSON and TOML too. E.g. for JSON it might be used as a comment, so there is still value provided. And it is nice to be able to transparently convert flags definition between formats so they remain valid.Description
Closes issue(s)
Resolve #
Checklist
README.mdand/website/docs)