rls: propagate headers received in RLS response to backends #5883
Conversation
balancer/rls/picker.go
Outdated
| return res, err | ||
| } | ||
| if res.Metatada == nil { | ||
| res.Metatada = metadata.Pairs("x-google-rls-data", dcEntry.headerData) |
balancer/rls/picker_test.go
Outdated
| EmptyCallF: func(ctx context.Context, in *testpb.Empty) (*testpb.Empty, error) { | ||
| gotHeaderData := metadata.ValueFromIncomingContext(ctx, "x-google-rls-data") | ||
| if len(gotHeaderData) != 1 || gotHeaderData[0] != headerDataContents { | ||
| rpcErrCh <- fmt.Errorf("got metadata in `X-Google-RLS-Data` is %v, want %s", gotHeaderData, headerDataContents) |
There was a problem hiding this comment.
It seems like you could simplify this by just returning the error directly from the RPC?
balancer/rls/picker_test.go
Outdated
| t.Fatalf("Failed to start backend: %v", err) | ||
| } | ||
| t.Logf("Started TestService backend at: %q", backend.Address) | ||
| t.Cleanup(func() { backend.Stop() }) |
There was a problem hiding this comment.
defer backend.Stop()? Or t.Cleanup(backend.Stop) instead?
|
|
||
| // Start a test backend which expects the header data contents sent from the | ||
| // RLS server to be part of RPC metadata as X-Google-RLS-Data header. | ||
| const headerDataContents = "foo,bar,baz" |
There was a problem hiding this comment.
I find this test case particularly interesting since header values are comma separated, aren't they?
There was a problem hiding this comment.
The actual value does not matter to the LB policy. So, I just put something in :)
Do you have a recommendation for a better value?
There was a problem hiding this comment.
No, I just thought it was interesting.
Just to confirm: this feature allows us to set exactly one header to exactly one value, right? I.e. if the config has commas as the value, we aren't expected to add multiple values to the same header key, right?
There was a problem hiding this comment.
this feature allows us to set exactly one header to exactly one value, right?
That's right.
|
|
||
| // Start a test backend which expects the header data contents sent from the | ||
| // RLS server to be part of RPC metadata as X-Google-RLS-Data header. | ||
| const headerDataContents = "foo,bar,baz" |
There was a problem hiding this comment.
No, I just thought it was interesting.
Just to confirm: this feature allows us to set exactly one header to exactly one value, right? I.e. if the config has commas as the value, we aren't expected to add multiple values to the same header key, right?
Follow-up from #5853.
RELEASE NOTES: