[installinator] Only use installinator doc (#9410)

We're now past the check point for deprecating non-installinator doc
updates.

Author: labbott

installinator/src/artifact.rs

@@ -4,7 +4,7 @@
 
 use std::{net::SocketAddr, time::Duration};
 
-use anyhow::{Context, Result};
+use anyhow::{Context, Result, bail};
 use clap::Args;
 use futures::StreamExt;
 use installinator_client::ClientError;
@@ -23,7 +23,7 @@ pub(crate) struct ArtifactIdOpts {
     /// Retrieve artifact ID from IPCC
     #[clap(
         long,
-        required_unless_present_any = ["update_id", "host_phase_2", "control_plane", "installinator_doc"]
+        required_unless_present_any = ["update_id", "installinator_doc"]
     )]
     from_ipcc: bool,
 
@@ -36,22 +36,8 @@ pub(crate) struct ArtifactIdOpts {
 
     #[clap(
         long,
-        conflicts_with_all = ["from_ipcc", "installinator_doc"],
-        required_unless_present_any = ["from_ipcc", "installinator_doc"],
-    )]
-    host_phase_2: Option<ArtifactHash>,
-
-    #[clap(
-        long,
-        conflicts_with_all = ["from_ipcc", "installinator_doc"],
-        required_unless_present_any = ["from_ipcc", "installinator_doc"],
-    )]
-    control_plane: Option<ArtifactHash>,
-
-    #[clap(
-        long,
-        conflicts_with_all = ["from_ipcc", "host_phase_2", "control_plane"],
-        required_unless_present_any = ["from_ipcc", "host_phase_2", "control_plane"],
+        conflicts_with_all = ["from_ipcc"],
+        required_unless_present_any = ["from_ipcc"],
     )]
     installinator_doc: Option<ArtifactHash>,
 }
@@ -63,52 +49,38 @@ impl ArtifactIdOpts {
             let image_id = ipcc
                 .installinator_image_id()
                 .context("error retrieving installinator image ID")?;
-            Ok(LookupId::from_image_id(&image_id))
+            LookupId::from_image_id(&image_id)
         } else {
             let update_id = self.update_id.unwrap();
-            let kind =
-                if let Some(installinator_doc_hash) = self.installinator_doc {
-                    LookupIdKind::Document(installinator_doc_hash)
-                } else {
-                    LookupIdKind::Hashes {
-                        host_phase_2: self.host_phase_2.unwrap(),
-                        control_plane: self.control_plane.unwrap(),
-                    }
-                };
-
-            Ok(LookupId { update_id, kind })
+            let document = self
+                .installinator_doc
+                .context("error retrieving installinator doc hash")?;
+
+            Ok(LookupId { update_id, document })
         }
     }
 }
 
 /// Identifiers used by installinator to retrieve artifacts.
 pub(crate) struct LookupId {
     pub(crate) update_id: MupdateUuid,
-    pub(crate) kind: LookupIdKind,
+    pub(crate) document: ArtifactHash,
 }
 
 impl LookupId {
-    fn from_image_id(image_id: &InstallinatorImageId) -> Self {
+    fn from_image_id(image_id: &InstallinatorImageId) -> Result<Self> {
         // This sentinel hash is used to indicate that the host phase 2 hash is
         // actually the hash to the installinator document.
-        let kind = if image_id.control_plane == ArtifactHash([0; 32]) {
-            LookupIdKind::Document(image_id.host_phase_2)
-        } else {
-            LookupIdKind::Hashes {
-                host_phase_2: image_id.host_phase_2,
-                control_plane: image_id.control_plane,
-            }
-        };
 
-        Self { update_id: image_id.update_id, kind }
-    }
-}
+        if image_id.control_plane != ArtifactHash([0; 32]) {
+            bail!("non-zero control plane hash, this isn't using doc format");
+        }
 
-/// Either an installinator document hash, or host phase 2 and control plane
-/// hashes.
-pub(crate) enum LookupIdKind {
-    Document(ArtifactHash),
-    Hashes { host_phase_2: ArtifactHash, control_plane: ArtifactHash },
+        Ok(Self {
+            update_id: image_id.update_id,
+            document: image_id.host_phase_2,
+        })
+    }
 }
 
 /// The host phase 2 and control plane hashes to download.

