Merge lp:~cyphermox/maas/maas-enlist-wget-not-curl into lp:~maas-maintainers/maas/maas-enlist

[Mathieu Trudel-Lapierre]

Use wget instead of curl.

At the very least, this allows maas-enlist to work as a d-i module, for use with "Multiple installs using MAAS" from the server CD boot menu.

[Mike Pontillo (mpontillo)]

In this case it looks like the original curl implementation is more robust, since it URL encodes the parameters properly, whereas the wget implementation will just concatenate them into the URL string.

I would rather split this into enlist_node_wget() and enlist_node_curl() and have a --wget parameter to choose the wget backend if desired, leaving the curl backend the default.

review: Needs Fixing

[Mike Pontillo (mpontillo)]

I just tested both implementations and confirmed the issue. (the wget implementation, for example, doesn't work with power parameters whose passwords have the '&' character in them).

I noticed another difference as well: the original `curl` implementation helpfully prints the JSON output from the MAAS API call to stdout, whereas the wget version redirects it to /dev/null, leaving the user with no real indication of success or failure. That should be fixed as well before this lands.

[Mike Pontillo (mpontillo)]

(to be clear, I'm okay with NOT fixing all the issues with the wget version, as long as it has it's own separate --wget option to select wget for the I/O.)

[Mathieu Trudel-Lapierre (cyphermox)]

If you want to do that, you will need to explicitly change also the maas-enlist call in maas-enlist-udeb.postinst to use --wget.

Note, it doesn't seem like the url encoding is really useful in this case, and it is still possible to urlencode programmatically outside wget before passing the parameters. I chose not to, because the MAC address could be passed without encoding (received just fine with the ':').

[Mike Pontillo (mpontillo)]

Perhaps we'll just prefer curl, if installed, and fall back to wget if not found, then.

=== modified file 'bin/maas-enlist'
--- bin/maas-enlist	2014-02-25 15:52:42 +0000
+++ bin/maas-enlist	2015-09-03 20:21:26 +0000
@@ -37,12 +37,12 @@
 	echo "$mac"
 }
 
-get_mac_address_curl_parms() {
+get_mac_address_parms() {
 	local args="" input_string="$1"
 	OIFS=$IFS; IFS=","; set -- $input_string; IFS=$OIFS
 	for i in "$@";
 	do
-		args="${args} --data-urlencode mac_addresses=${i}"
+		args="${args}&mac_addresses=${i}"
 		#mac_address="$mac_address""&mac_addresses=""${i}";
 	done
 	echo "${args# }"
@@ -78,6 +78,7 @@
 	_RET=${_RET%%/*};
 	echo "$_RET";
 }
+
 enlist_node() {
 	serverurl="${1}"
 	mac="${2}"
@@ -88,19 +89,20 @@
 	power_params="${7}"
 
 	local macparms=""
-	macparms=$(get_mac_address_curl_parms "$mac")
-
-	curl \
-	    --data-urlencode "op=new" \
-        --data-urlencode "autodetect_nodegroup=1" \
-	    --data-urlencode "hostname=${hostname}" \
-	    --data-urlencode "architecture=${arch}" \
-	    --data-urlencode "subarchitecture=${subarch}" \
-	    --data-urlencode "power_type=${power_type}" \
-	    --data-urlencode "power_parameters=${power_params}" \
-	    ${macparms} \
-	    "${serverurl}"
-
+	macparms=$(get_mac_address_parms "$mac")
+ 
+	local params="op=new"
+	params="${params}&autodetect_nodegroup=1"
+	params="${params}&hostname=${hostname}"
+	params="${params}&architecture=${arch}"
+	params="${params}&subarchitecture=${subarch}"
+	params="${params}&power_type=${power_type}"
+	params="${params}&power_parameters=${power_params}"
+	params="${params}${macparms}"
+
+	wget -q -O/dev/null \
+	    "${serverurl}" \
+	    --post-data="$params"
 }
 
 Error () {