Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove ssh key name requirement from testing. #135

Merged
merged 2 commits into from
Mar 12, 2018
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: 2 additions & 0 deletions config/mash_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,5 @@ credentials:
credentials_directory: /var/lib/mash/credentials/
obs:
download_directory: /var/tmp/mash/images
testing:
ssh_private_key_file: /etc/mash/testing_key
Copy link
Collaborator

Choose a reason for hiding this comment

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

We do not want to write the private key to disk if it can be avoided and for mash, since it is generated, based on the discussions this week, it should live in /var/lib/mash/keys/ssh

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The idea is that this is a setting and not something that needs to be auto generated. In setting up a MASH pipeline we setup a ssh private/public key pair and point to the file location. /var/lib/mash/keys/ssh/testing_key could be default but it's nice to give the user options.

This can be discussed in #164 .

19 changes: 19 additions & 0 deletions mash/services/testing/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
# along with mash. If not, see <http://www.gnu.org/licenses/>
#

from mash.mash_exceptions import MashConfigException
from mash.services.base_config import BaseConfig


Expand All @@ -32,3 +33,21 @@ class TestingConfig(BaseConfig):

def __init__(self, config_file=None):
super(TestingConfig, self).__init__(config_file)

def get_ssh_private_key_file(self):
"""
Return the path to the ssh private key file.

:rtype: string
"""
private_key_file = self._get_attribute(
attribute='ssh_private_key_file', element='testing'
)

if not private_key_file:
raise MashConfigException(
'ssh_private_key_file is required in configuration file for '
'testing service.'
)

return private_key_file
11 changes: 5 additions & 6 deletions mash/services/testing/ec2_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ class EC2TestingJob(TestingJob):
__test__ = False

def __init__(
self, id, provider, test_regions, tests, utctime, job_file=None,
credentials=None, description=None, distro=None, instance_type=None,
ssh_user='ec2-user'
self, id, provider, ssh_private_key_file, test_regions, tests, utctime,
job_file=None, credentials=None, description=None, distro=None,
instance_type=None, ssh_user='ec2-user'
):
super(EC2TestingJob, self).__init__(
id, provider, test_regions, tests, utctime,
id, provider, ssh_private_key_file, test_regions, tests, utctime,
job_file=job_file, description=description, distro=distro,
instance_type=instance_type
)
Expand All @@ -59,8 +59,7 @@ def _run_tests(self):
'instance_type': self.instance_type,
'region': region,
'secret_access_key': creds['secret_access_key'],
'ssh_key_name': creds['ssh_key_name'],
'ssh_private_key': creds['ssh_private_key'],
'ssh_private_key': self.ssh_private_key_file,
'ssh_user': self.ssh_user,
'tests': self.tests
}
Expand Down
15 changes: 1 addition & 14 deletions mash/services/testing/ipa_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,29 +20,17 @@
import threading

from ipa.ipa_controller import test_image
from ipa.ipa_utils import generate_public_ssh_key

from mash.services.status_levels import EXCEPTION, FAILED, SUCCESS
from mash.utils.ec2 import get_client


def ipa_test(
results, provider=None, access_key_id=None, description=None, distro=None,
image_id=None, instance_type=None, region=None, secret_access_key=None,
ssh_key_name=None, ssh_private_key=None, ssh_user=None, tests=None
ssh_private_key=None, ssh_user=None, tests=None
):
name = threading.current_thread().getName()
try:
client = get_client('ec2', access_key_id, secret_access_key, region)
try:
client.describe_key_pairs(KeyNames=[ssh_key_name])
except Exception:
# If key pair does not exist create it in the region.
pub_key = generate_public_ssh_key(ssh_private_key)
client.import_key_pair(
KeyName=ssh_key_name, PublicKeyMaterial=pub_key
)

