Skip to content

Commit

Permalink
Merge pull request #1972 from input-output-hk/djo/1925/deprecate_cert…
Browse files Browse the repository at this point in the history
…ificate_pending

Deprecate certificate pending
  • Loading branch information
Alenar authored Oct 3, 2024
2 parents 7e0b764 + 5cef73c commit a80a413
Show file tree
Hide file tree
Showing 17 changed files with 64 additions and 357 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ As a minor extension, we have adopted a slightly different versioning convention

- Signer compute what to sign independently of the aggregator.

- Deprecate aggregator `/certificate-pending` route as the signer does not need it anymore.

- Crates versions:

| Crate | Version |
Expand Down
8 changes: 4 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion mithril-aggregator/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "mithril-aggregator"
version = "0.5.72"
version = "0.5.73"
description = "A Mithril Aggregator server"
authors = { workspace = true }
edition = { workspace = true }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![allow(deprecated)]
use mithril_common::entities::CardanoDbBeacon;
use mithril_common::{
entities::{CertificatePending, SignedEntityType},
Expand Down
2 changes: 1 addition & 1 deletion mithril-common/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "mithril-common"
version = "0.4.62"
version = "0.4.63"
description = "Common types, interfaces, and utilities for Mithril nodes."
authors = { workspace = true }
edition = { workspace = true }
Expand Down
5 changes: 5 additions & 0 deletions mithril-common/src/messages/certificate_pending.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
#![allow(deprecated)]
use serde::{Deserialize, Serialize};

use crate::entities::{CardanoDbBeacon, Epoch, ProtocolParameters, SignedEntityType};
use crate::messages::SignerMessagePart;

/// Structure to transport [crate::entities::CertificatePending] data.
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
#[deprecated(
since = "0.4.63",
note = "Exists only for backward-compatibility, will be removed in the future"
)]
pub struct CertificatePendingMessage {
/// Current Epoch
pub epoch: Epoch,
Expand Down
1 change: 1 addition & 0 deletions mithril-common/src/messages/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ pub use certificate::CertificateMessage;
pub use certificate_list::{
CertificateListItemMessage, CertificateListItemMessageMetadata, CertificateListMessage,
};
#[allow(deprecated)]
pub use certificate_pending::CertificatePendingMessage;
pub use epoch_settings::EpochSettingsMessage;
pub use interface::*;
Expand Down
52 changes: 31 additions & 21 deletions mithril-common/src/test_utils/apispec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ mod tests {

use super::*;
use crate::entities;
use crate::messages::{CertificatePendingMessage, SignerMessagePart};
use crate::messages::{AggregatorFeaturesMessage, SignerMessagePart};
use crate::test_utils::{fake_data, TempDir};

fn build_empty_response(status_code: u16) -> Response<Bytes> {
Expand Down Expand Up @@ -420,9 +420,19 @@ components:

#[test]
fn test_validate_a_response_without_body() {
APISpec::from_file(&APISpec::get_default_spec_file())
let file = get_temp_openapi_filename("validate_a_response_without_body", line!());
let paths = r#"
/empty-route:
get:
responses:
"204":
description: no pending certificate available
"#;
write_minimal_open_api_file("1.0.0", &file, paths, "");

APISpec::from_file(file.to_str().unwrap())
.method(Method::GET.as_str())
.path("/certificate-pending")
.path("/empty-route")
.validate_request(&Null)
.unwrap()
.validate_response(&build_empty_response(204))
Expand All @@ -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("/")
.validate_request(&Null)
.unwrap()
.validate_response(&build_json_response(
200,
CertificatePendingMessage::dummy(),
AggregatorFeaturesMessage::dummy(),
))
.unwrap();
}
Expand Down Expand Up @@ -480,7 +490,7 @@ components:
let mut api_spec = APISpec::from_file(&APISpec::get_default_spec_file());
let result = api_spec
.method(Method::GET.as_str())
.path("/certificate-pending")
.path("/")
.validate_request(&Null)
.unwrap()
.validate_status(&response, &StatusCode::OK);
Expand All @@ -505,7 +515,7 @@ components:

APISpec::from_file(&APISpec::get_default_spec_file())
.method(Method::GET.as_str())
.path("/certificate-pending")
.path("/")
.validate_request(&Null)
.unwrap()
.validate_status(&response, &StatusCode::INTERNAL_SERVER_ERROR)
Expand All @@ -532,21 +542,21 @@ components:
let mut api_spec = APISpec::from_file(&APISpec::get_default_spec_file());
let result = api_spec
.method(Method::OPTIONS.as_str())
.path("/certificate-pending")
.path("/certificates")
.validate_response(&build_response(200, b"abcdefgh"));

assert!(result.is_err());
assert_eq!(
result.err(),
Some("Unmatched path and method: /certificate-pending OPTIONS".to_string())
Some("Unmatched path and method: /certificates OPTIONS".to_string())
);
}
#[test]
fn test_validate_returns_error_when_route_exists_but_expects_non_empty_response() {
let mut api_spec = APISpec::from_file(&APISpec::get_default_spec_file());
let result = api_spec
.method(Method::GET.as_str())
.path("/certificate-pending")
.path("/certificates")
.validate_response(&build_empty_response(200));

assert!(result.is_err());
Expand Down Expand Up @@ -588,7 +598,7 @@ components:
let mut api_spec = APISpec::from_file(&APISpec::get_default_spec_file());
let result = api_spec
.method(Method::GET.as_str())
.path("/certificate-pending")
.path("/certificates")
.validate_request(&Null)
.unwrap()
.validate_response(&build_response(200, b"not a json"));
Expand All @@ -602,7 +612,7 @@ components:
fn test_validate_returns_errors_when_route_exists_but_does_not_expect_request_body() {
assert!(APISpec::from_file(&APISpec::get_default_spec_file())
.method(Method::GET.as_str())
.path("/certificate-pending")
.path("/certificates")
.validate_request(&fake_data::beacon())
.is_err());
}
Expand All @@ -620,7 +630,7 @@ components:
let mut api_spec = APISpec::from_file(&APISpec::get_default_spec_file());
let result = api_spec
.method(Method::GET.as_str())
.path("/certificate-pending")
.path("/certificates")
.content_type("whatever")
.validate_request(&Null)
.unwrap()
Expand All @@ -629,7 +639,7 @@ components:
assert!(result.is_err());
assert_eq!(
result.err().unwrap().to_string(),
"Expected content type 'whatever' but spec is '{\"application/json\":{\"schema\":{\"$ref\":\"#/components/schemas/CertificatePendingMessage\"}}}'",
"Expected content type 'whatever' but spec is '{\"application/json\":{\"schema\":{\"$ref\":\"#/components/schemas/CertificateListMessage\"}}}'",
);
}

Expand Down Expand Up @@ -707,24 +717,24 @@ components:
APISpec::verify_conformity(
APISpec::get_all_spec_files(),
Method::GET.as_str(),
"/certificate-pending",
"/",
"application/json",
&Null,
&build_json_response(200, CertificatePendingMessage::dummy()),
&build_json_response(200, AggregatorFeaturesMessage::dummy()),
&StatusCode::OK,
)
.unwrap()
}

#[test]
fn test_verify_conformity_with_non_expected_status_returns_error() {
let response = build_json_response(200, CertificatePendingMessage::dummy());
let response = build_json_response(200, AggregatorFeaturesMessage::dummy());

let spec_file = APISpec::get_default_spec_file();
let result = APISpec::verify_conformity(
vec![spec_file.clone()],
Method::GET.as_str(),
"/certificate-pending",
"/",
"application/json",
&Null,
&response,
Expand All @@ -737,7 +747,7 @@ components:
StatusCode::OK.as_u16()
);
let error_message = format!(
"OpenAPI invalid response in {spec_file} on route /certificate-pending, reason: {error_reason}\nresponse: {response:#?}"
"OpenAPI invalid response in {spec_file} on route /, reason: {error_reason}\nresponse: {response:#?}"
);
assert!(result.is_err());
assert_eq!(result.err().unwrap().to_string(), error_message);
Expand All @@ -748,10 +758,10 @@ components:
let result = APISpec::verify_conformity(
vec![],
Method::GET.as_str(),
"/certificate-pending",
"/",
"application/json",
&Null,
&build_json_response(200, CertificatePendingMessage::dummy()),
&build_json_response(200, AggregatorFeaturesMessage::dummy()),
&StatusCode::OK,
);

Expand Down
2 changes: 1 addition & 1 deletion mithril-signer/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "mithril-signer"
version = "0.2.190"
version = "0.2.191"
description = "A Mithril Signer"
authors = { workspace = true }
edition = { workspace = true }
Expand Down
4 changes: 1 addition & 3 deletions mithril-signer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ pub mod store;

pub use configuration::{Configuration, DefaultConfiguration};
pub use entities::SignerEpochSettings;
pub use message_adapters::{
FromEpochSettingsAdapter, FromPendingCertificateMessageAdapter, ToRegisterSignerMessageAdapter,
};
pub use message_adapters::{FromEpochSettingsAdapter, ToRegisterSignerMessageAdapter};
pub use metrics::*;
pub use runtime::*;

Expand Down

This file was deleted.

2 changes: 0 additions & 2 deletions mithril-signer/src/message_adapters/mod.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
mod from_epoch_settings;
mod from_pending_certificate_message;
mod to_register_signature_message;
mod to_register_signer_message;

pub use from_epoch_settings::FromEpochSettingsAdapter;
pub use from_pending_certificate_message::FromPendingCertificateMessageAdapter;
pub use to_register_signature_message::ToRegisterSignatureMessageAdapter;
pub use to_register_signer_message::ToRegisterSignerMessageAdapter;
Loading

0 comments on commit a80a413

Please sign in to comment.