-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Generate certificates at runtime for runner test #3527
Conversation
@@ -937,7 +945,6 @@ func GenerateTLSCertificate(t *testing.T, host string, notBefore time.Time, vali | |||
NotAfter: notAfter, | |||
|
|||
KeyUsage: keyUsage, | |||
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, |
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.
Beyond the new logic to have support for CA cert & key (if parent == nil { ... }
), this is the only line from the original method that I have modified (removed), because with this line the test doesn't work (because the generated cert cannot be used to sign later) and without it everything works as expected, so 👌🏻
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3527 +/- ##
==========================================
- Coverage 72.55% 72.50% -0.05%
==========================================
Files 276 274 -2
Lines 20840 20835 -5
==========================================
- Hits 15120 15107 -13
- Misses 4758 4762 +4
- Partials 962 966 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
What?
It modifies the
TestVUIntegrationClientCerts
test onrunner_test.go
in order to generate certificates at runtime, instead of using pre-generated ones.Why?
This test is currently failing when
runtime.GOOS == "darwin"
, because it expects the"certificate signed by unknown authority"
error but it gets"certificate is not standards compliant"
instead, because the pre-generated certificate has a validity of 1000 years since 2022, and that's not considered standards compliant by Apple.So, instead of having a pre-generated certificate that expires every year (so we'd need to update it every year), this makes the test to generate the required certificates at runtime (so always up to date), which also seems to be fine for MacOS.
Checklist
make lint
) and all checks pass.make tests
) and all tests pass.Related PR(s)/Issue(s)
I think it fixes #2578, cause with this I no longer see errors on MacOS.