Fix behaviour of module_utils/ec2 compare_policies when dealing with bare bools and ints. (#61115)
* module_utils/ec2: (unit tests) Move unit tests for module_utils/ec2.py into test/units/module_utils - compare_policies was refactored from s3_bucket - "ec2_utils" doesn't seem to have ever existed * module_utils/ec2: (unit tests) Add unit test for comparing quoted and unquoted bools and numbers within policies As per https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_grammar.html "Values are enclosed in quotation marks. Quotation marks are optional for numeric and Boolean values." * module_utils/ec2: Explicitly convert bools and ints to strings when comparing policies See also: https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_grammar.html
This commit is contained in:
Mark Chappell
Sloane Hertel
parent
646a0b9024
commit
1f38a12057
6 changed files with 254 additions and 180 deletions
|
|
@ -0,0 +1,2 @@
|
|||
bugfixes:
|
||||
- compare_policies() Explicitly convert bools and ints to strings when comparing AWS IAM policies
|
||||
|
|
@ -554,6 +554,12 @@ def _hashable_policy(policy, policy_list):
|
|||
('Version', (u'2012-10-17',)))]
|
||||
|
||||
"""
|
||||
# Amazon will automatically convert bool and int to strings for us
|
||||
if isinstance(policy, bool):
|
||||
return tuple([str(policy).lower()])
|
||||
elif isinstance(policy, int):
|
||||
return tuple([str(policy)])
|
||||
|
||||
if isinstance(policy, list):
|
||||
for each in policy:
|
||||
tupleified = _hashable_policy(each, [])
|
||||
|
|
|
|||
|
|
@ -6191,6 +6191,8 @@ test/units/module_utils/test_database.py future-import-boilerplate
|
|||
test/units/module_utils/test_database.py metaclass-boilerplate
|
||||
test/units/module_utils/test_distro.py future-import-boilerplate
|
||||
test/units/module_utils/test_distro.py metaclass-boilerplate
|
||||
test/units/module_utils/test_ec2.py future-import-boilerplate
|
||||
test/units/module_utils/test_ec2.py metaclass-boilerplate
|
||||
test/units/module_utils/test_hetzner.py future-import-boilerplate
|
||||
test/units/module_utils/test_hetzner.py metaclass-boilerplate
|
||||
test/units/module_utils/test_kubevirt.py future-import-boilerplate
|
||||
|
|
@ -6218,8 +6220,6 @@ test/units/modules/cloud/amazon/test_data_pipeline.py future-import-boilerplate
|
|||
test/units/modules/cloud/amazon/test_data_pipeline.py metaclass-boilerplate
|
||||
test/units/modules/cloud/amazon/test_ec2_group.py future-import-boilerplate
|
||||
test/units/modules/cloud/amazon/test_ec2_group.py metaclass-boilerplate
|
||||
test/units/modules/cloud/amazon/test_ec2_utils.py future-import-boilerplate
|
||||
test/units/modules/cloud/amazon/test_ec2_utils.py metaclass-boilerplate
|
||||
test/units/modules/cloud/amazon/test_ec2_vpc_nat_gateway.py future-import-boilerplate
|
||||
test/units/modules/cloud/amazon/test_ec2_vpc_nat_gateway.py metaclass-boilerplate
|
||||
test/units/modules/cloud/amazon/test_ec2_vpc_nat_gateway.py pylint:blacklisted-name
|
||||
|
|
@ -6236,8 +6236,6 @@ test/units/modules/cloud/amazon/test_redshift_cross_region_snapshots.py future-i
|
|||
test/units/modules/cloud/amazon/test_redshift_cross_region_snapshots.py metaclass-boilerplate
|
||||
test/units/modules/cloud/amazon/test_route53_zone.py future-import-boilerplate
|
||||
test/units/modules/cloud/amazon/test_route53_zone.py metaclass-boilerplate
|
||||
test/units/modules/cloud/amazon/test_s3_bucket.py future-import-boilerplate
|
||||
test/units/modules/cloud/amazon/test_s3_bucket.py metaclass-boilerplate
|
||||
test/units/modules/cloud/amazon/test_s3_bucket_notification.py future-import-boilerplate
|
||||
test/units/modules/cloud/amazon/test_s3_bucket_notification.py metaclass-boilerplate
|
||||
test/units/modules/cloud/cloudstack/test_cs_traffic_type.py future-import-boilerplate
|
||||
|
|
|
|||
244
test/units/module_utils/test_ec2.py
Normal file
244
test/units/module_utils/test_ec2.py
Normal file
|
|
@ -0,0 +1,244 @@
|
|||
# (c) 2017 Red Hat Inc.
|
||||
#
|
||||
# This file is part of Ansible
|
||||
#
|
||||
# Ansible 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.
|
||||
#
|
||||
# Ansible 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 Ansible. If not, see <http://www.gnu.org/licenses/>.
|
||||
|
||||
# (c) 2017 Red Hat Inc.
|
||||
import unittest
|
||||
|
||||
from ansible.module_utils.ec2 import map_complex_type, compare_policies
|
||||
|
||||
|
||||
class Ec2Utils(unittest.TestCase):
|
||||
|
||||
def setUp(self):
|
||||
# A pair of simple IAM Trust relationships using bools, the first a
|
||||
# native bool the second a quoted string
|
||||
self.bool_policy_bool = {
|
||||
'Version': '2012-10-17',
|
||||
'Statement': [
|
||||
{
|
||||
"Action": "sts:AssumeRole",
|
||||
"Condition": {
|
||||
"Bool": {"aws:MultiFactorAuthPresent": True}
|
||||
},
|
||||
"Effect": "Allow",
|
||||
"Principal": {"AWS": "arn:aws:iam::XXXXXXXXXXXX:root"},
|
||||
"Sid": "AssumeRoleWithBoolean"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
self.bool_policy_string = {
|
||||
'Version': '2012-10-17',
|
||||
'Statement': [
|
||||
{
|
||||
"Action": "sts:AssumeRole",
|
||||
"Condition": {
|
||||
"Bool": {"aws:MultiFactorAuthPresent": "true"}
|
||||
},
|
||||
"Effect": "Allow",
|
||||
"Principal": {"AWS": "arn:aws:iam::XXXXXXXXXXXX:root"},
|
||||
"Sid": "AssumeRoleWithBoolean"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
# A pair of simple bucket policies using numbers, the first a
|
||||
# native int the second a quoted string
|
||||
self.numeric_policy_number = {
|
||||
'Version': '2012-10-17',
|
||||
'Statement': [
|
||||
{
|
||||
"Action": "s3:ListBucket",
|
||||
"Condition": {
|
||||
"NumericLessThanEquals": {"s3:max-keys": 15}
|
||||
},
|
||||
"Effect": "Allow",
|
||||
"Resource": "arn:aws:s3:::examplebucket",
|
||||
"Sid": "s3ListBucketWithNumericLimit"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
self.numeric_policy_string = {
|
||||
'Version': '2012-10-17',
|
||||
'Statement': [
|
||||
{
|
||||
"Action": "s3:ListBucket",
|
||||
"Condition": {
|
||||
"NumericLessThanEquals": {"s3:max-keys": "15"}
|
||||
},
|
||||
"Effect": "Allow",
|
||||
"Resource": "arn:aws:s3:::examplebucket",
|
||||
"Sid": "s3ListBucketWithNumericLimit"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
self.small_policy_one = {
|
||||
'Version': '2012-10-17',
|
||||
'Statement': [
|
||||
{
|
||||
'Action': 's3:PutObjectAcl',
|
||||
'Sid': 'AddCannedAcl2',
|
||||
'Resource': 'arn:aws:s3:::test_policy/*',
|
||||
'Effect': 'Allow',
|
||||
'Principal': {'AWS': ['arn:aws:iam::XXXXXXXXXXXX:user/username1', 'arn:aws:iam::XXXXXXXXXXXX:user/username2']}
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
# The same as small_policy_one, except the single resource is in a list and the contents of Statement are jumbled
|
||||
self.small_policy_two = {
|
||||
'Version': '2012-10-17',
|
||||
'Statement': [
|
||||
{
|
||||
'Effect': 'Allow',
|
||||
'Action': 's3:PutObjectAcl',
|
||||
'Principal': {'AWS': ['arn:aws:iam::XXXXXXXXXXXX:user/username1', 'arn:aws:iam::XXXXXXXXXXXX:user/username2']},
|
||||
'Resource': ['arn:aws:s3:::test_policy/*'],
|
||||
'Sid': 'AddCannedAcl2'
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
self.larger_policy_one = {
|
||||
"Version": "2012-10-17",
|
||||
"Statement": [
|
||||
{
|
||||
"Sid": "Test",
|
||||
"Effect": "Allow",
|
||||
"Principal": {
|
||||
"AWS": [
|
||||
"arn:aws:iam::XXXXXXXXXXXX:user/testuser1",
|
||||
"arn:aws:iam::XXXXXXXXXXXX:user/testuser2"
|
||||
]
|
||||
},
|
||||
"Action": "s3:PutObjectAcl",
|
||||
"Resource": "arn:aws:s3:::test_policy/*"
|
||||
},
|
||||
{
|
||||
"Effect": "Allow",
|
||||
"Principal": {
|
||||
"AWS": "arn:aws:iam::XXXXXXXXXXXX:user/testuser2"
|
||||
},
|
||||
"Action": [
|
||||
"s3:PutObject",
|
||||
"s3:PutObjectAcl"
|
||||
],
|
||||
"Resource": "arn:aws:s3:::test_policy/*"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
# The same as larger_policy_one, except having a list of length 1 and jumbled contents
|
||||
self.larger_policy_two = {
|
||||
"Version": "2012-10-17",
|
||||
"Statement": [
|
||||
{
|
||||
"Principal": {
|
||||
"AWS": ["arn:aws:iam::XXXXXXXXXXXX:user/testuser2"]
|
||||
},
|
||||
"Effect": "Allow",
|
||||
"Resource": "arn:aws:s3:::test_policy/*",
|
||||
"Action": [
|
||||
"s3:PutObject",
|
||||
"s3:PutObjectAcl"
|
||||
]
|
||||
},
|
||||
{
|
||||
"Action": "s3:PutObjectAcl",
|
||||
"Principal": {
|
||||
"AWS": [
|
||||
"arn:aws:iam::XXXXXXXXXXXX:user/testuser1",
|
||||
"arn:aws:iam::XXXXXXXXXXXX:user/testuser2"
|
||||
]
|
||||
},
|
||||
"Sid": "Test",
|
||||
"Resource": "arn:aws:s3:::test_policy/*",
|
||||
"Effect": "Allow"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
# Different than larger_policy_two: a different principal is given
|
||||
self.larger_policy_three = {
|
||||
"Version": "2012-10-17",
|
||||
"Statement": [
|
||||
{
|
||||
"Principal": {
|
||||
"AWS": ["arn:aws:iam::XXXXXXXXXXXX:user/testuser2"]
|
||||
},
|
||||
"Effect": "Allow",
|
||||
"Resource": "arn:aws:s3:::test_policy/*",
|
||||
"Action": [
|
||||
"s3:PutObject",
|
||||
"s3:PutObjectAcl"]
|
||||
},
|
||||
{
|
||||
"Action": "s3:PutObjectAcl",
|
||||
"Principal": {
|
||||
"AWS": [
|
||||
"arn:aws:iam::XXXXXXXXXXXX:user/testuser1",
|
||||
"arn:aws:iam::XXXXXXXXXXXX:user/testuser3"
|
||||
]
|
||||
},
|
||||
"Sid": "Test",
|
||||
"Resource": "arn:aws:s3:::test_policy/*",
|
||||
"Effect": "Allow"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
def test_map_complex_type_over_dict(self):
|
||||
complex_type = {'minimum_healthy_percent': "75", 'maximum_percent': "150"}
|
||||
type_map = {'minimum_healthy_percent': 'int', 'maximum_percent': 'int'}
|
||||
complex_type_mapped = map_complex_type(complex_type, type_map)
|
||||
complex_type_expected = {'minimum_healthy_percent': 75, 'maximum_percent': 150}
|
||||
self.assertEqual(complex_type_mapped, complex_type_expected)
|
||||
|
||||
def test_compare_small_policies_without_differences(self):
|
||||
""" Testing two small policies which are identical except for:
|
||||
* The contents of the statement are in different orders
|
||||
* The second policy contains a list of length one whereas in the first it is a string
|
||||
"""
|
||||
self.assertFalse(compare_policies(self.small_policy_one, self.small_policy_two))
|
||||
|
||||
def test_compare_large_policies_without_differences(self):
|
||||
""" Testing two larger policies which are identical except for:
|
||||
* The statements are in different orders
|
||||
* The contents of the statements are also in different orders
|
||||
* The second contains a list of length one for the Principal whereas in the first it is a string
|
||||
"""
|
||||
self.assertFalse(compare_policies(self.larger_policy_one, self.larger_policy_two))
|
||||
|
||||
def test_compare_larger_policies_with_difference(self):
|
||||
""" Testing two larger policies which are identical except for:
|
||||
* one different principal
|
||||
"""
|
||||
self.assertTrue(compare_policies(self.larger_policy_two, self.larger_policy_three))
|
||||
|
||||
def test_compare_smaller_policy_with_larger(self):
|
||||
""" Testing two policies of different sizes """
|
||||
self.assertTrue(compare_policies(self.larger_policy_one, self.small_policy_one))
|
||||
|
||||
def test_compare_boolean_policy_bool_and_string_are_equal(self):
|
||||
""" Testing two policies one using a quoted boolean, the other a bool """
|
||||
self.assertFalse(compare_policies(self.bool_policy_string, self.bool_policy_bool))
|
||||
|
||||
def test_compare_numeric_policy_number_and_string_are_equal(self):
|
||||
""" Testing two policies one using a quoted number, the other an int """
|
||||
self.assertFalse(compare_policies(self.numeric_policy_string, self.numeric_policy_number))
|
||||
|
|
@ -1,12 +0,0 @@
|
|||
import unittest
|
||||
|
||||
from ansible.module_utils.ec2 import map_complex_type
|
||||
|
||||
|
||||
class Ec2Utils(unittest.TestCase):
|
||||
def test_map_complex_type_over_dict(self):
|
||||
complex_type = {'minimum_healthy_percent': "75", 'maximum_percent': "150"}
|
||||
type_map = {'minimum_healthy_percent': 'int', 'maximum_percent': 'int'}
|
||||
complex_type_mapped = map_complex_type(complex_type, type_map)
|
||||
complex_type_expected = {'minimum_healthy_percent': 75, 'maximum_percent': 150}
|
||||
self.assertEqual(complex_type_mapped, complex_type_expected)
|
||||
|
|
@ -1,164 +0,0 @@
|
|||
# (c) 2017 Red Hat Inc.
|
||||
#
|
||||
# This file is part of Ansible
|
||||
#
|
||||
# Ansible 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.
|
||||
#
|
||||
# Ansible 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 Ansible. If not, see <http://www.gnu.org/licenses/>.
|
||||
|
||||
# (c) 2017 Red Hat Inc.
|
||||
|
||||
from ansible.modules.cloud.amazon.s3_bucket import compare_policies
|
||||
|
||||
small_policy_one = {
|
||||
'Version': '2012-10-17',
|
||||
'Statement': [
|
||||
{
|
||||
'Action': 's3:PutObjectAcl',
|
||||
'Sid': 'AddCannedAcl2',
|
||||
'Resource': 'arn:aws:s3:::test_policy/*',
|
||||
'Effect': 'Allow',
|
||||
'Principal': {'AWS': ['arn:aws:iam::XXXXXXXXXXXX:user/username1', 'arn:aws:iam::XXXXXXXXXXXX:user/username2']}
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
# The same as small_policy_one, except the single resource is in a list and the contents of Statement are jumbled
|
||||
small_policy_two = {
|
||||
'Version': '2012-10-17',
|
||||
'Statement': [
|
||||
{
|
||||
'Effect': 'Allow',
|
||||
'Action': 's3:PutObjectAcl',
|
||||
'Principal': {'AWS': ['arn:aws:iam::XXXXXXXXXXXX:user/username1', 'arn:aws:iam::XXXXXXXXXXXX:user/username2']},
|
||||
'Resource': ['arn:aws:s3:::test_policy/*'],
|
||||
'Sid': 'AddCannedAcl2'
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
larger_policy_one = {
|
||||
"Version": "2012-10-17",
|
||||
"Statement": [
|
||||
{
|
||||
"Sid": "Test",
|
||||
"Effect": "Allow",
|
||||
"Principal": {
|
||||
"AWS": [
|
||||
"arn:aws:iam::XXXXXXXXXXXX:user/testuser1",
|
||||
"arn:aws:iam::XXXXXXXXXXXX:user/testuser2"
|
||||
]
|
||||
},
|
||||
"Action": "s3:PutObjectAcl",
|
||||
"Resource": "arn:aws:s3:::test_policy/*"
|
||||
},
|
||||
{
|
||||
"Effect": "Allow",
|
||||
"Principal": {
|
||||
"AWS": "arn:aws:iam::XXXXXXXXXXXX:user/testuser2"
|
||||
},
|
||||
"Action": [
|
||||
"s3:PutObject",
|
||||
"s3:PutObjectAcl"
|
||||
],
|
||||
"Resource": "arn:aws:s3:::test_policy/*"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
# The same as larger_policy_one, except having a list of length 1 and jumbled contents
|
||||
larger_policy_two = {
|
||||
"Version": "2012-10-17",
|
||||
"Statement": [
|
||||
{
|
||||
"Principal": {
|
||||
"AWS": ["arn:aws:iam::XXXXXXXXXXXX:user/testuser2"]
|
||||
},
|
||||
"Effect": "Allow",
|
||||
"Resource": "arn:aws:s3:::test_policy/*",
|
||||
"Action": [
|
||||
"s3:PutObject",
|
||||
"s3:PutObjectAcl"
|
||||
]
|
||||
},
|
||||
{
|
||||
"Action": "s3:PutObjectAcl",
|
||||
"Principal": {
|
||||
"AWS": [
|
||||
"arn:aws:iam::XXXXXXXXXXXX:user/testuser1",
|
||||
"arn:aws:iam::XXXXXXXXXXXX:user/testuser2"
|
||||
]
|
||||
},
|
||||
"Sid": "Test",
|
||||
"Resource": "arn:aws:s3:::test_policy/*",
|
||||
"Effect": "Allow"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
# Different than larger_policy_two: a different principal is given
|
||||
larger_policy_three = {
|
||||
"Version": "2012-10-17",
|
||||
"Statement": [
|
||||
{
|
||||
"Principal": {
|
||||
"AWS": ["arn:aws:iam::XXXXXXXXXXXX:user/testuser2"]
|
||||
},
|
||||
"Effect": "Allow",
|
||||
"Resource": "arn:aws:s3:::test_policy/*",
|
||||
"Action": [
|
||||
"s3:PutObject",
|
||||
"s3:PutObjectAcl"]
|
||||
},
|
||||
{
|
||||
"Action": "s3:PutObjectAcl",
|
||||
"Principal": {
|
||||
"AWS": [
|
||||
"arn:aws:iam::XXXXXXXXXXXX:user/testuser1",
|
||||
"arn:aws:iam::XXXXXXXXXXXX:user/testuser3"
|
||||
]
|
||||
},
|
||||
"Sid": "Test",
|
||||
"Resource": "arn:aws:s3:::test_policy/*",
|
||||
"Effect": "Allow"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
|
||||
def test_compare_small_policies_without_differences():
|
||||
""" Testing two small policies which are identical except for:
|
||||
* The contents of the statement are in different orders
|
||||
* The second policy contains a list of length one whereas in the first it is a string
|
||||
"""
|
||||
assert compare_policies(small_policy_one, small_policy_two) is False
|
||||
|
||||
|
||||
def test_compare_large_policies_without_differences():
|
||||
""" Testing two larger policies which are identical except for:
|
||||
* The statements are in different orders
|
||||
* The contents of the statements are also in different orders
|
||||
* The second contains a list of length one for the Principal whereas in the first it is a string
|
||||
"""
|
||||
assert compare_policies(larger_policy_one, larger_policy_two) is False
|
||||
|
||||
|
||||
def test_compare_larger_policies_with_difference():
|
||||
""" Testing two larger policies which are identical except for:
|
||||
* one different principal
|
||||
"""
|
||||
assert compare_policies(larger_policy_two, larger_policy_three)
|
||||
|
||||
|
||||
def test_compare_smaller_policy_with_larger():
|
||||
""" Testing two policies of different sizes """
|
||||
assert compare_policies(larger_policy_one, small_policy_one)
|
||||
Loading…
Reference in a new issue