-
Notifications
You must be signed in to change notification settings - Fork 41
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
Deprecate certificate pending #1972
Conversation
60fe7bd
to
c4a3558
Compare
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
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
@@ -433,12 +443,12 @@ components: | |||
fn test_validate_ok_when_request_without_body_and_expects_response() { | |||
APISpec::from_file(&APISpec::get_default_spec_file()) | |||
.method(Method::GET.as_str()) | |||
.path("/certificate-pending") | |||
.path("/") |
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.
It should be interesting to not use the openapi.yaml of the project for those tests.
We are just testing our tools here.
This PR would not have to modify this file if we have used a separate file or a file created by the test.
Ideally (probably too much work), we could create a refactoring PR to not use the production file in those tests and then this PR changes only what is impacted by the modification.
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.
Agree with that 👍 , as you say we should do a refactoring PR to solve that problem.
@@ -214,36 +205,6 @@ impl AggregatorClient for AggregatorHTTPClient { | |||
} | |||
} | |||
|
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.
The /certificate_pending still exist in openapi.yaml even if it was deprecated.
Is it normal to remove all tests ? What happen when the route is called ?
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.
Yes it's normal since one of this PR goal is to remove support for the certificate pending in the mithril-signer
. Its the first step that will allow to remove it from the aggregator and the openapi in the near future.
6353393
to
2cc3fd3
Compare
As the signer can now compute what to sign on its own.
As they will be removed in the future
c4a3558
to
6db1cd6
Compare
6db1cd6
to
92b0a16
Compare
* OpenApi from `0.1.31` to `0.1.32` * Mithril-aggregator from `0.5.72` to `0.5.73` * Mithril-signer from `0.2.190` to `0.2.191` * Mithril-common from `0.4.62` to `0.4.63` * Mithril-end-to-end from `0.4.32` to `0.4.33`
92b0a16
to
5cef73c
Compare
Content
This PR deprecate the
CertificatePendingMessage
both in rust code and in the openapi. It also remove the remaining certificate pending code inmithril-signer
.Pre-submit checklist
Comments
This is a follow up on #1970, so it must be merged after it.
Issue(s)
Closes #1925