stop logging cloud_init_bytes in sled-agent

[iliana]

aka "want sled-agent to stop screaming at base64 floppy disks"

Fixes #6387.

1. Create a NoDebug wrapper struct for this purpose in omicron-common. (Note that the OpenAPI schema for sled-agent does not change; schemars seems to honor #[serde(transparent)].)
2. Apply NoDebug inside the cloud_init_bytes Option. Thus we log everything else about the instance hardware, and whether or not there is cidata, but not the cidata itself.
3. Demote an INFO-level log of the Propolis client's InstanceEnsureRequest to DEBUG.

[sunshowers]

I wrote https://docs.rs/debug-ignore a bit ago which we use :)

[iliana]

The trick is that it must also implement JsonSchema for this particular use case, and anything else we ever want to use it in a struct for as well. For that reason it seems to make sense to keep it within Omicron so we can adjust it as needed.

[sunshowers]

sighhhh, yeah.

[sunshowers]

(Could you add a comment about this?)

[hawkw]

Thank you for doing this, the giant base64 strings were getting on my nerves a bit!

[hawkw]

world's most transparent type :)

[hawkw]

An alternative solution, rather than adding a no-debug wrapper type, could be to manually implement Debug for InstanceHardware using fmt::DebugStruct, and formatting everything normally but skipping the cloud_init_bytes field (or rendering it as Some(..)/None).

I'm not sure which is better off the top of my head --- the wrapper type seems a bit unfortunate: the "actual" type is Option<String>, and it's kinda sad to change the type just to control formatting behavior. But it might be better in the long run, since we don't have to remember to add any new fields to the manual debug implementation. So, I think this is fine, just felt like talking through alternatives.

---

Incidentally, there is nice technique for avoiding issues where new fields aren't added to a manual Debug implementation, though (credit to my friend @cratelyn for this one). You can use a destructuring assignment on the Self type in the Debug impl, like:

impl fmt::Debug for InstanceHardware {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        let Self {
            properties,
            nics,
            source_nat,
            ephemeral_ip,
            floating_ips,
            firewall_rules,
            dhcp_config,
            disks,
            cloud_init_bytes,
        } = self;
        f.debug_struct("InstanceHardware")
            .field("properties", properties)
            .field("nics", nics)
            // ... and so on ...
            .finish()
    }
}

and, because the destructuring pattern binds all the fields (since it doesn't have a ..), any newly-added field will cause it to fail to compile, serving as a nice reminder to add the new field to the Debug impl. But, this is still more work than #[derive(Debug)], so...

[sunshowers]

FWIW I prefer the wrapper struct over the two other alternatives I'm aware of, the manual impl you mentioned and the derive-more crate. But there are definitely places where I'd use one of the other approaches.

[hawkw]

For the record, the wrapper struct is fine with me!