This is an automated email from the ASF dual-hosted git repository. smolnar82 pushed a commit to branch knox_idf in repository https://gitbox.apache.org/repos/asf/knox.git
commit f82dcf75224e65d518c0b9d7549b3c3f46e1d1fd Author: Raghav Maheshwari <[email protected]> AuthorDate: Fri Jun 12 20:43:31 2026 +0530 KNOX-3335: Add pylint pre-push linting for .github/workflows/tests Python integration tests (#1260) (cherry picked from commit 10eedcc34a8bfd8d09c946ad95f6801d9ada6c35) --- .github/workflows/compose/docker-compose.yml | 9 ++-- .github/workflows/tests/common_utils.py | 8 +++- .github/workflows/tests/requirements.txt | 1 + .github/workflows/tests/test_health.py | 13 +++--- ..._LDAP.py => test_knox_auth_service_and_ldap.py} | 49 +++++++++++++--------- .github/workflows/tests/test_knox_configs.py | 18 ++++---- .../tests/test_knoxauth_preauth_and_paths.py | 4 +- .github/workflows/tests/test_remote_auth.py | 19 ++++----- .../test_remoteauth_extauthz_additional_path.py | 11 ++++- 9 files changed, 80 insertions(+), 52 deletions(-) diff --git a/.github/workflows/compose/docker-compose.yml b/.github/workflows/compose/docker-compose.yml index e03e90e5a..3364c7f08 100644 --- a/.github/workflows/compose/docker-compose.yml +++ b/.github/workflows/compose/docker-compose.yml @@ -37,16 +37,17 @@ services: - ldap tests: - image: python:3.9-slim + image: python:3.10-slim working_dir: /tests volumes: - ../tests:/tests environment: - KNOX_GATEWAY_URL=https://knox:8443/ command: > - bash -c "pip install -r requirements.txt - && echo 'Waiting for knox...' - && sleep 30 + bash -c "pip install -r requirements.txt + && pylint *.py + && echo 'Waiting for knox...' + && sleep 30 && pytest --junitxml=test-results.xml" depends_on: - knox diff --git a/.github/workflows/tests/common_utils.py b/.github/workflows/tests/common_utils.py index 0b544b4e9..e80193374 100644 --- a/.github/workflows/tests/common_utils.py +++ b/.github/workflows/tests/common_utils.py @@ -13,6 +13,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +"""Shared helpers for Knox gateway integration tests.""" + from __future__ import annotations import base64 @@ -56,7 +58,10 @@ def knox_post(url: str, **kwargs: Any) -> requests.Response: def collect_actor_group_values( response: requests.Response, prefix: str = "x-knox-actor-groups" ) -> list[str]: - """Comma-split values from all response headers whose names start with prefix (case-insensitive).""" + """Comma-split values from response headers whose names start with prefix. + + Matching is case-insensitive. + """ prefix_lower = prefix.lower() all_groups: list[str] = [] for name in response.headers: @@ -66,6 +71,7 @@ def collect_actor_group_values( def assert_hsts_header(testcase: unittest.TestCase, response: requests.Response) -> None: + """Assert the response includes the expected Strict-Transport-Security header.""" testcase.assertIn(HSTS_HEADER_NAME, response.headers) testcase.assertEqual(response.headers[HSTS_HEADER_NAME], HSTS_EXPECTED_VALUE) diff --git a/.github/workflows/tests/requirements.txt b/.github/workflows/tests/requirements.txt index 9967c535d..9f223fca6 100644 --- a/.github/workflows/tests/requirements.txt +++ b/.github/workflows/tests/requirements.txt @@ -1,2 +1,3 @@ requests==2.32.4 pytest==8.3.4 +pylint==4.0.5 \ No newline at end of file diff --git a/.github/workflows/tests/test_health.py b/.github/workflows/tests/test_health.py index a62e2f2bd..cb99e3495 100644 --- a/.github/workflows/tests/test_health.py +++ b/.github/workflows/tests/test_health.py @@ -12,6 +12,9 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + +"""Integration tests for the Knox gateway health REST API.""" + import json import unittest @@ -44,11 +47,11 @@ class TestKnoxHealth(unittest.TestCase): assert_hsts_header(self, response) except requests.exceptions.ConnectionError: self.fail("Failed to connect to Knox on port 8443 - Connection refused") - except Exception as e: - self.fail(f"Health check failed with unexpected error: {e}") + except requests.exceptions.RequestException as exc: + self.fail(f"Health check failed with unexpected error: {exc}") def test_health_metrics_returns_json(self): - """Metrics with pretty=true returns 200 and a JSON object with application/json content type.""" + """Metrics with pretty=true returns 200 and JSON with application/json type.""" url = self.base_url + "gateway/health/v1/metrics?pretty=true" response = knox_get(url) self.assertEqual(response.status_code, 200) @@ -71,7 +74,7 @@ class TestKnoxHealth(unittest.TestCase): ) def test_health_metrics_without_pretty_returns_json(self): - """Metrics without pretty still returns 200, parseable JSON, and the same top-level keys as pretty.""" + """Metrics without pretty returns 200, parseable JSON, and the same top-level keys.""" url = self.base_url + "gateway/health/v1/metrics" response = knox_get(url) self.assertEqual(response.status_code, 200) @@ -92,6 +95,6 @@ class TestKnoxHealth(unittest.TestCase): content_type = response.headers.get("Content-Type", "") self.assertIn("text/plain", content_type) + if __name__ == '__main__': unittest.main() - diff --git a/.github/workflows/tests/test_knox_auth_service_and_LDAP.py b/.github/workflows/tests/test_knox_auth_service_and_ldap.py similarity index 81% rename from .github/workflows/tests/test_knox_auth_service_and_LDAP.py rename to .github/workflows/tests/test_knox_auth_service_and_ldap.py index 575d63a92..3f3ccea6e 100644 --- a/.github/workflows/tests/test_knox_auth_service_and_LDAP.py +++ b/.github/workflows/tests/test_knox_auth_service_and_ldap.py @@ -12,22 +12,19 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + +"""Integration tests for Knox Auth Service with LDAP authentication.""" + import unittest + from requests.auth import HTTPBasicAuth from common_utils import collect_actor_group_values, gateway_base_url, knox_get -######################################################## -# This test is verifying the behavior of the Knox Auth Service + LDAP authentication. -# It is using the 'auth/api/v1/pre' endpoint to get the actor ID and group headers. -# It is using the 'guest' user to get the guest user headers. -# It is using the 'admin' user to get the admin user headers. -# It is verifying that the actor ID and group headers are correct. -# It is verifying that the actor ID and group headers are not empty. -# It is verifying that the actor ID and group headers are not None. -######################################################## class TestKnoxAuthService(unittest.TestCase): + """Verify actor ID and group headers from the knoxldap preauth endpoint.""" + def setUp(self): self.base_url = gateway_base_url() # The topology name is based on the filename knoxldap.xml @@ -42,12 +39,13 @@ class TestKnoxAuthService(unittest.TestCase): self.topology_url, auth=HTTPBasicAuth('guest', 'guest-password'), ) - + print(f"Status Code: {response.status_code}") self.assertEqual(response.status_code, 200) - + # Check for Actor ID header - # The config in knoxldap.xml sets 'preauth.auth.header.actor.id.name' to 'x-knox-actor-username' + # The config in knoxldap.xml sets 'preauth.auth.header.actor.id.name' + # to 'x-knox-actor-username' actor_id_header = 'x-knox-actor-username' self.assertIn(actor_id_header, response.headers) self.assertEqual(response.headers[actor_id_header], 'guest') @@ -56,7 +54,11 @@ class TestKnoxAuthService(unittest.TestCase): # Check for Actor Group header - should be empty for guest prefix = 'x-knox-actor-groups' all_groups = collect_actor_group_values(response, prefix=prefix) - self.assertEqual(len(all_groups), 0, f"Guest user should not have any group headers starting with {prefix}") + self.assertEqual( + len(all_groups), + 0, + f"Guest user should not have any group headers starting with {prefix}", + ) def test_auth_service_admin_groups(self): """ @@ -67,26 +69,32 @@ class TestKnoxAuthService(unittest.TestCase): self.topology_url, auth=HTTPBasicAuth('admin', 'admin-password'), ) - + print(f"Status Code: {response.status_code}") self.assertEqual(response.status_code, 200) - + # Check for Actor ID header actor_id_header = 'x-knox-actor-username' self.assertIn(actor_id_header, response.headers) self.assertEqual(response.headers[actor_id_header], 'admin') print(f"Verified {actor_id_header}: {response.headers[actor_id_header]}") - + # Config: 'preauth.auth.header.actor.groups.prefix' = 'x-knox-actor-groups' # We mapped admin to 'longGroupName1,longGroupName2,longGroupName3,longGroupName4' prefix = 'x-knox-actor-groups' all_groups = collect_actor_group_values(response, prefix=prefix) self.assertTrue(len(all_groups) > 0, f"No headers found starting with {prefix}") - for h in response.headers: - if h.lower().startswith(prefix.lower()): - print(f"Found group header {h}: {response.headers[h]}") + for header_name in response.headers: + if header_name.lower().startswith(prefix.lower()): + print(f"Found group header {header_name}: {response.headers[header_name]}") - expected_groups = ['admin', 'longGroupName1', 'longGroupName2', 'longGroupName3', 'longGroupName4'] + expected_groups = [ + 'admin', + 'longGroupName1', + 'longGroupName2', + 'longGroupName3', + 'longGroupName4', + ] for group in expected_groups: self.assertIn(group, all_groups) @@ -119,5 +127,6 @@ class TestKnoxAuthService(unittest.TestCase): self.assertIn(group, all_groups) print(f"Verified recursive groups: {all_groups}") + if __name__ == '__main__': unittest.main() diff --git a/.github/workflows/tests/test_knox_configs.py b/.github/workflows/tests/test_knox_configs.py index 0a9b5e78c..752b1dc5d 100644 --- a/.github/workflows/tests/test_knox_configs.py +++ b/.github/workflows/tests/test_knox_configs.py @@ -12,19 +12,19 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + +"""Integration tests for global Knox gateway configuration.""" + import unittest + from requests.auth import HTTPBasicAuth from common_utils import assert_hsts_header, gateway_base_url, knox_get -######################################################## -# This test is verifying the global HSTS headers for 404 response. -# It executes new GET request on non-existent Knox path -# It verifies header is present with the correct value. -######################################################## - class TestKnoxConfigs(unittest.TestCase): + """Verify global gateway settings such as HSTS on error responses.""" + def setUp(self): self.base_url = gateway_base_url() self.non_existent_path = self.base_url + "gateway/not-exists" @@ -33,7 +33,7 @@ class TestKnoxConfigs(unittest.TestCase): """ Verifies header is present with the correct value """ - print(f"\nTesting global HSTS config for 404 response") + print("\nTesting global HSTS config for 404 response") response = knox_get( self.non_existent_path, auth=HTTPBasicAuth('admin', 'admin-password'), @@ -43,5 +43,5 @@ class TestKnoxConfigs(unittest.TestCase): self.assertEqual(response.status_code, 404) assert_hsts_header(self, response) - print(f"Verified Strict-Transport-Security: {response.headers['Strict-Transport-Security']}") - + hsts_value = response.headers['Strict-Transport-Security'] + print(f"Verified Strict-Transport-Security: {hsts_value}") diff --git a/.github/workflows/tests/test_knoxauth_preauth_and_paths.py b/.github/workflows/tests/test_knoxauth_preauth_and_paths.py index 45d51a5c8..28a3f0940 100644 --- a/.github/workflows/tests/test_knoxauth_preauth_and_paths.py +++ b/.github/workflows/tests/test_knoxauth_preauth_and_paths.py @@ -12,6 +12,9 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + +"""Integration tests for KnoxLDAP preauth and extauthz path handling.""" + import unittest from requests.auth import HTTPBasicAuth @@ -84,4 +87,3 @@ class TestKnoxAuthServicePreAuthAndPaths(unittest.TestCase): if __name__ == "__main__": unittest.main() - diff --git a/.github/workflows/tests/test_remote_auth.py b/.github/workflows/tests/test_remote_auth.py index ae87c5393..8e3120ff5 100644 --- a/.github/workflows/tests/test_remote_auth.py +++ b/.github/workflows/tests/test_remote_auth.py @@ -12,21 +12,19 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + +"""Integration tests for the RemoteAuthProvider topology.""" + import unittest + from requests.auth import HTTPBasicAuth from common_utils import collect_actor_group_values, gateway_base_url, knox_get -######################################################## -# This test is verifying the behavior of the RemoteAuthProvider. -# It is using the 'auth/api/v1/pre' endpoint to get the actor ID and group headers. -# It is using the 'guest' user to get the guest user headers. -# It is using the 'admin' user to get the admin user headers. -# It is verifying that the actor ID and group headers are correct. -# It is verifying that the actor ID and group headers are not empty. -# It is verifying that the actor ID and group headers are not None. -######################################################## + class TestRemoteAuth(unittest.TestCase): + """Verify RemoteAuthProvider actor ID and group headers via the pre endpoint.""" + def setUp(self): self.base_url = gateway_base_url() self.topology_url = self.base_url + "gateway/remoteauth/auth/api/v1/pre" @@ -67,7 +65,7 @@ class TestRemoteAuth(unittest.TestCase): # knoxldap maps admin to: longGroupName1,longGroupName2,longGroupName3,longGroupName4 # RemoteAuthFilter picks these up from x-knox-actor-groups-* # And KNOX-AUTH-SERVICE echoes them back in X-Knox-Actor-Groups-* - + all_groups = collect_actor_group_values(response) print(f"Found groups: {all_groups}") @@ -87,5 +85,6 @@ class TestRemoteAuth(unittest.TestCase): # When remote auth fails (knoxldap returns 401), RemoteAuthFilter should return 401 self.assertEqual(response.status_code, 401) + if __name__ == '__main__': unittest.main() diff --git a/.github/workflows/tests/test_remoteauth_extauthz_additional_path.py b/.github/workflows/tests/test_remoteauth_extauthz_additional_path.py index 069dbc166..1abeb4233 100644 --- a/.github/workflows/tests/test_remoteauth_extauthz_additional_path.py +++ b/.github/workflows/tests/test_remoteauth_extauthz_additional_path.py @@ -12,6 +12,9 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + +"""Integration tests for RemoteAuth extauthz path handling.""" + import unittest from requests.auth import HTTPBasicAuth @@ -20,11 +23,14 @@ from common_utils import gateway_base_url, knox_get class TestRemoteAuthExtAuthzAdditionalPath(unittest.TestCase): + """Verify RemoteAuth extauthz success, path handling, and auth failures.""" + def setUp(self): self.base_url = gateway_base_url() self.extauthz_url = self.base_url + "gateway/remoteauth/auth/api/v1/extauthz" def test_extauthz_success(self): + """Valid credentials on extauthz return 200 and X-Knox-Actor-ID.""" response = knox_get( self.extauthz_url, auth=HTTPBasicAuth("guest", "guest-password"), @@ -34,6 +40,7 @@ class TestRemoteAuthExtAuthzAdditionalPath(unittest.TestCase): self.assertEqual(response.headers["X-Knox-Actor-ID"], "guest") def test_extauthz_additional_path_is_ignored(self): + """Extra path segments under extauthz still authenticate successfully.""" response = knox_get( self.extauthz_url + "/some/extra/path", auth=HTTPBasicAuth("guest", "guest-password"), @@ -43,6 +50,7 @@ class TestRemoteAuthExtAuthzAdditionalPath(unittest.TestCase): self.assertEqual(response.headers["X-Knox-Actor-ID"], "guest") def test_extauthz_bad_credentials_unauthorized(self): + """Invalid credentials on extauthz return 401.""" response = knox_get( self.extauthz_url, auth=HTTPBasicAuth("baduser", "badpass"), @@ -50,11 +58,10 @@ class TestRemoteAuthExtAuthzAdditionalPath(unittest.TestCase): self.assertEqual(response.status_code, 401) def test_extauthz_missing_credentials(self): - # No Authorization header: 401 (no longer hit the NPE because of missing cache key from the header). + """Missing Authorization header on extauthz returns 401.""" response = knox_get(self.extauthz_url) self.assertEqual(response.status_code, 401) if __name__ == "__main__": unittest.main() -
