Commit 07423ed9 authored by Pit Kleyersburg's avatar Pit Kleyersburg Committed by maxbecke
Browse files

Incorporate PR review by maxbecke

This includes the following changes:

* Reduce inline comments that were mostly redundant to the code it
described

* Added another example to the man-page to make clear that TLS is
supported for CNAMEs. For that matter, all other options will also work
without change, given that internally the CNAME will only be resolved to
an FS-ID before handing back over to the same logic that is already in
place.

* Small modifications to better adhere to the given codestyle.

* Adjusted log messages as suggested.
parent e9af4aa4
......@@ -12,16 +12,13 @@ package, which simplifies using EFS file systems\&.
\fBmount\&.efs\fR is meant to be used through the \
\fBmount\fR(8) command for mounting EFS file systems\&.
.sp
\fIfs-id-or-dns-name\fR has to be of any of the following \
three forms:
\fIfs-id-or-dns-name\fR has to be of one of the following \
two forms:
.P
.IP \(bu
An EFS filesystem ID in the form of "fs\-abcd1234", generated \
when the file system is created\&.
.IP \(bu
A fully-qualified EFS DNS name in the form of \
"fs\-abcd1234\&.efs\&.us-east-1\&.amazonaws\&.com"\&.
.IP \(bu
A domain name that has a resolvable DNS-CNAME record, \
which in turn points to a fully-qualified EFS DNS name \
in the form of "fs\-abcd1234\&.efs\&.us-east-1\&.amazonaws\&.com"\&.
......@@ -100,12 +97,6 @@ sudo mount -t efs -o tls,verify=0 fs-abcd1234 /mnt/efs
Mount an EFS file system with file system ID "fs-abcd1234" at mount point \
"/mnt/efs" using encryption of data in transit and a verify level of 0\&.
.TP
sudo mount -t efs fs-abcd1234.efs.us-east-1.amazonaws.com /mnt/efs
Mount an EFS file system with the fully-qualified EFS DNS name \
"fs\-abcd1234\&.efs\&.us-east-1\&.amazonaws\&.com" at mount point \
"/mnt/efs" without encryption of data in transit\&. If the region \
is correct, this is identical to mounting with just the file system ID\&.
.TP
sudo mount -t efs custom-cname.example.com /mnt/efs
Mount an EFS file system using the custom DNS name \
"custom-cname\&.example\&.com" \(em which has to \
......@@ -113,6 +104,14 @@ resolve to a fully-qualified EFS DNS name such as \
"fs\-abcd1234\&.efs\&.us-east-1\&.amazonaws\&.com" \
\(em at mount point "/mnt/efs" without encryption \
of data in transit\&.
.TP
sudo mount -t efs -o tls custom-cname.example.com /mnt/efs
Mount an EFS file system using the custom DNS name \
"custom-cname\&.example\&.com" \(em which has to \
resolve to a fully-qualified EFS DNS name such as \
"fs\-abcd1234\&.efs\&.us-east-1\&.amazonaws\&.com" \
\(em at mount point "/mnt/efs" using encryption \
of data in transit\&.
.SH "FILES"
.TP
\fI/sbin/mount.efs\fR
......
......@@ -575,45 +575,17 @@ def get_dns_name(config, fs_id):
def match_device(config, device):
"""
Return the EFS id and the remote path to mount.
:param config: the current configuration
:param device: the device descriptor, separating an EFS id or a DNS name and the remote path to mount by a colon
:return: a two element tuple of the EFS id and the remote path to mount
"""
"""Return the EFS id and the remote path to mount"""
# The device descriptor as specified separates the remote filesystem by the path to mount by a colon. Since colons
# are not allowed in either an EFS id or in a domain name, we can left-split once. (If left-splitting fails, the
# user didn't specify a path and we use '/' as the default.)
try:
remote, path = device.split(':', 1)
except ValueError:
remote = device
path = '/'
# The simplest case is that the remote is already the EFS id. If so, we return it as is.
if FS_ID_RE.match(remote):
return remote, path
# If the user did not specify an EFS id, we first check for the special case where the user supplied us with the
# FQDN of the EFS.
efs_fqdn_match = EFS_FQDN_RE.match(remote)
if efs_fqdn_match:
fs_id = efs_fqdn_match.group('fs_id')
expected_dns_name = get_dns_name(config, fs_id)
if remote == expected_dns_name:
return fs_id, path
else:
fatal_error(
'Fully qualified EFS domain name specified "%s", but it didn\'t match the expected value "%s"'
% (remote, expected_dns_name),
'EFS FQDN "%s" didn\'t match expected "%s"' % (remote, expected_dns_name)
)
# For the final case we now assume that the user specified a DNS resolvable name with a CNAME record that points to
# a valid EFS FQDN. To verify this, we'll use `socket.gethostbyname_ex()` which returns an alias-list for, among
# other things, a CNAME.
try:
primary, secondaries, _ = socket.gethostbyname_ex(remote)
hostnames = filter(lambda e: e is not None, [primary] + secondaries)
......@@ -626,18 +598,22 @@ def match_device(config, device):
if not hostnames:
fatal_error(
'The specified domain name "%s" returned no entries, where at least one was expected' % remote
'The specified domain name "%s" did not resolve to an EFS mount target' % remote
)
for hostname in hostnames:
efs_fqdn_match = EFS_FQDN_RE.match(hostname)
if efs_fqdn_match:
fs_id = efs_fqdn_match.group('fs_id')
expected_dns_name = get_dns_name(config, fs_id)
if hostname == expected_dns_name:
return fs_id, path
else:
fatal_error('The specified domain name "%s" resolved to no valid/expected EFS DNS name' % remote)
fatal_error('The specified CNAME "%s" did not resolve to a valid DNS name for an EFS mount target. '
'Please refer to the EFS documentation for mounting with DNS names for examples: '
'https://docs.aws.amazon.com/efs/latest/ug/mounting-fs-mount-cmd-dns-name.' % remote)
def mount_tls(config, init_system, dns_name, path, fs_id, mountpoint, options):
......
......@@ -17,12 +17,6 @@ CORRECT_DEVICE_DESCRIPTORS_FS_ID = [
('fs-deadbeef:/some/subpath', ('fs-deadbeef', '/some/subpath')),
('fs-deadbeef:/some/subpath/with:colons', ('fs-deadbeef', '/some/subpath/with:colons')),
]
CORRECT_DEVICE_DESCRIPTORS_EFS_FQDN = [
('fs-deadbeef.efs.us-east-1.amazonaws.com', ('fs-deadbeef', '/')),
('fs-deadbeef.efs.us-east-1.amazonaws.com:/', ('fs-deadbeef', '/')),
('fs-deadbeef.efs.us-east-1.amazonaws.com:/some/subpath', ('fs-deadbeef', '/some/subpath')),
('fs-deadbeef.efs.us-east-1.amazonaws.com:/some/subpath/with:colons', ('fs-deadbeef', '/some/subpath/with:colons')),
]
CORRECT_DEVICE_DESCRIPTORS_CNAME_DNS = [
('custom-cname.example.com', ('fs-deadbeef', '/')),
('custom-cname.example.com:/', ('fs-deadbeef', '/')),
......@@ -36,13 +30,6 @@ def test_match_device_correct_descriptors_fs_id(mocker):
assert (fs_id, path) == mount_efs.match_device(None, device)
def test_match_device_correct_descriptors_efs_fqdn(mocker):
get_dns_name_mock = mocker.patch('mount_efs.get_dns_name', return_value='fs-deadbeef.efs.us-east-1.amazonaws.com')
for device, (fs_id, path) in CORRECT_DEVICE_DESCRIPTORS_EFS_FQDN:
assert (fs_id, path) == mount_efs.match_device(None, device)
get_dns_name_mock.assert_called()
def test_match_device_correct_descriptors_cname_dns_primary(mocker):
get_dns_name_mock = mocker.patch('mount_efs.get_dns_name', return_value='fs-deadbeef.efs.us-east-1.amazonaws.com')
gethostbyname_ex_mock = mocker.patch('socket.gethostbyname_ex',
......@@ -87,18 +74,6 @@ def test_match_device_correct_descriptors_cname_dns_amongst_invalid(mocker):
gethostbyname_ex_mock.assert_called()
def test_match_device_wrong_efs_dns_name(mocker, capsys):
get_dns_name_mock = mocker.patch('mount_efs.get_dns_name', return_value='fs-deadbeef.efs.us-west-1.amazonaws.com')
with pytest.raises(SystemExit) as ex:
mount_efs.match_device(None, 'fs-deadbeef.efs.us-east-1.amazonaws.com:/')
assert 0 != ex.value.code
out, err = capsys.readouterr()
assert 'Fully qualified EFS domain name specified' in err
assert 'didn\'t match the expected value' in err
get_dns_name_mock.assert_called()
def test_match_device_unresolvable_domain(mocker, capsys):
mocker.patch('socket.gethostbyname_ex', side_effect=socket.gaierror)
with pytest.raises(SystemExit) as ex:
......@@ -117,7 +92,7 @@ def test_match_device_no_hostnames(mocker, capsys):
assert 0 != ex.value.code
out, err = capsys.readouterr()
assert 'returned no entries' in err
assert 'did not resolve to an EFS mount target' in err
gethostbyname_ex_mock.assert_called()
......@@ -129,7 +104,7 @@ def test_match_device_no_hostnames2(mocker, capsys):
assert 0 != ex.value.code
out, err = capsys.readouterr()
assert 'returned no entries' in err
assert 'did not resolve to an EFS mount target' in err
gethostbyname_ex_mock.assert_called()
......@@ -141,7 +116,7 @@ def test_match_device_resolve_to_invalid_efs_dns_name(mocker, capsys):
assert 0 != ex.value.code
out, err = capsys.readouterr()
assert 'resolved to no valid/expected EFS DNS name' in err
assert 'did not resolve to a valid DNS name' in err
gethostbyname_ex_mock.assert_called()
......@@ -154,6 +129,6 @@ def test_match_device_resolve_to_unexpected_efs_dns_name(mocker, capsys):
assert 0 != ex.value.code
out, err = capsys.readouterr()
assert 'resolved to no valid/expected EFS DNS name' in err
assert 'did not resolve to a valid DNS name' in err
get_dns_name_mock.assert_called()
gethostbyname_ex_mock.assert_called()
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment