diff --git a/.gitignore b/.gitignore index 72364f99..bd19bc46 100644 --- a/.gitignore +++ b/.gitignore @@ -87,3 +87,9 @@ ENV/ # Rope project settings .ropeproject + +# symbols file for pytest +symbols.yaml + +# PyCharm directory +.idea diff --git a/.pylintrc b/.pylintrc index b029a22e..a0eefca5 100644 --- a/.pylintrc +++ b/.pylintrc @@ -76,7 +76,9 @@ disable=locally-disabled, too-few-public-methods, too-many-instance-attributes, too-many-arguments, - too-many-public-methods + too-many-public-methods, + too-many-branches, + useless-object-inheritance [REPORTS] diff --git a/README.md b/README.md index dadd6b16..83669d6b 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,9 @@ Creating issues is good, creating good issues is even better. Please provide: # Contributing -This project is used internally by other F5 projects; we're not yet ready to accept contributions. Please check back later or see if another project, such as https://github.com/F5Networks/f5-common-python would be a good place for your contribution. +This project is used internally by other F5 projects; we're not yet ready to accept contributions. +Please check back later or see if another project, such as https://github.com/F5Networks/f5-common-python +would be a good place for your contribution. # Copyright Copyright (c) 2017,2018, F5 Networks, Inc. diff --git a/f5_cccl/exceptions.py b/f5_cccl/exceptions.py index ea958abc..cfba0e4f 100644 --- a/f5_cccl/exceptions.py +++ b/f5_cccl/exceptions.py @@ -50,7 +50,7 @@ class F5CcclValidationError(F5CcclError): def __init__(self, msg): """Initialize with base config does not match schema message.""" super(F5CcclValidationError, self).__init__(msg) - self.msg = 'Service congifuration provided does not match schema: ' + \ + self.msg = 'Service configuration provided does not match schema: ' + \ msg diff --git a/f5_cccl/resource/ltm/app_service.py b/f5_cccl/resource/ltm/app_service.py index c6b9ca72..cfd21136 100644 --- a/f5_cccl/resource/ltm/app_service.py +++ b/f5_cccl/resource/ltm/app_service.py @@ -66,7 +66,7 @@ def __eq__(self, other): return False for key in self._data: - if key == "variables" or key == "tables": + if key in ["variables", "tables"]: # already compared continue if self._data[key] != other.data.get(key, None): diff --git a/f5_cccl/resource/ltm/node.py b/f5_cccl/resource/ltm/node.py index 51d09c66..a521a39c 100644 --- a/f5_cccl/resource/ltm/node.py +++ b/f5_cccl/resource/ltm/node.py @@ -40,7 +40,7 @@ def __init__(self, name, partition, **properties): super(Node, self).__init__(name, partition, **properties) for key, value in self.properties.items(): - if key == "name" or key == "partition": + if key in ["name", "partition"]: continue self._data[key] = properties.get(key, value) diff --git a/f5_cccl/resource/ltm/policy/condition.py b/f5_cccl/resource/ltm/policy/condition.py index e229ae00..2079624c 100644 --- a/f5_cccl/resource/ltm/policy/condition.py +++ b/f5_cccl/resource/ltm/policy/condition.py @@ -38,6 +38,7 @@ class Condition(Resource): "endsWith": None, "startsWith": None, "contains": None, + "matches": None, "not": None, "missing": None, @@ -55,6 +56,9 @@ class Condition(Resource): "httpHeader": False, "httpCookie": False, + "tcp": True, + "address": False, + "tmName": None, "values": list() } @@ -66,7 +70,6 @@ def __init__(self, name, data): values = sorted(data.get('values', list())) tm_name = data.get('tmName', None) - condition_map = dict() # Does this rule match the HTTP hostname? if data.get('httpHost', False): @@ -99,6 +102,23 @@ def __init__(self, name, data): condition_map = { 'httpCookie': True, 'tmName': tm_name, 'values': values} + # Does this rule match a TCP related setting? + elif data.get('tcp', False): + condition_map = {'tcp': True, 'values': values} + + if data.get('external', False): + condition_map['external'] = True + elif data.get('internal', False): + condition_map['internal'] = True + + if data.get('matches', False): + condition_map['matches'] = True + + if data.get('address', False): + condition_map['address'] = True + else: + raise ValueError("must specify address for TCP matching " + "condition") else: # This class does not support the condition type; however, # we want to create in order to manage the policy. @@ -111,7 +131,9 @@ def __init__(self, name, data): # For example, having a comparison option set to 'None' will conflict # with the one that is set to 'True' match_options = ['not', 'missing', 'caseSensitive'] - comparisons = ['contains', 'equals', 'startsWith', 'endsWith'] + comparisons = [ + 'contains', 'equals', 'startsWith', 'endsWith', 'matches' + ] for key in match_options + comparisons: value = data.get(key, None) if value: diff --git a/f5_cccl/resource/ltm/policy/policy.py b/f5_cccl/resource/ltm/policy/policy.py index 31d29c19..7ed04eea 100644 --- a/f5_cccl/resource/ltm/policy/policy.py +++ b/f5_cccl/resource/ltm/policy/policy.py @@ -61,7 +61,7 @@ def __eq__(self, other): """Check the equality of the two objects. Only compare the properties as defined in the - properties class dictionany. + properties class dictionary. """ if not isinstance(other, Policy): return False @@ -155,7 +155,7 @@ def _flatten_policy(self, data): if 'items' in rulesReference: policy['rules'] = self._flatten_rules( rulesReference['items']) - elif key == 'name' or key == 'partition': + elif key in ['name', 'partition']: pass else: policy[key] = data.get(key) diff --git a/f5_cccl/resource/ltm/policy/rule.py b/f5_cccl/resource/ltm/policy/rule.py index e8d89694..8d09e758 100644 --- a/f5_cccl/resource/ltm/policy/rule.py +++ b/f5_cccl/resource/ltm/policy/rule.py @@ -67,7 +67,7 @@ def __eq__(self, other): return False for key in self.properties: - if key == 'actions' or key == 'conditions': + if key in ['actions', 'conditions']: if len(self._data[key]) != len(other.data[key]): return False for index, obj in enumerate(self._data[key]): diff --git a/f5_cccl/resource/ltm/policy/test/test_condition.py b/f5_cccl/resource/ltm/policy/test/test_condition.py index 001400ac..4362bd91 100644 --- a/f5_cccl/resource/ltm/policy/test/test_condition.py +++ b/f5_cccl/resource/ltm/policy/test/test_condition.py @@ -74,6 +74,12 @@ 'tmName': "Host", 'contains': True, 'values': ["www.acme.com"] + }, + 'tcp_address': { + 'tcp': True, + 'address': True, + 'matches': True, + 'values': ["10.10.10.10/32", "10.0.0.0/16"] } } @@ -180,13 +186,13 @@ def test_create_http_uri_unsupported_match(): name="0" with pytest.raises(ValueError): - condition = Condition(name, conditions['http_uri_unsupported']) + Condition(name, conditions['http_uri_unsupported']) def test_create_http_unsupported_operand_type(): name="0" with pytest.raises(ValueError): - condition = Condition(name, conditions['http_unsupported_operand_type']) + Condition(name, conditions['http_unsupported_operand_type']) def test_create_http_uri_path_segment_match(): @@ -350,3 +356,32 @@ def test_uri_path(bigip): with pytest.raises(NotImplementedError): condition._uri_path(bigip) + + +def test_create_tcp_address_match(): + name="0" + condition = Condition(name, conditions['tcp_address']) + data = condition.data + + assert condition.name == "0" + assert not condition.partition + + assert data.get('tcp') + assert data.get('values') == ["10.0.0.0/16", "10.10.10.10/32"] + + assert 'httpHost' not in data + assert 'httpUri' not in data + assert 'httpCookie' not in data + + assert not data.get('equals') + assert not data.get('startsWith') + assert not data.get('endsWith') + assert data.get('matches') + + assert not data.get('missing') + assert not data.get('not') + assert not data.get('caseSensitive') + + assert not data.get('index') + assert not data.get('path') + assert not data.get('pathSegment') diff --git a/f5_cccl/resource/ltm/pool.py b/f5_cccl/resource/ltm/pool.py index dcd156cd..a1228c58 100644 --- a/f5_cccl/resource/ltm/pool.py +++ b/f5_cccl/resource/ltm/pool.py @@ -42,7 +42,7 @@ def __init__(self, name, partition, members=None, **properties): super(Pool, self).__init__(name, partition, **properties) for key, value in self.properties.items(): - if key == "name" or key == "partition": + if key in ["name", "partition"]: continue self._data[key] = properties.get(key, value) @@ -65,7 +65,7 @@ def __eq__(self, other): return False for key in self.properties: - if key == 'membersReference' or key == 'monitor': + if key in ['membersReference', 'monitor']: continue if isinstance(self._data[key], list): @@ -112,7 +112,7 @@ def __init__(self, name, partition, default_route_domain, **properties): """Parse the CCCL schema input.""" pool_config = dict() for k, v in properties.items(): - if k == "members" or k == "monitors": + if k in ["members", "monitors"]: continue pool_config[k] = v diff --git a/f5_cccl/resource/ltm/pool_member.py b/f5_cccl/resource/ltm/pool_member.py index 8c8cac99..bb752c5b 100644 --- a/f5_cccl/resource/ltm/pool_member.py +++ b/f5_cccl/resource/ltm/pool_member.py @@ -51,7 +51,7 @@ def __init__(self, name, partition, pool=None, **properties): self._pool = pool for key, value in self.properties.items(): - if key == 'name' or key == 'partition': + if key in ['name', 'partition']: continue self._data[key] = properties.get(key, value) diff --git a/f5_cccl/resource/ltm/virtual.py b/f5_cccl/resource/ltm/virtual.py index 71bad869..3f5d2129 100644 --- a/f5_cccl/resource/ltm/virtual.py +++ b/f5_cccl/resource/ltm/virtual.py @@ -191,8 +191,7 @@ def __eq__(self, other): len(self._data[key]) != len(other.data.get(key, list())): return False - if key == 'vlans' or key == 'policies' or key == 'rules' \ - or key == 'metadata': + if key in ['vlans', 'policies', 'rules', 'metadata']: if sorted(self._data[key]) != \ sorted(other.data.get(key, list())): return False diff --git a/f5_cccl/resource/net/arp.py b/f5_cccl/resource/net/arp.py index 30a6456f..f2e421c0 100644 --- a/f5_cccl/resource/net/arp.py +++ b/f5_cccl/resource/net/arp.py @@ -36,7 +36,7 @@ def __init__(self, name, partition, **data): super(Arp, self).__init__(name, partition) for key, value in self.properties.items(): - if key == "name" or key == "partition": + if key in ["name", "partition"]: continue self._data[key] = data.get(key, value) diff --git a/f5_cccl/resource/net/fdb/tunnel.py b/f5_cccl/resource/net/fdb/tunnel.py index 9f8bb960..dd383605 100644 --- a/f5_cccl/resource/net/fdb/tunnel.py +++ b/f5_cccl/resource/net/fdb/tunnel.py @@ -53,10 +53,9 @@ def __eq__(self, other): for record in self._data[key]: if record not in other.data[key]: return False - else: - idx = other.data[key].index(record) - if record != other.data[key][idx]: - return False + idx = other.data[key].index(record) + if record != other.data[key][idx]: + return False continue if self._data[key] != other.data.get(key): return False diff --git a/f5_cccl/resource/resource.py b/f5_cccl/resource/resource.py index 4b1151ae..ecf89a54 100644 --- a/f5_cccl/resource/resource.py +++ b/f5_cccl/resource/resource.py @@ -378,7 +378,7 @@ def _handle_http_error(self, error): raise cccl_exc.F5CcclResourceNotFoundError(str(error)) elif code == 409: raise cccl_exc.F5CcclResourceConflictError(str(error)) - elif code >= 400 and code < 500: + elif 400 <= code < 500: raise cccl_exc.F5CcclResourceRequestError(str(error)) else: raise cccl_exc.F5CcclError(str(error)) diff --git a/f5_cccl/schemas/cccl-ltm-api-schema.yml b/f5_cccl/schemas/cccl-ltm-api-schema.yml index 39ffe8fe..9174e64c 100644 --- a/f5_cccl/schemas/cccl-ltm-api-schema.yml +++ b/f5_cccl/schemas/cccl-ltm-api-schema.yml @@ -268,7 +268,7 @@ definitions: type: "object" description: | Defines an L7 Policy condition. - The supported operands are http-host, http-uri, http-header, and http-cookie + The supported operands are http-host, http-uri, http-header, http-cookie, and tcp Operands httpHost -- Provides all or part of the HTTP Host header, only match against @@ -282,6 +282,8 @@ definitions: to match against is required. httpHeader -- Returns the value of a particular header. If matching a HTTP header, tmName must be defined. A list of values to match against is required. + tcp -- Returns the value of a particular TCP related attribute. A list of values + to match against is required. Supported match operators are equals, startsWith, endsWith, and contains. These match operators can be negated by setting 'not' to true. @@ -290,7 +292,7 @@ definitions: # Match modifiers caseSensitive: default: false - description: "The match string is case-sensive" + description: "The match string is case-sensitive" type: "boolean" missing: default: false @@ -314,6 +316,9 @@ definitions: contains: description: "The value matches if it contains a certain string." type: "boolean" + matches: + description: "The value matches if it matches certain values." + type: "boolean" # Operand types httpUri: @@ -328,6 +333,9 @@ definitions: httpHeader: description: "Name of the HTTP header that contains the value to compare." type: "boolean" + tcp: + description: "Inspect and match on various TCP properties of a connection." + type: "boolean" # Operand values tmName: @@ -354,6 +362,9 @@ definitions: index: description: "The specified index is used to match a specific segment." type: "integer" + address: + description: "Match on IP address" + type: "boolean" oneOf: - required: @@ -385,6 +396,10 @@ definitions: - "httpUri" - "host" - "values" + - required: + - "tcp" + - "address" + - "values" l7PolicyType: id: "#/definitions/l7PolicyType" @@ -545,7 +560,7 @@ definitions: type: "string" loadBalancingMode: default: "round-robin" - description: "Loadbalancing algorithm to use on pool." + description: "Load balancing algorithm to use on pool." enum: - "dynamic-ratio-member" - "dynamic-ratio-node" diff --git a/f5_cccl/schemas/tests/ltm_service.json b/f5_cccl/schemas/tests/ltm_service.json index 32b3596f..0db05b4e 100644 --- a/f5_cccl/schemas/tests/ltm_service.json +++ b/f5_cccl/schemas/tests/ltm_service.json @@ -264,6 +264,26 @@ "location": "http://www.othersite.com" } ] + }, + { + "conditions": [ + { + "tcp": true, + "address": true, + "matches": true, + "external": true, + "values": ["10.10.10.10/32"] + } + ], + "name": "rule3", + "actions": [ + { + "request": true, + "redirect": true, + "httpReply": true, + "location": "http://www.othersite.com" + } + ] } ] }], diff --git a/f5_cccl/service/validation.py b/f5_cccl/service/validation.py index 051d85da..ae739986 100644 --- a/f5_cccl/service/validation.py +++ b/f5_cccl/service/validation.py @@ -52,11 +52,10 @@ def read_yaml_or_json(target): """Read json or yaml, return a dict.""" if target.lower().endswith('.json'): return read_json(target) - elif target.lower().endswith('.yaml') or target.lower().endswith('.yml'): + if target.lower().endswith('.yaml') or target.lower().endswith('.yml'): return read_yaml(target) - else: - raise cccl_exc.F5CcclError( - 'CCCL API schema json or yaml file expected.') + raise cccl_exc.F5CcclError( + 'CCCL API schema json or yaml file expected.') class ServiceConfigValidator(object): diff --git a/f5_cccl/utils/resource_merge.py b/f5_cccl/utils/resource_merge.py index add9b82d..649c5576 100644 --- a/f5_cccl/utils/resource_merge.py +++ b/f5_cccl/utils/resource_merge.py @@ -88,7 +88,7 @@ def _merge_list(dst, src): if not dst: return src - elif isinstance(dst[0], dict): + if isinstance(dst[0], dict): dst = _merge_list_of_dict_by_name(dst, src) elif isinstance(dst[0], list): # May cause duplicates (what is a duplicate for lists of lists?) diff --git a/requirements.test.txt b/requirements.test.txt index 6e9c1977..c8eb29ea 100644 --- a/requirements.test.txt +++ b/requirements.test.txt @@ -1,7 +1,7 @@ git+https://github.com/F5Networks/pytest-symbols.git # Test Requirements -f5-sdk==3.0.3 +f5-sdk==3.0.20 ipaddress==1.0.17 PyJWT==1.4.0 mock==2.0.0 @@ -21,3 +21,4 @@ jsonschema jsonpatch==1.16 python-coveralls==2.7.0 requests==2.9.1 +urllib3==1.23 diff --git a/setup_requirements.txt b/setup_requirements.txt index 9e646376..1a78904e 100644 --- a/setup_requirements.txt +++ b/setup_requirements.txt @@ -1,6 +1,6 @@ # F5-CCCL Install Requirements -f5-icontrol-rest==1.3.4 -f5-sdk==3.0.3 +f5-icontrol-rest==1.3.11 +f5-sdk==3.0.20 ipaddress==1.0.17 netaddr==0.7.19 PyJWT==1.4.0 @@ -10,3 +10,4 @@ simplejson==3.10.0 jsonpatch==1.16 jsonpointer==2.0 jsonschema +urllib3==1.23 diff --git a/test/f5_cccl/conftest.py b/test/f5_cccl/conftest.py index c96ea68a..f3a93754 100644 --- a/test/f5_cccl/conftest.py +++ b/test/f5_cccl/conftest.py @@ -54,8 +54,9 @@ def bigip(): hostname = pytest.symbols.bigip_mgmt_ip username = pytest.symbols.bigip_username password = pytest.symbols.bigip_password + port = pytest.symbols.bigip_port - bigip_fix = ManagementRoot(hostname, username, password) + bigip_fix = ManagementRoot(hostname, username, password, port=port) bigip_fix = instrument_bigip(bigip_fix) else: bigip_fix = MagicMock() @@ -87,19 +88,19 @@ def partition(bigip): except iControlUnexpectedHTTPError as icr_error: pass try: - bigip.tm.sys.folders.folder.load(name=name).delete() + bigip.tm.auth.partitions.partition.load(name=name).delete() except iControlUnexpectedHTTPError as icr_error: pass try: - partition = bigip.tm.sys.folders.folder.create(subPath="/", name=name) + partition = bigip.tm.auth.partitions.partition.create(subPath="/", name=name) except iControlUnexpectedHTTPError as icr_error: code = icr_error.response.status_code if code == 400: print("Can't create partition {}".format(name)) elif code == 409: print("Partition {} already exists".format(name)) - partition = bigip.tm.sys.folders.folder.load(subPath="/", name=name) + partition = bigip.tm.auth.partitions.partition.load(subPath="/", name=name) else: print("Unknown error creating partition.") print(icr_error) @@ -119,7 +120,7 @@ def partition(bigip): def cccl(bigip, partition): cccl = F5CloudServiceManager(bigip, partition) yield cccl - cccl.apply_config({}) + cccl.apply_ltm_config({}) @pytest.fixture() diff --git a/test/f5_cccl/perf/test_perf.py b/test/f5_cccl/perf/test_perf.py index 80dfe978..78e5850f 100644 --- a/test/f5_cccl/perf/test_perf.py +++ b/test/f5_cccl/perf/test_perf.py @@ -15,61 +15,51 @@ # limitations under the License. # -from copy import deepcopy import pytest import requests -import pdb - -from mock import MagicMock - -from f5.bigip import ManagementRoot - -from f5.sdk_exception import F5SDKError -from icontrol.exceptions import iControlUnexpectedHTTPError - -import f5_cccl.exceptions as exceptions -from f5_cccl.resource.ltm.virtual import VirtualServer from pprint import pprint requests.packages.urllib3.disable_warnings() -req_symbols = ['bigip_mgmt_ip', 'bigip_username', 'bigip_password'] +req_symbols = ['bigip_mgmt_ip', 'bigip_username', 'bigip_password', 'bigip_port'] def missing_bigip_symbols(): for sym in req_symbols: if not hasattr(pytest.symbols, sym): return True return False + pytestmark = pytest.mark.skipif(missing_bigip_symbols(), reason="Need symbols pointing at a real bigip.") + def _make_svc_config(partition, num_virtuals=0, num_members=0): base_virtual = { 'name': 'Virtual-1', - 'destination': '/Test/1.2.3.4:80', + 'destination': '/{}/1.2.3.4:80'.format(partition), 'ipProtocol': 'tcp', 'profiles': [ - {'name': "tcp", - 'partition': "Common", - 'context': "all"} + { + 'name': "tcp", + 'partition': "Common", + 'context': "all" + } ], "enabled": True, "vlansEnabled": True, "sourceAddressTranslation": { - "type": "automap", + "type": "automap", } } base_pool = { "name": "pool1", "monitors": ["/Common/http"] - }, - base_member ={ + } + base_member = { "address": "172.16.0.100%0", "port": 8080 } - - - cfg={ + cfg = { 'virtualServers': [], 'pools': [], } @@ -77,7 +67,7 @@ def _make_svc_config(partition, num_virtuals=0, num_members=0): v = {} v.update(base_virtual) v['name'] = "virtual-{}".format(i) - v['pool'] = "/{}/pool-{}".format(partition,i) + v['pool'] = "/{}/pool-{}".format(partition, i) cfg['virtualServers'].append(v) p = {} @@ -85,16 +75,16 @@ def _make_svc_config(partition, num_virtuals=0, num_members=0): p['name'] = "pool-{}".format(i) members = [] - for i in range(num_members): + for j in range(num_members): m = {} m.update(base_member) - m['address'] = '172.16.0.{}'.format(i) + m['address'] = '172.16.0.{}'.format(j) members.append(m) p['members'] = members cfg['pools'].append(p) - return cfg + testdata = [ (1, 1), (10, 10), @@ -103,17 +93,15 @@ def _make_svc_config(partition, num_virtuals=0, num_members=0): ] - @pytest.mark.parametrize("nv,nm", testdata) @pytest.mark.benchmark(group="apply-new") def test_apply_new(partition, cccl, bigip_rest_counters, benchmark, nv, nm): cfg = _make_svc_config(partition, num_virtuals=nv, num_members=nm) def setup(): - cccl.apply_config({}) + cccl.apply_ltm_config({}) def apply(): - cccl.apply_config(cfg) + cccl.apply_ltm_config(cfg) benchmark.pedantic(apply, setup=setup, rounds=2, iterations=1) - pprint(bigip_rest_counters) @pytest.mark.parametrize("nv,nm", testdata) @@ -121,7 +109,7 @@ def apply(): def test_apply_no_change(partition, cccl, bigip_rest_counters, benchmark, nv, nm): cfg = _make_svc_config(partition, num_virtuals=nv, num_members=nm) def apply(): - cccl.apply_config(cfg) + cccl.apply_ltm_config(cfg) apply() benchmark.pedantic(apply, rounds=2, iterations=1) pprint(bigip_rest_counters) diff --git a/test/f5_cccl/resource/ltm/policy/test_policy.py b/test/f5_cccl/resource/ltm/policy/test_policy.py index e547e6a6..522a7ad3 100644 --- a/test/f5_cccl/resource/ltm/policy/test_policy.py +++ b/test/f5_cccl/resource/ltm/policy/test_policy.py @@ -118,6 +118,12 @@ 'tmName': "Host", 'contains': True, 'values': ["www.acme.com"] + }, + 'tcp_address': { + 'tcp': True, + 'address': True, + 'matches': True, + 'values': ['1.1.1.1/32', '2.2.2.0/24'] } } @@ -535,7 +541,6 @@ def test_create_policy_supported_conditions(self, bigip, partition): except exceptions.F5CcclError as e: print(e) - def test_create_policy_supported_actions(self, bigip, partition, pool): """Create a policy with supported actions""" if isinstance(bigip, MagicMock): diff --git a/tox.ini b/tox.ini index 0011758a..a452c76c 100644 --- a/tox.ini +++ b/tox.ini @@ -23,12 +23,12 @@ deps = # commands = # Misc tests - unit: py.test ./f5_cccl --cov=f5_cccl/ {posargs} + unit: py.test --cov=f5_cccl/ {posargs:./f5_cccl} style: flake8 {posargs:.} style: pylint f5_cccl/ coverage: coveralls flake: flake8 {posargs:.} - functional: py.test ./test + functional: py.test {posargs:./test} docs: bash ./devtools/bin/build-docs.sh usedevelop = true