Fix logging and new name for allowlist (Azure#27673)

* Fix logging and new name for allowlist * Tests and everything * Hide the old doc * CI happyness * Actual test * azure-mgmt-core * mgmt version * CI fix * Rename to cls * TBH, trigger CI
azure-sdk · Nov 30, 2022 · 774c9a7 · 774c9a7
1 parent 3c04b76
commit 774c9a7
Show file tree

Hide file tree

Showing 11 changed files with 49 additions and 11 deletions.
diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md
@@ -10,6 +10,9 @@
 
 ### Other Changes
 
+- Add "x-ms-error-code" as secure header to log
+- Rename "DEFAULT_HEADERS_WHITELIST" to "DEFAULT_HEADERS_ALLOWLIST". Added a backward compatible alias.
+
 ## 1.26.1 (2022-11-03)
 
 ### Other Changes

diff --git a/sdk/core/azure-core/azure/core/pipeline/policies/_universal.py b/sdk/core/azure-core/azure/core/pipeline/policies/_universal.py
@@ -351,15 +351,27 @@ def on_response(self, request, response):
         except Exception as err:  # pylint: disable=broad-except
             _LOGGER.debug("Failed to log response: %s", repr(err))
 
+class _HiddenClassProperties(type):
+    # Backward compatible for DEFAULT_HEADERS_WHITELIST
+    # https://github.com/Azure/azure-sdk-for-python/issues/26331
 
-class HttpLoggingPolicy(SansIOHTTPPolicy):
+    @property
+    def DEFAULT_HEADERS_WHITELIST(cls):
+        return cls.DEFAULT_HEADERS_ALLOWLIST
+
+    @DEFAULT_HEADERS_WHITELIST.setter
+    def DEFAULT_HEADERS_WHITELIST(cls, value):
+        cls.DEFAULT_HEADERS_ALLOWLIST = value
+
+class HttpLoggingPolicy(SansIOHTTPPolicy, metaclass=_HiddenClassProperties):
     """The Pipeline policy that handles logging of HTTP requests and responses.
     """
 
-    DEFAULT_HEADERS_WHITELIST = set([
+    DEFAULT_HEADERS_ALLOWLIST = set([
         "x-ms-request-id",
         "x-ms-client-request-id",
         "x-ms-return-client-request-id",
+        "x-ms-error-code",
         "traceparent",
         "Accept",
         "Cache-Control",
@@ -390,7 +402,7 @@ def __init__(self, logger=None, **kwargs):  # pylint: disable=unused-argument
             "azure.core.pipeline.policies.http_logging_policy"
         )
         self.allowed_query_params = set()
-        self.allowed_header_names = set(self.__class__.DEFAULT_HEADERS_WHITELIST)
+        self.allowed_header_names = set(self.__class__.DEFAULT_HEADERS_ALLOWLIST)
 
     def _redact_query_param(self, key, value):
         lower_case_allowed_query_params = [

diff --git a/sdk/core/azure-core/tests/async_tests/test_pipeline_async.py b/sdk/core/azure-core/tests/async_tests/test_pipeline_async.py
@@ -174,6 +174,7 @@ def test_default_http_logging_policy():
     pipeline = pipeline_client._build_pipeline(config)
     http_logging_policy = pipeline._impl_policies[-1]._policy
     assert http_logging_policy.allowed_header_names == HttpLoggingPolicy.DEFAULT_HEADERS_WHITELIST
+    assert http_logging_policy.allowed_header_names == HttpLoggingPolicy.DEFAULT_HEADERS_ALLOWLIST
 
 def test_pass_in_http_logging_policy():
     config = Configuration()
@@ -187,6 +188,7 @@ def test_pass_in_http_logging_policy():
     pipeline = pipeline_client._build_pipeline(config)
     http_logging_policy = pipeline._impl_policies[-1]._policy
     assert http_logging_policy.allowed_header_names == HttpLoggingPolicy.DEFAULT_HEADERS_WHITELIST.union({"x-ms-added-header"})
+    assert http_logging_policy.allowed_header_names == HttpLoggingPolicy.DEFAULT_HEADERS_ALLOWLIST.union({"x-ms-added-header"})
 
 @pytest.mark.asyncio
 @pytest.mark.parametrize("http_request", HTTP_REQUESTS)

diff --git a/sdk/core/azure-core/tests/test_http_logging_policy.py b/sdk/core/azure-core/tests/test_http_logging_policy.py
@@ -43,6 +43,7 @@ def emit(self, record):
     universal_request = http_request('GET', 'http://localhost/')
     http_response = create_http_response(http_response, universal_request, None)
     http_response.status_code = 202
+    http_response.headers["x-ms-error-code"] = "ERRORCODE"
     request = PipelineRequest(universal_request, PipelineContext(None))
 
     # Basics
@@ -61,6 +62,7 @@ def emit(self, record):
     assert messages_request[3] == 'No body was attached to the request'
     assert messages_response[0] == 'Response status: 202'
     assert messages_response[1] == 'Response headers:'
+    assert messages_response[2] == "    'x-ms-error-code': 'ERRORCODE'"
 
     mock_handler.reset()
 

diff --git a/sdk/core/azure-core/tests/test_pipeline.py b/sdk/core/azure-core/tests/test_pipeline.py
@@ -64,7 +64,11 @@ def test_default_http_logging_policy(http_request):
     pipeline = pipeline_client._build_pipeline(config)
     http_logging_policy = pipeline._impl_policies[-1]._policy
     assert http_logging_policy.allowed_header_names == HttpLoggingPolicy.DEFAULT_HEADERS_WHITELIST
+    assert http_logging_policy.allowed_header_names == HttpLoggingPolicy.DEFAULT_HEADERS_ALLOWLIST
     assert "WWW-Authenticate" in http_logging_policy.allowed_header_names
+    # Testing I can replace the set entirely
+    HttpLoggingPolicy.DEFAULT_HEADERS_ALLOWLIST = set(HttpLoggingPolicy.DEFAULT_HEADERS_ALLOWLIST)
+    HttpLoggingPolicy.DEFAULT_HEADERS_WHITELIST = set(HttpLoggingPolicy.DEFAULT_HEADERS_ALLOWLIST)
 
 @pytest.mark.parametrize("http_request", HTTP_REQUESTS)
 def test_pass_in_http_logging_policy(http_request):
@@ -79,6 +83,7 @@ def test_pass_in_http_logging_policy(http_request):
     pipeline = pipeline_client._build_pipeline(config)
     http_logging_policy = pipeline._impl_policies[-1]._policy
     assert http_logging_policy.allowed_header_names == HttpLoggingPolicy.DEFAULT_HEADERS_WHITELIST.union({"x-ms-added-header"})
+    assert http_logging_policy.allowed_header_names == HttpLoggingPolicy.DEFAULT_HEADERS_ALLOWLIST.union({"x-ms-added-header"})
 
 
 @pytest.mark.parametrize("http_request", HTTP_REQUESTS)

diff --git a/sdk/core/azure-mgmt-core/CHANGELOG.md b/sdk/core/azure-mgmt-core/CHANGELOG.md
@@ -1,5 +1,17 @@
 # Release History
 
+## 1.3.3 (Unreleased)
+
+### Features Added
+
+### Breaking Changes
+
+### Bugs Fixed
+
+### Other Changes
+
+- Rename "DEFAULT_HEADERS_WHITELIST" to "DEFAULT_HEADERS_ALLOWLIST". Added a backward compatible alias.
+
 ## 1.3.2 (2022-08-11)
 
 ### Other Changes

diff --git a/sdk/core/azure-mgmt-core/azure/mgmt/core/_version.py b/sdk/core/azure-mgmt-core/azure/mgmt/core/_version.py
@@ -9,4 +9,4 @@
 # regenerated.
 # --------------------------------------------------------------------------
 
-VERSION = "1.3.2"
+VERSION = "1.3.3"
diff --git a/sdk/core/azure-mgmt-core/azure/mgmt/core/policies/__init__.py b/sdk/core/azure-mgmt-core/azure/mgmt/core/policies/__init__.py
@@ -33,7 +33,7 @@ class ARMHttpLoggingPolicy(HttpLoggingPolicy):
     """HttpLoggingPolicy with ARM specific safe headers fopr loggers.
     """
 
-    DEFAULT_HEADERS_WHITELIST = HttpLoggingPolicy.DEFAULT_HEADERS_WHITELIST | set([
+    DEFAULT_HEADERS_ALLOWLIST = HttpLoggingPolicy.DEFAULT_HEADERS_ALLOWLIST | set([
         # https://docs.microsoft.com/azure/azure-resource-manager/management/request-limits-and-throttling#remaining-requests
         "x-ms-ratelimit-remaining-subscription-reads",
         "x-ms-ratelimit-remaining-subscription-writes",

diff --git a/sdk/core/azure-mgmt-core/setup.py b/sdk/core/azure-mgmt-core/setup.py
@@ -45,15 +45,15 @@
     author_email='[email protected]',
     url='https://github.com/Azure/azure-sdk-for-python/tree/main/sdk/core/azure-mgmt-core',
     classifiers=[
-        "Development Status :: 4 - Beta",
+        "Development Status :: 5 - Production/Stable",
         'Programming Language :: Python',
         'Programming Language :: Python :: 3 :: Only',
         'Programming Language :: Python :: 3',
-        'Programming Language :: Python :: 3.5',
-        'Programming Language :: Python :: 3.6',
         'Programming Language :: Python :: 3.7',
         'Programming Language :: Python :: 3.8',
         'Programming Language :: Python :: 3.9',
+        'Programming Language :: Python :: 3.10',
+        'Programming Language :: Python :: 3.11',
         'License :: OSI Approved :: MIT License',
     ],
     zip_safe=False,
@@ -67,7 +67,7 @@
         'pytyped': ['py.typed'],
     },
     install_requires=[
-        "azure-core<2.0.0,>=1.24.0",
+        "azure-core<2.0.0,>=1.26.2",
     ],
-    python_requires=">=3.6",
+    python_requires=">=3.7",
 )
diff --git a/sdk/core/azure-mgmt-core/tests/test_policies.py b/sdk/core/azure-mgmt-core/tests/test_policies.py
@@ -173,6 +173,7 @@ def test_default_http_logging_policy():
     pipeline_client = ARMPipelineClient(base_url="test", config=config)
     http_logging_policy = pipeline_client._pipeline._impl_policies[-1]._policy
     assert http_logging_policy.allowed_header_names == ARMHttpLoggingPolicy.DEFAULT_HEADERS_WHITELIST
+    assert http_logging_policy.allowed_header_names == ARMHttpLoggingPolicy.DEFAULT_HEADERS_ALLOWLIST
 
 def test_pass_in_http_logging_policy():
     config = Configuration()
@@ -184,4 +185,5 @@ def test_pass_in_http_logging_policy():
 
     pipeline_client = ARMPipelineClient(base_url="test", config=config)
     http_logging_policy = pipeline_client._pipeline._impl_policies[-1]._policy
+    assert http_logging_policy.allowed_header_names == ARMHttpLoggingPolicy.DEFAULT_HEADERS_ALLOWLIST.union({"x-ms-added-header"})
     assert http_logging_policy.allowed_header_names == ARMHttpLoggingPolicy.DEFAULT_HEADERS_WHITELIST.union({"x-ms-added-header"})
diff --git a/shared_requirements.txt b/shared_requirements.txt
@@ -133,7 +133,7 @@ chardet<5,>=3.0.2
 yarl<2.0,>=1.0
 #override azure-core typing-extensions>=4.0.1
 #override azure azure-keyvault~=1.0
-#override azure-mgmt-core azure-core<2.0.0,>=1.24.0
+#override azure-mgmt-core azure-core<2.0.0,>=1.26.2
 #override azure-containerregistry azure-core>=1.24.0,<2.0.0
 #override azure-containerregistry msrest>=0.7.1
 #override azure-core-tracing-opencensus azure-core<2.0.0,>=1.13.0