Merge ~ahasenack/ubuntu/+source/base-files:eoan-base-file-handle-empty-cloud-id-1841597 into ubuntu/+source/base-files:ubuntu/devel
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Andreas Hasenack | ||||
Approved revision: | f1d54c80e8841754b37eb7b17caf5d93157166e1 | ||||
Merged at revision: | f1d54c80e8841754b37eb7b17caf5d93157166e1 | ||||
Proposed branch: | ~ahasenack/ubuntu/+source/base-files:eoan-base-file-handle-empty-cloud-id-1841597 | ||||
Merge into: | ubuntu/+source/base-files:ubuntu/devel | ||||
Diff against target: |
56 lines (+19/-5) 2 files modified
debian/changelog (+7/-0) motd/50-motd-news (+12/-5) |
||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Christian Ehrhardt (community) | Approve | ||
Canonical Server | Pending | ||
Review via email: mp+371905@code.launchpad.net |
Description of the change
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=
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)' ']'e 0 -o -z '"\'\'';id nasty things
$(id)'o '"\'\'';id nasty things
++ cut -c -40
++ tr -c -d '[:alnum:]'
+ cloud_id=
+ read up idle
+ uptime=
+ USER_AGENT=
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_
+ 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.
Two minor comments inline - one a potential rework here one a cloud-init bug/feature if you like it, but generally LGTM already +1