Conversation
| # future false negatives. | ||
| # | ||
|
|
||
| [[dependency_filter_rules]] |
There was a problem hiding this comment.
huge thanks to Dave and John for helping puzzle through the ls-apis issues we tripped over now that propolis-server depends on sled-agent-client. I believe John is writing up some issues there and I have a PR to do as well after this one.
whoever reads through the notes here: please do shout if this does not make sense! this seems like it's at the intersection of at least one bug (intra-deployment-unit dependency cycles are considered impossible to deploy, but they are very possible) and another.. should be bug (API consumption is determined without considering filter rules breaking dependency paths, leading to overly-broad API consumption claims)
`cargo xtask ls-apis check` will report an error if the software component graph contains a cycle, as well as if the deployment units form a cycle, but prints only the name of a node in the graph that was in the cycle. If you've added a cycle to either of these graphs, the error and node can tell you about the what that is wrong. Going a bit further and printing an example cycle including the troublesome node can help with the how you got there. With this change, the first approach at #10231 (which introduced a cycle in, but all in one deployment unit: sled-agent -> propolis-server -> sled-agent) errors with a more clear message about propolis-server and sled-agent being the problems: ``` Error: graph of server-managed API dependencies between components has a cycle (includes node: "propolis-server", example cycle: propolis-server, omicron-sled-agent) ```
|
leaving a note here: I've put a TUF from 50642c7 on I think the platform measurement differing makes sense since we've been testing against a relatively old Omicron meaning relatively old RoT and SP builds, so them changing relative to the image's baked in measurement makes sense? if @jordanhendricks or @flihp want to cross-check me on that, that's my one remaining wrinkle, but I'm pretty sure that all checks out and I think we're good to go here! edit: from checking the TUF contents between a build off of |
|
thanks for checking @iximeow, seems right to me |
hawkw
left a comment
hawkw
left a comment
There was a problem hiding this comment.
Propolis bump seems great! I had a couple questions about the ls-apis rules and the notes explaining them.
| This filter should be removed when the intra-deployment relationship between | ||
| propolis-server and sled-agent can be tolerated in the component graph check. |
There was a problem hiding this comment.
nit: perhaps this should reference #10234 as the issue which would allow us to remove this filter?
There was a problem hiding this comment.
yes, and yes below! we'd just written this well before the issues were filed. given the potentially long turnaround (anxious about the last helios/deploy time...) on CI I'm gonna merge this and push another commit after.
| propolis-server through to propolis-client. Once we've fixed the issue requiring | ||
| the dice-verifier rule above, we should probably keep this dependency filter | ||
| rule. |
There was a problem hiding this comment.
Am I correct that when #10232 (inter-deployment-unit exceptions) is fixed, this rule also becomes unnecessary? Should we note that here as well?
6 participants
since last time:
plus another few commits that should be non-functional changes:
that is to say, this brings the VM RoT/attestation work, a bunch of the vsock fixes for issues we'd found in testing, and a fix for the virtio nic issues that Windows saw in R18.