Skip to content

tls: remove stale comment#23846

Merged
lizan merged 1 commit intoenvoyproxy:mainfrom
davidben:stale-comment
Nov 7, 2022
Merged

tls: remove stale comment#23846
lizan merged 1 commit intoenvoyproxy:mainfrom
davidben:stale-comment

Conversation

@davidben
Copy link
Copy Markdown
Contributor

@davidben davidben commented Nov 4, 2022

Commit Message:
Both things the comment says are inaccurate: After #23320, this mechanism no longer uses the X509_STORE callback, but a flag. After #21417, Envoy is also not (always) using
SSL_CTX_set_cert_verify_callback, but a different one.

Just remove the comment altogether, as it doesn't seem to provide any value. All it's saying is that, because Envoy still uses store at the end of the day, configuring things on store does stuff. But the whole function configures things on store, so it's not specific to that block anyway.

Signed-off-by: David Benjamin davidben@google.com

Additional Description:
Risk Level: none (comment-only change)
Testing: CI (comment-only change)
Docs Changes: n/a (comment-only change)
Release Notes: n/a (comment-only change)
Platform Specific Features: n/a (comment-only change)
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Both things the comment says are inaccurate: After envoyproxy#23320, this
mechanism no longer uses the X509_STORE callback, but a flag.
After envoyproxy#21417, Envoy is also not (always) using
SSL_CTX_set_cert_verify_callback, but a different one.

Just remove the comment altogether, as it doesn't seem to provide any
value. All it's saying is that, because Envoy still uses `store` at the
end of the day, configuring things on `store` does stuff. But the whole
function configures things on `store`, so it's not specific to that
block anyway.

Signed-off-by: David Benjamin <davidben@google.com>
@KBaichoo
Copy link
Copy Markdown
Contributor

KBaichoo commented Nov 5, 2022

/assign @lizan

@lizan lizan merged commit 19636ab into envoyproxy:main Nov 7, 2022
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.

3 participants