From e88c14287c07a55a43efa3b2063d927baaa6aae2 Mon Sep 17 00:00:00 2001 From: Xiaotong Luo Date: Mon, 14 Sep 2020 12:26:29 -0700 Subject: [PATCH 1/5] Add error message and NSX time for failed authentication In addition to status code, also log error message and local NSX time for failed NSX authentication for easier troubleshooting for failed JWT authentication and potential clock drift between NSX, VC and Master node. Change-Id: Icf31477bffda85ba73a1123232b6aa5503066922 (cherry picked from commit 8e7075b69d4870e6ff1d123507164cc325735efb) --- vmware_nsxlib/v3/cluster.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/vmware_nsxlib/v3/cluster.py b/vmware_nsxlib/v3/cluster.py index 8facbe6a..d162855b 100644 --- a/vmware_nsxlib/v3/cluster.py +++ b/vmware_nsxlib/v3/cluster.py @@ -281,8 +281,11 @@ class NSXRequestsHTTPProvider(AbstractHTTPProvider): else: if resp.status_code != 200 and resp.status_code != 201: LOG.warning("Session create failed for endpoint %s with " - "response %s", - provider.url, resp.status_code) + "response %s, error message: %s, " + "local NSX time: %s", + provider.url, resp.status_code, + resp.json().get('error_message'), + resp.headers['Date']) # this may will later cause the endpoint to be Down else: for header_name in resp.headers: From 6da897f1360fcc715ce1371b812646b4b680a233 Mon Sep 17 00:00:00 2001 From: Shawn Wang Date: Fri, 25 Sep 2020 14:35:38 -0700 Subject: [PATCH 2/5] Fix Sensitive Header Censorship in Log - Add censoring of sensitive headers from being logged in _proxy() - Fix issue where Cookie and X-XSRF-TOKEN were not censored as intended Change-Id: I14b422a25b40d0014c05226f9ae4fe8be75e33fb (cherry picked from commit 6b34416026120c72fa63e260c228f3fce3497460) --- vmware_nsxlib/v3/cluster.py | 6 +++++- vmware_nsxlib/v3/utils.py | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/vmware_nsxlib/v3/cluster.py b/vmware_nsxlib/v3/cluster.py index d162855b..13339838 100644 --- a/vmware_nsxlib/v3/cluster.py +++ b/vmware_nsxlib/v3/cluster.py @@ -762,9 +762,13 @@ class ClusteredAPI(object): kwargs['headers'] = kwargs.get('headers', {}) kwargs['headers'].update(conn.default_headers) if not self._silent: + # To censor sensitive headers before logging + kwargs_copy = copy.copy(kwargs) + kwargs_copy['headers'] = utils.censor_headers( + kwargs_copy['headers']) LOG.debug("API cluster proxy %s %s to %s with %s. " "Waited conn: %2.4f, rate: %2.4f", - proxy_for.upper(), uri, url, kwargs, + proxy_for.upper(), uri, url, kwargs_copy, conn_data.conn_wait, conn_data.rate_wait) # call the actual connection method to do the diff --git a/vmware_nsxlib/v3/utils.py b/vmware_nsxlib/v3/utils.py index a024fd62..bf4c3f73 100644 --- a/vmware_nsxlib/v3/utils.py +++ b/vmware_nsxlib/v3/utils.py @@ -64,7 +64,7 @@ def set_inject_headers_callback(callback): def censor_headers(headers): - censored_headers = ['authorization'] + censored_headers = ['authorization', 'x-xsrf-token', 'cookie'] result = {} for name, value in headers.items(): if name.lower() in censored_headers: From bd983d2673c975fb062d0377b432ee07ab7ced11 Mon Sep 17 00:00:00 2001 From: Shih-Hao Li Date: Mon, 24 Aug 2020 03:28:43 -0700 Subject: [PATCH 3/5] NSXT: Add rule tag support Expose firewall rule rule_tag property. Change-Id: Iec6848e325bb7e1eb43b83d060ba9486897cc93a (cherry picked from commit 3ba085fec36668e70dd1a26a12d5c7be83b5156b) --- vmware_nsxlib/tests/unit/v3/test_security.py | 20 ++++++++++++++++++++ vmware_nsxlib/v3/security.py | 5 ++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/vmware_nsxlib/tests/unit/v3/test_security.py b/vmware_nsxlib/tests/unit/v3/test_security.py index 1f57e082..64a6df3e 100644 --- a/vmware_nsxlib/tests/unit/v3/test_security.py +++ b/vmware_nsxlib/tests/unit/v3/test_security.py @@ -63,6 +63,26 @@ class TestNsxLibFirewallSection(nsxlib_testcase.NsxLibTestCase): } self.assertEqual(expected, result) + def test_get_rule_dict(self): + result = self.nsxlib.firewall_section.get_rule_dict( + 'display_name', sources='sources', destinations='destinations', + direction=const.IN_OUT, ip_protocol=const.IPV4_IPV6, + services='services', action=const.FW_ACTION_ALLOW, + logged=True, disabled=True, applied_tos='applied_tos', + rule_tag='rule_tag') + expected = {'display_name': 'display_name', + 'sources': 'sources', + 'destinations': 'destinations', + 'direction': const.IN_OUT, + 'ip_protocol': const.IPV4_IPV6, + 'services': 'services', + 'action': const.FW_ACTION_ALLOW, + 'logged': True, + 'disabled': True, + 'applied_tos': 'applied_tos', + 'rule_tag': 'rule_tag'} + self.assertEqual(expected, result) + def test_create_rules_with_protocol(self): with mock.patch("vmware_nsxlib.v3.security.NsxLibFirewallSection" ".add_rules") as add_rules: diff --git a/vmware_nsxlib/v3/security.py b/vmware_nsxlib/v3/security.py index 47bb3b14..bbfeb02c 100644 --- a/vmware_nsxlib/v3/security.py +++ b/vmware_nsxlib/v3/security.py @@ -407,7 +407,8 @@ class NsxLibFirewallSection(utils.NsxLibApiBase): def get_rule_dict(self, display_name, sources=None, destinations=None, direction=consts.IN_OUT, ip_protocol=consts.IPV4_IPV6, services=None, action=consts.FW_ACTION_ALLOW, - logged=False, disabled=False, applied_tos=None): + logged=False, disabled=False, applied_tos=None, + rule_tag=None): rule_dict = {'display_name': display_name, 'direction': direction, 'ip_protocol': ip_protocol, @@ -419,6 +420,8 @@ class NsxLibFirewallSection(utils.NsxLibApiBase): 'services': services or []} if applied_tos is not None: rule_dict['applied_tos'] = applied_tos + if rule_tag is not None: + rule_dict['rule_tag'] = rule_tag return rule_dict def add_rule(self, rule, section_id, operation=consts.FW_INSERT_BOTTOM): From 65ad77d6baab061b438bb676180cf297b158c688 Mon Sep 17 00:00:00 2001 From: Danting Liu Date: Tue, 18 Aug 2020 20:48:12 -0700 Subject: [PATCH 4/5] Add force delete option for IP block subnet If IP block is created by Policy API, we need {'X-Allow-Overwrite': 'true'} in header to delete subnet Change-Id: Ib05437fef32d2ea206ee5af67b441f710d6f5016 (cherry picked from commit be35d1dc1140deaa61724027b0a144f900fae365) --- .../tests/unit/v3/test_load_balancer.py | 23 +++++++++++-------- .../unit/v3/test_qos_switching_profile.py | 3 ++- vmware_nsxlib/tests/unit/v3/test_resources.py | 11 +++++++++ vmware_nsxlib/tests/unit/v3/test_security.py | 5 ++-- vmware_nsxlib/v3/core_resources.py | 8 +++++-- vmware_nsxlib/v3/utils.py | 8 +++---- 6 files changed, 40 insertions(+), 18 deletions(-) diff --git a/vmware_nsxlib/tests/unit/v3/test_load_balancer.py b/vmware_nsxlib/tests/unit/v3/test_load_balancer.py index c96e31de..2a826549 100644 --- a/vmware_nsxlib/tests/unit/v3/test_load_balancer.py +++ b/vmware_nsxlib/tests/unit/v3/test_load_balancer.py @@ -152,7 +152,8 @@ class TestApplicationProfile(nsxlib_testcase.NsxClientTestCase): self.nsxlib.load_balancer.application_profile.delete( fake_profile['id']) delete.assert_called_with( - 'loadbalancer/application-profiles/%s' % fake_profile['id']) + 'loadbalancer/application-profiles/%s' % fake_profile['id'], + headers=None) class TestPersistenceProfile(nsxlib_testcase.NsxClientTestCase): @@ -193,7 +194,8 @@ class TestPersistenceProfile(nsxlib_testcase.NsxClientTestCase): self.nsxlib.load_balancer.persistence_profile.delete( fake_profile['id']) delete.assert_called_with( - 'loadbalancer/persistence-profiles/%s' % fake_profile['id']) + 'loadbalancer/persistence-profiles/%s' % fake_profile['id'], + headers=None) class TestRule(nsxlib_testcase.NsxClientTestCase): @@ -227,7 +229,7 @@ class TestRule(nsxlib_testcase.NsxClientTestCase): fake_rule = consts.FAKE_RULE.copy() self.nsxlib.load_balancer.rule.delete(fake_rule['id']) delete.assert_called_with( - 'loadbalancer/rules/%s' % fake_rule['id']) + 'loadbalancer/rules/%s' % fake_rule['id'], headers=None) class TestClientSslProfile(nsxlib_testcase.NsxClientTestCase): @@ -265,7 +267,8 @@ class TestClientSslProfile(nsxlib_testcase.NsxClientTestCase): self.nsxlib.load_balancer.client_ssl_profile.delete( fake_profile['id']) delete.assert_called_with( - 'loadbalancer/client-ssl-profiles/%s' % fake_profile['id']) + 'loadbalancer/client-ssl-profiles/%s' % fake_profile['id'], + headers=None) class TestServerSslProfile(nsxlib_testcase.NsxClientTestCase): @@ -303,7 +306,8 @@ class TestServerSslProfile(nsxlib_testcase.NsxClientTestCase): self.nsxlib.load_balancer.server_ssl_profile.delete( fake_profile['id']) delete.assert_called_with( - 'loadbalancer/server-ssl-profiles/%s' % fake_profile['id']) + 'loadbalancer/server-ssl-profiles/%s' % fake_profile['id'], + headers=None) class TestMonitor(nsxlib_testcase.NsxClientTestCase): @@ -341,7 +345,7 @@ class TestMonitor(nsxlib_testcase.NsxClientTestCase): fake_monitor = consts.FAKE_MONITOR.copy() self.nsxlib.load_balancer.monitor.delete(fake_monitor['id']) delete.assert_called_with( - 'loadbalancer/monitors/%s' % fake_monitor['id']) + 'loadbalancer/monitors/%s' % fake_monitor['id'], headers=None) class TestPool(nsxlib_testcase.NsxClientTestCase): @@ -378,7 +382,7 @@ class TestPool(nsxlib_testcase.NsxClientTestCase): fake_profile = consts.FAKE_POOL.copy() self.nsxlib.load_balancer.pool.delete(fake_profile['id']) delete.assert_called_with( - 'loadbalancer/pools/%s' % fake_profile['id']) + 'loadbalancer/pools/%s' % fake_profile['id'], headers=None) def test_remove_monitor_from_pool(self): fake_pool = consts.FAKE_POOL.copy() @@ -463,7 +467,8 @@ class TestVirtualServer(nsxlib_testcase.NsxClientTestCase): self.nsxlib.load_balancer.virtual_server.delete( fake_virtual_server['id']) delete.assert_called_with( - 'loadbalancer/virtual-servers/%s' % fake_virtual_server['id']) + 'loadbalancer/virtual-servers/%s' % fake_virtual_server['id'], + headers=None) def test_add_rule(self): fake_virtual_server = consts.FAKE_VIRTUAL_SERVER.copy() @@ -633,7 +638,7 @@ class TestService(nsxlib_testcase.NsxClientTestCase): fake_service = consts.FAKE_SERVICE.copy() self.nsxlib.load_balancer.service.delete(fake_service['id']) delete.assert_called_with( - 'loadbalancer/services/%s' % fake_service['id']) + 'loadbalancer/services/%s' % fake_service['id'], headers=None) def test_get_usage(self): with mock.patch.object(self.nsxlib.client, 'get') as get: diff --git a/vmware_nsxlib/tests/unit/v3/test_qos_switching_profile.py b/vmware_nsxlib/tests/unit/v3/test_qos_switching_profile.py index ffe6a410..77ed8039 100644 --- a/vmware_nsxlib/tests/unit/v3/test_qos_switching_profile.py +++ b/vmware_nsxlib/tests/unit/v3/test_qos_switching_profile.py @@ -223,7 +223,8 @@ class NsxLibQosTestCase(nsxlib_testcase.NsxClientTestCase): test_constants.FAKE_QOS_PROFILE['id']) delete.assert_called_with( 'switching-profiles/%s' - % test_constants.FAKE_QOS_PROFILE['id']) + % test_constants.FAKE_QOS_PROFILE['id'], + headers=None) def test_qos_switching_profile_set_shaping(self): """Test updating a qos-switching profile diff --git a/vmware_nsxlib/tests/unit/v3/test_resources.py b/vmware_nsxlib/tests/unit/v3/test_resources.py index 053e4b21..fc393f99 100644 --- a/vmware_nsxlib/tests/unit/v3/test_resources.py +++ b/vmware_nsxlib/tests/unit/v3/test_resources.py @@ -2356,6 +2356,17 @@ class NsxLibIpBlockSubnetTestCase(BaseTestResource): (mocked_resource.uri_segment, block_id), headers=self.default_headers()) + def test_force_delete(self): + mocked_resource = self.get_mocked_resource() + subnet_id = 'subnet1' + headers = self.default_headers() + headers['X-Allow-Overwrite'] = 'true' + mocked_resource.delete(subnet_id, allow_overwrite=True) + test_client.assert_json_call( + 'delete', mocked_resource, + 'https://1.2.3.4/api/v1/pools/ip-subnets/%s' % subnet_id, + headers=headers) + class NsxLibIpBlockTestCase(BaseTestResource): diff --git a/vmware_nsxlib/tests/unit/v3/test_security.py b/vmware_nsxlib/tests/unit/v3/test_security.py index 64a6df3e..a369e042 100644 --- a/vmware_nsxlib/tests/unit/v3/test_security.py +++ b/vmware_nsxlib/tests/unit/v3/test_security.py @@ -453,7 +453,8 @@ class TestNsxLibIPSet(nsxlib_testcase.NsxClientTestCase): with mock.patch.object(self.nsxlib.client, 'delete') as delete: fake_ip_set = test_constants.FAKE_IP_SET.copy() self.nsxlib.ip_set.delete(fake_ip_set['id']) - delete.assert_called_with('ip-sets/%s' % fake_ip_set['id']) + delete.assert_called_with('ip-sets/%s' % fake_ip_set['id'], + headers=None) def test_update_ip_set(self): fake_ip_set = test_constants.FAKE_IP_SET.copy() @@ -614,7 +615,7 @@ class TestNsxLibNSGroup(nsxlib_testcase.NsxClientTestCase): 'delete') as del_mock: self.nsxlib.ns_group.delete(ns_group_id) del_mock.assert_called_with( - 'ns-groups/%s?force=true' % ns_group_id) + 'ns-groups/%s?force=true' % ns_group_id, headers=None) def test_update_nsgroup_and_section(self): name = 'name' diff --git a/vmware_nsxlib/v3/core_resources.py b/vmware_nsxlib/v3/core_resources.py index 6a3cbe63..ac7c0421 100644 --- a/vmware_nsxlib/v3/core_resources.py +++ b/vmware_nsxlib/v3/core_resources.py @@ -1086,9 +1086,13 @@ class NsxLibIpBlockSubnet(utils.NsxLibApiBase): headers = {'X-Allow-Overwrite': 'true'} return self.client.create(self.get_path(), body, headers=headers) - def delete(self, subnet_id): + def delete(self, subnet_id, allow_overwrite=False): """Delete a IP block subnet on the backend.""" - self._delete_with_retry(subnet_id) + headers = None + if allow_overwrite: + # Need to force delete subnet if IpBlock is created by Policy API + headers = {'X-Allow-Overwrite': 'true'} + self._delete_with_retry(subnet_id, headers=headers) def list(self, ip_block_id): resource = '%s?block_id=%s' % (self.get_path(), ip_block_id) diff --git a/vmware_nsxlib/v3/utils.py b/vmware_nsxlib/v3/utils.py index bf4c3f73..224387af 100644 --- a/vmware_nsxlib/v3/utils.py +++ b/vmware_nsxlib/v3/utils.py @@ -477,15 +477,15 @@ class NsxLibApiBase(object): action_params=action_params, update_payload_cbk=update_payload_cbk) - def _delete_with_retry(self, resource): - self._delete_by_path_with_retry(self.get_path(resource)) + def _delete_with_retry(self, resource, headers=None): + self._delete_by_path_with_retry(self.get_path(resource), headers) - def _delete_by_path_with_retry(self, path): + def _delete_by_path_with_retry(self, path, headers=None): # Using internal method so we can access max_attempts in the decorator @retry_upon_exception(nsxlib_exc.StaleRevision, max_attempts=self.max_attempts) def _do_delete(): - self.client.delete(path) + self.client.delete(path, headers=headers) _do_delete() From 0f90a705966dc2bfb71c0fbed83eb64b846eb2d6 Mon Sep 17 00:00:00 2001 From: Erica Liu Date: Tue, 13 Oct 2020 17:07:36 -0700 Subject: [PATCH 5/5] Adding support for customzing ep value in segement creation In case of none default enforcement point value, the Segment create might fail, because it is hard coded to use default ep for querying transport zone, if specified. This change add support for creating segment with transport zone in none default enforcement point Change-Id: Id122f9591c2bded5edc43fad514e6e1e9e6a9fa3 (cherry picked from commit ba0994042982c81dede2f7e7800bc479ca23d962) --- .../tests/unit/v3/policy/test_resources.py | 14 +++++++++++++- vmware_nsxlib/v3/policy/core_defs.py | 3 ++- vmware_nsxlib/v3/policy/core_resources.py | 2 ++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/vmware_nsxlib/tests/unit/v3/policy/test_resources.py b/vmware_nsxlib/tests/unit/v3/policy/test_resources.py index 4cc19af2..a34e122f 100644 --- a/vmware_nsxlib/tests/unit/v3/policy/test_resources.py +++ b/vmware_nsxlib/tests/unit/v3/policy/test_resources.py @@ -4093,7 +4093,8 @@ class TestPolicySegment(NsxPolicyLibTestCase): def _test_create(self, tier1_id=None, tier0_id=None, mdproxy=None, dhcp_server=None, admin_state=None, - ip_pool_id='external-ip-pool', ls_id=None): + ip_pool_id='external-ip-pool', ls_id=None, + tz_id=None, ep_id=None): name = 'test' description = 'desc' subnets = [core_defs.Subnet(gateway_address="2.2.2.0/24")] @@ -4118,6 +4119,11 @@ class TestPolicySegment(NsxPolicyLibTestCase): if ls_id: kwargs['ls_id'] = ls_id + if tz_id: + kwargs['transport_zone_id'] = tz_id + if ep_id: + kwargs['ep_id'] = ep_id + with mock.patch.object(self.policy_api, "create_or_update") as api_call: result = self.resourceApi.create_or_overwrite(name, **kwargs) @@ -4171,6 +4177,12 @@ class TestPolicySegment(NsxPolicyLibTestCase): def test_create_with_ls_id(self): self._test_create(ls_id='lsid1') + def test_create_with_transport_zone_id(self): + self._test_create(tz_id='tz_id1', ep_id='ep_id1') + + def test_create_with_transport_zone_id_and_default_ep(self): + self._test_create(tz_id='tz_id1') + def test_delete(self): segment_id = '111' with mock.patch.object(self.policy_api, "delete") as api_call: diff --git a/vmware_nsxlib/v3/policy/core_defs.py b/vmware_nsxlib/v3/policy/core_defs.py index 2d7c2e4a..45426b2d 100644 --- a/vmware_nsxlib/v3/policy/core_defs.py +++ b/vmware_nsxlib/v3/policy/core_defs.py @@ -996,7 +996,8 @@ class SegmentDef(BaseSegmentDef): if self.get_attr('transport_zone_id'): tz = TransportZoneDef( tz_id=self.get_attr('transport_zone_id'), - ep_id=constants.DEFAULT_ENFORCEMENT_POINT, + ep_id=self.get_attr( + 'ep_id') or constants.DEFAULT_ENFORCEMENT_POINT, tenant=self.get_tenant()) path = tz.get_resource_full_path() self._set_attr_if_specified(body, 'transport_zone_id', diff --git a/vmware_nsxlib/v3/policy/core_resources.py b/vmware_nsxlib/v3/policy/core_resources.py index a7f8e632..05ed9523 100644 --- a/vmware_nsxlib/v3/policy/core_resources.py +++ b/vmware_nsxlib/v3/policy/core_resources.py @@ -1947,6 +1947,7 @@ class NsxPolicySegmentApi(NsxPolicyResourceBase): dhcp_server_config_id=IGNORE, admin_state=IGNORE, ls_id=IGNORE, + ep_id=IGNORE, tags=IGNORE, tenant=constants.POLICY_INFRA_TENANT): @@ -1971,6 +1972,7 @@ class NsxPolicySegmentApi(NsxPolicyResourceBase): dhcp_server_config_id=dhcp_server_config_id, admin_state=admin_state, ls_id=ls_id, + ep_id=ep_id, tags=tags, tenant=tenant) self._create_or_store(segment_def)