status, result = test_image(
provider.upper(), # TODO remove uppercase when IPA update released
access_key_id=access_key_id,
Expand All @@ -54,7 +42,6 @@ def ipa_test(
log_level=logging.WARNING,
region=region,
secret_access_key=secret_access_key,
ssh_key_name=ssh_key_name,
ssh_private_key=ssh_private_key,
ssh_user=ssh_user,
tests=tests
Expand Down
5 changes: 3 additions & 2 deletions mash/services/testing/job.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ class TestingJob(object):
__test__ = False

def __init__(
self, id, provider, test_regions, tests, utctime, job_file=None,
description=None, distro=None, instance_type=None
self, id, provider, ssh_private_key_file, test_regions, tests, utctime,
job_file=None, description=None, distro=None, instance_type=None
):
self.cloud_image_name = None
self.job_file = job_file
Expand All @@ -45,6 +45,7 @@ def __init__(
self.provider = self.validate_provider(provider)
self.status = UNKOWN
self.source_regions = None
self.ssh_private_key_file = ssh_private_key_file
self.test_regions = self.validate_test_regions(test_regions)
self.tests = self.validate_tests(tests)
self.utctime = self.validate_timestamp(utctime)
Expand Down
2 changes: 2 additions & 0 deletions mash/services/testing/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ def post_init(self):
"""
self.config = TestingConfig()
self.set_logfile(self.config.get_log_file(self.service_exchange))
self.ssh_private_key_file = self.config.get_ssh_private_key_file()

self.jobs = {}

Expand Down Expand Up @@ -105,6 +106,7 @@ def _create_job(self, job_class, job_config):
3. Store config file if not stored already.
4. Bind to job listener queue.
"""
job_config['ssh_private_key_file'] = self.ssh_private_key_file
try:
job = job_class(**job_config)
except Exception as e:
Expand Down
2 changes: 2 additions & 0 deletions test/data/mash_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,5 @@ services:
- pint
non_cred_services:
- obs
testing:
ssh_private_key_file: /etc/mash/testing_key
13 changes: 13 additions & 0 deletions test/unit/services/testing/config_test.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,23 @@
from pytest import raises

from mash.mash_exceptions import MashConfigException
from mash.services.testing.config import TestingConfig


class TestTestingConfig(object):
def setup(self):
self.empty_config = TestingConfig('../data/empty_mash_config.yaml')
self.config = TestingConfig('../data/mash_config.yaml')

def test_get_log_file(self):
assert self.empty_config.get_log_file('testing') == \
'/var/log/mash/testing_service.log'

def test_get_ssh_private_key_file(self):
assert self.config.get_ssh_private_key_file() == \
'/etc/mash/testing_key'

with raises(MashConfigException) as error:
self.empty_config.get_ssh_private_key_file()

assert str(error.value)
27 changes: 5 additions & 22 deletions test/unit/services/testing/ec2_job_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from unittest.mock import call, Mock, patch
from unittest.mock import call, patch

from mash.services.testing.ec2_job import EC2TestingJob

Expand All @@ -8,25 +8,17 @@ def setup(self):
self.job_config = {
'id': '1',
'provider': 'ec2',
'ssh_private_key_file': 'private_ssh_key.file',
'test_regions': {'us-east-1': 'test-aws'},
'tests': 'test_stuff',
'utctime': 'now',
}

@patch('mash.services.testing.ipa_helper.generate_public_ssh_key')
@patch('mash.services.testing.ipa_helper.get_client')
@patch('mash.services.testing.ipa_helper.test_image')
@patch.object(EC2TestingJob, 'send_log')
def test_testing_run_test(
self, mock_send_log, mock_test_image, mock_get_client,
mock_generate_ssh_key
self, mock_send_log, mock_test_image
):
client = Mock()
client.describe_key_pairs.side_effect = Exception('Key not found.')
mock_get_client.return_value = client

mock_generate_ssh_key.return_value = 'fakekey'

mock_test_image.return_value = (
0,
{
Expand All @@ -43,20 +35,12 @@ def test_testing_run_test(
job.credentials = {
'test-aws': {
'access_key_id': '123',
'secret_access_key': '321',
'ssh_key_name': 'my-key',
'ssh_private_key': 'my-key.file'
'secret_access_key': '321'
}
}
job.source_regions = {'us-east-1': 'ami-123'}
job._run_tests()

client.describe_key_pairs.assert_called_once_with(
KeyNames=['my-key']
)
client.import_key_pair.assert_called_once_with(
KeyName='my-key', PublicKeyMaterial='fakekey'
)
mock_test_image.assert_called_once_with(
'EC2',
cleanup=True,
Expand All @@ -68,8 +52,7 @@ def test_testing_run_test(
log_level=30,
region='us-east-1',
secret_access_key='321',
ssh_key_name='my-key',
ssh_private_key='my-key.file',
ssh_private_key='private_ssh_key.file',
ssh_user='ec2-user',
tests=['test_stuff']
)
Expand Down
1 change: 1 addition & 0 deletions test/unit/services/testing/job_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ def setup(self):
self.job_config = {
'id': '1',
'provider': 'ec2',
'ssh_private_key_file': 'private_ssh_key.file',
'test_regions': {'us-east-1': 'test-aws'},
'tests': 'test_stuff',
'utctime': 'now'
Expand Down
5 changes: 4 additions & 1 deletion test/unit/services/testing/service_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ def setup(
self.testing.service_queue = 'service'
self.testing.job_document_key = 'job_document'
self.testing.listener_queue = 'listener'
self.testing.ssh_private_key_file = 'private_ssh_key.file'

self.error_message = '{"testing_result": ' \
'{"id": "1", "status": "error"}}'
Expand Down Expand Up @@ -117,7 +118,9 @@ def test_testing_create_job(
job_config = {'id': '1', 'provider': 'ec2'}
self.testing._create_job(job_class, job_config)

job_class.assert_called_once_with(id='1', provider='ec2')
job_class.assert_called_once_with(
id='1', provider='ec2', ssh_private_key_file='private_ssh_key.file'
)
job.set_log_callback.assert_called_once_with(
self.testing.log_job_message
)
Expand Down