advancedtls: unexport parts of API not meant to be public #7118
Conversation
gtcooke94
added
Type: Internal Cleanup
| @@ -192,25 +188,25 @@ func x509NameHash(r pkix.RDNSequence) string { | |||
| return fmt.Sprintf("%08x", fileHash) | |||
| } | |||
|
|
|||
| // CheckRevocation checks the connection for revoked certificates based on RFC5280. | |||
| // checkRevocation checks the connection for revoked certificates based on RFC5280. | |||
There was a problem hiding this comment.
I don't see any usages of it inside the codebase, but it looks like it can be used as an embedded part of tls.Config. In case of OSS users use it that way, would it make sense just to mark it as deprecated (in favor of new CRL solution) and remove later?
There was a problem hiding this comment.
Can you link what you mean? I didn't see any usage in grpc-go or in google
There was a problem hiding this comment.
grpc-go/security/advancedtls/crl_test.go
Line 739 in 0baa668
There was a problem hiding this comment.
For me, I think we should unexport - we don't want anyone depending on this. If someone is using it in this way currently, they can stay on a version of the advancedTLS package that has it until they move off.
It's not really something to deprecate, rather it's an internal API that was written with a capital letter
@dfawley what do you think here?
dfawley
changed the title
ServerNameOverride is test-only
RevocationStatus, CheckRevocation, and CheckChainRevocation are not used externally and should not be exported
RELEASE NOTES: none