From 19ad11128b40fccfe4a19fc22171ef388f94781d Mon Sep 17 00:00:00 2001
From: John Gallagher <john@oxidecomputer.com>
Date: Thu, 7 Mar 2024 17:45:53 -0500
Subject: [PATCH] `omdb nexus blueprints target set`: take an `enabled` setting
 (#5224)

I made this a required argument because I'm nervous to default it to
`Enabled` when we've talked so much about manually setting a disabled
blueprint. If folks feel strongly this should be an optional
`--enabled={true,false,inherit}` defaulting to `true` instead, I
wouldn't put up a fight.

Addresses the first part of #5210.
---
 dev-tools/omdb/src/bin/omdb/nexus.rs | 49 +++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 9 deletions(-)

diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs
index 692931db51..4705da2a32 100644
--- a/dev-tools/omdb/src/bin/omdb/nexus.rs
+++ b/dev-tools/omdb/src/bin/omdb/nexus.rs
@@ -11,6 +11,7 @@ use chrono::SecondsFormat;
 use chrono::Utc;
 use clap::Args;
 use clap::Subcommand;
+use clap::ValueEnum;
 use futures::TryStreamExt;
 use nexus_client::types::ActivationReason;
 use nexus_client::types::BackgroundTask;
@@ -77,7 +78,7 @@ enum BlueprintsCommands {
     Diff(BlueprintIdsArgs),
     /// Delete a blueprint
     Delete(BlueprintIdArgs),
-    /// Set the current target blueprint
+    /// Interact with the current target blueprint
     Target(BlueprintsTargetArgs),
     /// Generate an initial blueprint from a specific inventory collection
     GenerateFromCollection(CollectionIdArgs),
@@ -116,7 +117,25 @@ enum BlueprintTargetCommands {
     /// Show the current target blueprint
     Show,
     /// Change the current target blueprint
-    Set(BlueprintIdArgs),
+    Set(BlueprintTargetSetArgs),
+}
+
+#[derive(Debug, Args)]
+struct BlueprintTargetSetArgs {
+    /// id of blueprint to make target
+    blueprint_id: Uuid,
+    /// whether this blueprint should be enabled
+    enabled: BlueprintTargetSetEnabled,
+}
+
+#[derive(Debug, Clone, Copy, ValueEnum)]
+enum BlueprintTargetSetEnabled {
+    /// set the new current target as enabled
+    Enabled,
+    /// set the new current target as disabled
+    Disabled,
+    /// use the enabled setting from the parent blueprint
+    Inherit,
 }
 
 #[derive(Debug, Args)]
@@ -914,14 +933,26 @@ async fn cmd_nexus_blueprints_target_show(
 
 async fn cmd_nexus_blueprints_target_set(
     client: &nexus_client::Client,
-    args: &BlueprintIdArgs,
+    args: &BlueprintTargetSetArgs,
 ) -> Result<(), anyhow::Error> {
-    // Try to preserve the value of "enabled", if possible.
-    let enabled = client
-        .blueprint_target_view()
-        .await
-        .map(|current| current.into_inner().enabled)
-        .unwrap_or(true);
+    let enabled = match args.enabled {
+        BlueprintTargetSetEnabled::Enabled => true,
+        BlueprintTargetSetEnabled::Disabled => false,
+        // There's a small TOCTOU race with "inherit": What if the user wants to
+        // inherit the parent blueprint enabled bit but the current target
+        // blueprint enabled bit is flipped or the current target blueprint is
+        // changed? We expect neither of these to be problematic in practice:
+        // the only way for the `enable` bit to be set to anything at all is via
+        // `omdb`, so the user would have to be racing with another `omdb`
+        // operator. (In the case of the current target blueprint being changed
+        // entirely, that will result in a failure to set the current target
+        // below, because its parent will no longer be the current target.)
+        BlueprintTargetSetEnabled::Inherit => client
+            .blueprint_target_view()
+            .await
+            .map(|current| current.into_inner().enabled)
+            .context("failed to fetch current target blueprint")?,
+    };
     client
         .blueprint_target_set(&nexus_client::types::BlueprintTargetSet {
             target_id: args.blueprint_id,