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
10 changes: 9 additions & 1 deletion cloudinit/distros/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
from cloudinit import subp
from cloudinit import util

from cloudinit.features import \
ALLOW_EC2_MIRRORS_ON_NON_AWS_INSTANCE_TYPES

from cloudinit.distros.parsers import hosts
from .networking import LinuxNetworking

Expand Down Expand Up @@ -849,7 +852,12 @@ def _get_package_mirror_info(mirror_info, data_source=None,
# ec2 availability zones are named cc-direction-[0-9][a-d] (us-east-1b)
# the region is us-east-1. so region = az[0:-1]
if _EC2_AZ_RE.match(data_source.availability_zone):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think maybe we can just drop this re.match and instead just specificall check data_source.platform_type == "ec2".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

But wouldn't that interfere with past release ? We would like to have past releases still being able to user ec2 mirrors, right ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You are right, darn backward compatibility.

subst['ec2_region'] = "%s" % data_source.availability_zone[0:-1]
ec2_region = data_source.availability_zone[0:-1]

if ALLOW_EC2_MIRRORS_ON_NON_AWS_INSTANCE_TYPES:
subst['ec2_region'] = "%s" % ec2_region
elif data_source.platform_type == "ec2":
subst['ec2_region'] = "%s" % ec2_region

if data_source and data_source.region:
subst['region'] = data_source.region
Expand Down
35 changes: 28 additions & 7 deletions cloudinit/distros/tests/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ def test_failsafe_used_if_all_search_results_filtered_out(self):
assert {'primary': 'http://other'} == _get_package_mirror_info(
mirror_info, mirror_filter=lambda x: False)

@pytest.mark.parametrize('allow_ec2_mirror, platform_type', [
(True, 'ec2')
])
@pytest.mark.parametrize('availability_zone,region,patterns,expected', (
# Test ec2_region alone
('fk-fake-1f', None, ['http://EC2-%(ec2_region)s/ubuntu'],
Expand Down Expand Up @@ -120,16 +123,34 @@ def test_failsafe_used_if_all_search_results_filtered_out(self):
['http://%(region)s/ubuntu'], ['http://fk-fake-1/ubuntu'])
for invalid_char in INVALID_URL_CHARS
))
def test_substitution(self, availability_zone, region, patterns, expected):
def test_valid_substitution(self,
allow_ec2_mirror,
platform_type,
availability_zone,
region,
patterns,
expected):
"""Test substitution works as expected."""
flag_path = "cloudinit.distros." \
"ALLOW_EC2_MIRRORS_ON_NON_AWS_INSTANCE_TYPES"

m_data_source = mock.Mock(
availability_zone=availability_zone, region=region
availability_zone=availability_zone,
region=region,
platform_type=platform_type
)
mirror_info = {'search': {'primary': patterns}}

ret = _get_package_mirror_info(
mirror_info,
data_source=m_data_source,
mirror_filter=lambda x: x
)
with mock.patch(flag_path, allow_ec2_mirror):
ret = _get_package_mirror_info(
mirror_info,
data_source=m_data_source,
mirror_filter=lambda x: x
)
print(allow_ec2_mirror)
print(platform_type)
print(availability_zone)
print(region)
print(patterns)
print(expected)
assert {'primary': expected} == ret
11 changes: 11 additions & 0 deletions cloudinit/features.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,17 @@
This flag can be removed after Focal is no longer supported
"""


ALLOW_EC2_MIRRORS_ON_NON_AWS_INSTANCE_TYPES = False
"""
When configuring apt mirrors, old behavior is to allow
the use of ec2 mirrors if the datasource availability_zone format
matches one of the possible aws ec2 regions. After the 20.2 release, we
no longer publish ec2 region mirror urls on non-AWS cloud platforms.
Besides feature_overrides.py, users can override this by providing
#cloud-config apt directives.
"""

try:
# pylint: disable=wildcard-import
from cloudinit.feature_overrides import * # noqa
Expand Down
186 changes: 113 additions & 73 deletions tests/unittests/test_distros/test_generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from cloudinit.tests import helpers

import os
import pytest
import shutil
import tempfile
from unittest import mock
Expand Down Expand Up @@ -37,24 +38,6 @@

class TestGenericDistro(helpers.FilesystemMockingTestCase):

def return_first(self, mlist):
if not mlist:
return None
return mlist[0]

def return_second(self, mlist):
if not mlist:
return None
return mlist[1]

def return_none(self, _mlist):
return None

def return_last(self, mlist):
if not mlist:
return None
return(mlist[-1])

def setUp(self):
super(TestGenericDistro, self).setUp()
# Make a temp directoy for tests to use.
Expand Down Expand Up @@ -145,61 +128,6 @@ def test_arch_package_mirror_info_known(self):
arch_mirrors = gapmi(package_mirrors, arch="amd64")
self.assertEqual(package_mirrors[0], arch_mirrors)

def test_get_package_mirror_info_az_ec2(self):
arch_mirrors = gapmi(package_mirrors, arch="amd64")
data_source_mock = mock.Mock(availability_zone="us-east-1a")

results = gpmi(arch_mirrors, data_source=data_source_mock,
mirror_filter=self.return_first)
self.assertEqual(results,
{'primary': 'http://us-east-1.ec2/',
'security': 'http://security-mirror1-intel'})

results = gpmi(arch_mirrors, data_source=data_source_mock,
mirror_filter=self.return_second)
self.assertEqual(results,
{'primary': 'http://us-east-1a.clouds/',
'security': 'http://security-mirror2-intel'})

results = gpmi(arch_mirrors, data_source=data_source_mock,
mirror_filter=self.return_none)
self.assertEqual(results, package_mirrors[0]['failsafe'])

def test_get_package_mirror_info_az_non_ec2(self):
arch_mirrors = gapmi(package_mirrors, arch="amd64")
data_source_mock = mock.Mock(availability_zone="nova.cloudvendor")

results = gpmi(arch_mirrors, data_source=data_source_mock,
mirror_filter=self.return_first)
self.assertEqual(results,
{'primary': 'http://nova.cloudvendor.clouds/',
'security': 'http://security-mirror1-intel'})

results = gpmi(arch_mirrors, data_source=data_source_mock,
mirror_filter=self.return_last)
self.assertEqual(results,
{'primary': 'http://nova.cloudvendor.clouds/',
'security': 'http://security-mirror2-intel'})

def test_get_package_mirror_info_none(self):
arch_mirrors = gapmi(package_mirrors, arch="amd64")
data_source_mock = mock.Mock(availability_zone=None)

# because both search entries here replacement based on
# availability-zone, the filter will be called with an empty list and
# failsafe should be taken.
results = gpmi(arch_mirrors, data_source=data_source_mock,
mirror_filter=self.return_first)
self.assertEqual(results,
{'primary': 'http://fs-primary-intel',
'security': 'http://security-mirror1-intel'})

results = gpmi(arch_mirrors, data_source=data_source_mock,
mirror_filter=self.return_last)
self.assertEqual(results,
{'primary': 'http://fs-primary-intel',
'security': 'http://security-mirror2-intel'})

def test_systemd_in_use(self):
cls = distros.fetch("ubuntu")
d = cls("ubuntu", {}, None)
Expand Down Expand Up @@ -259,4 +187,116 @@ def test_expire_passwd_freebsd_uses_pw_command(self):
["pw", "usermod", "myuser", "-p", "01-Jan-1970"])


class TestGetPackageMirrors:

def return_first(self, mlist):
if not mlist:
return None
return mlist[0]

def return_second(self, mlist):
if not mlist:
return None

return mlist[1] if len(mlist) > 1 else None

def return_none(self, _mlist):
return None

def return_last(self, mlist):
if not mlist:
return None
return(mlist[-1])

@pytest.mark.parametrize(
"allow_ec2_mirror, platform_type, mirrors",
[
(True, "ec2", [
{'primary': 'http://us-east-1.ec2/',
'security': 'http://security-mirror1-intel'},
{'primary': 'http://us-east-1a.clouds/',
'security': 'http://security-mirror2-intel'}
]),
(True, "other", [
{'primary': 'http://us-east-1.ec2/',
'security': 'http://security-mirror1-intel'},
{'primary': 'http://us-east-1a.clouds/',
'security': 'http://security-mirror2-intel'}
]),
(False, "ec2", [
{'primary': 'http://us-east-1.ec2/',
'security': 'http://security-mirror1-intel'},
{'primary': 'http://us-east-1a.clouds/',
'security': 'http://security-mirror2-intel'}
]),
(False, "other", [
{'primary': 'http://us-east-1a.clouds/',
'security': 'http://security-mirror1-intel'},
{'primary': 'http://fs-primary-intel',
'security': 'http://security-mirror2-intel'}
])
])
def test_get_package_mirror_info_az_ec2(self,
allow_ec2_mirror,
platform_type,
mirrors):
flag_path = "cloudinit.distros." \
"ALLOW_EC2_MIRRORS_ON_NON_AWS_INSTANCE_TYPES"
with mock.patch(flag_path, allow_ec2_mirror):
arch_mirrors = gapmi(package_mirrors, arch="amd64")
data_source_mock = mock.Mock(
availability_zone="us-east-1a",
platform_type=platform_type)

results = gpmi(arch_mirrors, data_source=data_source_mock,
mirror_filter=self.return_first)
assert(results == mirrors[0])

results = gpmi(arch_mirrors, data_source=data_source_mock,
mirror_filter=self.return_second)
assert(results == mirrors[1])

results = gpmi(arch_mirrors, data_source=data_source_mock,
mirror_filter=self.return_none)
assert(results == package_mirrors[0]['failsafe'])

def test_get_package_mirror_info_az_non_ec2(self):
arch_mirrors = gapmi(package_mirrors, arch="amd64")
data_source_mock = mock.Mock(availability_zone="nova.cloudvendor")

results = gpmi(arch_mirrors, data_source=data_source_mock,
mirror_filter=self.return_first)
assert(results == {
'primary': 'http://nova.cloudvendor.clouds/',
'security': 'http://security-mirror1-intel'}
)

results = gpmi(arch_mirrors, data_source=data_source_mock,
mirror_filter=self.return_last)
assert(results == {
'primary': 'http://nova.cloudvendor.clouds/',
'security': 'http://security-mirror2-intel'}
)

def test_get_package_mirror_info_none(self):
arch_mirrors = gapmi(package_mirrors, arch="amd64")
data_source_mock = mock.Mock(availability_zone=None)

# because both search entries here replacement based on
# availability-zone, the filter will be called with an empty list and
# failsafe should be taken.
results = gpmi(arch_mirrors, data_source=data_source_mock,
mirror_filter=self.return_first)
assert(results == {
'primary': 'http://fs-primary-intel',
'security': 'http://security-mirror1-intel'}
)

results = gpmi(arch_mirrors, data_source=data_source_mock,
mirror_filter=self.return_last)
assert(results == {
'primary': 'http://fs-primary-intel',
'security': 'http://security-mirror2-intel'}
)

# vi: ts=4 expandtab
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,17 @@ def apt_source_list(self, distro, mirror, mirrorcheck=None):
cfg = {'apt_mirror_search': mirror}
else:
cfg = {'apt_mirror': mirror}

mycloud = self._get_cloud(distro)

with mock.patch.object(util, 'write_file') as mockwf:
with mock.patch.object(util, 'load_file',
return_value="faketmpl") as mocklf:
with mock.patch.object(os.path, 'isfile',
return_value=True) as mockisfile:
with mock.patch.object(templater, 'render_string',
return_value="fake") as mockrnd:
with mock.patch.object(
templater, 'render_string',
return_value='fake') as mockrnd:
with mock.patch.object(util, 'rename'):
cc_apt_configure.handle("test", cfg, mycloud,
LOG, None)
Expand Down
7 changes: 7 additions & 0 deletions tests/unittests/test_handler/test_handler_apt_source_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,17 @@ def update_package_sources(self):
return


class FakeDatasource:
"""Fake Datasource helper object"""
def __init__(self):
self.region = 'region'


class FakeCloud(object):
"""Fake Cloud helper object"""
def __init__(self):
self.distro = FakeDistro()
self.datasource = FakeDatasource()


class TestAptSourceConfig(TestCase):
Expand Down
Loading