-
Notifications
You must be signed in to change notification settings - Fork 347
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
gatewayapi: validate tls secret in listener certificateRef #868
gatewayapi: validate tls secret in listener certificateRef #868
Conversation
bcaf619
to
0855114
Compare
Codecov Report
@@ Coverage Diff @@
## main #868 +/- ##
==========================================
+ Coverage 63.89% 63.99% +0.09%
==========================================
Files 50 51 +1
Lines 6523 6526 +3
==========================================
+ Hits 4168 4176 +8
+ Misses 2093 2090 -3
+ Partials 262 260 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Ok, sorry for all the pushed commits and fixups. I should finish my coffee before doing early morning PR's 😄. I fixed up the lint issue reported by CI and the broken gatewayapi tests. With the new validation logic a few of the tests with TLS now require valid tls secrets for the inputs and expected outputs so I have adjusted those test so they match the new tls validation behavior. |
Looks like my fixup commits didn't get the DCO. Let me know if you need me to squash those or I think the project squash merges so that should take care of them. |
@LanceEa each commit needs to be signed for the DCO job to pass. |
Previously, the tls secret referenced in a listener certifcateRef would only validate that data existed within the `tls.key` and `tls.cert` fields. This adds additional validation to ensure the data in the `tls.cert` is a valid pem encoded cert and the `tls.key` contains a valid pem encoded private key. The GatewayInvalidTLSConfiguration conformance test can now be enabled and passes. Signed-off-by: Lance Austin <laustin@datawire.io>
daefedf
to
e73ac3d
Compare
Sorry for the pain with the forced push, I went ahead and squashed the fixup commits to address the DCO and addressed feedback. I will be sure to get the DCO applied to fixup commits too, in the future. |
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.
LGTM, thanks for fixing this !
Previously, the tls secret referenced in a listener certifcateRef would only validate that data existed within the
tls.key
andtls.cert
fields. This adds additional validation to ensure the data in thetls.cert
is a valid pem encoded cert and thetls.key
contains a valid pem encoded private key. The GatewayInvalidTLSConfiguration conformance test can now be enabled and passes.Fixes #783