From b3e81f22e4a7e3431a04a4cc963770f4d9d78845 Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Wed, 29 Jan 2014 14:01:33 -0800 Subject: [PATCH 1/3] fix(image_to_vm): Wrap upload_image in vm_upload. This keeps all references to VM_GENERATED_FILES inside of build_library/vm_image_util.sh so changes that impact it more obvious. --- build_library/vm_image_util.sh | 4 ++++ image_to_vm.sh | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/build_library/vm_image_util.sh b/build_library/vm_image_util.sh index 2a67635f1e..a678d52d4d 100644 --- a/build_library/vm_image_util.sh +++ b/build_library/vm_image_util.sh @@ -631,6 +631,10 @@ vm_cleanup() { sudo rm -rf "${VM_TMP_DIR}" } +vm_upload() { + upload_image "${VM_GENERATED_FILES[@]}" +} + print_readme() { local filename info "Files written to $(relpath "$(dirname "${VM_DST_IMG}")")" diff --git a/image_to_vm.sh b/image_to_vm.sh index d2bf6823b5..37d25de020 100755 --- a/image_to_vm.sh +++ b/image_to_vm.sh @@ -106,7 +106,7 @@ vm_cleanup trap - EXIT # Optionally upload all of our hard work -upload_image "${VM_GENERATED_FILES[@]}" +vm_upload # Ready to set sail! okboat From feb59db9f58f1e02c0acdf6488453f826c322be1 Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Wed, 29 Jan 2014 15:56:29 -0800 Subject: [PATCH 2/3] fix(release_util): Add argument to specify the name to use for .DIGESTS For multi-file uploads we should explicitly declare what the name of the .DIGESTS file should be instead of using the first file name. Relying on the ordering was subtle and easy to break. --- build_image | 3 ++- build_library/release_util.sh | 29 ++++++++++++++++++++-------- build_library/vm_image_util.sh | 3 ++- common.sh | 35 +++++++++++++++++++--------------- 4 files changed, 45 insertions(+), 25 deletions(-) diff --git a/build_image b/build_image index 521fdd6c0b..8649fe7c5f 100755 --- a/build_image +++ b/build_image @@ -170,7 +170,8 @@ COREOS_PATCH=${COREOS_PATCH} COREOS_SDK_VERSION=${COREOS_SDK_VERSION} EOF -upload_image "${BUILD_DIR}/au-generator.zip" "${BUILD_DIR}/version.txt" +upload_image -d "${BUILD_DIR}/au-generator.zip.DIGESTS" \ + "${BUILD_DIR}/au-generator.zip" "${BUILD_DIR}/version.txt" # Create a named symlink. LINK_NAME="${FLAGS_output_root}/${BOARD}/${FLAGS_symlink}" diff --git a/build_library/release_util.sh b/build_library/release_util.sh index 8d96e04cff..b9790bb7fc 100644 --- a/build_library/release_util.sh +++ b/build_library/release_util.sh @@ -85,12 +85,25 @@ upload_packages() { upload_files packages ${def_upload_path} "pkgs/" "${board_packages}"/* } -# Upload a image along with optional supporting files -# The image file must be the first argument +# Upload a set of files (usually images) and digest, optionally w/ gpg sig +# If more than one file is specified -d must be the first argument +# Usage: upload_image [-d file.DIGESTS] file1 [file2...] upload_image() { [[ ${FLAGS_upload} -eq ${FLAGS_TRUE} ]] || return 0 [[ -n "${BOARD}" ]] || die "board_options.sh must be sourced first" + # The name to use for .DIGESTS and .DIGESTS.asc must be explicit if + # there is more than one file to upload to avoid potential confusion. + local digests + if [[ "$1" == "-d" ]]; then + [[ -n "$2" ]] || die "-d requires an argument" + digests="$2" + shift 2 + else + [[ $# -eq 1 ]] || die "-d is required for multi-file uploads" + digests="${1}.DIGESTS" + fi + local uploads=() local filename for filename in "$@"; do @@ -110,18 +123,18 @@ upload_image() { # For consistency generate a .DIGESTS file similar to the one catalyst # produces for the SDK tarballs and up upload it too. - make_digests "${uploads[@]}" - uploads+=( "${uploads[0]}.DIGESTS" ) + make_digests -d "${digests}" "${uploads[@]}" + uploads+=( "${digests}" ) # Create signature as ...DIGESTS.asc as Gentoo does. if [[ -n "${FLAGS_sign_digests}" ]]; then - rm -f "${uploads[0]}.DIGESTS.asc" + rm -f "${digests}.asc" gpg --batch --local-user "${FLAGS_sign_digests}" \ - --clearsign "${uploads[0]}.DIGESTS" || die "gpg failed" - uploads+=( "${uploads[0]}.DIGESTS.asc" ) + --clearsign "${digests}" || die "gpg failed" + uploads+=( "${digests}.asc" ) fi - local log_msg="${1##*/}" + local log_msg=$(basename "$digests" .DIGESTS) local def_upload_path="${UPLOAD_ROOT}/${BOARD}/${COREOS_VERSION_STRING}" upload_files "${log_msg}" "${def_upload_path}" "" "${uploads[@]}" } diff --git a/build_library/vm_image_util.sh b/build_library/vm_image_util.sh index a678d52d4d..0c8cb46d62 100644 --- a/build_library/vm_image_util.sh +++ b/build_library/vm_image_util.sh @@ -632,7 +632,8 @@ vm_cleanup() { } vm_upload() { - upload_image "${VM_GENERATED_FILES[@]}" + local digests="${VM_GENERATED_FILES[0]}.DIGESTS" + upload_image -d "${digests}" "${VM_GENERATED_FILES[@]}" } print_readme() { diff --git a/common.sh b/common.sh index 6e638e175c..bd94e09500 100644 --- a/common.sh +++ b/common.sh @@ -693,40 +693,45 @@ enable_rw_mount() { # Generate a DIGESTS file, as normally used by Gentoo. # This is an alternative to shash which doesn't know how to report errors. -# Usage: make_digests file1 [file2...] -# Output: file1.DIGESTS -# Any extra files be hashed and listed in file1.DIGESTS +# Usage: make_digests -d file.DIGESTS file1 [file2...] _digest_types="md5 sha1 sha512" make_digests() { - local dirname=$(dirname "$1") - local basename=$(basename "$1") + [[ "$1" == "-d" ]] || die + local digests="$(readlink -f "$2")" + shift 2 - pushd "${dirname}" >/dev/null - echo -n > "${basename}.DIGESTS" + pushd "$(dirname "$1")" >/dev/null + echo -n > "${digests}" for filename in "$@"; do filename=$(basename "$filename") info "Computing DIGESTS for ${filename}" for hash_type in $_digest_types; do - echo "# $hash_type HASH" | tr "a-z" "A-Z" >> "${basename}.DIGESTS" - ${hash_type}sum "${filename}" >> "${basename}.DIGESTS" + echo "# $hash_type HASH" | tr "a-z" "A-Z" >> "${digests}" + ${hash_type}sum "${filename}" >> "${digests}" done done popd >/dev/null } # Validate a DIGESTS file. Essentially the inverse of make_digests. -# Usage: verify_digests file1 [file2...] -# Checks the hash of all given files using file1.DIGESTS +# Usage: verify_digests [-d file.DIGESTS] file1 [file2...] +# If -d is not specified file1.DIGESTS will be used verify_digests() { - local dirname=$(dirname "$1") - local basename=$(basename "$1") + local digests + if [[ "$1" == "-d" ]]; then + [[ -n "$2" ]] || die "-d requires an argument" + digests="$(readlink -f "$2")" + shift 2 + else + digests=$(basename "${1}.DIGESTS") + fi - pushd "${dirname}" >/dev/null + pushd "$(dirname "$1")" >/dev/null for filename in "$@"; do filename=$(basename "$filename") info "Validating DIGESTS for ${filename}" for hash_type in $_digest_types; do - grep -A1 -i "^# ${hash_type} HASH$" "${basename}.DIGESTS" | \ + grep -A1 -i "^# ${hash_type} HASH$" "${digests}" | \ grep "$filename$" | ${hash_type}sum -c - --strict || return 1 # Also check that none of the greps failed in the above pipeline [[ -z ${PIPESTATUS[*]#0} ]] || return 1 From 8a15840d6a4c3af92d08f634d4589cb0fe054e12 Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Wed, 29 Jan 2014 16:38:48 -0800 Subject: [PATCH 3/3] fix(vm_image_util): Use more consistent names for vm .DIGESTS Switch from naming DIGESTS based in disk image name to a common prefix. old: coreos_production_qemu_image.img.DIGESTS -> new: coreos_production_qemu.DIGESTS The old behavior wasn't very consistent since plain disk images aren't used by all types and the code implementing that was easy to brake, namely by mistake coreos_production_pxe_image.cpio.gz.DIGESTS became coreos_production_pxe.vmlinuz.DIGESTS a couple releases ago. The old names will continue to exist as well for the time being to avoid breaking existing install/download scripts and the original pxe DIGESTS name is back. --- build_library/vm_image_util.sh | 35 +++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/build_library/vm_image_util.sh b/build_library/vm_image_util.sh index 0c8cb46d62..f6b1da68d0 100644 --- a/build_library/vm_image_util.sh +++ b/build_library/vm_image_util.sh @@ -632,8 +632,41 @@ vm_cleanup() { } vm_upload() { - local digests="${VM_GENERATED_FILES[0]}.DIGESTS" + local digests="$(_dst_dir)/$(_dst_name .DIGESTS)" upload_image -d "${digests}" "${VM_GENERATED_FILES[@]}" + + # FIXME(marineam): Temporary alternate name for .DIGESTS + # This used to be derived from the first file listed in + # ${VM_GENERATED_FILES[@]}", usually $VM_DST_IMG or similar. + # Since not everything actually uploads $VM_DST_IMG this was not very + # consistent and relying on ordering was breakable. + # Now the common prefix, output by $(_dst_name) is used above. + # Some download/install scripts may still refer to the old name. + local uploaded legacy_digests + for uploaded in "${VM_GENERATED_FILES[@]}"; do + if [[ "${uploaded}" == "${VM_DST_IMG}" ]]; then + legacy_digests="$(_dst_dir)/$(basename ${VM_DST_IMG}).DIGESTS" + break + fi + done + + # Since depending on the ordering of $VM_GENERATED_FILES is brittle only + # use it if $VM_DST_IMG isn't included in the uploaded files. + if [[ -z "${legacy_digests}" ]]; then + legacy_digests="${VM_GENERATED_FILES[0]}.DIGESTS" + fi + + [[ "${legacy_digests}" != "${digests}" ]] || return + + local legacy_uploads=( "${legacy_digests}" ) + cp "${digests}" "${legacy_digests}" + if [[ -e "${digests}.asc" ]]; then + legacy_uploads+=( "${legacy_digests}.asc" ) + cp "${digests}.asc" "${legacy_digests}.asc" + fi + + local def_upload_path="${UPLOAD_ROOT}/${BOARD}/${COREOS_VERSION_STRING}" + upload_files "$(_dst_name)" "${def_upload_path}" "" "${legacy_uploads[@]}" } print_readme() {