Report the OctoPrint API as what it is, and let the operator name it
Reporting a detected printer as `prusa_link` asserted a product nothing on
the wire mentions. What a probe observes is that something speaks the
OctoPrint REST API; PrusaLink is one implementation of it, OctoPrint itself
is another, and without a credential the two are indistinguishable - every
PrusaLink identity endpoint requires digest auth.
So discovery now reports `vendor = "octoprint"`, the protocol it actually
saw, and carries `ambiguous_with` so the rendered entry prints a comment
offering `prusa_link` directly above the `vendor` line. Changing it is a
one-word edit in the file the operator is already pasting into, which is the
right place for a question only they can answer. The exception is a body
that names itself: `/api/version`'s `text` field reads "PrusaLink 0.7.0" or
"OctoPrint 1.9.3", so on the rare unauthenticated 200 the answer is known
and no choice is offered.
For that to mean anything, `octoprint` had to become a vendor `fleet.rs`
accepts - otherwise discovery printed a block that would not load. It maps
to the same client as `prusa_link`, because they speak the same API, with an
`OctoPrintFlavour` recording which. The flavour is stored now, while the
client is still a stub, precisely because no probe can settle it: it is the
operator's answer, and without somewhere to keep it the real client would
have to ask them a second time. `PrusaLinkPrinter` is accordingly now
`OctoPrintPrinter` - one client for the API family - since a struct called
PrusaLink holding a flavour that says "Generic" is a contradiction someone
would later have to unpick.
**Found by the new test, and the reason it exists:** `fleet::load` did *not*
reject an entry whose credential was blank. `require_field` took an
`Option`, so `access_code = ""` arrived as `Some("")` and loaded happily -
which falsified the claim this feature was built on, that printing a block
with an empty credential is safe because pasting it in unfinished fails
loudly at startup. It does now. An empty credential is not a usable one; it
only moves the failure somewhere worse, where a Bambu printer is refused at
MQTT and a PrusaLink one 401s on every poll with nothing saying the field is
blank. This is the same trap as `??`-instead-of-`||` on an environment
variable that ships empty, which the backend's `auth/edge_secret.ts` and
`s3/index.ts` both document; it bites identically on a TOML key.
tests/discovery_to_fleet.rs is new and closes the loop: discovery and fleet
are coupled by nothing but a string, unit tests on either side cannot catch
a drift between them, and the symptom of a drift is an operator pasting in
exactly what they were told to and the gateway refusing to start. It renders
a candidate for every vendor, fills in the credential the way a human would,
loads it for real, and checks the handle matches the vendor asked for - not
merely that something loaded, since `octoprint` and `prusa_link` share a
client and a mapping that collapsed them would still load and still be
wrong.
Verification: the Moonraker probe now runs end to end over real HTTP against
a stand-in serving Moonraker's documented `/printer/info`, and correctly
reports a klipper entry named from the returned hostname. Port 80 cannot be
bound without root here, so the PrusaLink path is covered by a test that
takes a real socket answering a real digest challenge - the one link unit
tests on the classifier cannot reach, and the one PrusaLink is identified by.
This commit is contained in:
@@ -0,0 +1,136 @@
|
||||
//! Proves the blocks `discovery` prints are blocks `fleet` can load.
|
||||
//!
|
||||
//! The two are coupled by nothing but a string - `Vendor::toml_value` has to
|
||||
//! match what `fleet::build_printer` matches on, and the fields rendered have
|
||||
//! to be the fields it requires. Unit tests on either side cannot catch a
|
||||
//! drift between them, and the symptom of a drift is the worst kind: an
|
||||
//! operator pastes in exactly what they were told to and the gateway refuses
|
||||
//! to start.
|
||||
//!
|
||||
//! So this renders a candidate for every vendor, fills in the credential the
|
||||
//! way a human would, and loads the result for real.
|
||||
|
||||
use std::io::Write;
|
||||
|
||||
use continuum_proxy::discovery::{Candidate, Source, Vendor};
|
||||
use continuum_proxy::fleet;
|
||||
use continuum_proxy::printer::{GenericPrinter, PrinterHandle};
|
||||
|
||||
/// Fills in the credential a human is told to supply, so the entry loads.
|
||||
fn with_credentials(entry: &str) -> String {
|
||||
entry
|
||||
.replace("access_code = \"\"", "access_code = \"12345678\"")
|
||||
.replace("api_key = \"\"", "api_key = \"an-api-key\"")
|
||||
}
|
||||
|
||||
fn load(toml: &str) -> anyhow::Result<Vec<PrinterHandle>> {
|
||||
let mut file = tempfile();
|
||||
file.write_all(toml.as_bytes()).expect("write the fleet file");
|
||||
fleet::load(&file.path)
|
||||
}
|
||||
|
||||
/// A self-deleting temporary file, rather than a dependency for four lines.
|
||||
struct TempToml {
|
||||
path: std::path::PathBuf,
|
||||
handle: Option<std::fs::File>,
|
||||
}
|
||||
|
||||
impl TempToml {
|
||||
fn write_all(&mut self, bytes: &[u8]) -> std::io::Result<()> {
|
||||
self.handle.as_mut().expect("still open").write_all(bytes)?;
|
||||
self.handle.as_mut().expect("still open").flush()
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for TempToml {
|
||||
fn drop(&mut self) {
|
||||
self.handle.take();
|
||||
let _ = std::fs::remove_file(&self.path);
|
||||
}
|
||||
}
|
||||
|
||||
fn tempfile() -> TempToml {
|
||||
// The pid and a counter, so two tests in one binary cannot collide.
|
||||
use std::sync::atomic::{AtomicUsize, Ordering};
|
||||
static NEXT: AtomicUsize = AtomicUsize::new(0);
|
||||
let path =
|
||||
std::env::temp_dir().join(format!("continuum-fleet-{}-{}.toml", std::process::id(), NEXT.fetch_add(1, Ordering::Relaxed)));
|
||||
let handle = std::fs::File::create(&path).expect("create a temp fleet file");
|
||||
TempToml { path, handle: Some(handle) }
|
||||
}
|
||||
|
||||
fn candidate(vendor: Vendor, host: &str) -> Candidate {
|
||||
let mut candidate = Candidate::new(host, vendor, Source::HttpApi);
|
||||
// Bambu needs one, and for every other vendor it is ignored.
|
||||
candidate.serial = Some("01S09B441900143".to_string());
|
||||
candidate
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn every_rendered_entry_loads_as_the_printer_it_claims_to_be() {
|
||||
let cases = [
|
||||
(Vendor::Bambu, "192.168.0.20"),
|
||||
(Vendor::Klipper, "192.168.0.50"),
|
||||
(Vendor::PrusaLink, "192.168.0.51"),
|
||||
(Vendor::Octoprint, "192.168.0.60"),
|
||||
(Vendor::PrusaSerial, "/dev/ttyACM0"),
|
||||
];
|
||||
for (vendor, host) in cases {
|
||||
let entry = candidate(vendor, host).to_toml_entry("found");
|
||||
let fleet = load(&with_credentials(&entry))
|
||||
.unwrap_or_else(|err| panic!("{vendor:?} rendered an entry fleet::load rejects: {err}\n{entry}"));
|
||||
assert_eq!(fleet.len(), 1, "{vendor:?}");
|
||||
assert_eq!(fleet[0].base().id, "found", "{vendor:?}");
|
||||
|
||||
// The vendor string has to resolve to the matching handle, not just
|
||||
// to *a* handle - `octoprint` and `prusa_link` share a client, so a
|
||||
// mapping that collapsed them would still load and still be wrong.
|
||||
let matched = matches!(
|
||||
(vendor, &fleet[0]),
|
||||
(Vendor::Bambu, PrinterHandle::Bambu(_))
|
||||
| (Vendor::Klipper, PrinterHandle::Klipper(_))
|
||||
| (Vendor::PrusaLink | Vendor::Octoprint, PrinterHandle::OctoPrint(_))
|
||||
| (Vendor::PrusaSerial, PrinterHandle::PrusaSerial(_))
|
||||
);
|
||||
assert!(matched, "{vendor:?} loaded as the wrong handle");
|
||||
}
|
||||
}
|
||||
|
||||
/// The flavour is the operator's answer to a question no probe can settle, so
|
||||
/// it has to survive the trip from the file into the printer.
|
||||
#[test]
|
||||
fn the_octoprint_flavour_an_operator_chose_reaches_the_client() {
|
||||
use continuum_proxy::printer::OctoPrintFlavour;
|
||||
|
||||
for (vendor, expected) in
|
||||
[(Vendor::Octoprint, OctoPrintFlavour::Generic), (Vendor::PrusaLink, OctoPrintFlavour::PrusaLink)]
|
||||
{
|
||||
let entry = with_credentials(&candidate(vendor, "192.168.0.51").to_toml_entry("found"));
|
||||
let fleet = load(&entry).expect("loads");
|
||||
let PrinterHandle::OctoPrint(printer) = &fleet[0] else {
|
||||
panic!("{vendor:?} should load as an OctoPrint-family client");
|
||||
};
|
||||
assert_eq!(printer.flavour, expected, "{vendor:?}");
|
||||
}
|
||||
}
|
||||
|
||||
/// An entry straight out of discovery, credential still blank, must be
|
||||
/// refused - and refused in a way that names the printer and the field.
|
||||
#[test]
|
||||
fn an_entry_with_the_credential_left_blank_is_refused_by_name() {
|
||||
for vendor in [Vendor::Bambu, Vendor::PrusaLink, Vendor::Octoprint] {
|
||||
let entry = candidate(vendor, "192.168.0.20").to_toml_entry("unfinished");
|
||||
let Err(err) = load(&entry) else {
|
||||
panic!("{vendor:?}: a blank credential must not load:\n{entry}");
|
||||
};
|
||||
let message = err.to_string();
|
||||
// This is the whole reason discovery prints instead of writing the
|
||||
// file: the failure is loud, and it says which printer and which
|
||||
// field.
|
||||
assert!(message.contains("unfinished"), "{vendor:?}: {message}");
|
||||
assert!(
|
||||
message.contains("access_code") || message.contains("api_key"),
|
||||
"{vendor:?} must name the missing field: {message}"
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user