Skip to content

Commit

Permalink
Fix err 500 on listing /api/v1/namespaces with browsable api enabled (a…
Browse files Browse the repository at this point in the history
…nsible#1915)

* fix 500 on namespaces endpoint
* add custom renderer with disabled forms for non superuser
* add CustomBrowsableAPI to dev, stage and prod
* LegacyRole queryset to LegacyRoleVersionsViewSet
* add test_custom_browsable_format
* add test_v1_role_versions
* test with basic_user
Issue: AAH-2733
  • Loading branch information
jerabekjiri authored Oct 23, 2023
1 parent 739f2ee commit 9823b29
Show file tree
Hide file tree
Showing 7 changed files with 124 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGES/2733.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed server error 500 on ``/api/v1/namespaces`` if browsable api is enabled
1 change: 1 addition & 0 deletions galaxy_ng/app/api/v1/viewsets/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"])
Expand Down
16 changes: 16 additions & 0 deletions galaxy_ng/app/dynaconf_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -29,6 +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(settings))
data.update(configure_password_validators(settings))
data.update(configure_api_base_path(settings))
data.update(configure_legacy_roles(settings))
Expand Down Expand Up @@ -533,6 +535,20 @@ def configure_authentication_backends(settings: Dynaconf) -> Dict[str, Any]:
return data


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]:
"""Set the feature flag for legacy roles from the setting"""
data = {}
Expand Down
11 changes: 11 additions & 0 deletions galaxy_ng/app/renderers.py
Original file line number Diff line number Diff line change
@@ -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 super().show_form_for_method(view, method, request, obj)
return False
5 changes: 5 additions & 0 deletions galaxy_ng/app/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]

Expand Down
41 changes: 41 additions & 0 deletions galaxy_ng/tests/integration/api/test_community.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import pytest
import subprocess

from ansible.errors import AnsibleError

from urllib.parse import urlparse

from ..utils import (
Expand Down Expand Up @@ -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)
50 changes: 49 additions & 1 deletion galaxy_ng/tests/integration/community/test_v1_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

import pytest

from ansible.errors import AnsibleError

from ..utils import (
ansible_galaxy,
get_client,
Expand All @@ -12,7 +14,6 @@
cleanup_social_user,
)


pytestmark = pytest.mark.qa # noqa: F821


Expand Down Expand Up @@ -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 basic user
config = ansible_config("basic_user")
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)

0 comments on commit 9823b29

Please sign in to comment.