From e04bb171f6b0016b4d1ec4e1ebb42f07277d2be2 Mon Sep 17 00:00:00 2001 From: Adam Leventhal Date: Thu, 30 Mar 2023 12:27:25 -0700 Subject: [PATCH] cli: handle optional body parameters (#401) --- progenitor-impl/src/cli.rs | 47 ++++- progenitor-impl/tests/output/nexus-cli.out | 185 ++++++++++++++++++ .../tests/output/propolis-server-cli.out | 7 +- 3 files changed, 235 insertions(+), 4 deletions(-) diff --git a/progenitor-impl/src/cli.rs b/progenitor-impl/src/cli.rs index 194150a..a5c6cee 100644 --- a/progenitor-impl/src/cli.rs +++ b/progenitor-impl/src/cli.rs @@ -254,8 +254,8 @@ impl Generator { let body_arg = maybe_body_param.map(|param| { let OperationParameterType::Type(type_id) = ¶m.typ else { - unreachable!(); - }; + unreachable!(); + }; let body_args = self.type_space.get_type(type_id).unwrap(); @@ -273,11 +273,52 @@ impl Generator { .type_space .get_type(&prop_type_id) .unwrap(); + + // TODO this is maybe a kludge--not completely sure + // of the right way to handle option types. On one + // hand, we could want types from this interface to + // never show us Option types--we could let the + // `required` field give us that information. On + // the other hand, there might be Option types that + // are required ... at least in the JSON sense, + // meaning that we need to include `"foo": null` + // rather than omitting the field. Back to the + // first hand: is that last point just a serde + // issue rather than an interface one? + let maybe_inner_type = + if let typify::TypeDetails::Option( + inner_type_id, + ) = prop_type.details() + { + let inner_type = self + .type_space + .get_type(&inner_type_id) + .unwrap(); + Some(inner_type) + } else { + None + }; + + let prop_type = + if let Some(inner_type) = maybe_inner_type { + inner_type + } else { + prop_type + }; + let prop_type_ident = prop_type.ident(); let good = prop_type.has_impl(TypeSpaceImpl::FromStr); let prop_name = prop_name.to_kebab_case(); - // assert!(good || !required); + + // println!( + // "{}::{}: {}; good: {}; required: {}", + // body_args.name(), + // prop_name, + // prop_type.name(), + // good, + // required, + // ); good.then(|| { let help = diff --git a/progenitor-impl/tests/output/nexus-cli.out b/progenitor-impl/tests/output/nexus-cli.out index dec7a5c..8e1c961 100644 --- a/progenitor-impl/tests/output/nexus-cli.out +++ b/progenitor-impl/tests/output/nexus-cli.out @@ -545,6 +545,18 @@ impl Cli { .value_parser(clap::value_parser!(types::Name)) .help("The organization's unique name."), ) + .arg( + clap::Arg::new("description") + .long("description") + .required(false) + .value_parser(clap::value_parser!(String)), + ) + .arg( + clap::Arg::new("name") + .long("name") + .required(false) + .value_parser(clap::value_parser!(types::Name)), + ) .about("Update an organization\n\nUse `PUT /v1/organizations/{organization}` instead") } @@ -676,6 +688,18 @@ impl Cli { .value_parser(clap::value_parser!(types::Name)) .help("The project's unique name within the organization."), ) + .arg( + clap::Arg::new("description") + .long("description") + .required(false) + .value_parser(clap::value_parser!(String)), + ) + .arg( + clap::Arg::new("name") + .long("name") + .required(false) + .value_parser(clap::value_parser!(types::Name)), + ) .about("Update a project\n\nUse `PUT /v1/projects/{project}` instead") } @@ -1336,6 +1360,16 @@ impl Cli { .required(true) .value_parser(clap::value_parser!(String)), ) + .arg( + clap::Arg::new("ip") + .long("ip") + .required(false) + .value_parser(clap::value_parser!(std::net::IpAddr)) + .help( + "The IP address for the interface. One will be auto-assigned if not \ + provided.", + ), + ) .arg( clap::Arg::new("name") .long("name") @@ -1414,6 +1448,18 @@ impl Cli { .required(true) .value_parser(clap::value_parser!(types::Name)), ) + .arg( + clap::Arg::new("description") + .long("description") + .required(false) + .value_parser(clap::value_parser!(String)), + ) + .arg( + clap::Arg::new("name") + .long("name") + .required(false) + .value_parser(clap::value_parser!(types::Name)), + ) .arg( clap::Arg::new("primary") .long("primary") @@ -1838,6 +1884,18 @@ impl Cli { .required(true) .value_parser(clap::value_parser!(types::Name)), ) + .arg( + clap::Arg::new("ipv6-prefix") + .long("ipv6-prefix") + .required(false) + .value_parser(clap::value_parser!(types::Ipv6Net)) + .help( + "The IPv6 prefix for this VPC.\n\nAll IPv6 subnets created from this VPC \ + must be taken from this range, which sould be a Unique Local Address in \ + the range `fd00::/48`. The default VPC Subnet will have the first `/64` \ + range from this prefix.", + ), + ) .arg( clap::Arg::new("name") .long("name") @@ -1890,6 +1948,24 @@ impl Cli { .required(true) .value_parser(clap::value_parser!(types::Name)), ) + .arg( + clap::Arg::new("description") + .long("description") + .required(false) + .value_parser(clap::value_parser!(String)), + ) + .arg( + clap::Arg::new("dns-name") + .long("dns-name") + .required(false) + .value_parser(clap::value_parser!(types::Name)), + ) + .arg( + clap::Arg::new("name") + .long("name") + .required(false) + .value_parser(clap::value_parser!(types::Name)), + ) .about("Update a VPC") } @@ -2088,6 +2164,18 @@ impl Cli { .required(true) .value_parser(clap::value_parser!(types::Name)), ) + .arg( + clap::Arg::new("description") + .long("description") + .required(false) + .value_parser(clap::value_parser!(String)), + ) + .arg( + clap::Arg::new("name") + .long("name") + .required(false) + .value_parser(clap::value_parser!(types::Name)), + ) .about("Update a router") } @@ -2270,6 +2358,18 @@ impl Cli { .required(true) .value_parser(clap::value_parser!(types::Name)), ) + .arg( + clap::Arg::new("description") + .long("description") + .required(false) + .value_parser(clap::value_parser!(String)), + ) + .arg( + clap::Arg::new("name") + .long("name") + .required(false) + .value_parser(clap::value_parser!(types::Name)), + ) .about("Update a route") } @@ -2381,6 +2481,18 @@ impl Cli { existing subnet in the VPC.", ), ) + .arg( + clap::Arg::new("ipv6-block") + .long("ipv6-block") + .required(false) + .value_parser(clap::value_parser!(types::Ipv6Net)) + .help( + "The IPv6 address range for this subnet.\n\nIt must be allocated from the \ + RFC 4193 Unique Local Address range, with the prefix equal to the parent \ + VPC's prefix. A random `/64` block will be assigned if one is not \ + provided. It must not overlap with any existing subnet in the VPC.", + ), + ) .arg( clap::Arg::new("name") .long("name") @@ -2445,6 +2557,18 @@ impl Cli { .required(true) .value_parser(clap::value_parser!(types::Name)), ) + .arg( + clap::Arg::new("description") + .long("description") + .required(false) + .value_parser(clap::value_parser!(String)), + ) + .arg( + clap::Arg::new("name") + .long("name") + .required(false) + .value_parser(clap::value_parser!(types::Name)), + ) .about("Update a subnet") } @@ -2983,6 +3107,18 @@ impl Cli { .required(true) .value_parser(clap::value_parser!(types::Name)), ) + .arg( + clap::Arg::new("description") + .long("description") + .required(false) + .value_parser(clap::value_parser!(String)), + ) + .arg( + clap::Arg::new("name") + .long("name") + .required(false) + .value_parser(clap::value_parser!(types::Name)), + ) .about("Update an IP Pool") } @@ -3169,6 +3305,20 @@ impl Cli { pub fn cli_silo_create() -> clap::Command { clap::Command::new("") + .arg( + clap::Arg::new("admin-group-name") + .long("admin-group-name") + .required(false) + .value_parser(clap::value_parser!(String)) + .help( + "If set, this group will be created during Silo creation and granted the \ + \"Silo Admin\" role. Identity providers can assert that users belong to \ + this group and those users can log in and further initialize the \ + Silo.\n\nNote that if configuring a SAML based identity provider, \ + group_attribute_name must be set for users to be considered part of a \ + group. See [`SamlIdentityProviderCreate`] for more information.", + ), + ) .arg( clap::Arg::new("description") .long("description") @@ -3331,6 +3481,17 @@ impl Cli { .required(true) .value_parser(clap::value_parser!(String)), ) + .arg( + clap::Arg::new("group-attribute-name") + .long("group-attribute-name") + .required(false) + .value_parser(clap::value_parser!(String)) + .help( + "If set, SAML attributes with this name will be considered to denote a \ + user's group membership, where the attribute value(s) should be a \ + comma-separated list of group names.", + ), + ) .arg( clap::Arg::new("idp-entity-id") .long("idp-entity-id") @@ -4094,6 +4255,18 @@ impl Cli { .required(true) .value_parser(clap::value_parser!(types::NameOrId)), ) + .arg( + clap::Arg::new("description") + .long("description") + .required(false) + .value_parser(clap::value_parser!(String)), + ) + .arg( + clap::Arg::new("name") + .long("name") + .required(false) + .value_parser(clap::value_parser!(types::Name)), + ) .about("Update an organization") } @@ -4208,6 +4381,18 @@ impl Cli { .required(false) .value_parser(clap::value_parser!(types::NameOrId)), ) + .arg( + clap::Arg::new("description") + .long("description") + .required(false) + .value_parser(clap::value_parser!(String)), + ) + .arg( + clap::Arg::new("name") + .long("name") + .required(false) + .value_parser(clap::value_parser!(types::Name)), + ) .about("Update a project") } diff --git a/progenitor-impl/tests/output/propolis-server-cli.out b/progenitor-impl/tests/output/propolis-server-cli.out index 47794ee..9709cb4 100644 --- a/progenitor-impl/tests/output/propolis-server-cli.out +++ b/progenitor-impl/tests/output/propolis-server-cli.out @@ -27,7 +27,12 @@ impl Cli { } pub fn cli_instance_ensure() -> clap::Command { - clap::Command::new("") + clap::Command::new("").arg( + clap::Arg::new("cloud-init-bytes") + .long("cloud-init-bytes") + .required(false) + .value_parser(clap::value_parser!(String)), + ) } pub fn cli_instance_issue_crucible_snapshot_request() -> clap::Command {