installinator/src/dispatch.rs

@@ -25,7 +25,7 @@ use update_engine::StepResult;
 
 use crate::{
     ArtifactWriter, WriteDestination,
-    artifact::{ArtifactIdOpts, ArtifactsToDownload, LookupIdKind},
+    artifact::{ArtifactIdOpts, ArtifactsToDownload},
     fetch::{FetchArtifactBackend, FetchedArtifact, HttpFetchBackend},
     peers::DiscoveryMechanism,
     reporter::{HttpProgressBackend, ProgressReporter, ReportProgressBackend},
@@ -209,121 +209,102 @@ impl InstallOpts {
 
         let engine = UpdateEngine::new(log, event_sender);
 
-        let to_download = match lookup_id.kind {
-            LookupIdKind::Document(hash) => {
-                engine
-                    .new_step(
-                        InstallinatorComponent::InstallinatorDocument,
-                        InstallinatorStepId::Download,
-                        "Downloading installinator document",
-                        async move |cx| {
-                            let installinator_doc_id = ArtifactHashId {
-                                kind: KnownArtifactKind::InstallinatorDocument
-                                    .into(),
-                                hash,
-                            };
-                            let installinator_doc = fetch_artifact(
-                                &cx,
-                                &installinator_doc_id,
-                                discovery,
-                                log,
-                            )
-                            .await?;
+        let to_download = engine
+            .new_step(
+                InstallinatorComponent::InstallinatorDocument,
+                InstallinatorStepId::Download,
+                "Downloading installinator document",
+                async move |cx| {
+                    let installinator_doc_id = ArtifactHashId {
+                        kind: KnownArtifactKind::InstallinatorDocument.into(),
+                        hash: lookup_id.document,
+                    };
+                    let installinator_doc = fetch_artifact(
+                        &cx,
+                        &installinator_doc_id,
+                        discovery,
+                        log,
+                    )
+                    .await?;
 
-                            // Check that the sha256 of the data we got from
-                            // wicket matches the data we asked for.
-                            //
-                            // If this fails, we fail the entire installation
-                            // rather than trying to fetch the artifact again,
-                            // because we're fetching data from wicketd (cached
-                            // in a temp dir) over TCP to ourselves (in memory),
-                            // so the only cases where this could fail are
-                            // disturbing enough (memory corruption, corruption
-                            // under TCP, or wicketd gave us something other
-                            // than what we requested) we want to know
-                            // immediately and not retry: it's likely an
-                            // operator could miss any warnings we emit if a
-                            // retry succeeds.
-                            check_downloaded_artifact_hash(
-                                "installinator document",
-                                installinator_doc.artifact.clone(),
-                                installinator_doc_id.hash,
-                            )
-                            .await?;
+                    // Check that the sha256 of the data we got from
+                    // wicket matches the data we asked for.
+                    //
+                    // If this fails, we fail the entire installation
+                    // rather than trying to fetch the artifact again,
+                    // because we're fetching data from wicketd (cached
+                    // in a temp dir) over TCP to ourselves (in memory),
+                    // so the only cases where this could fail are
+                    // disturbing enough (memory corruption, corruption
+                    // under TCP, or wicketd gave us something other
+                    // than what we requested) we want to know
+                    // immediately and not retry: it's likely an
+                    // operator could miss any warnings we emit if a
+                    // retry succeeds.
+                    check_downloaded_artifact_hash(
+                        "installinator document",
+                        installinator_doc.artifact.clone(),
+                        installinator_doc_id.hash,
+                    )
+                    .await?;
 
-                            // Read the document as JSON.
-                            //
-                            // Going via the reader is slightly less efficient
-                            // than operating purely in-memory, but serde_json
-                            // doesn't have a good way to pass in a BufList
-                            // directly.
-                            let json: InstallinatorDocument =
-                                serde_json::from_reader(buf_list::Cursor::new(
-                                    &installinator_doc.artifact,
-                                ))
-                                .context(
-                                    "error deserializing \
+                    // Read the document as JSON.
+                    //
+                    // Going via the reader is slightly less efficient
+                    // than operating purely in-memory, but serde_json
+                    // doesn't have a good way to pass in a BufList
+                    // directly.
+                    let json: InstallinatorDocument = serde_json::from_reader(
+                        buf_list::Cursor::new(&installinator_doc.artifact),
+                    )
+                    .context(
+                        "error deserializing \
                                      installinator document",
-                                )?;
-
-                            // Every valid installinator document must have the
-                            // host phase 2 and control plane hashes.
-                            let mut host_phase_2_hash = None;
-                            let mut control_plane_hash = None;
-                            for artifact in &json.artifacts {
-                                match artifact.kind {
-                                    InstallinatorArtifactKind::HostPhase2 => {
-                                        host_phase_2_hash = Some(artifact.hash);
-                                    }
-                                    InstallinatorArtifactKind::ControlPlane => {
-                                        control_plane_hash =
-                                            Some(artifact.hash);
-                                    }
-                                }
-                            }
-
-                            // Return an error if either the host phase 2 or the
-                            // control plane artifacts are missing.
-                            let mut missing = Vec::new();
-                            if host_phase_2_hash.is_none() {
-                                missing.push(
-                                    InstallinatorArtifactKind::HostPhase2,
-                                );
+                    )?;
+
+                    // Every valid installinator document must have the
+                    // host phase 2 and control plane hashes.
+                    let mut host_phase_2_hash = None;
+                    let mut control_plane_hash = None;
+                    for artifact in &json.artifacts {
+                        match artifact.kind {
+                            InstallinatorArtifactKind::HostPhase2 => {
+                                host_phase_2_hash = Some(artifact.hash);
                             }
-                            if control_plane_hash.is_none() {
-                                missing.push(
-                                    InstallinatorArtifactKind::ControlPlane,
-                                );
+                            InstallinatorArtifactKind::ControlPlane => {
+                                control_plane_hash = Some(artifact.hash);
                             }
-                            if !missing.is_empty() {
-                                bail!(
-                                    "installinator document missing \
+                        }
+                    }
+
+                    // Return an error if either the host phase 2 or the
+                    // control plane artifacts are missing.
+                    let mut missing = Vec::new();
+                    if host_phase_2_hash.is_none() {
+                        missing.push(InstallinatorArtifactKind::HostPhase2);
+                    }
+                    if control_plane_hash.is_none() {
+                        missing.push(InstallinatorArtifactKind::ControlPlane);
+                    }
+                    if !missing.is_empty() {
+                        bail!(
+                            "installinator document missing \
                                      required artifacts: {:?}",
-                                    missing
-                                );
-                            }
+                            missing
+                        );
+                    }
 
-                            StepSuccess::new(ArtifactsToDownload {
-                                host_phase_2: host_phase_2_hash.expect(
-                                    "host phase 2 is Some, checked above",
-                                ),
-                                control_plane: control_plane_hash.expect(
-                                    "control plane is Some, checked above",
-                                ),
-                            })
-                            .into()
-                        },
-                    )
-                    .register()
-            }
-            LookupIdKind::Hashes { host_phase_2, control_plane } => {
-                StepHandle::ready(ArtifactsToDownload {
-                    host_phase_2,
-                    control_plane,
-                })
-            }
-        }
-        .into_shared();
+                    StepSuccess::new(ArtifactsToDownload {
+                        host_phase_2: host_phase_2_hash
+                            .expect("host phase 2 is Some, checked above"),
+                        control_plane: control_plane_hash
+                            .expect("control plane is Some, checked above"),
+                    })
+                    .into()
+                },
+            )
+            .register()
+            .into_shared();
 
         let to_download_2 = to_download.clone();
         let to_download_3 = to_download_2.clone();