migrate to hyper v1 #6580
migrate to hyper v1 #6580
Conversation
| @@ -13,6 +13,7 @@ | |||
| [libraries."libgcc_s.so.1"] | |||
| [libraries."libipcc.so.1"] | |||
| [libraries."libkstat.so.1"] | |||
| [libraries."liblzma.so.5"] | |||
There was a problem hiding this comment.
@papertigers can I just... add this? @rcgoodfellow this is due to a new dep from falcon from oxidecomputer/falcon#88
There was a problem hiding this comment.
That's an explicit allow list, so as long as we expect that lib to be present in the gz and ngz it's the right place to stick it. If it's a lib we don't want to leak out of a single binary we can add it an allow list similar to what libipcc has.
| || error.to_string().contains("self-signed certificate") | ||
| || error.to_string().contains("self signed certificate") | ||
| ); | ||
| assert!(error.chain().to_string().contains("self-signed certificate")); |
There was a problem hiding this comment.
@jmpesp I think this was you; does this new check suffice?
There was a problem hiding this comment.
It's been so long I don't remember doing this haha!
I'm not sure this check is right: it looks like this part of the test is meant to check that a client can't connect using the wrong certificate, and this check is asserting it won't trust a self-signed certificate. That's not quite the same thing. I think we need either add_root_certificate or danger_accept_invalid_certs in order to test that the client accepts the self-signed cert but fails due to the incorrect certificate being used.
| @@ -834,8 +833,8 @@ mod test { | |||
| // The DNS server is running, but has no records. Expect a failure. | |||
| let err = client.test_endpoint().await.unwrap_err(); | |||
| assert!( | |||
| err.to_string().contains("no record found"), | |||
| "Unexpected Error (expected 'no record found'): {err}", | |||
| err.to_string().contains("error sending request"), | |||
There was a problem hiding this comment.
Why did this bit change? If the server is running, but doesn't have any records, shouldn't the "send part" succeed?
There was a problem hiding this comment.
I think I can improve this check (insofar as it's useful). The part we were looking for is just farther down the nested chain of errors. I'll do that in a follow on
There's going to need to be some coordination with these:
I hope to merge this and then get crucible, propolis, and maghemite depending on this updated version so that we can return the reqwest 0.11 dependency over there.
After merging this I intend to temporarily depend on a maghemite branch (see oxidecomputer/maghemite#378) to navigate past the omicron-common test failure.