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

grpcreflect: add flag to make Client try to be lenient in the face of missing dependencies #604

Merged
merged 1 commit into from
Apr 9, 2024

Conversation

jhump
Copy link
Owner

@jhump jhump commented Apr 9, 2024

Ever since v1.15 was released, the grpcreflect package inadvertently became more strict:

  1. It suddenly wraps the protobuf-go runtime's protoreflect descriptors, which means creating descriptors uses the protobuf-go runtime's protodesc package. That package is more strict and does more validation than this repo's desc package.
  2. Before v1.61.0 of the grpc-go runtime, the reflection service would send back invalid file descriptors -- "placeholder" values that represent missing dependencies. Pre-v1.15 of this repo, if the file was actually unused or only provided custom options, no problem. The desc package was doing enough validation to realize the descriptor was junk. But starting with v1.15, this would result in an error.

So, if a file was imported only for custom options (for things like OpenAPI annotations, validation options, etc), it was previously allowed to be missing. But that is no longer the case.

This has actually caused some issues for users of grpcurl and grpcui, which use the grpcreflect package in this repo. See, for example, fullstorydev/grpcurl#423, fullstorydev/grpcurl#432, fullstorydev/grpcurl#451, and fullstorydev/grpcui#279.

In these cases, prior to grpcurl pulling in v1.15 of this repo, things worked and the files that would later cause issues were effectively ignored (and okay to ignore because they were just providing custom options, not necessary for the descriptors to actually be linked). But since v1.15, they fail.

This branch adds a new method to *grpcreflect.Client, allowing callers to opt-in to lenient behavior that will effectively ignore "file not found" errors when downloading the schema from the server. If the missing file is not actually necessary to link the descriptors, the client will not complain about the missing file and instead return the requested schema (though any custom options provided by the missing files will be uninterpretable).

…descriptors even in the face of missing dependencies
@jhump jhump merged commit de71523 into main Apr 9, 2024
6 checks passed
@jhump jhump deleted the jh/allow-missing-files-in-grpcreflect-client branch April 9, 2024 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant