Merge lp:~lazypower/charms/precise/nrpe/fix-lp-1287393 into lp:charms/nrpe

[Charles Butler]

replaces charm-helpers-sh call with nslookup

https://codereview.appspot.com/69980048/

[Charles Butler (lazypower)]

Reviewers: mp+209137_code.launchpad.net,

Message:
Please take a look.

Description:
replaces charm-helpers-sh call with nslookup

https://code.launchpad.net/~lazypower/charms/precise/nrpe/fix-lp-1287393/+merge/209137

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/69980048/

Affected files (+3, -1 lines):
   A [revision details]
   M hooks/nrpe-relation-joined

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: hooks/nrpe-relation-joined
=== modified file 'hooks/nrpe-relation-joined'
--- hooks/nrpe-relation-joined 2012-07-05 22:22:37 +0000
+++ hooks/nrpe-relation-joined 2014-03-03 22:05:22 +0000
@@ -7,7 +7,7 @@
  . hooks/nrpe.common.bash

  # NRPE requires IP
-address=$(ch_get_ip $(relation-get private-address))
+address=`nslookup $(unit-get private-address) | grep Add | grep -v '#' |
cut -f 2 -d ' '`

  juju-log "Adding $address to allowed_hosts."

[Matt Bruzek (mbruzek)]

I have a comment/question about the new command you are using.

https://codereview.appspot.com/69980048/diff/1/hooks/nrpe-relation-joined
File hooks/nrpe-relation-joined (right):

https://codereview.appspot.com/69980048/diff/1/hooks/nrpe-relation-joined#newcode10
hooks/nrpe-relation-joined:10: address=`nslookup $(unit-get
private-address) | grep Add | grep -v '#' | cut -f 2 -d ' '`
I ran this nslookup command on my own system and I believe the two greps
are cancelling each out.

$ nslookup 192.168.168.13
Server: 127.0.1.1
Address: 127.0.1.1#53

** server can't find 13.168.168.192.in-addr.arpa.: NXDOMAIN

$ nslookup 192.168.168.13 | grep Add
Address: 127.0.1.1#53

$ nslookup 192.168.168.13 | grep Add | grep -v '#'
(no match)

Are you sure this command works in all cases? Am I using the command
incorrectly here, or should we consider refining the command to work in
this case too?

https://codereview.appspot.com/69980048/

[Matt Bruzek (mbruzek)]

This change needs more information.

Add a comment to make it more clear what IP address the code is trying to get. The new code gets the unit address rather than the relation address (which I realize may be the same one).

Since there appears to be a possiblity of getting an empty address. Either fall back to the old way of getting the address or log a warning, or error if the address resolves to an empty string.

review: Needs Information

[lp:~lazypower/charms/precise/nrpe/fix-lp-1287393]

29. By Charles Butler on 2014-03-19

Embed lib/net.sh instead of rewriting the world

[Charles Butler (lazypower)]

Embedded net.sh with an addition of returning $HOST if found.

Refactored out the cowboy nslookup that fails on local. Give it another go please

[lp:~lazypower/charms/precise/nrpe/fix-lp-1287393]

30. By Charles Butler on 2014-03-19

remove stray return 1 for debugging

[Charles Butler (lazypower)]

Please take a look.

https://codereview.appspot.com/69980048/

[Matt Bruzek (mbruzek)]

+1 LGTM

review: Approve

[Charles Butler (lazypower)]

Merged

review: Approve

=== modified file 'hooks/nrpe-relation-joined'
--- hooks/nrpe-relation-joined	2012-07-05 22:22:37 +0000
+++ hooks/nrpe-relation-joined	2014-03-19 12:23:25 +0000
@@ -3,7 +3,7 @@
 
 set -e
 
-. /usr/share/charm-helper/sh/net.sh
+. lib/net.sh
 . hooks/nrpe.common.bash
 
 # NRPE requires IP

