dapr / go-sdk

Dapr SDK for go
Apache License 2.0
446 stars 171 forks source link

Adds `DAPR_GRPC_ENPOINT` support to client #475

Closed JoshVanL closed 11 months ago

JoshVanL commented 11 months ago

PR adds DAPR_GRPC_ENPOINT environment variable support to client.

Address parser respects http[s] schemes, and TLS query options, as per 0008-S-sidecar-endpoint-tls.md.

DAPR_GRPC_ENDPONT takes precedence over DAPR_GRPC_PORT.

codecov[bot] commented 11 months ago

Codecov Report

Attention: 28 lines in your changes are missing coverage. Please review.

Comparison is base (ae8becf) 68.96% compared to head (d5e92fc) 69.71%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #475 +/- ## ========================================== + Coverage 68.96% 69.71% +0.74% ========================================== Files 34 35 +1 Lines 2707 2843 +136 ========================================== + Hits 1867 1982 +115 - Misses 732 746 +14 - Partials 108 115 +7 ``` | [Files](https://app.codecov.io/gh/dapr/go-sdk/pull/475?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dapr) | Coverage Δ | | |---|---|---| | [client/internal/parse.go](https://app.codecov.io/gh/dapr/go-sdk/pull/475?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dapr#diff-Y2xpZW50L2ludGVybmFsL3BhcnNlLmdv) | `92.37% <92.37%> (ø)` | | | [client/client.go](https://app.codecov.io/gh/dapr/go-sdk/pull/475?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dapr#diff-Y2xpZW50L2NsaWVudC5nbw==) | `65.46% <34.48%> (-4.79%)` | :arrow_down: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mikeee commented 11 months ago

Are the grpc and grpcs schemas intentional? (They're not parsed)

JoshVanL commented 11 months ago

@mikeee cpuld you elaborate? The schema should be (hopefully!) following https://github.com/dapr/proposals/blob/main/0008-S-sidecar-endpoint-tls.md

mikeee commented 11 months ago

In the parser lines 168-169 the grpc/s schemas are permitted yet not parsed as there is no handling for those schemas e.g. grpcs://localhost:443 would result in no target. (I understand this is not what the proposal includes - reading this I would be slightly confused if not for the switch statement not explicitly catching tbe rest of the valid schemas)