Skip to content

Commit 8372d55

Browse files
sunshowerssunshowers
authored
add versioned tests that skip middle version + introduce incompatible change (#7)
Also fix a bug + add a test that if you delete a symlink, it is recreated to the expected location (which is the blessed hash, not the generated hash). Tricky!
1 parent 71ecafc commit 8372d55

5 files changed

Lines changed: 601 additions & 114 deletions

File tree

‎CLAUDE.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# Instructions for dropshot-api-manager
2+
3+
## General instructions
4+
5+
* Always use `cargo nextest run` to run tests. Never use `cargo test`.
6+
* Wrap comments to 80 characters.
7+
* Always end comments with a period.

‎crates/dropshot-api-manager/src/resolved.rs‎

Lines changed: 111 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -672,7 +672,19 @@ fn resolve_api<'a>(
672672
let latest_generated = api_generated.latest_link().expect(
673673
"\"generated\" source should always have a \"latest\" link",
674674
);
675-
let symlink = match api_local.and_then(|l| l.latest_link()) {
675+
let generated_version =
676+
latest_generated.version().expect("versioned APIs have a version");
677+
let resolution =
678+
by_version.get(generated_version).unwrap_or_else(|| {
679+
panic!(
680+
"by_version map should have a version \
681+
corresponding to latest_generated ({})",
682+
latest_generated
683+
)
684+
});
685+
686+
let latest_local = api_local.and_then(|l| l.latest_link());
687+
let symlink = match latest_local {
676688
Some(latest_local) => {
677689
if latest_local == latest_generated {
678690
None
@@ -689,95 +701,123 @@ fn resolve_api<'a>(
689701
// 1. latest_local is blessed, latest_generated has the same
690702
// version as latest_local, and it has wire-compatible
691703
// changes. In that case, don't update the symlink.
704+
//
692705
// 2. latest_local is blessed, latest_generated has the same
693706
// version as latest_local, and latest_generated has
694707
// wire-*incompatible* changes. In that case, we'd have
695708
// returned errors in the by_version map above, and we
696709
// wouldn't want to update the symlink in any case.
710+
//
697711
// 3. latest_local is blessed, and latest_generated is
698712
// blessed but a *different* version. This means that
699713
// the latest version was retired. In this case,
700714
// we want to update the symlink to the blessed hash
701715
// corresponding to the latest generated version.
716+
//
702717
// 4. latest_local is not blessed. In that case, we do
703718
// want to update the symlink.
704-
let generated_version = latest_generated
705-
.version()
706-
.expect("versioned APIs have a version");
707719
let local_version = latest_local
708720
.version()
709721
.expect("versioned APIs have a version");
710-
if let Some(resolution) = by_version.get(generated_version)
711-
{
712-
match resolution.kind() {
713-
ResolutionKind::Lockstep => {
714-
unreachable!("this is a versioned API");
715-
}
716-
// Case 1 and 2 above.
717-
ResolutionKind::Blessed
718-
if generated_version == local_version =>
719-
{
720-
// latest_generated is blessed and the same
721-
// version as latest_local, so don't update the
722-
// symlink.
723-
None
724-
}
725-
ResolutionKind::Blessed => {
726-
// latest_generated is blessed, and has a
727-
// different version from latest_local. In this
728-
// case, we want to update the symlink to the
729-
// blessed version matching latest_generated
730-
// (not latest_generated, in case it's different
731-
// from the blessed version in a wire-compatible
732-
// way!)
733-
let api_blessed =
734-
api_blessed.unwrap_or_else(|| {
735-
panic!(
736-
"for {}, Blessed means \
737-
api_blessed exists",
738-
api.ident()
739-
)
740-
});
741-
let blessed = api_blessed
742-
.versions()
743-
.get(generated_version)
744-
.unwrap_or_else(|| {
745-
panic!(
746-
"for {} v{}, Blessed means \
747-
generated_version exists",
748-
api.ident(),
749-
generated_version
750-
);
751-
});
752-
Some(Problem::LatestLinkStale {
753-
api_ident: api.ident().clone(),
754-
link: blessed.spec_file_name(),
755-
found: latest_local,
756-
})
757-
}
758-
ResolutionKind::NewLocally => {
759-
// latest_generated is not blessed, so update
760-
// the symlink.
761-
Some(Problem::LatestLinkStale {
762-
api_ident: api.ident().clone(),
763-
link: latest_generated,
764-
found: latest_local,
765-
})
766-
}
722+
match resolution.kind() {
723+
ResolutionKind::Lockstep => {
724+
unreachable!("this is a versioned API");
725+
}
726+
// Case 1 and 2 above.
727+
ResolutionKind::Blessed
728+
if generated_version == local_version =>
729+
{
730+
// latest_generated is blessed and the same
731+
// version as latest_local, so don't update the
732+
// symlink.
733+
None
734+
}
735+
ResolutionKind::Blessed => {
736+
// latest_generated is blessed, and has a
737+
// different version from latest_local. In this
738+
// case, we want to update the symlink to the
739+
// blessed version matching latest_generated
740+
// (not latest_generated, in case it's different
741+
// from the blessed version in a wire-compatible
742+
// way!)
743+
let api_blessed =
744+
api_blessed.unwrap_or_else(|| {
745+
panic!(
746+
"for {}, Blessed means \
747+
api_blessed exists",
748+
api.ident()
749+
)
750+
});
751+
let blessed = api_blessed
752+
.versions()
753+
.get(generated_version)
754+
.unwrap_or_else(|| {
755+
panic!(
756+
"for {} v{}, Blessed means \
757+
generated_version exists",
758+
api.ident(),
759+
generated_version
760+
);
761+
});
762+
Some(Problem::LatestLinkStale {
763+
api_ident: api.ident().clone(),
764+
link: blessed.spec_file_name(),
765+
found: latest_local,
766+
})
767+
}
768+
ResolutionKind::NewLocally => {
769+
// latest_generated is not blessed, so update
770+
// the symlink.
771+
Some(Problem::LatestLinkStale {
772+
api_ident: api.ident().clone(),
773+
link: latest_generated,
774+
found: latest_local,
775+
})
767776
}
768-
} else {
769-
unreachable!(
770-
"by_version map should have a version \
771-
corresponding to latest_generated ({})",
772-
latest_generated
773-
)
774777
}
775778
}
776779
}
777-
None => Some(Problem::LatestLinkMissing {
778-
api_ident: api.ident().clone(),
779-
link: latest_generated,
780-
}),
780+
None => {
781+
// As in case 3 above, if the resolution is blessed, we want to
782+
// update the symlink to the *blessed() hash corresponding to
783+
// the latest generated version.
784+
match resolution.kind() {
785+
ResolutionKind::Lockstep => {
786+
unreachable!("this is a versioned API");
787+
}
788+
ResolutionKind::Blessed => {
789+
let api_blessed = api_blessed.unwrap_or_else(|| {
790+
panic!(
791+
"for {}, Blessed means api_blessed exists",
792+
api.ident()
793+
)
794+
});
795+
let blessed = api_blessed
796+
.versions()
797+
.get(generated_version)
798+
.unwrap_or_else(|| {
799+
panic!(
800+
"for {} v{}, Blessed means \
801+
generated_version exists",
802+
api.ident(),
803+
generated_version
804+
);
805+
});
806+
Some(Problem::LatestLinkMissing {
807+
api_ident: api.ident().clone(),
808+
link: blessed.spec_file_name(),
809+
})
810+
}
811+
ResolutionKind::NewLocally => {
812+
// latest_generated is not blessed, so update
813+
// the symlink to the generated version.
814+
Some(Problem::LatestLinkMissing {
815+
api_ident: api.ident().clone(),
816+
link: latest_generated,
817+
})
818+
}
819+
}
820+
}
781821
};
782822

783823
(by_version, symlink)

‎crates/integration-tests/src/common/fixtures.rs‎

Lines changed: 130 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,7 @@ pub mod versioned_health {
163163
use super::*;
164164
use dropshot_api_manager_types::api_versions;
165165

166-
api_versions!(
167-
[(3, WITH_METRICS), (2, WITH_DETAILED_STATUS), (1, INITIAL),]
168-
);
166+
api_versions!([(3, WITH_METRICS), (2, WITH_DETAILED_STATUS), (1, INITIAL)]);
169167

170168
#[dropshot::api_description]
171169
pub trait VersionedHealthApi {
@@ -457,7 +455,7 @@ pub mod versioned_health_reduced {
457455
use super::*;
458456
use dropshot_api_manager_types::api_versions;
459457

460-
api_versions!([(2, WITH_DETAILED_STATUS), (1, INITIAL),]);
458+
api_versions!([(2, WITH_DETAILED_STATUS), (1, INITIAL)]);
461459

462460
#[dropshot::api_description]
463461
pub trait VersionedHealthApi {
@@ -491,3 +489,131 @@ pub mod versioned_health_reduced {
491489
DependencyStatus, DetailedHealthStatus, HealthStatusV1,
492490
};
493491
}
492+
493+
/// Versioned health API fixture that skips the middle version (2.0.0). This has
494+
/// versions 3.0.0 and 1.0.0 only, simulating retirement of an older blessed
495+
/// version.
496+
pub mod versioned_health_skip_middle {
497+
use super::*;
498+
use dropshot_api_manager_types::api_versions;
499+
500+
api_versions!([(3, WITH_METRICS), (1, INITIAL)]);
501+
502+
#[dropshot::api_description]
503+
pub trait VersionedHealthApi {
504+
type Context;
505+
506+
/// Check if the service is healthy (all versions).
507+
#[endpoint {
508+
method = GET,
509+
path = "/health",
510+
operation_id = "health_check",
511+
versions = "1.0.0"..
512+
}]
513+
async fn health_check(
514+
rqctx: RequestContext<Self::Context>,
515+
) -> Result<HttpResponseOk<HealthStatusV1>, HttpError>;
516+
517+
/// Get detailed health status (v2+, but only available in v3 since we skip v2).
518+
#[endpoint {
519+
method = GET,
520+
path = "/health/detailed",
521+
operation_id = "detailed_health_check",
522+
versions = "3.0.0"..
523+
}]
524+
async fn detailed_health_check(
525+
rqctx: RequestContext<Self::Context>,
526+
) -> Result<HttpResponseOk<DetailedHealthStatus>, HttpError>;
527+
528+
/// Get service metrics (v3+).
529+
#[endpoint {
530+
method = GET,
531+
path = "/metrics",
532+
operation_id = "get_metrics",
533+
versions = "3.0.0"..
534+
}]
535+
async fn get_metrics(
536+
rqctx: RequestContext<Self::Context>,
537+
) -> Result<HttpResponseOk<ServiceMetrics>, HttpError>;
538+
}
539+
540+
// Reuse the same response types from the main versioned_health module.
541+
pub use super::versioned_health::{
542+
DependencyStatus, DetailedHealthStatus, HealthStatusV1, ServiceMetrics,
543+
};
544+
}
545+
546+
/// Versioned health API with incompatible changes - this breaks backward
547+
/// compatibility by changing the response schema of an existing endpoint.
548+
pub mod versioned_health_incompatible {
549+
use super::*;
550+
use dropshot_api_manager_types::api_versions;
551+
552+
api_versions!([(3, WITH_METRICS), (2, WITH_DETAILED_STATUS), (1, INITIAL)]);
553+
554+
#[dropshot::api_description]
555+
pub trait VersionedHealthApi {
556+
type Context;
557+
558+
/// Check if the service is healthy (all versions).
559+
#[endpoint {
560+
method = GET,
561+
path = "/health",
562+
operation_id = "health_check",
563+
versions = "1.0.0"..
564+
}]
565+
async fn health_check(
566+
rqctx: RequestContext<Self::Context>,
567+
) -> Result<HttpResponseOk<HealthStatusV1>, HttpError>;
568+
569+
/// Get detailed health status (v2+).
570+
#[endpoint {
571+
method = GET,
572+
path = "/health/detailed",
573+
operation_id = "detailed_health_check",
574+
versions = "2.0.0"..
575+
}]
576+
async fn detailed_health_check(
577+
rqctx: RequestContext<Self::Context>,
578+
) -> Result<HttpResponseOk<DetailedHealthStatus>, HttpError>;
579+
580+
/// Get service metrics (v3+).
581+
#[endpoint {
582+
method = GET,
583+
path = "/metrics",
584+
operation_id = "get_metrics",
585+
versions = "3.0.0"..
586+
}]
587+
async fn get_metrics(
588+
rqctx: RequestContext<Self::Context>,
589+
) -> Result<HttpResponseOk<ServiceMetrics>, HttpError>;
590+
591+
/// Get system info (v3+): new endpoint added to existing version.
592+
///
593+
/// This breaks backward compatibility by adding a new endpoint to
594+
/// v3.0.0.
595+
#[endpoint {
596+
method = GET,
597+
path = "/system/info",
598+
operation_id = "get_system_info",
599+
versions = "3.0.0"..
600+
}]
601+
async fn get_system_info(
602+
rqctx: RequestContext<Self::Context>,
603+
) -> Result<HttpResponseOk<SystemInfo>, HttpError>;
604+
}
605+
606+
/// System information response for the new endpoint.
607+
#[derive(JsonSchema, Serialize)]
608+
pub struct SystemInfo {
609+
pub version: String,
610+
pub build_time: DateTime<Utc>,
611+
pub environment: String,
612+
}
613+
614+
// Reuse response types from the main versioned_health module for other
615+
// endpoints.
616+
pub use super::versioned_health::{
617+
DependencyStatus, DetailedHealthStatus, HealthStatusV1, ServiceMetrics,
618+
};
619+
}

0 commit comments

Comments
 (0)