feat: tls connectivity for grpc syncs #398
Closed
Kavindu-Dodan wants to merge 4 commits intoopen-feature:mainfrom
Closed
feat: tls connectivity for grpc syncs #398Kavindu-Dodan wants to merge 4 commits intoopen-feature:mainfrom
Kavindu-Dodan wants to merge 4 commits intoopen-feature:mainfrom
Conversation
Kavindu-Dodan
force-pushed
the
feature/grpc-tls
branch
3 times, most recently
from
63f3b50 to
62eff66
Compare
Codecov Report
@@ Coverage Diff @@
## main #398 +/- ##
==========================================
+ Coverage 63.92% 66.64% +2.71%
==========================================
Files 11 11
Lines 1325 1364 +39
==========================================
+ Hits 847 909 +62
+ Misses 428 400 -28
- Partials 50 55 +5
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Kavindu-Dodan
force-pushed
the
feature/grpc-tls
branch
2 times, most recently
from
f17fa4a to
033b8f7
Compare
Kavindu-Dodan
force-pushed
the
feature/grpc-tls
branch
4 times, most recently
from
11e012d to
34787d2
Compare
Kavindu-Dodan
requested review from
AlexsJones,
james-milligan and
toddbaert
as code owners
Kavindu-Dodan
added a commit
that referenced
this pull request
Feb 15, 2023
## This PR Attempts to resolve #249 by introducing grpc sync provider to flagd. OFEP [approved] - https://github.com/open-feature/ofep/blob/main/OFEP-flagd-grpc-sync.md ### How to test/run ? Flagd acts as the grpc client, hence you need at least a minimal mock server. For this, you can utilize this [1] server implementation. Startup arguments of flagd now support grpc target uri. This can be provided with `grpc://` , for example, `./flagd start --uri grpc://localhost:8090` ### Technical highlights - GRPC protobuf definitions are available in buf [2] and are backed by the schema repository (https://github.com/open-feature/schemas) - Initial connection must be successful (i.e- grpc server/target must be accepting connections) - Subsequent server connection losses will not result in a runtime failure and connection re-establishment attempts will be performed ### What is not included (follow up improvements) - Connection security: This version does not enforce connection security. This will be addressed with follow-up improvements (ex:- TLS enabled connections). Hence, strongly recommends not using this version in production scenarios (fixed by #398) - Server implementations: This sync provider was designed to be open and connects to any server implementation. Hence there is no default server implementation. You may create your own server implementation based on grpc schemas. [1] - https://github.com/Kavindu-Dodan/flagd-grpc-sync-server [2] - https://buf.build/open-feature/flagd --------- Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com> Signed-off-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com> Co-authored-by: James Milligan <75740990+james-milligan@users.noreply.github.com> Co-authored-by: Skye Gill <gill.skye95@gmail.com>
Kavindu-Dodan
force-pushed
the
feature/grpc-tls
branch
2 times, most recently
from
0bd19c3 to
f39c27a
Compare
toddbaert
reviewed
Feb 15, 2023
toddbaert
reviewed
Feb 15, 2023
toddbaert
approved these changes
Feb 15, 2023
Member
toddbaert
left a comment
toddbaert
left a comment
There was a problem hiding this comment.
Left a few minor comments, but it looks good to me. Tests also seem good, uncovered lines look to be mostly unrecoverable error conditions and coverage is good otherwise.
Kavindu-Dodan
force-pushed
the
feature/grpc-tls
branch
3 times, most recently
from
7124482 to
63ae33e
Compare
toddbaert
reviewed
Feb 22, 2023
Kavindu-Dodan
force-pushed
the
feature/grpc-tls
branch
from
8da0719 to
3e20341
Compare
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Kavindu-Dodan
force-pushed
the
feature/grpc-tls
branch
from
3e20341 to
8c3eb74
Compare
skyerus
reviewed
Feb 27, 2023
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Kavindu-Dodan
force-pushed
the
feature/grpc-tls
branch
from
3ee0c9e to
ace6e23
Compare
Contributor
Author
|
Closing as diverged too much from main 😞 Will work on a fresh PR when time permits. |
raphael-wigoutschnigg-dt
pushed a commit
to open-feature-forking/flagd
that referenced
this pull request
Mar 11, 2025
Signed-off-by: Matt Vinall <boyvinall@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR
Note - based on open pr #297
Introduce TLS connectivity for GRPC sync provider.
TLS can be enabled using schema
grpcs://. For example,./flagd start --uri grpcs://localhost:8090Further, a self-sign certificate can be provided for TLS connectivity using startup arguments
-g, --grpc-cert-path string. For example,./flagd start --uri grpcs://localhost:8090 -g <CA_CERT>Additional
How to test
Start mock server impl - https://github.com/Kavindu-Dodan/flagd-grpc-sync-server & then run flagd with grpc tls