Skip to content
Permalink
Browse files

Move Koji tag expansion to validate_from_tag().

This finds the latest builds in a tag in the from_tag validator and adds
them to request.validated. This is necessary because the list of builds
is further used in validate_acls() to decide if a user is allowed to
submit an update for the involved builds.

Signed-off-by: Nils Philippsen <nils@redhat.com>
  • Loading branch information...
nphilipp committed May 17, 2019
1 parent 30b392c commit 311e06557c7aacda4b3c5bcf8d5d94de22034911
Showing with 143 additions and 19 deletions.
  1. +6 −10 bodhi/server/services/updates.py
  2. +34 −5 bodhi/server/validators.py
  3. +103 −4 bodhi/tests/server/test_validators.py
@@ -26,7 +26,7 @@
from sqlalchemy.sql import or_
from requests import RequestException, Timeout as RequestsTimeout

from bodhi.server import buildsys, log, security
from bodhi.server import log, security
from bodhi.server.exceptions import BodhiException, LockedUpdateException
from bodhi.server.models import (
Update,
@@ -434,7 +434,8 @@ def new_update(request):
If the ``from_tag`` parameter is specified and ``builds`` is missing or
empty, the list of builds will be filled with the latest builds in this
Koji tag.
Koji tag. This is done by validate_from_tag() because the list of builds
needs to be available in validate_acls().
Args:
request (pyramid.request): The current request.
@@ -446,17 +447,12 @@ def new_update(request):
# it since the models don't care about a csrf argument.
data.pop('csrf_token')

# Same here, but it can be missing.
data.pop('builds_from_tag', None)

build_nvrs = data.get('builds', [])
from_tag = data.get('from_tag')

if from_tag and not build_nvrs:
# If from_tag is set and build_nvrs (`builds` in the request) is empty,
# fill from latest builds in the submitted Koji tag. This lets users
# submit the builds when editing in order to skip this step.
koji = buildsys.get_session()

build_nvrs = [b['nvr'] for b in koji.listTagged(from_tag, latest=True)]

caveats = []
try:

@@ -55,6 +55,7 @@
taskotron_results,
)
from bodhi.server.config import config
from bodhi.server.exceptions import BodhiException


