-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: FlagD Provider #19
Conversation
This work is in progress and in internal review, - Envoy nameresolver, testcontainer impl does not work yet Signed-off-by: Eren Atas <[email protected]>
Signed-off-by: Eren Atas <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, this is very impressive. Nice job @erenatas!
Honestly, a lot of this is beyond my Rust skills, so I mainly focused on the configurations, documentation, and general structure.
@@ -1,9 +1,45 @@ | |||
[package] | |||
name = "open-feature-flagd" | |||
version = "0.1.0" | |||
version = "0.0.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a follow up, we'll want to wire up Release Please so this version can be automatically managed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add something like
name: Release Please
on:
push:
branches:
- main
jobs:
release:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Generate Release Pull Request
uses: googleapis/release-please-action@v2
with:
token: ${{ secrets.GITHUB_TOKEN }}
release-type: cargo
If that sounds good?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can handle this in a follow-up pr. I have an embarrassing amount of experience with release please at this point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also update rust build yaml to install protoc however I believe I need approval to test.
assert_eq!(struct_result.value.fields["number"], Value::Int(42)); | ||
} | ||
|
||
// TODO: MAKE THIS WORK |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this relevant in this PR or is it something to follow up on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to get this to work, but I couldn't. I would like to have 2 flagd containers where I would like to pass one of them to the other as a source with a selector so that I can E2E test selector functionality but I couldn't get this to work as much as I tried. I tried to replicate what is written in flagd documentation, I couldn't get it to work. I either get something like: "timeout while waiting for all sync sources to complete their initial sync. continuing sync service", or "sync server stopped". I tried doing:
$ echo '{
"$schema": "https://flagd.dev/schema/v0/flags.json",
"flags": {
"scoped-flag": {
"state": "ENABLED",
"variants": {
"on": true,
"off": false
},
"defaultVariant": "on",
"source": "test-scope"
}
}
}' > config.json
$ docker run --network flagd-net -v $(pwd)/config.json:/etc/flagd/config.json ghcr.io/open-feature/flagd start --port 8013 --sources='[{"uri":"/etc/flagd/config.json","provider":"file"},{"uri":"flagd-source:8013","provider":"grpc","selector":"test-scope"}]'
$ docker run --network flagd-net -v $(pwd)/config.json:/etc/flagd/config.json -p 8018:8013 -p 8020:8015 -p 8021:8016 ghcr.io/open-feature/flagd start --port 8013 --sources='[{"uri":"/etc/flagd/config.json","provider":"file"},{"uri":"flagd-source:8016/flags.json","provider":"http","selector":"test-scope"}]'
It didn't work unfortunately. If you have any tips I would be very happy to hear.
Signed-off-by: Eren Atas <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not done yet
@@ -1,9 +1,45 @@ | |||
[package] | |||
name = "open-feature-flagd" | |||
version = "0.1.0" | |||
version = "0.0.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add something like
name: Release Please
on:
push:
branches:
- main
jobs:
release:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Generate Release Pull Request
uses: googleapis/release-please-action@v2
with:
token: ${{ secrets.GITHUB_TOKEN }}
release-type: cargo
If that sounds good?
crates/flagd/src/resolver/rest.rs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once I understand what's difference of an "OFREP" provider exactly I doubt it would be that difficult, however is it ok if we do this separately? I would be happy to work on it and abstract this part away from this crate.
Another thing is as I stated over the PR description I originally intended to generate the client over openapi yaml however I could not find a suitable client generator that supports 3.1.0 yet. From what I have seenprogenitor
is most used client generator and support is not there yet.
assert_eq!(struct_result.value.fields["number"], Value::Int(42)); | ||
} | ||
|
||
// TODO: MAKE THIS WORK |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to get this to work, but I couldn't. I would like to have 2 flagd containers where I would like to pass one of them to the other as a source with a selector so that I can E2E test selector functionality but I couldn't get this to work as much as I tried. I tried to replicate what is written in flagd documentation, I couldn't get it to work. I either get something like: "timeout while waiting for all sync sources to complete their initial sync. continuing sync service", or "sync server stopped". I tried doing:
$ echo '{
"$schema": "https://flagd.dev/schema/v0/flags.json",
"flags": {
"scoped-flag": {
"state": "ENABLED",
"variants": {
"on": true,
"off": false
},
"defaultVariant": "on",
"source": "test-scope"
}
}
}' > config.json
$ docker run --network flagd-net -v $(pwd)/config.json:/etc/flagd/config.json ghcr.io/open-feature/flagd start --port 8013 --sources='[{"uri":"/etc/flagd/config.json","provider":"file"},{"uri":"flagd-source:8013","provider":"grpc","selector":"test-scope"}]'
$ docker run --network flagd-net -v $(pwd)/config.json:/etc/flagd/config.json -p 8018:8013 -p 8020:8015 -p 8021:8016 ghcr.io/open-feature/flagd start --port 8013 --sources='[{"uri":"/etc/flagd/config.json","provider":"file"},{"uri":"flagd-source:8016/flags.json","provider":"http","selector":"test-scope"}]'
It didn't work unfortunately. If you have any tips I would be very happy to hear.
Signed-off-by: Eren Atas <[email protected]>
Signed-off-by: Eren Atas <[email protected]>
Signed-off-by: Eren Atas <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless someone objects, I'm going to try and merge this tomorrow. It's still marked as experimental, so we can make changes as necessary but at least we can tackle them in small chunks.
Before squash, I would appreacite if you add my colleagues' mails who helped review:
I would like to note down here that I appreciate all of their help! |
Signed-off-by: Eren Atas <[email protected]>
Signed-off-by: Eren Atas <[email protected]>
@beeme1mr build+tests passed after recursive submodule pull, two notes:
I really couldn't figure out how to pass all of them at once properly, I really tried |
Signed-off-by: Eren Atas <[email protected]>
9457450
to
4304e71
Compare
This PR
Implements FlagD provider, including support for RPC evaluation, In-Process, OFREP and Offline modes.
Configuration and relevant feature set should be 100% compliant with testbed. Includes E2E testing done with testcontainers-rs and flagd and envoy containers.
Notes
Linked issues
Should resolve #5
How to test
Follow instructions on README.md.