Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion debian/control
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Description: CloudStack server library

Package: cloudstack-agent
Architecture: all
Depends: ${misc:Depends}, ${python:Depends}, openjdk-8-jre-headless | openjdk-7-jre-headless, cloudstack-common (= ${source:Version}), lsb-base (>= 4.0), libcommons-daemon-java, openssh-client, qemu-kvm (>= 1.0), libvirt-bin (>= 0.9.8), uuid-runtime, iproute, ebtables, vlan, jsvc, ipset, python-libvirt, ethtool, iptables, perl-modules
Depends: ${misc:Depends}, ${python:Depends}, openjdk-8-jre-headless | openjdk-7-jre-headless, cloudstack-common (= ${source:Version}), lsb-base (>= 4.0), libcommons-daemon-java, openssh-client, qemu-kvm (>= 1.0), libvirt-bin (>= 0.9.8), uuid-runtime, iproute, ebtables, vlan, jsvc, ipset, python-libvirt, ethtool, iptables
Conflicts: cloud-agent, cloud-agent-libs, cloud-agent-deps, cloud-agent-scripts
Description: CloudStack agent
The CloudStack agent is in charge of managing shared computing resources in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -633,9 +633,9 @@ public boolean configure(final String name, final Map<String, Object> params) th
throw new ConfigurationException("Unable to find versions.sh");
}

_patchViaSocketPath = Script.findScript(kvmScriptsDir + "/patch/", "patchviasocket.pl");
_patchViaSocketPath = Script.findScript(kvmScriptsDir + "/patch/", "patchviasocket.py");
if (_patchViaSocketPath == null) {
throw new ConfigurationException("Unable to find patchviasocket.pl");
throw new ConfigurationException("Unable to find patchviasocket.py");
}

_heartBeatPath = Script.findScript(kvmScriptsDir, "kvmheartbeat.sh");
Expand Down
2 changes: 1 addition & 1 deletion scripts/installer/windows/client.wxs
Original file line number Diff line number Diff line change
Expand Up @@ -1055,7 +1055,7 @@
<File Id="fil47212822DDAFFCD71C79615CF4583BAF" KeyPath="yes" Source="!(wix.SourceClient)\WEB-INF\classes\scripts\vm\hypervisor\kvm\kvmheartbeat.sh" />
</Component>
<Component Id="cmpF2BBDD336FEC0B34B3C744ACF1E4B959" Guid="{56D8ECF7-49F8-4B26-A8F4-662252C0A647}">
<File Id="filD809C7F728AC5D1BD36E7DB403BFA141" KeyPath="yes" Source="!(wix.SourceClient)\WEB-INF\classes\scripts\vm\hypervisor\kvm\patchviasocket.pl" />
<File Id="filD809C7F728AC5D1BD36E7DB403BFA141" KeyPath="yes" Source="!(wix.SourceClient)\WEB-INF\classes\scripts\vm\hypervisor\kvm\patchviasocket.py" />
</Component>
<Component Id="cmp2F4D4D81563D153E86B0A652A83D363A" Guid="{7BFC7637-E33D-4BC4-8B25-3CDEA601110C}">
<File Id="fil349420D6088A01C9F63E27634623F5BE" KeyPath="yes" Source="!(wix.SourceClient)\WEB-INF\classes\scripts\vm\hypervisor\kvm\setup_agent.sh" />
Expand Down
58 changes: 0 additions & 58 deletions scripts/vm/hypervisor/kvm/patchviasocket.pl

This file was deleted.

76 changes: 76 additions & 0 deletions scripts/vm/hypervisor/kvm/patchviasocket.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
#!/usr/bin/env python
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

#
# This script connects to the system vm socket and writes the
# authorized_keys and cmdline data to it. The system VM then
# reads it from /dev/vport0p1 in cloud_early_config
#

import argparse
import os
import socket

SOCK_FILE = "/var/lib/libvirt/qemu/{name}.agent"
PUB_KEY_FILE = "/root/.ssh/id_rsa.pub.cloud"
MESSAGE = "pubkey:{key}\ncmdline:{cmdline}\n"


def send_to_socket(sock_file, key_file, cmdline):
if not os.path.exists(key_file):
print("ERROR: ssh public key not found on host at {0}".format(key_file))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why throw this error out of read_pub_key and consolidate the error handling/return in the exception handler on line 64?

return 1

try:
with open(key_file, "r") as f:
pub_key = f.read()
except IOError as e:
print("ERROR: unable to open {0} - {1}".format(key_file, e.strerror))
return 1

# Keep old substitution from perl code:
cmdline = cmdline.replace("%", " ")

msg = MESSAGE.format(key=pub_key, cmdline=cmdline)

if not os.path.exists(sock_file):
print("ERROR: {0} socket not found".format(sock_file))
return 1