csrf_error_message = """CSRF tokens do not match. This happens if you have
@@ -1262,7 +1263,10 @@ def validate_request(request, **kwargs):

@postschema_validator
def validate_from_tag(request: pyramid.request.Request, **kwargs: dict):
"""Ensure that from_tag exists and is known and valid as a release tag.
"""Check that an existing from_tag is valid and set `builds`.
Ensure that `from_tag` is a valid Koji tag and set `builds` to latest
builds from this tag if unset.
Args:
request: The current request.
@@ -1271,8 +1275,33 @@ def validate_from_tag(request: pyramid.request.Request, **kwargs: dict):
koji_tag = request.validated.get('from_tag')

if koji_tag:
koji = buildsys.get_session()
koji_client = buildsys.get_session()

if request.validated.get('builds'):
# Builds were specified explicitly, verify that the tag exists in Koji.
if not koji_client.getTag(koji_tag):
request.errors.add('body', 'from_tag', "The supplied from_tag doesn't exist.")
return

# Verify that the tag exists in Koji.
if not koji.getTag(koji_tag):
request.errors.add('body', 'from_tag', "The supplied from_tag doesn't exist.")
# Flag that `builds` wasn't filled from the Koji tag.
request.validated['builds_from_tag'] = False
else:
# Builds weren't specified explicitly, pull the list of latest NVRs here, as it is
# necessary for later validation of ACLs pertaining the respective components.
try:
request.validated['builds'] = [
b['nvr'] for b in koji_client.listTagged(koji_tag, latest=True)
]
except koji.GenericError as e:
if "invalid taginfo" in str(e).lower():
request.errors.add('body', 'from_tag', "The supplied from_tag doesn't exist.")
else:
raise BodhiException("Encountered error while requesting tagged builds from "
f"Koji: '{e}'") from e

if not request.validated['builds']:
request.errors.add('body', 'from_tag',
"The supplied from_tag doesn't contain any builds.")
else:
# Flag that `builds` was filled from the Koji tag.
request.validated['builds_from_tag'] = True
@@ -22,7 +22,9 @@

from cornice.errors import Errors
from fedora_messaging import api, testing as fml_testing
import koji
from pyramid import exceptions
import pytest

from bodhi.tests.server.base import BasePyTestCase, BaseTestCase
from bodhi.server import buildsys, models, validators
@@ -667,6 +669,30 @@ def test_empty_from_tag(self):
class TestValidateFromTag(BasePyTestCase):
"""Test the validate_from_tag() function."""

class UnknownTagDevBuildsys(buildsys.DevBuildsys):

def listTagged(self, tag, *args, **kwargs):
raise koji.GenericError(f"Invalid tagInfo: {tag!r}")

def getTag(self, tag, **kwargs):
return None

class NoBuildsDevBuildsys(buildsys.DevBuildsys):

def listTagged(self, tag, *args, **kwargs):
return []

class UnknownKojiGenericError(buildsys.DevBuildsys):

def listTagged(self, tag, *args, **kwargs):
raise koji.GenericError("foo")

@classmethod
def mock_get_session_for_class(cls, buildsys_cls):
def mock_get_session():
return buildsys_cls()
return mock_get_session

def setup_method(self, method):
"""Sets up the environment for each test method call."""
super().setup_method(method)
@@ -676,14 +702,87 @@ def setup_method(self, method):
self.request.errors = Errors()
self.request.validated = {'from_tag': 'f17-updates-candidate'}

def test_unknown(self):
"""An unknown from_tag should add an error to the request."""
# DevBuildSys acts as if it doesn't know tags starting with 'epel'.
self.request.validated['from_tag'] = 'epel7'
# Successful validations

def test_known_with_builds(self):
"""Test with known from_tag and with builds set in request.validated.
This test expects that builds_from_tag is set to False after calling
the validator."""
self.request.validated['builds'] = ['foo-1-1']

validators.validate_from_tag(self.request)

assert self.request.validated['builds_from_tag'] == False
assert not self.request.errors

def test_known_without_builds(self):
"""Test with known from_tag but without builds in request.validated.
This test expects that builds_from_tag is set to True, and builds are
filled after calling the validator."""
validators.validate_from_tag(self.request)

assert self.request.validated['builds_from_tag'] == True
assert len(self.request.validated['builds'])
assert not self.request.errors

def test_without_from_tag(self):
"""Test without from_tag supplied.
This makes the validator a no-op."""
del self.request.validated['from_tag']

validators.validate_from_tag(self.request)

# Error conditions

def test_known_with_empty_builds(self):
"""Test with known from_tag but with empty builds in request.validated.
This test expects an appropriate error to be added."""
with mock.patch('bodhi.server.validators.buildsys.get_session',
self.mock_get_session_for_class(self.NoBuildsDevBuildsys)):
validators.validate_from_tag(self.request)

assert self.request.errors == [
{'location': 'body', 'name': 'from_tag',
'description': "The supplied from_tag doesn't contain any builds."}
]

def test_unknown_with_builds(self):
"""An unknown from_tag should add an error to the request.
This test runs with a list of fills in request.validated."""
self.request.validated['builds'] = ['foo-1-1']

with mock.patch('bodhi.server.validators.buildsys.get_session',
self.mock_get_session_for_class(self.UnknownTagDevBuildsys)):
validators.validate_from_tag(self.request)

assert self.request.errors == [
{'location': 'body', 'name': 'from_tag',
'description': "The supplied from_tag doesn't exist."}
]

def test_unknown_without_builds(self):
"""An unknown from_tag should add an error to the request.
This test runs without a list of fills in request.validated."""
with mock.patch('bodhi.server.validators.buildsys.get_session',
self.mock_get_session_for_class(self.UnknownTagDevBuildsys)):
validators.validate_from_tag(self.request)

assert self.request.errors == [
{'location': 'body', 'name': 'from_tag',
'description': "The supplied from_tag doesn't exist."}
]

def test_unknown_koji_genericerror(self):
"""An unknown koji.GenericError should be re-raised."""
with pytest.raises(koji.GenericError) as excinfo:
with mock.patch('bodhi.server.validators.buildsys.get_session',
self.mock_get_session_for_class(self.UnknownKojiGenericError)):
validators.validate_from_tag(self.request)

assert str(excinfo.value) == 'foo'

0 comments on commit 311e065

Please sign in to comment.
You can’t perform that action at this time.