Skip to content

Tracking issue for cleaning up ulitebox broker design #911

Description

@jaybosamiya-ms

Just a tracking issue for things that need to be cleaned up in subsequent PRs from #880 ; I plan to resolve these as I go along. In some sense, I'd also prefer if we can set up something where some tooling can be vigilant about these, so I've tried to classify issues I've noticed into various classes here.

Broad stuff that I think would probably benefit from some vigilance setup:

  • Reduce broad pub use re-exports.
  • Remove unnecessary indirection layers for types; this is both type definitions, as well as get/set stuff for stuff that should be transparent/pub
  • Remove unnecessary indirection layers for functions / tiny helper functions that simply obscure things
  • Use From impls where it makes sense.
  • Keep methods close to the primary type definition
  • Use descriptive generic/type-parameter names like Channel instead of T. Also, keep trait boundaries closer at the struct level or such
  • Make API scopes tighter: pub -> pub(crate) -> pub(super) -> private, wherever it makes snese
  • Prevent multiple-bool constructors and such where confusing (hmm, maybe I need to pull in some of the named-bool stuff I'd setup for TRex?)
  • Use relevant existing crates where we're already using them elsewhere in the project (e.g., bitflags / thiserror / ...) rather than unnecessary manual reimplementation
  • Make sure ordering of enum things is consistent / readable, and not accidentally ordered
  • Prefer panicing on unrecoverable things, rather than forcing it to go into Err and have upper level be forced to panic on an internal irrecoverable thing
  • Clean up use std::env style module imports
  • Keep test-only helpers as part of tests; lock all test things behind a #cfg(test)] module

More specific to #880's code itself:

  • Remove the minor-version skew that will likely cause parser-differential issues in the future
  • Move to Vest
  • Clean up overuse + underuse of #[non_exhaustive]
  • Clean up unnecessary trivial constants
  • Consider using slotmap
  • Unknown + #[non_exhasutive] is a recipe for confusion, clean
  • Document policy engine is a placeholder for now
  • Rename unauthenticated to maybe host-guaranteed.
  • Remove unused readiness-cache complexity.
  • Remove readv/writev complexity.
  • Fix the unnecessary seccomp weakening that Initial broker design and implementation #880 is doing; broken broker is not something the guest is supposed to be guarding against

Stuff that needs more thought, and probably not in a set of cleanup PRs, but needs broader discussion about the #880 decisions:

  • Figure out the blocking / non-blocking broker interface design
  • Ordering of runner + broker invocation and how the tunnel between them gets set up

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions