Skip to content
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

fix: only propagate FTL headers #3358

Merged

Conversation

stuartwdouglas
Copy link
Collaborator

No description provided.

@stuartwdouglas stuartwdouglas added the run-all A PR with this label will run the full set of CI jobs in the PR rather than in the merge queue label Nov 7, 2024
This was referenced Nov 7, 2024
var headers = []string{DirectRoutingHeader, VerbHeader, RequestIDHeader, ParentRequestIDHeader}

// CopyRequest creates a new request with the same message as the original, but with only the FTL specific headers
func CopyRequest[T any](req *connect.Request[T]) *connect.Request[T] {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nit, this sounds like a complete copy rather than what it is? Maybe CopyRequestForForwarding?

@@ -853,7 +853,7 @@ func (s *Service) AcquireLease(ctx context.Context, stream *connect.BidiStream[f
}

func (s *Service) Call(ctx context.Context, req *connect.Request[ftlv1.CallRequest]) (*connect.Response[ftlv1.CallResponse], error) {
return s.callWithRequest(ctx, req, optional.None[model.RequestKey](), optional.None[model.RequestKey](), "")
return s.callWithRequest(ctx, headers.CopyRequest(req), optional.None[model.RequestKey](), optional.None[model.RequestKey](), "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@stuartwdouglas stuartwdouglas force-pushed the stuartwdouglas/really-fix-rbac-for-real-this-time branch from 80e8d89 to 244adf6 Compare November 7, 2024 22:19
@stuartwdouglas stuartwdouglas enabled auto-merge (squash) November 7, 2024 22:23
@stuartwdouglas stuartwdouglas merged commit c8a2244 into main Nov 7, 2024
93 of 94 checks passed
@stuartwdouglas stuartwdouglas deleted the stuartwdouglas/really-fix-rbac-for-real-this-time branch November 7, 2024 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-all A PR with this label will run the full set of CI jobs in the PR rather than in the merge queue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants