yarpc / yarpc-go

A message passing platform for Go
MIT License
401 stars 101 forks source link

tls-outbounds: make spiffe ids field as optional field #2216

Closed jronak closed 1 year ago

jronak commented 1 year ago

Earlier, tls outbound required at least one spiffe id as we wanted to use it to match the server id. Since managing server spiffe id across clients causes issues during the migration, this PR makes spiffe ids an optional field. Outbounds will accept any server certificate issued by the internal CA with any spiffe id if no spiffe ids configuration is provided.

codecov[bot] commented 1 year ago

Codecov Report

Patch and project coverage have no change.

Comparison is base (f345554) 85.36% compared to head (05f91cd) 85.36%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #2216 +/- ## ======================================= Coverage 85.36% 85.36% ======================================= Files 270 270 Lines 15479 15473 -6 ======================================= - Hits 13214 13209 -5 + Misses 1844 1843 -1 Partials 421 421 ``` | [Impacted Files](https://app.codecov.io/gh/yarpc/yarpc-go/pull/2216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yarpc) | Coverage Δ | | |---|---|---| | [transport/grpc/config.go](https://app.codecov.io/gh/yarpc/yarpc-go/pull/2216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yarpc#diff-dHJhbnNwb3J0L2dycGMvY29uZmlnLmdv) | `97.26% <ø> (-0.04%)` | :arrow_down: | | [transport/http/config.go](https://app.codecov.io/gh/yarpc/yarpc-go/pull/2216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yarpc#diff-dHJhbnNwb3J0L2h0dHAvY29uZmlnLmdv) | `95.65% <ø> (-0.10%)` | :arrow_down: | | [transport/tchannel/config.go](https://app.codecov.io/gh/yarpc/yarpc-go/pull/2216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yarpc#diff-dHJhbnNwb3J0L3RjaGFubmVsL2NvbmZpZy5nbw==) | `93.65% <ø> (-0.20%)` | :arrow_down: | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/yarpc/yarpc-go/pull/2216/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yarpc)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

lambdai commented 1 year ago

Can you also add into the description: When the empty spiffie id config is provided, what new server config is accepted

My interpretation is that it allow server providing any legit spiffie id but still need to be a legit spiffied id format

jronak commented 1 year ago

Can you also add into the description: When the empty spiffie id config is provided, what new server config is accepted

My interpretation is that it allow server providing any legit spiffie id but still need to be a legit spiffied id format

Updated, thanks!