Merge ~ahasenack/ubuntu/+source/base-files:eoan-base-file-handle-empty-cloud-id-1841597 into ubuntu/+source/base-files:ubuntu/devel

[Andreas Hasenack]

Set cloud-id to unknown if the tool exited with a non-zero status, or printed an empty string.

I wanted to use set -o pipefail, so I could check the exit status of cloud-id in this line below, but dash doesn't have this option:

cloud_id=$(/usr/bin/cloud-id 2>/dev/null | cut -c -40 | tr -c -d '[:alnum:]')

Therefore I have to call cloud-id first, check $?, and then proceed if that is zero, and this is this branch. I checked if shell chars in the cloud-id output would break the script, and looks like they don't, at least in this test where I replaced /usr/bin/cloud-id with a script of mine, outputting all sorts of nasty things:

++ /usr/bin/cloud-id
$(id)'d_id='"\'\'';id nasty things
$(id)' ']'e 0 -o -z '"\'\'';id nasty things
$(id)'o '"\'\'';id nasty things
++ cut -c -40
++ tr -c -d '[:alnum:]'
+ cloud_id=idnastythingsid
+ read up idle
+ uptime=uptime/15283.00/15247.00
+ USER_AGENT='curl/7.65.3-1ubuntu1 Ubuntu/Eoan/Ermine/(development/branch) GNU/Linux/4.15.0-58-generic/x86_64 Intel(R)/Core(TM)/i7-7600U/CPU/@/2.80GHz uptime/15283.00/15247.00 cloud_id/idnastythingsid'

I also got a suggestion to use head or tail -n 1 in the cloud-id output, but I think it's best to have the tr call delete the \n and join multiple lines if that ever happens, which it does today. I rather not overoptimize this client-side branch, and deal with this on the server if I have to, unless it becomes a real problem.

Just to remember, the server side will do this kind of parsing:
+ RewriteEngine on
+ # list in the regexp all the clouds which should get motd support
+ RewriteCond "%{HTTP_USER_AGENT}" "cloud_id/(aws|azure|gce)\b"
+ RewriteRule "^/$" "/index-%1.txt" [L]

Specific for those 3 clouds at the moment, spelled like that. And if the corresponding index-$cloud.txt file doesn't exist, the 404 document is index.txt, which exists as that is the common motd message content.

[Christian Ehrhardt  (paelzer)]

Two minor comments inline - one a potential rework here one a cloud-init bug/feature if you like it, but generally LGTM already +1

review: Approve

[Andreas Hasenack (ahasenack)]

I pushed a new alternative, using a tempfile for the cloud-id data, please take another look

[Andreas Hasenack (ahasenack)]

Lastly I renamed CLOUD_ID (the tempfile) to just CLOUD, to avoid confusion with the variable cloud_id.

Tagging and uploading f1d54c80e8841754b37eb7b17caf5d93157166e1

$ git push pkg upload/10.2ubuntu6
Enumerating objects: 14, done.
Counting objects: 100% (14/14), done.
Delta compression using up to 2 threads
Compressing objects: 100% (9/9), done.
Writing objects: 100% (9/9), 1.30 KiB | 222.00 KiB/s, done.
Total 9 (delta 6), reused 0 (delta 0)
To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/base-files
 * [new tag] upload/10.2ubuntu6 -> upload/10.2ubuntu6

$ dput ubuntu ../base-files_10.2ubuntu6_source.changes
Checking signature on .changes
gpg: ../base-files_10.2ubuntu6_source.changes: Valid signature from AC983EB5BF6BCBA9
Checking signature on .dsc
gpg: ../base-files_10.2ubuntu6.dsc: Valid signature from AC983EB5BF6BCBA9
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading base-files_10.2ubuntu6.dsc: done.
  Uploading base-files_10.2ubuntu6.tar.xz: done.
  Uploading base-files_10.2ubuntu6_source.buildinfo: done.
  Uploading base-files_10.2ubuntu6_source.changes: done.
Successfully uploaded packages.

diff --git a/debian/changelog b/debian/changelog
index 5054e04..c5dd058 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+base-files (10.2ubuntu6) eoan; urgency=medium
+
+  * motd/50-motd-news: set cloud-id to unknown if the tool exited with a
+    non-zero status, or printed an empty string (LP: #1841597)
+
+ -- Andreas Hasenack <[e-mail]>  Tue, 27 Aug 2019 15:31:30 -0300
+
 base-files (10.2ubuntu5) eoan; urgency=medium
 
   * motd/50-motd-news: add cloud_id to user-agent string (LP: #1840946)
diff --git a/motd/50-motd-news b/motd/50-motd-news
index 026429f..838eeb0 100644
--- a/motd/50-motd-news
+++ b/motd/50-motd-news
@@ -67,7 +67,8 @@ fi
 # Generate our temp files, clean up when done
 NEWS=$(mktemp) || exit 1
 ERR=$(mktemp) || exit 1
-trap "rm -f $NEWS $ERR" HUP INT QUIT ILL TRAP KILL BUS TERM
+CLOUD=$(mktemp) || exit 1
+trap "rm -f $NEWS $ERR $CLOUD" HUP INT QUIT ILL TRAP KILL BUS TERM
 
 # Construct a user agent, similar to Firefox/Chrome/Safari/IE to
 # ensure a proper, tailored, accurate message of the day
@@ -84,10 +85,16 @@ codename="$DISTRIB_CODENAME"
 platform="$(uname -o)/$(uname -r)/$(uname -m)"
 arch="$(uname -m)"
 cpu="$(grep -m1 "^model name" /proc/cpuinfo | sed -e "s/.*: //" -e "s:\s\+:/:g")"
+cloud_id="unknown"
 if [ -x /usr/bin/cloud-id ]; then
-    cloud_id=$(/usr/bin/cloud-id 2>/dev/null | cut -c -40 | tr -c -d '[:alnum:]')
-else
-    cloud_id="unknown"
+    /usr/bin/cloud-id > "$CLOUD" 2>/dev/null
+    if [ "$?" -eq "0" ]; then
+        # sanitize it a bit, just in case
+        cloud_id=$(cut -c -40 "${CLOUD}" | tr -c -d '[:alnum:]')
+        if [ -z "${cloud_id}" ]; then
+            cloud_id="unknown"
+        fi
+    fi
 fi
 
 # Some messages may only be pertinent before or after some amount of uptime
@@ -124,5 +131,5 @@ for u in $URLS; do
 		: > "$CACHE"
  	fi
 done
-rm -f "$NEWS" "$ERR"
+rm -f "$NEWS" "$ERR" "$CLOUD"
 exit 0