=== added directory 'lib'
=== added file 'lib/net.sh'
--- lib/net.sh	1970-01-01 00:00:00 +0000
+++ lib/net.sh	2014-03-19 12:23:25 +0000
@@ -0,0 +1,188 @@
+#!/bin/sh
+
+##
+# Copyright 2011 Marco Ceppi <[e-mail]>
+#
+# This file is part of Charm Helpers.
+#
+# Charm Helpers is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Charm Helpers is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Charm Helpers.  If not, see <http://www.gnu.org/licenses/>.
+##
+
+##
+# Globally overridable settings. Make sure to set them before sourcing
+# this file.
+CH_WGET_ARGS=${CH_WGET_ARGS:-"-q --content-disposition"}
+
+##
+# Get File
+# Retrives a file and compares the file to a hash
+#
+# param FILE URL or Path to file
+# param HASH URL, Path, or HASH string for file's hash
+#
+# return filepath|null
+##
+ch_get_file()
+{
+	local FILE=${1:-""}
+	local HASH=${2:-""}
+	local CH_DOWNLOAD_DIR=${CH_DOWNLOAD_DIR:-"`mktemp -d /tmp/ch-downloads.XXXXXX`"}
+
+	if [ `ch_is_url "$FILE"` ]; then
+		wget $CH_WGET_ARGS --directory-prefix="$CH_DOWNLOAD_DIR/" "$FILE"
+		FILE=$CH_DOWNLOAD_DIR/$(ls -tr $CH_DOWNLOAD_DIR|head -n 1)
+	fi
+
+	if [ ! -f "$FILE" ]; then
+		return 2
+	fi
+
+	if [ -z "$HASH" ];then
+		#echo "Warning, no has specified. The file will be downloaded but not cryptographically checked!" > 2
+		echo "$FILE"
+		return 0
+	elif [ `ch_is_url "$HASH"` ]; then
+		local HASHNAME=$(basename $HASH)
+		wget $CH_WGET_ARGS "$HASH" -O /tmp/$HASHNAME
+		HASH=$(cat /tmp/$HASHNAME | awk '{ print $1 }')
+	elif [ -f "$HASH" ]; then
+		HASH=$(cat "$HASH" | awk '{ print $1 }')
+	fi
+
+	local HASH_TYPE=$(ch_type_hash $HASH)
+
+	if [ -z "$HASH_TYPE" ]; then
+		return 3
+	else
+		local FILE_HASH=$(${HASH_TYPE}sum $FILE | awk '{ print $1 }')
+
+		if [ "$FILE_HASH" != "$HASH" ]; then
+			return 4
+		fi
+	fi
+
+	echo "$FILE"
+	return 0
+}
+
+##
+# Hash Type
+# Determine, using best approximation, if the hash is valid and what type
+# of hashing algorithm was used
+#
+# param HASH
+#
+# return type|false
+##
+ch_type_hash()
+{
+	local DIRTY="$1"
+
+	case $DIRTY in
+		*[![:xdigit:]]* | "" )
+			echo ""
+			return 1
+		;;
+		* )
+			case ${#DIRTY} in
+				32 )
+					echo md5
+				;;
+				40 )
+					echo sha1
+				;;
+				64 )
+					echo sha256
+				;;
+			esac
+		;;
+	esac
+}
+
+##
+# Is URL?
+# Checks if the string passed is a valid URL (http(s), ftp)
+#
+# param URL The URL to be checked
+#
+# return boolean
+##
+ch_is_url()
+{
+	local DIRTY="$1"
+
+	case "$DIRTY" in
+		"http://"* | "https://"* | "ftp://"*)
+			echo true
+			return 0
+		;;
+		*)
+			return 1
+		;;
+	esac
+}
+
+##
+# Is IP?
+# Checks if the string passed is an IP address
+#
+# param IP The IP addressed to be checked
+#
+# return boolean
+##
+ch_is_ip()
+{
+	local DIRTY="$1"
+	local IP=$(echo $DIRTY | awk -F. '$1 <=255 && $2 <= 255 && $3 <= 255 && $4 <= 255 ')
+
+	if [ -z "$IP" ]; then
+		return 1
+	else
+		echo true
+		return 0
+	fi
+}
+
+##
+# Get IP
+# Returns the first IP match to the hostname provided
+#
+# param HOSTNAME Host for which to retrieve an IP
+#
+# return IP|false
+##
+ch_get_ip()
+{
+	local HOST="$1"
+	#So, if there's multiple IP addresses, just grab the first
+	# for now.
+	if [ `ch_is_ip "$HOST"` ]; then
+		echo "$HOST"
+		return 0
+	fi
+
+	local CHECK_IP=$(host -t A $HOST | awk 'NR==1{ print $4 }')
+
+	if [ ! `ch_is_ip "$CHECK_IP"` ]; then
+		# Try a dig, why not?
+		CHECK_IP=$(dig +short $HOST | awk 'NR==1{ print $1 }')
+
+		if [ ! `ch_is_ip "$CHECK_IP"` ]; then
+			return 1
+		fi
+	fi
+
+	echo $CHECK_IP
+	return 0
+}