From c76692192ad25dbbfc9db9cf239f69a8c80a1062 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Thu, 29 Oct 2020 13:57:19 -0400
Subject: [PATCH 1/9] nixos ec2/create-amis.sh: shellcheck: quote region
 references

---
 nixos/maintainers/scripts/ec2/create-amis.sh | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/nixos/maintainers/scripts/ec2/create-amis.sh b/nixos/maintainers/scripts/ec2/create-amis.sh
index 89e24f2ccfd..0a0b07577c2 100755
--- a/nixos/maintainers/scripts/ec2/create-amis.sh
+++ b/nixos/maintainers/scripts/ec2/create-amis.sh
@@ -1,5 +1,6 @@
 #!/usr/bin/env nix-shell
 #!nix-shell -p awscli -p jq -p qemu -i bash
+# shellcheck shell=bash
 
 # Uploads and registers NixOS images built from the
 # <nixos/release.nix> amazonImage attribute. Images are uploaded and
@@ -111,7 +112,7 @@ wait_for_import() {
     log "Waiting for import task $task_id to be completed"
     while true; do
         read state progress snapshot_id < <(
-            aws ec2 describe-import-snapshot-tasks --region $region --import-task-ids "$task_id" | \
+            aws ec2 describe-import-snapshot-tasks --region "$region" --import-task-ids "$task_id" | \
                 jq -r '.ImportSnapshotTasks[].SnapshotTaskDetail | "\(.Status) \(.Progress) \(.SnapshotId)"'
         )
         log " ... state=$state progress=$progress snapshot_id=$snapshot_id"
@@ -139,7 +140,7 @@ wait_for_image() {
 
     while true; do
         read state < <(
-            aws ec2 describe-images --image-ids "$ami_id" --region $region | \
+            aws ec2 describe-images --image-ids "$ami_id" --region "$region" | \
                 jq -r ".Images[].State"
         )
         log " ... state=$state"
@@ -163,7 +164,7 @@ make_image_public() {
     local region=$1
     local ami_id=$2
 
-    wait_for_image $region "$ami_id"
+    wait_for_image "$region" "$ami_id"
 
     log "Making image $ami_id public"
 
@@ -185,7 +186,7 @@ upload_image() {
         log "Checking for image on S3"
         if ! aws s3 ls --region "$region" "s3://${bucket}/${aws_path}" >&2; then
             log "Image missing from aws, uploading"
-            aws s3 cp --region $region "$image_file" "s3://${bucket}/${aws_path}" >&2
+            aws s3 cp --region "$region" "$image_file" "s3://${bucket}/${aws_path}" >&2
         fi
 
         log "Importing image from S3 path s3://$bucket/$aws_path"
@@ -197,7 +198,7 @@ upload_image() {
               \"S3Bucket\": \"$bucket\",
               \"S3Key\": \"$aws_path\"
           }
-        }" --region $region | jq -r '.ImportTaskId')
+        }" --region "$region" | jq -r '.ImportTaskId')
 
         write_state "$state_key" task_id "$task_id"
     fi
@@ -230,7 +231,7 @@ upload_image() {
             aws ec2 register-image \
                 --name "$image_name" \
                 --description "$image_description" \
-                --region $region \
+                --region "$region" \
                 --architecture $amazon_arch \
                 --block-device-mappings "${block_device_mappings[@]}" \
                 "${extra_flags[@]}" \
@@ -240,7 +241,7 @@ upload_image() {
         write_state "$state_key" ami_id "$ami_id"
     fi
 
-    make_image_public $region "$ami_id"
+    make_image_public "$region" "$ami_id"
 
     echo "$ami_id"
 }
@@ -268,7 +269,7 @@ copy_to_region() {
         write_state "$state_key" ami_id "$ami_id"
     fi
 
-    make_image_public $region "$ami_id"
+    make_image_public "$region" "$ami_id"
 
     echo "$ami_id"
 }

From f5994c208df64f3db0f4f69efffb3a2868e688b3 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Thu, 29 Oct 2020 13:58:37 -0400
Subject: [PATCH 2/9] nixos ec2/create-amis.sh: shellcheck: quote state_dir
 reference

---
 nixos/maintainers/scripts/ec2/create-amis.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nixos/maintainers/scripts/ec2/create-amis.sh b/nixos/maintainers/scripts/ec2/create-amis.sh
index 0a0b07577c2..6be9cd59400 100755
--- a/nixos/maintainers/scripts/ec2/create-amis.sh
+++ b/nixos/maintainers/scripts/ec2/create-amis.sh
@@ -101,7 +101,7 @@ write_state() {
     local type=$2
     local val=$3
 
-    mkdir -p $state_dir
+    mkdir -p "$state_dir"
     echo "$val" > "$state_dir/$state_key.$type"
 }
 

From baf7ed3f2446c433885d0b99ace294b333358183 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Thu, 29 Oct 2020 13:59:34 -0400
Subject: [PATCH 3/9] nixos ec2/create-amis.sh: shellcheck: SC2155: Declare and
 assign separately to avoid masking return values.

---
 nixos/maintainers/scripts/ec2/create-amis.sh | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/nixos/maintainers/scripts/ec2/create-amis.sh b/nixos/maintainers/scripts/ec2/create-amis.sh
index 6be9cd59400..085de5a647d 100755
--- a/nixos/maintainers/scripts/ec2/create-amis.sh
+++ b/nixos/maintainers/scripts/ec2/create-amis.sh
@@ -178,9 +178,12 @@ upload_image() {
     local aws_path=${image_file#/}
 
     local state_key="$region.$image_label.$image_system"
-    local task_id=$(read_state "$state_key" task_id)
-    local snapshot_id=$(read_state "$state_key" snapshot_id)
-    local ami_id=$(read_state "$state_key" ami_id)
+    local task_id
+    task_id=$(read_state "$state_key" task_id)
+    local snapshot_id
+    snapshot_id=$(read_state "$state_key" snapshot_id)
+    local ami_id
+    ami_id=$(read_state "$state_key" ami_id)
 
     if [ -z "$task_id" ]; then
         log "Checking for image on S3"

From a66a22ca545ce94a7b69aefe163fea2c20241f9d Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Thu, 29 Oct 2020 14:01:22 -0400
Subject: [PATCH 4/9] nixos ec2/create-amis.sh: shellcheck: read without -r
 mangles backslashes

---
 nixos/maintainers/scripts/ec2/create-amis.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/nixos/maintainers/scripts/ec2/create-amis.sh b/nixos/maintainers/scripts/ec2/create-amis.sh
index 085de5a647d..6e2836a7a97 100755
--- a/nixos/maintainers/scripts/ec2/create-amis.sh
+++ b/nixos/maintainers/scripts/ec2/create-amis.sh
@@ -111,7 +111,7 @@ wait_for_import() {
     local state snapshot_id
     log "Waiting for import task $task_id to be completed"
     while true; do
-        read state progress snapshot_id < <(
+        read -r state progress snapshot_id < <(
             aws ec2 describe-import-snapshot-tasks --region "$region" --import-task-ids "$task_id" | \
                 jq -r '.ImportSnapshotTasks[].SnapshotTaskDetail | "\(.Status) \(.Progress) \(.SnapshotId)"'
         )
@@ -139,7 +139,7 @@ wait_for_image() {
     log "Waiting for image $ami_id to be available"
 
     while true; do
-        read state < <(
+        read -r state < <(
             aws ec2 describe-images --image-ids "$ami_id" --region "$region" | \
                 jq -r ".Images[].State"
         )

From 7dac8470cf69d10bcea3a61b1919cebf12a94fe6 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Thu, 29 Oct 2020 14:01:43 -0400
Subject: [PATCH 5/9] nixos ec2/create-amis.sh: shellcheck: explicitly make the
 additions to block_device_mappings single strings

---
 nixos/maintainers/scripts/ec2/create-amis.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/nixos/maintainers/scripts/ec2/create-amis.sh b/nixos/maintainers/scripts/ec2/create-amis.sh
index 6e2836a7a97..16aa5ce35f4 100755
--- a/nixos/maintainers/scripts/ec2/create-amis.sh
+++ b/nixos/maintainers/scripts/ec2/create-amis.sh
@@ -225,10 +225,10 @@ upload_image() {
             --virtualization-type hvm
         )
 
-        block_device_mappings+=(DeviceName=/dev/sdb,VirtualName=ephemeral0)
-        block_device_mappings+=(DeviceName=/dev/sdc,VirtualName=ephemeral1)
-        block_device_mappings+=(DeviceName=/dev/sdd,VirtualName=ephemeral2)
-        block_device_mappings+=(DeviceName=/dev/sde,VirtualName=ephemeral3)
+        block_device_mappings+=("DeviceName=/dev/sdb,VirtualName=ephemeral0")
+        block_device_mappings+=("DeviceName=/dev/sdc,VirtualName=ephemeral1")
+        block_device_mappings+=("DeviceName=/dev/sdd,VirtualName=ephemeral2")
+        block_device_mappings+=("DeviceName=/dev/sde,VirtualName=ephemeral3")
 
         ami_id=$(
             aws ec2 register-image \

From f92a883ddb2e26e34244cf764fbfa63a4e580afa Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Thu, 29 Oct 2020 14:02:11 -0400
Subject: [PATCH 6/9] nixos ec2/create-amis.sh: shellcheck: $ is not needed in
 arithmetic

---
 nixos/maintainers/scripts/ec2/create-amis.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nixos/maintainers/scripts/ec2/create-amis.sh b/nixos/maintainers/scripts/ec2/create-amis.sh
index 16aa5ce35f4..6cac4bb58d5 100755
--- a/nixos/maintainers/scripts/ec2/create-amis.sh
+++ b/nixos/maintainers/scripts/ec2/create-amis.sh
@@ -65,7 +65,7 @@ image_logical_bytes=$(read_image_info .logical_bytes)
 
 # Derived attributes
 
-image_logical_gigabytes=$((($image_logical_bytes-1)/1024/1024/1024+1)) # Round to the next GB
+image_logical_gigabytes=$(((image_logical_bytes-1)/1024/1024/1024+1)) # Round to the next GB
 
 case "$image_system" in
     aarch64-linux)

From e253de8a772d8c73173807fef2e82ebce59c5153 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Fri, 30 Oct 2020 11:40:58 -0400
Subject: [PATCH 7/9] create-amis.sh: log the full response if describing the
 import snapshot tasks fails

---
 nixos/maintainers/scripts/ec2/create-amis.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/nixos/maintainers/scripts/ec2/create-amis.sh b/nixos/maintainers/scripts/ec2/create-amis.sh
index 6cac4bb58d5..d8bdf022d61 100755
--- a/nixos/maintainers/scripts/ec2/create-amis.sh
+++ b/nixos/maintainers/scripts/ec2/create-amis.sh
@@ -126,6 +126,8 @@ wait_for_import() {
                 ;;
             *)
                 log "Unexpected snapshot import state: '${state}'"
+                log "Full response: "
+                aws ec2 describe-import-snapshot-tasks --region "$region" --import-task-ids "$task_id" >&2
                 exit 1
                 ;;
         esac

From 2bf1fc034502dd89f1026f8cf7bbdcf250d82550 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Fri, 30 Oct 2020 11:59:09 -0400
Subject: [PATCH 8/9] create-amis: allow customizing the service role name

The complete setup on the AWS end can be configured
with the following Terraform configuration. It generates
a ./credentials.sh which I just copy/pasted in to the
create-amis.sh script near the top. Note: the entire stack
of users and bucket can be destroyed at the end of the
import.

    variable "region" {
      type = string
    }
    variable "availability_zone" {
      type = string
    }

    provider "aws" {
      region = var.region
    }

    resource "aws_s3_bucket" "nixos-amis" {
      bucket_prefix = "nixos-amis-"
      lifecycle_rule {
        enabled = true
        abort_incomplete_multipart_upload_days = 1
        expiration {
          days = 7
        }
      }
    }

    resource "local_file" "credential-file" {
      file_permission = "0700"
      filename = "${path.module}/credentials.sh"
      sensitive_content = <<SCRIPT
    export service_role_name="${aws_iam_role.vmimport.name}"
    export bucket="${aws_s3_bucket.nixos-amis.bucket}"
    export AWS_ACCESS_KEY_ID="${aws_iam_access_key.uploader.id}"
    export AWS_SECRET_ACCESS_KEY="${aws_iam_access_key.uploader.secret}"
    SCRIPT
    }

    # The following resources are for the *uploader*
    resource "aws_iam_user" "uploader" {
      name = "nixos-amis-uploader"
    }

    resource "aws_iam_access_key" "uploader" {
      user = aws_iam_user.uploader.name
    }

    resource "aws_iam_user_policy" "upload-to-nixos-amis" {
      user = aws_iam_user.uploader.name

      policy = data.aws_iam_policy_document.upload-policy-document.json
    }

    data "aws_iam_policy_document" "upload-policy-document" {
      statement {
        effect = "Allow"

        actions = [
          "s3:ListBucket",
          "s3:GetBucketLocation",
        ]

        resources = [
          aws_s3_bucket.nixos-amis.arn
        ]
      }

      statement {
        effect = "Allow"

        actions = [
          "s3:PutObject",
          "s3:GetObject",
          "s3:DeleteObject",
        ]

        resources = [
          "${aws_s3_bucket.nixos-amis.arn}/*"
        ]
      }

      statement {
        effect = "Allow"
        actions = [
          "ec2:ImportSnapshot",
          "ec2:DescribeImportSnapshotTasks",
          "ec2:DescribeImportSnapshotTasks",
          "ec2:RegisterImage",
          "ec2:DescribeImages"
        ]
        resources = [
          "*"
        ]
      }
    }

    # The following resources are for the *vmimport service user*
    # See: https://docs.aws.amazon.com/vm-import/latest/userguide/vmie_prereqs.html#vmimport-role
    resource "aws_iam_role" "vmimport" {
      assume_role_policy = data.aws_iam_policy_document.vmimport-trust.json
    }

    resource "aws_iam_role_policy" "vmimport-access" {
      role = aws_iam_role.vmimport.id
      policy = data.aws_iam_policy_document.vmimport-access.json
    }

    data "aws_iam_policy_document" "vmimport-access" {
      statement {
        effect = "Allow"
        actions = [
          "s3:GetBucketLocation",
          "s3:GetObject",
          "s3:ListBucket",
        ]
        resources = [
          aws_s3_bucket.nixos-amis.arn,
          "${aws_s3_bucket.nixos-amis.arn}/*"
        ]
      }
      statement {
        effect = "Allow"
        actions = [
          "ec2:ModifySnapshotAttribute",
          "ec2:CopySnapshot",
          "ec2:RegisterImage",
          "ec2:Describe*"
        ]
        resources = [
          "*"
        ]
      }
    }

    data "aws_iam_policy_document" "vmimport-trust" {
      statement {
        effect = "Allow"
        principals {
          type = "Service"
          identifiers = [ "vmie.amazonaws.com" ]
        }

        actions = [
          "sts:AssumeRole"
        ]

        condition {
          test = "StringEquals"
          variable = "sts:ExternalId"
          values = [ "vmimport" ]
        }
      }
    }
---
 nixos/maintainers/scripts/ec2/create-amis.sh | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/nixos/maintainers/scripts/ec2/create-amis.sh b/nixos/maintainers/scripts/ec2/create-amis.sh
index d8bdf022d61..fdcaba4e2e7 100755
--- a/nixos/maintainers/scripts/ec2/create-amis.sh
+++ b/nixos/maintainers/scripts/ec2/create-amis.sh
@@ -6,9 +6,10 @@
 # <nixos/release.nix> amazonImage attribute. Images are uploaded and
 # registered via a home region, and then copied to other regions.
 
-# The home region requires an s3 bucket, and a "vmimport" IAM role
-# with access to the S3 bucket.  Configuration of the vmimport role is
-# documented in
+# The home region requires an s3 bucket, and an IAM role named, by default,
+# "vmimport" IAM role with access to the S3 bucket. The name can be
+# configured with the "service_role_name" variable. Configuration of the
+# vmimport role is documented in
 # https://docs.aws.amazon.com/vm-import/latest/userguide/vmimport-image-import.html
 
 # set -x
@@ -18,6 +19,7 @@ set -euo pipefail
 state_dir=$HOME/amis/ec2-images
 home_region=eu-west-1
 bucket=nixos-amis
+service_role_name=vmimport
 
 regions=(eu-west-1 eu-west-2 eu-west-3 eu-central-1 eu-north-1
          us-east-1 us-east-2 us-west-1 us-west-2
@@ -196,7 +198,7 @@ upload_image() {
 
         log "Importing image from S3 path s3://$bucket/$aws_path"
 
-        task_id=$(aws ec2 import-snapshot --disk-container "{
+        task_id=$(aws ec2 import-snapshot --role-name "$service_role_name" --disk-container "{
           \"Description\": \"nixos-image-${image_label}-${image_system}\",
           \"Format\": \"vhd\",
           \"UserBucket\": {

From 74a577b2936180508a129aafd75b88489e6fd730 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Fri, 30 Oct 2020 12:40:17 -0400
Subject: [PATCH 9/9] create-amis: improve wording around the service name's
 IAM role

Co-authored-by: Cole Helbling <cole.e.helbling@outlook.com>
---
 nixos/maintainers/scripts/ec2/create-amis.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/nixos/maintainers/scripts/ec2/create-amis.sh b/nixos/maintainers/scripts/ec2/create-amis.sh
index fdcaba4e2e7..ec2eb536679 100755
--- a/nixos/maintainers/scripts/ec2/create-amis.sh
+++ b/nixos/maintainers/scripts/ec2/create-amis.sh
@@ -6,8 +6,8 @@
 # <nixos/release.nix> amazonImage attribute. Images are uploaded and
 # registered via a home region, and then copied to other regions.
 
-# The home region requires an s3 bucket, and an IAM role named, by default,
-# "vmimport" IAM role with access to the S3 bucket. The name can be
+# The home region requires an s3 bucket, and an IAM role named "vmimport"
+# (by default) with access to the S3 bucket. The name can be
 # configured with the "service_role_name" variable. Configuration of the
 # vmimport role is documented in
 # https://docs.aws.amazon.com/vm-import/latest/userguide/vmimport-image-import.html