Require both IPv4 and IPv6 blocks on VPC subnets

[david-crespo]

Leaving as a draft because I'm not sure whether we want to do this, but I wanted to see how it would look. @rmustacc's says here:

So IIRC we had some discussion around this earlier, I think when @teisenbe was working on the nexus/Omicron end. So hopefully what I'm about to say will agree with what I talked about then. The long term goal is to allow someone to have an entire VPC or an individual VPC subnet with only one of IPv4 or IPv6; however, to simplify the initial implementation we're requiring that both an IPv4 and IPv6 subnet be present at this time.

@teisenbe how does this comport with your memory of the conversation?

@bnaecker it looks like you originally wrote this comment, what do you remember?

omicron/nexus/src/external_api/views.rs

Lines 114 to 116 in 939adec

	// TODO-design: RFD 21 says that V4 subnets are currently required, and V6 are optional. If a
	// V6 address is _not_ specified, one is created with a prefix that depends on the VPC and a
	// unique subnet-specific portion of the prefix (40 and 16 bits for each, respectively).

[david-crespo]

The fact that the IP blocks use Some but name and description do not suggests something is off here, though it may be that the former are correct and the latter are wrong.

It's a bit odd that name and description are optional on IdentityMetadataUpdateParams given that I thought we decided to go with the more strictly correct PUT scheme in which all fields are expected to be present in the request body. We probably just haven't updated all the code in accordance with that determination.

omicron/common/src/api/external/mod.rs

Lines 635 to 640 in 939adec

	#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
	#[serde(rename_all = "camelCase")]
	pub struct IdentityMetadataUpdateParams {
	pub name: Option<Name>,
	pub description: Option<String>,
	}

If name and description are made optional in the params, I would expect the fields to remain optional on the diesel changeset because we can use that internally to make queries that only update some fields but not others.

#[derive(AsChangeset)]
#[table_name = "vpc_subnet"]
pub struct VpcSubnetUpdate {
    pub name: Option<Name>,
    pub description: Option<String>,
    pub time_modified: DateTime<Utc>,
    pub ipv4_block: Option<Ipv4Net>,
    pub ipv6_block: Option<Ipv6Net>,
}

In that case the resolution to the above asymmetry will be to wrap name and description in Some:

name: Some(Name(params.identity.name)),
description: Some(params.identity.description),

[david-crespo]

Confirmed there's an issue for bringing our PUTs into line: #333

[bnaecker]

My recollection is as that comment describes: that IPv4 was (at the time I wrote that) currently required, and IPv6 optional. But since the ultimate goal is that both should be optional, I made them Options. I understood that the requirement of IPv4 was to be enforced by the software at runtime, but the types in the API were optional to reflect that we'd ultimately relax that requirement without changing the API.

[david-crespo]

Ok. After thinking about it a bit more, the long term solution is probably something like the below, and then in the DB model both remain optional like they are today. Unfortunately when converting from the DB model to the view, you have to assume at least one of IPv4 and IPv6 is there because we enforce that on the input side.

I confirmed that these get turned into the oneOf you'd expect in the OpenAPI spec. I'll close this and will either make an attempt at implementing the below or at least make an issue for it.

// params
pub enum VpcSubnetCreate {
    IPv4 {
        #[serde(flatten)]
        identity: IdentityMetadataCreateParams,
        ipv4_block: Ipv4Net,
    },
    IPv6 {
        #[serde(flatten)]
        identity: IdentityMetadataCreateParams,
        ipv6_block: Ipv6Net,
    },
    Ipv4Ipv6 {
        #[serde(flatten)]
        identity: IdentityMetadataCreateParams,
        ipv4_block: Ipv4Net,
        ipv6_block: Ipv6Net,
    },
}

// view
pub enum VpcSubnet {
    IPv4 {
        #[serde(flatten)]
        identity: IdentityMetadata,
        vpc_id: Uuid,
        ipv4_block: Ipv4Net,
    },
    IPv6 {
        #[serde(flatten)]
        identity: IdentityMetadata,
        vpc_id: Uuid,
        ipv6_block: Ipv6Net,
    },
    Ipv4Ipv6 {
        #[serde(flatten)]
        identity: IdentityMetadata,
        vpc_id: Uuid,
        ipv4_block: Ipv4Net,
        ipv6_block: Ipv6Net,
    },
}