Add Invoice skip list #176
Merged
+156
−32
Conversation
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
sputn1ck
force-pushed
the
invoice_skip_list
branch
from
86ddf38 to
0b5dd11
Compare
bhandras
reviewed
May 14, 2025
proxy/service.go
Outdated
| // given request. | ||
| func (s *Service) SkipInvoiceCreation(r *http.Request) bool { | ||
| for _, pathRegexp := range s.AuthSkipInvoiceCreationPaths { | ||
| pathRegexp := regexp.MustCompile(pathRegexp) |
There was a problem hiding this comment.
Perhaps this could be precompiled for better performance, see:
func MustCompile(str string) *Regexp {
regexp, err := Compile(str)
if err != nil {
panic(`regexp: Compile(` + quote(str) + `): ` + err.Error())
}
return regexp
}
There was a problem hiding this comment.
Can probably rebase this on the other PR and then use pre-compiled patterns here too.
bhandras
reviewed
May 14, 2025
proxy/proxy.go
Outdated
| @@ -182,6 +183,14 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |||
| // resources. | |||
| acceptAuth := p.authenticator.Accept(&r.Header, resourceName) | |||
| if !acceptAuth { | |||
| if skipInvoiceCreation { | |||
| addCorsHeaders(w.Header()) | |||
| sendDirectResponse(w, r, http.StatusUnauthorized, | |||
sputn1ck
force-pushed
the
invoice_skip_list
branch
2 times, most recently
from
e9ab581 to
1cda069
Compare
guggero
approved these changes
May 14, 2025
| @@ -135,6 +135,12 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |||
| return | |||
| } | |||
|
|
|||
| // If the request is a gRPC request, we need to set the Content-Type | |||
| // header to application/grpc. | |||
| if strings.HasPrefix(r.Header.Get(hdrContentType), hdrTypeGrpc) { | |||
There was a problem hiding this comment.
Hmm, this might fix a long-standing error that we see sometimes. I just hope it doesn't introduce any side effects (I remember the whole header handling for gRPC being super brittle with many weird behaviors).
proxy/service.go
Outdated
| // given request. | ||
| func (s *Service) SkipInvoiceCreation(r *http.Request) bool { | ||
| for _, pathRegexp := range s.AuthSkipInvoiceCreationPaths { | ||
| pathRegexp := regexp.MustCompile(pathRegexp) |
There was a problem hiding this comment.
Can probably rebase this on the other PR and then use pre-compiled patterns here too.
Prior we would not set the Content-Type header for grpc request, which on failure events would cause the response to be invalid gRPC responses, as the grpc code path in sendDirectResponse expects the Content-Type header to be set. This change fixes the problem by setting the Content-Type header to application/grpc for gRPC requests.
sputn1ck
force-pushed
the
invoice_skip_list
branch
from
1cda069 to
9023abc
Compare
guggero
approved these changes
May 14, 2025
sputn1ck
force-pushed
the
invoice_skip_list
branch
from
9023abc to
b1b0dc5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 adds a new list for the config which will allow skipping the creation of invoices on these specific calls.
This is useful for streaming rpcs, as we can't handle the l402 handshake there (yet?).