From 423a3716d73c8d3c6051fc348a12b08842292557 Mon Sep 17 00:00:00 2001 From: jerabekjiri Date: Tue, 3 Oct 2023 18:05:41 +0200 Subject: [PATCH 01/10] fix 500 on namespaces endpoint Issue: AAH-2733 --- CHANGES/2733.bugfix | 1 + galaxy_ng/app/access_control/access_policy.py | 7 +++++++ 2 files changed, 8 insertions(+) create mode 100644 CHANGES/2733.bugfix diff --git a/CHANGES/2733.bugfix b/CHANGES/2733.bugfix new file mode 100644 index 0000000000..bc6f0f59c2 --- /dev/null +++ b/CHANGES/2733.bugfix @@ -0,0 +1 @@ +Fixed server error 500 on ``/api/v1/namespaces`` if browsable api is enabled diff --git a/galaxy_ng/app/access_control/access_policy.py b/galaxy_ng/app/access_control/access_policy.py index b2d91c4443..9baa584bf2 100644 --- a/galaxy_ng/app/access_control/access_policy.py +++ b/galaxy_ng/app/access_control/access_policy.py @@ -778,6 +778,13 @@ def is_namespace_owner(self, request, viewset, action): if user.is_superuser: return True + # for some reason + # "is_namespace_owner" is called on GET endpoint if browsable api is enabled + # on visiting endpoint "request.method" is "POST", "action" is "create" + # even though request.META["REQUEST_METHOD"] is GET + if request.META["REQUEST_METHOD"] == "GET" and request.accepted_renderer.format == "api": + return True + namespace = None github_user = None kwargs = request.parser_context['kwargs'] From 923325f130d7ed0f3c8497a11f1438c4954054af Mon Sep 17 00:00:00 2001 From: jerabekjiri Date: Wed, 4 Oct 2023 18:56:52 +0200 Subject: [PATCH 02/10] add custom renderer with disabled forms for non superuser Issue: AAH-2733 --- galaxy_ng/app/access_control/access_policy.py | 7 ------- galaxy_ng/app/dynaconf_hooks.py | 11 +++++++++++ galaxy_ng/app/renderers.py | 11 +++++++++++ 3 files changed, 22 insertions(+), 7 deletions(-) create mode 100644 galaxy_ng/app/renderers.py diff --git a/galaxy_ng/app/access_control/access_policy.py b/galaxy_ng/app/access_control/access_policy.py index 9baa584bf2..b2d91c4443 100644 --- a/galaxy_ng/app/access_control/access_policy.py +++ b/galaxy_ng/app/access_control/access_policy.py @@ -778,13 +778,6 @@ def is_namespace_owner(self, request, viewset, action): if user.is_superuser: return True - # for some reason - # "is_namespace_owner" is called on GET endpoint if browsable api is enabled - # on visiting endpoint "request.method" is "POST", "action" is "create" - # even though request.META["REQUEST_METHOD"] is GET - if request.META["REQUEST_METHOD"] == "GET" and request.accepted_renderer.format == "api": - return True - namespace = None github_user = None kwargs = request.parser_context['kwargs'] diff --git a/galaxy_ng/app/dynaconf_hooks.py b/galaxy_ng/app/dynaconf_hooks.py index 5455793cca..0fbb391f6e 100755 --- a/galaxy_ng/app/dynaconf_hooks.py +++ b/galaxy_ng/app/dynaconf_hooks.py @@ -29,6 +29,7 @@ def post(settings: Dynaconf) -> Dict[str, Any]: data.update(configure_cors(settings)) data.update(configure_pulp_ansible(settings)) data.update(configure_authentication_backends(settings)) + data.update(configure_renderers()) data.update(configure_password_validators(settings)) data.update(configure_api_base_path(settings)) data.update(configure_legacy_roles(settings)) @@ -533,6 +534,16 @@ def configure_authentication_backends(settings: Dynaconf) -> Dict[str, Any]: return data +def configure_renderers() -> Dict[str, Any]: + "Set default renderer classes." + return { + "DEFAULT_RENDERER_CLASSES": [ + 'rest_framework.renderers.JSONRenderer', + 'galaxy_ng.app.renderers.CustomBrowsableAPIRenderer' + ] + } + + def configure_legacy_roles(settings: Dynaconf) -> Dict[str, Any]: """Set the feature flag for legacy roles from the setting""" data = {} diff --git a/galaxy_ng/app/renderers.py b/galaxy_ng/app/renderers.py new file mode 100644 index 0000000000..34114c0235 --- /dev/null +++ b/galaxy_ng/app/renderers.py @@ -0,0 +1,11 @@ +from rest_framework.renderers import BrowsableAPIRenderer + + +class CustomBrowsableAPIRenderer(BrowsableAPIRenderer): + """Overrides the standard DRF Browsable API renderer.""" + + def show_form_for_method(self, view, method, request, obj): + """Display forms only for superuser.""" + if request.user.is_superuser: + return True + return False From 9315eb3f58ca47f2962d3bbdba5829e448160871 Mon Sep 17 00:00:00 2001 From: jerabekjiri Date: Wed, 4 Oct 2023 19:55:19 +0200 Subject: [PATCH 03/10] fix env variable name Issue: AAH-2733 --- galaxy_ng/app/dynaconf_hooks.py | 2 +- galaxy_ng/app/renderers.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/galaxy_ng/app/dynaconf_hooks.py b/galaxy_ng/app/dynaconf_hooks.py index 0fbb391f6e..a683062ca5 100755 --- a/galaxy_ng/app/dynaconf_hooks.py +++ b/galaxy_ng/app/dynaconf_hooks.py @@ -537,7 +537,7 @@ def configure_authentication_backends(settings: Dynaconf) -> Dict[str, Any]: def configure_renderers() -> Dict[str, Any]: "Set default renderer classes." return { - "DEFAULT_RENDERER_CLASSES": [ + "REST_FRAMEWORK__DEFAULT_RENDERER_CLASSES": [ 'rest_framework.renderers.JSONRenderer', 'galaxy_ng.app.renderers.CustomBrowsableAPIRenderer' ] diff --git a/galaxy_ng/app/renderers.py b/galaxy_ng/app/renderers.py index 34114c0235..be85e000b3 100644 --- a/galaxy_ng/app/renderers.py +++ b/galaxy_ng/app/renderers.py @@ -7,5 +7,5 @@ class CustomBrowsableAPIRenderer(BrowsableAPIRenderer): def show_form_for_method(self, view, method, request, obj): """Display forms only for superuser.""" if request.user.is_superuser: - return True + return super().show_form_for_method(view, method, request, obj) return False From 301737e2d2f81f185c779132f4b3c5940543a3f2 Mon Sep 17 00:00:00 2001 From: jerabekjiri Date: Wed, 11 Oct 2023 15:48:59 +0200 Subject: [PATCH 04/10] add CustomBrowsableAPI to dev, stage and prod Issue: AAH-2733 --- docker/etc/settings.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docker/etc/settings.py b/docker/etc/settings.py index 725ae47a4f..fff30389ff 100644 --- a/docker/etc/settings.py +++ b/docker/etc/settings.py @@ -2,6 +2,7 @@ # main galaxy_ng/app/settings.py file. # Split the configuration if necessary. import os +import re DEFAULT_FILE_STORAGE = 'storages.backends.s3boto3.S3Boto3Storage' @@ -41,6 +42,12 @@ REST_FRAMEWORK__DEFAULT_RENDERER_CLASSES = ['rest_framework.renderers.JSONRenderer'] +# add CustomBrowsableAPI only for community (galaxy.ansible.com, galaxy-stage, galaxy-dev)" +if re.search(r'galaxy(-dev|-stage)*.ansible.com', os.environ.get('PULP_CONTENT_ORIGIN', "")): + REST_FRAMEWORK__DEFAULT_RENDERER_CLASSES.append( + 'galaxy_ng.app.renderers.CustomBrowsableAPIRenderer' + ) + _enabled_handlers = ['console'] _extra_handlers = {} From a2d102f92afebd91ebacf91211965d97383a85aa Mon Sep 17 00:00:00 2001 From: jerabekjiri Date: Mon, 16 Oct 2023 13:43:01 +0200 Subject: [PATCH 05/10] invert settings Issue: AAH-2733 --- docker/etc/settings.py | 7 ------- galaxy_ng/app/dynaconf_hooks.py | 23 ++++++++++++++--------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/docker/etc/settings.py b/docker/etc/settings.py index fff30389ff..725ae47a4f 100644 --- a/docker/etc/settings.py +++ b/docker/etc/settings.py @@ -2,7 +2,6 @@ # main galaxy_ng/app/settings.py file. # Split the configuration if necessary. import os -import re DEFAULT_FILE_STORAGE = 'storages.backends.s3boto3.S3Boto3Storage' @@ -42,12 +41,6 @@ REST_FRAMEWORK__DEFAULT_RENDERER_CLASSES = ['rest_framework.renderers.JSONRenderer'] -# add CustomBrowsableAPI only for community (galaxy.ansible.com, galaxy-stage, galaxy-dev)" -if re.search(r'galaxy(-dev|-stage)*.ansible.com', os.environ.get('PULP_CONTENT_ORIGIN', "")): - REST_FRAMEWORK__DEFAULT_RENDERER_CLASSES.append( - 'galaxy_ng.app.renderers.CustomBrowsableAPIRenderer' - ) - _enabled_handlers = ['console'] _extra_handlers = {} diff --git a/galaxy_ng/app/dynaconf_hooks.py b/galaxy_ng/app/dynaconf_hooks.py index a683062ca5..abf92c87db 100755 --- a/galaxy_ng/app/dynaconf_hooks.py +++ b/galaxy_ng/app/dynaconf_hooks.py @@ -2,6 +2,7 @@ import ldap import pkg_resources import os +import re from typing import Any, Dict, List from django_auth_ldap.config import LDAPSearch from dynaconf import Dynaconf, Validator @@ -29,7 +30,7 @@ def post(settings: Dynaconf) -> Dict[str, Any]: data.update(configure_cors(settings)) data.update(configure_pulp_ansible(settings)) data.update(configure_authentication_backends(settings)) - data.update(configure_renderers()) + data.update(configure_renderers(settings)) data.update(configure_password_validators(settings)) data.update(configure_api_base_path(settings)) data.update(configure_legacy_roles(settings)) @@ -534,14 +535,18 @@ def configure_authentication_backends(settings: Dynaconf) -> Dict[str, Any]: return data -def configure_renderers() -> Dict[str, Any]: - "Set default renderer classes." - return { - "REST_FRAMEWORK__DEFAULT_RENDERER_CLASSES": [ - 'rest_framework.renderers.JSONRenderer', - 'galaxy_ng.app.renderers.CustomBrowsableAPIRenderer' - ] - } +def configure_renderers(settings) -> Dict[str, Any]: + """ + Add CustomBrowsableAPI only for community (galaxy.ansible.com, galaxy-stage, galaxy-dev)" + """ + if re.search( + r'galaxy(-dev|-stage)*.ansible.com', settings.get('CONTENT_ORIGIN', "") + ): + value = settings.get("REST_FRAMEWORK__DEFAULT_RENDERER_CLASSES", []) + value.append('galaxy_ng.app.renderers.CustomBrowsableAPIRenderer') + return {"REST_FRAMEWORK__DEFAULT_RENDERER_CLASSES": value} + + return {} def configure_legacy_roles(settings: Dynaconf) -> Dict[str, Any]: From 1b256b61d030bd146e7c6f0d45914e4eba38e6cf Mon Sep 17 00:00:00 2001 From: jerabekjiri Date: Wed, 18 Oct 2023 21:13:36 +0200 Subject: [PATCH 06/10] initial settings for dev env Issue: AAH-2733 --- galaxy_ng/app/settings.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/galaxy_ng/app/settings.py b/galaxy_ng/app/settings.py index 1afaf24895..533ac75801 100644 --- a/galaxy_ng/app/settings.py +++ b/galaxy_ng/app/settings.py @@ -64,6 +64,11 @@ "galaxy_ng.app.access_control.access_policy.AccessPolicyBase", ) +REST_FRAMEWORK__DEFAULT_RENDERER_CLASSES = [ + 'rest_framework.renderers.JSONRenderer', + 'galaxy_ng.app.renderers.CustomBrowsableAPIRenderer' +] + # Settings for insights mode # GALAXY_AUTHENTICATION_CLASSES = ["galaxy_ng.app.auth.auth.RHIdentityAuthentication"] From 812ef70cb8aad1f23b36d6eedeb4be751d98d995 Mon Sep 17 00:00:00 2001 From: jerabekjiri Date: Wed, 18 Oct 2023 23:01:47 +0200 Subject: [PATCH 07/10] LegacyRole queryset to LegacyRoleVersionsViewSet Issue: AAH-2733 --- galaxy_ng/app/api/v1/viewsets/roles.py | 1 + 1 file changed, 1 insertion(+) diff --git a/galaxy_ng/app/api/v1/viewsets/roles.py b/galaxy_ng/app/api/v1/viewsets/roles.py index 09aa914104..0992f593ee 100644 --- a/galaxy_ng/app/api/v1/viewsets/roles.py +++ b/galaxy_ng/app/api/v1/viewsets/roles.py @@ -159,6 +159,7 @@ class LegacyRoleVersionsViewSet(viewsets.GenericViewSet, mixins.RetrieveModelMix permission_classes = [LegacyAccessPolicy] authentication_classes = GALAXY_AUTHENTICATION_CLASSES serializer_class = LegacyRoleVersionsSerializer + queryset = LegacyRole.objects.all() def get_object(self): return get_object_or_404(LegacyRole, id=self.kwargs["pk"]) From 0c59a97990fee7fbe528e3457815af946dbc6eb1 Mon Sep 17 00:00:00 2001 From: jerabekjiri Date: Wed, 18 Oct 2023 23:49:54 +0200 Subject: [PATCH 08/10] add test_custom_browsable_format Issue: AAH-2733 --- .../integration/community/test_v1_api.py | 50 ++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/galaxy_ng/tests/integration/community/test_v1_api.py b/galaxy_ng/tests/integration/community/test_v1_api.py index 50a38b9b9b..c6e69691f5 100644 --- a/galaxy_ng/tests/integration/community/test_v1_api.py +++ b/galaxy_ng/tests/integration/community/test_v1_api.py @@ -3,6 +3,8 @@ import pytest +from ansible.errors import AnsibleError + from ..utils import ( ansible_galaxy, get_client, @@ -12,7 +14,6 @@ cleanup_social_user, ) - pytestmark = pytest.mark.qa # noqa: F821 @@ -118,3 +119,50 @@ def test_v1_users_filter(ansible_config): assert resp["count"] == 0 cleanup_social_user(github_user, ansible_config) + + +@pytest.mark.deployment_community +def test_custom_browsable_format(ansible_config): + """" Test endpoints works with enabled browsable api """ + + # test as a admin + config = ansible_config("admin") + api_client = get_client( + config=config, + request_token=False, + require_auth=True, + ) + + resp = api_client("v1/namespaces") + assert isinstance(resp, dict) + assert "results" in resp + + resp = api_client("v1/namespaces?format=json") + assert isinstance(resp, dict) + assert "results" in resp + + with pytest.raises(AnsibleError) as html: + api_client("v1/namespaces", headers={"Accept": "text/html"}) + assert not isinstance(html.value, dict) + assert "results" in str(html.value) + + # test as a github user + config = ansible_config("github_user_1") + api_client = get_client( + config=config, + request_token=False, + require_auth=True, + ) + + resp = api_client("v1/namespaces") + assert isinstance(resp, dict) + assert "results" in resp + + resp = api_client("v1/namespaces?format=json") + assert isinstance(resp, dict) + assert "results" in resp + + with pytest.raises(AnsibleError) as html: + api_client("v1/namespaces", headers={"Accept": "text/html"}) + assert not isinstance(html.value, dict) + assert "results" in str(html.value) From 72d040cc3e01719d06c3457da3b80134419658d1 Mon Sep 17 00:00:00 2001 From: jerabekjiri Date: Thu, 19 Oct 2023 00:25:12 +0200 Subject: [PATCH 09/10] add test_v1_role_versions Issue: AAH-2733 --- .../tests/integration/api/test_community.py | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/galaxy_ng/tests/integration/api/test_community.py b/galaxy_ng/tests/integration/api/test_community.py index 9aceb58f3c..6b80625400 100644 --- a/galaxy_ng/tests/integration/api/test_community.py +++ b/galaxy_ng/tests/integration/api/test_community.py @@ -5,6 +5,8 @@ import pytest import subprocess +from ansible.errors import AnsibleError + from urllib.parse import urlparse from ..utils import ( @@ -768,3 +770,42 @@ def test_legacy_roles_ordering(ansible_config): resp = api_client('/api/v1/roles/?order_by=-name') sorted_names = [r["name"] for r in resp["results"]] assert sorted_names == sorted(names, reverse=True) + + +@pytest.mark.deployment_community +def test_v1_role_versions(ansible_config): + """ + Test that role versions endpoint doesn't fail on missing queryset + with enabled browsable api format. + """ + + config = ansible_config("admin") + api_client = get_client( + config=config, + request_token=False, + require_auth=True + ) + + # clean all roles + clean_all_roles(ansible_config) + + # start the sync + pargs = json.dumps({"github_user": "geerlingguy", "role_name": "ansible"}).encode('utf-8') + resp = api_client('/api/v1/sync/', method='POST', args=pargs) + assert isinstance(resp, dict) + assert resp.get('task') is not None + wait_for_v1_task(resp=resp, api_client=api_client) + + resp = api_client('/api/v1/roles/?username=geerlingguy&name=ansible') + assert resp['count'] == 1 + + id = resp["results"][0]["id"] + versions = resp["results"][0]["summary_fields"]["versions"] + + resp = api_client(f'/api/v1/roles/{id}/versions') + assert len(versions) == resp["count"] + + with pytest.raises(AnsibleError) as html: + api_client(f"v1/roles/{id}/versions", headers={"Accept": "text/html"}) + assert not isinstance(html.value, dict) + assert "results" in str(html.value) From 29e720a7cc027e94b9e268124a29f6c458c0675b Mon Sep 17 00:00:00 2001 From: jerabekjiri Date: Thu, 19 Oct 2023 13:46:37 +0200 Subject: [PATCH 10/10] test with basic_user Issue: AAH-2733 --- galaxy_ng/tests/integration/community/test_v1_api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/galaxy_ng/tests/integration/community/test_v1_api.py b/galaxy_ng/tests/integration/community/test_v1_api.py index c6e69691f5..dac5e2c5d9 100644 --- a/galaxy_ng/tests/integration/community/test_v1_api.py +++ b/galaxy_ng/tests/integration/community/test_v1_api.py @@ -146,8 +146,8 @@ def test_custom_browsable_format(ansible_config): assert not isinstance(html.value, dict) assert "results" in str(html.value) - # test as a github user - config = ansible_config("github_user_1") + # test as a basic user + config = ansible_config("basic_user") api_client = get_client( config=config, request_token=False,