try:
s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
s.connect(sock_file)
s.sendall(msg)
s.close()
except IOError as e:
Copy link
Contributor

@wido wido May 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is IOError the only exception which can occur here?

Wouldn't it be safer to but a 'return 0' in a 'finally' statement and otherwise return 1?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IOError is the parent of socket.error so this will catch all socket related errors.
The code as it stands returns 1 on any error and 0 on success - so there is no benefit of adding a finally statement there that I can see.

print("ERROR: unable to connect to {0} - {1}".format(sock_file, e.strerror))
return 1

return 0 # Success


if __name__ == "__main__":
parser = argparse.ArgumentParser(description="Send configuration to system VM socket")
parser.add_argument("-n", "--name", required=True, help="Name of VM")
parser.add_argument("-p", "--cmdline", required=True, help="Command line")

arguments = parser.parse_args()

socket_file = SOCK_FILE.format(name=arguments.name)

exit(send_to_socket(socket_file, PUB_KEY_FILE, arguments.cmdline))
144 changes: 144 additions & 0 deletions scripts/vm/hypervisor/kvm/test_patchviasocket.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
#!/usr/bin/env python
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

import patchviasocket

import getpass
import os
import socket
import tempfile
import time
import threading
import unittest

KEY_DATA = "I luv\nCloudStack\n"
CMD_DATA = "/run/this-for-me --please=TRUE! very%quickly"
NON_EXISTING_FILE = "must-not-exist"


def write_key_file():
_, tmpfile = tempfile.mkstemp(".sck")
with open(tmpfile, "w") as f:
f.write(KEY_DATA)
return tmpfile


class SocketThread(threading.Thread):
def __init__(self):
super(SocketThread, self).__init__()
self._data = ""
self._folder = tempfile.mkdtemp(".sck")
self._file = os.path.join(self._folder, "socket")
self._ready = False

def data(self):
return self._data

def file(self):
return self._file

def wait_until_ready(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be bounded by a maximum wait time? For example, if the ulimits are exceeded, this function could turn into an infinite loop.

while not self._ready:
time.sleep(0.050)

def run(self):
TIMEOUT = 0.314 # Very short time for tests that don't write to socket.
MAX_SIZE = 10 * 1024

s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
try:
s.bind(self._file)
s.listen(1)
s.settimeout(TIMEOUT)
try:
self._ready = True
client, address = s.accept()
self._data = client.recv(MAX_SIZE)
client.close()
except socket.timeout:
pass
finally:
s.close()
os.remove(self._file)
os.rmdir(self._folder)


class TestPatchViaSocket(unittest.TestCase):
def setUp(self):
self._key_file = write_key_file()

self._unreadable = write_key_file()
os.chmod(self._unreadable, 0)

self.assertFalse(os.path.exists(NON_EXISTING_FILE))
self.assertNotEqual("root", getpass.getuser(), "must be non-root user (to test access denied errors)")

def tearDown(self):
os.remove(self._key_file)
os.remove(self._unreadable)

def test_write_to_socket(self):
reader = SocketThread()
reader.start()
reader.wait_until_ready()
self.assertEquals(0, patchviasocket.send_to_socket(reader.file(), self._key_file, CMD_DATA))
reader.join()
data = reader.data()
self.assertIn(KEY_DATA, data)
self.assertIn(CMD_DATA.replace("%", " "), data)
self.assertNotIn("LUV", data)
self.assertNotIn("very%quickly", data) # Testing substitution

def test_host_key_error(self):
reader = SocketThread()
reader.start()
reader.wait_until_ready()
self.assertEquals(1, patchviasocket.send_to_socket(reader.file(), NON_EXISTING_FILE, CMD_DATA))
reader.join() # timeout

def test_host_key_access_denied(self):
reader = SocketThread()
reader.start()
reader.wait_until_ready()
self.assertEquals(1, patchviasocket.send_to_socket(reader.file(), self._unreadable, CMD_DATA))
reader.join() # timeout

def test_nonexistant_socket_error(self):
reader = SocketThread()
reader.start()
reader.wait_until_ready()
self.assertEquals(1, patchviasocket.send_to_socket(NON_EXISTING_FILE, self._key_file, CMD_DATA))
reader.join() # timeout

def test_invalid_socket_error(self):
reader = SocketThread()
reader.start()
reader.wait_until_ready()
self.assertEquals(1, patchviasocket.send_to_socket(self._key_file, self._key_file, CMD_DATA))
reader.join() # timeout

def test_access_denied_socket_error(self):
reader = SocketThread()
reader.start()
reader.wait_until_ready()
self.assertEquals(1, patchviasocket.send_to_socket(self._unreadable, self._key_file, CMD_DATA))
reader.join() # timeout


if __name__ == '__main__':
unittest.main()