-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
xds: fix the validation code to accept new-style CertificateProviderPluginInstance wherever used #8892
Conversation
…luginInstance wherever used
The fix in this PR verified as reported in #8885 (comment) |
private static boolean hasValidationProviderInstance(CommonTlsContext commonTlsContext) { | ||
return (commonTlsContext.hasValidationContext() && commonTlsContext.getValidationContext() | ||
.hasCaCertificateProviderInstance()) | ||
|| commonTlsContext.hasValidationContextCertificateProviderInstance(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this method hasValidationProviderInstance
is only used once, why not use it inline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to other functions defined here e.g. hasIdentityCertificateProviderInstance
or hasCertProviderValidationContext
. If I inline all functions used only once I'll end up with complex functions (with successive such inlining). In general if it helps in understanding the code I think it is good to have separate, appropriately named functions. The compiler will anyway inline such functions so I don't see the downside.
xds/src/main/java/io/grpc/xds/internal/sds/CommonTlsContextUtil.java
Outdated
Show resolved
Hide resolved
…luginInstance wherever used (grpc#8892)
…luginInstance wherever used (grpc#8892)
…luginInstance wherever used (grpc#8892)
Update
CommonTlsContextUtil
validation functions to accept new-styleCertificateProviderPluginInstance
fixes #8885