Discussion: Using copr for building SCLs
by Bohuslav Kabrda
Hi list,
I'd like to start yet another discussion about some functionality of copr, here it is:
One of nice use cases for copr would be building software collections [1]. These however require modified mock chroot (modified config_opts['root'] and config_opts['chroot_setup_cmd']).
I'm trying to think of a way how to do this:
- We would need to allow users to create their own chroots (or more precisely allow them to do certain modifications).
- We would need to be able to send these chroots from frontend to backend.
- We would need some mechanism for sane naming, so that we can distinguish between builds of collections for different releases and architectures. E.g. ruby193-x86_64 is not enough, we need to also store the information about os name (epel/fedora) and os version (epel 6, fedora 18, ...).
My proposal:
- Use chroot names like ruby193_fedora-18-x86_64.
- Allow users to create these chroots by modifying config_opts['chroot_setup_cmd'] (config_opts['root'] would be autogenerated from the chroot name).
- Store the modified information in the MockChroot table and create these on backend using communication channel proposed in [2] (parts of it already implemented in frontend).
- Allow selecting these on copr creation/modification as more options for mock chroots, by default hidden under some kind of fancy clickable JS dropdown.
Thoughts?
--
Regards,
Bohuslav "Slavek" Kabrda.
[1] https://fedorahosted.org/SoftwareCollections/
[2] https://fedorahosted.org/copr/ticket/8
10 years, 3 months
[copr] master: Enable raising legal flag on a copr. Fixes #46 - now make it show up in admin interface. (2aab1bf)
by bkabrda@fedoraproject.org
Repository : http://git.fedorahosted.org/cgit/copr.git
On branch : master
>---------------------------------------------------------------
commit 2aab1bfaaa82ade6a7fcbaf621dab209371eae90
Author: Bohuslav Kabrda <bkabrda(a)redhat.com>
Date: Fri Mar 29 14:14:52 2013 +0100
Enable raising legal flag on a copr. Fixes #46 - now make it show up in admin interface.
>---------------------------------------------------------------
coprs_frontend/coprs/forms.py | 2 ++
coprs_frontend/coprs/helpers.py | 2 +-
coprs_frontend/coprs/static/copr.css | 10 ++++++++++
.../coprs/templates/coprs/_coprs_forms.html | 14 ++++++++++++++
.../coprs/templates/coprs/detail/overview.html | 6 ++++++
.../coprs/views/coprs_ns/coprs_general.py | 19 ++++++++++++++++---
.../tests/test_logic/test_coprs_logic.py | 10 +++++++++-
7 files changed, 58 insertions(+), 5 deletions(-)
diff --git a/coprs_frontend/coprs/forms.py b/coprs_frontend/coprs/forms.py
index 7c7cf35..6b533cd 100644
--- a/coprs_frontend/coprs/forms.py
+++ b/coprs_frontend/coprs/forms.py
@@ -138,6 +138,8 @@ class BuildForm(wtf.Form):
validators = [wtf.NumberRange(min = constants.MIN_BUILD_TIMEOUT, max = constants.MAX_BUILD_TIMEOUT)],
default = constants.DEFAULT_BUILD_TIMEOUT)
+class CoprLegalForm(wtf.Form):
+ comment = wtf.TextAreaField('Comment')
class PermissionsApplierFormFactory(object):
@staticmethod
diff --git a/coprs_frontend/coprs/helpers.py b/coprs_frontend/coprs/helpers.py
index fdc87af..cad7324 100644
--- a/coprs_frontend/coprs/helpers.py
+++ b/coprs_frontend/coprs/helpers.py
@@ -37,7 +37,7 @@ class PermissionEnum(object):
class ActionTypeEnum(object):
__metaclass__ = EnumType
- vals = {'delete': 0, 'rename': 1}
+ vals = {'delete': 0, 'rename': 1, 'legal-flag': 2}
class BackendResultEnum(object):
__metaclass__ = EnumType
diff --git a/coprs_frontend/coprs/static/copr.css b/coprs_frontend/coprs/static/copr.css
index a2349a7..243b676 100644
--- a/coprs_frontend/coprs/static/copr.css
+++ b/coprs_frontend/coprs/static/copr.css
@@ -311,3 +311,13 @@ table.releases th.rightmost {
table.releases tr.release-end {
border-bottom: 3px solid #f2f2f2;
}
+
+form.legal-flag, form.legal-flag input {
+ color: #888888;
+ margin-top: 5px;
+}
+
+hr {
+ margin-top: 25px;
+ margin-bottom: 25px;
+}
diff --git a/coprs_frontend/coprs/templates/coprs/_coprs_forms.html b/coprs_frontend/coprs/templates/coprs/_coprs_forms.html
index a0f55e3..e8856e1 100644
--- a/coprs_frontend/coprs/templates/coprs/_coprs_forms.html
+++ b/coprs_frontend/coprs/templates/coprs/_coprs_forms.html
@@ -75,4 +75,18 @@
<dd><input type="submit" value="Save"></dd>
</form>
{% endif %}
+ {% endmacro %}
+
+{% macro copr_legal_form(form, copr) %}
+<form class="legal-flag" method="post" action={{ url_for('coprs_ns.copr_legal_flag', username=copr.owner.name, coprname=copr.name) }}>
+ If you think that this copr may be violating our policies, you can mark it for legal review.
+ You can add an optional comment describing what you think is wrong here.
+
+ <dl>
+ {{ render_field(form.comment, rows=5, cols=50) }}
+ <dt>
+ <input type="submit" value="Raise Legal Flag">
+ </dt>
+ </dl>
+ </form>
{% endmacro %}
diff --git a/coprs_frontend/coprs/templates/coprs/detail/overview.html b/coprs_frontend/coprs/templates/coprs/detail/overview.html
index 6152ddf..b0bf4e2 100644
--- a/coprs_frontend/coprs/templates/coprs/detail/overview.html
+++ b/coprs_frontend/coprs/templates/coprs/detail/overview.html
@@ -1,4 +1,7 @@
{% extends "coprs/detail.html" %}
+
+{% from "coprs/_coprs_forms.html" import copr_legal_form with context %}
+
{% block overview_selected %}selected{% endblock %}
{% block detail_body %}
@@ -46,4 +49,7 @@
{% endfor %}
</ul>
{% endif %}
+
+ <hr>
+ {{ copr_legal_form(form, copr) }}
{% endblock %}
diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
index cce699a..8c63eee 100644
--- a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
+++ b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
@@ -104,13 +104,15 @@ def copr_new():
@coprs_ns.route('/detail/<username>/<coprname>/')
def copr_detail(username, coprname):
query = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname, with_mock_chroots=True)
+ form = forms.CoprLegalForm()
try:
copr = query.one()
except sqlalchemy.orm.exc.NoResultFound:
return page_not_found('Copr with name {0} does not exist.'.format(coprname))
return flask.render_template('coprs/detail/overview.html',
- copr=copr)
+ copr=copr,
+ form=form)
@coprs_ns.route('/detail/<username>/<coprname>/permissions/')
def copr_permissions(username, coprname):
@@ -256,7 +258,18 @@ def copr_delete(username, coprname):
else:
return flask.render_template('coprs/detail/delete.html', form=form, copr=copr)
-(a)coprs_ns.route('/detail/<username>/<coprname>/legal_flag/')
+(a)coprs_ns.route('/detail/<username>/<coprname>/legal_flag/', methods=['POST'])
@login_required
def copr_legal_flag(username, coprname):
- pass
+ form = forms.CoprLegalForm()
+ copr = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname).first()
+
+ action = models.Action(action_type=helpers.ActionTypeEnum('legal-flag'),
+ object_type='copr',
+ object_id=copr.id,
+ message=form.comment.data,
+ created_on=int(time.time()))
+ db.session.add(action)
+ db.session.commit()
+ flask.flash('Admin was noticed about your report and will investigate the copr shortly.')
+ return flask.redirect(flask.url_for('coprs_ns.copr_detail', username=username, coprname=coprname))
diff --git a/coprs_frontend/tests/test_logic/test_coprs_logic.py b/coprs_frontend/tests/test_logic/test_coprs_logic.py
index c3f365b..2f6d7d8 100644
--- a/coprs_frontend/tests/test_logic/test_coprs_logic.py
+++ b/coprs_frontend/tests/test_logic/test_coprs_logic.py
@@ -1,7 +1,7 @@
import pytest
from coprs.exceptions import ActionInProgressException
-from coprs.helpers import BackendResultEnum
+from coprs.helpers import ActionTypeEnum, BackendResultEnum
from coprs.logic.coprs_logic import CoprsLogic
from tests.coprs_test_case import CoprsTestCase
@@ -12,3 +12,11 @@ class TestCoprsLogic(CoprsTestCase):
with pytest.raises(ActionInProgressException):
CoprsLogic.update(self.u1, self.c1)
self.db.session.rollback()
+
+ def test_legal_flag_doesnt_block_copr_functionality(self, f_users, f_coprs, f_db):
+ self.db.session.add(self.models.Action(object_type='copr',
+ object_id=self.c1.id,
+ action_type=ActionTypeEnum('legal-flag')))
+ self.db.session.commit()
+ # test will fail if this raises exception
+ CoprsLogic.raise_if_unfinished_blocking_action(None, self.c1, 'ha, failed')
10 years, 6 months
[copr] master: Generalize Action to be usable for different scenarios (42bdee5)
by bkabrda@fedoraproject.org
Repository : http://git.fedorahosted.org/cgit/copr.git
On branch : master
>---------------------------------------------------------------
commit 42bdee5b1752ada33eb2f8b6af4bdc396d311201
Author: Bohuslav Kabrda <bkabrda(a)redhat.com>
Date: Fri Mar 29 12:20:57 2013 +0100
Generalize Action to be usable for different scenarios
- rename backend_message to message
- add ended_on column
>---------------------------------------------------------------
.../versions/451e9507b866_generalize_action.py | 30 ++++++++++++++++++++
coprs_frontend/coprs/logic/builds_logic.py | 2 +-
coprs_frontend/coprs/logic/coprs_logic.py | 15 ++++++----
coprs_frontend/coprs/models.py | 8 +++--
.../coprs/views/backend_ns/backend_general.py | 5 ++-
.../coprs/views/coprs_ns/coprs_general.py | 5 +++
.../test_backend_ns/test_backend_general.py | 21 +++++++++----
7 files changed, 67 insertions(+), 19 deletions(-)
diff --git a/coprs_frontend/alembic/versions/451e9507b866_generalize_action.py b/coprs_frontend/alembic/versions/451e9507b866_generalize_action.py
new file mode 100644
index 0000000..203f4cc
--- /dev/null
+++ b/coprs_frontend/alembic/versions/451e9507b866_generalize_action.py
@@ -0,0 +1,30 @@
+"""generalize_action
+
+Revision ID: 451e9507b866
+Revises: 2a75f0a06d90
+Create Date: 2013-03-29 12:13:33.303584
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '451e9507b866'
+down_revision = '2a75f0a06d90'
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+ ### commands auto generated by Alembic - please adjust! ###
+ op.add_column('action', sa.Column('message', sa.Text(), nullable=True))
+ op.add_column('action', sa.Column('ended_on', sa.Integer(), nullable=True))
+ op.drop_column('action', u'backend_message')
+ ### end Alembic commands ###
+
+
+def downgrade():
+ ### commands auto generated by Alembic - please adjust! ###
+ op.add_column('action', sa.Column(u'backend_message', sa.TEXT(), nullable=True))
+ op.drop_column('action', 'ended_on')
+ op.drop_column('action', 'message')
+ ### end Alembic commands ###
diff --git a/coprs_frontend/coprs/logic/builds_logic.py b/coprs_frontend/coprs/logic/builds_logic.py
index 834217d..bbcd639 100644
--- a/coprs_frontend/coprs/logic/builds_logic.py
+++ b/coprs_frontend/coprs/logic/builds_logic.py
@@ -58,7 +58,7 @@ class BuildsLogic(object):
@classmethod
def add(cls, user, pkgs, copr):
- coprs_logic.CoprsLogic.raise_if_unfinished_action(user, copr,
+ coprs_logic.CoprsLogic.raise_if_unfinished_blocking_action(user, copr,
'Can\'t build while there is an operation in progress: {action}')
users_logic.UsersLogic.raise_if_cant_build_in_copr(user, copr,
'You don\'t have permissions to build in this copr.')
diff --git a/coprs_frontend/coprs/logic/coprs_logic.py b/coprs_frontend/coprs/logic/coprs_logic.py
index 663c5cd..e768ae0 100644
--- a/coprs_frontend/coprs/logic/coprs_logic.py
+++ b/coprs_frontend/coprs/logic/coprs_logic.py
@@ -93,7 +93,7 @@ class CoprsLogic(object):
@classmethod
def update(cls, user, copr, check_for_duplicates = True):
- cls.raise_if_unfinished_action(user, copr,
+ cls.raise_if_unfinished_blocking_action(user, copr,
'Can\'t change this Copr name, another operation is in progress: {action}')
users_logic.UsersLogic.raise_if_cant_update_copr(user, copr,
'Only owners and admins may update their Coprs.')
@@ -120,7 +120,7 @@ class CoprsLogic(object):
if not copr.owner == user:
raise exceptions.InsufficientRightsException('Only owners may delete their Coprs.')
# TODO: do we want to dump the information somewhere, so that we can search it in future?
- cls.raise_if_unfinished_action(user, copr,
+ cls.raise_if_unfinished_blocking_action(user, copr,
'Can\'t delete this Copr, another operation is in progress: {action}')
action = models.Action(action_type=helpers.ActionTypeEnum('delete'),
object_type='copr',
@@ -147,19 +147,22 @@ class CoprsLogic(object):
update({models.Copr.build_count: models.Copr.build_count + 1})
@classmethod
- def unfinished_actions_for(cls, user, copr):
+ def unfinished_blocking_actions_for(cls, user, copr):
+ blocking_actions = [helpers.ActionTypeEnum('rename'),
+ helpers.ActionTypeEnum('delete')]
actions = models.Action.query.filter(models.Action.object_type=='copr').\
filter(models.Action.object_id==copr.id).\
- filter(models.Action.backend_result==helpers.BackendResultEnum('waiting'))
+ filter(models.Action.backend_result==helpers.BackendResultEnum('waiting')).\
+ filter(models.Action.action_type.in_(blocking_actions))
return actions
@classmethod
- def raise_if_unfinished_action(cls, user, copr, message):
+ def raise_if_unfinished_blocking_action(cls, user, copr, message):
"""This method raises ActionInProgressException if given copr has an unfinished
action. Returns None otherwise.
"""
- unfinished_actions = cls.unfinished_actions_for(user, copr).all()
+ unfinished_actions = cls.unfinished_blocking_actions_for(user, copr).all()
if unfinished_actions:
raise exceptions.ActionInProgressException(message, unfinished_actions[0])
diff --git a/coprs_frontend/coprs/models.py b/coprs_frontend/coprs/models.py
index 22e8efe..c092e53 100644
--- a/coprs_frontend/coprs/models.py
+++ b/coprs_frontend/coprs/models.py
@@ -269,7 +269,7 @@ class CoprChroot(db.Model, Serializer):
cascade='all,delete,delete-orphan'))
class Action(db.Model, Serializer):
- """Representation of a custom action that needs backends cooperation"""
+ """Representation of a custom action that needs backends cooperation/admin attention/..."""
id = db.Column(db.Integer, primary_key=True)
# delete, rename, ...; see helpers.ActionTypeEnum
action_type = db.Column(db.Integer, nullable=False)
@@ -282,10 +282,12 @@ class Action(db.Model, Serializer):
new_value = db.Column(db.String(255))
# backend result, see helpers.BackendResultEnum
backend_result = db.Column(db.Integer, default=helpers.BackendResultEnum('waiting'))
- # optional message from the backend
- backend_message = db.Column(db.Text)
+ # optional message from the backend/whatever
+ message = db.Column(db.Text)
# time created as returned by int(time.time())
created_on = db.Column(db.Integer)
+ # time ended as returned by int(time.time())
+ ended_on = db.Column(db.Integer)
def __str__(self):
return self.__unicode__()
diff --git a/coprs_frontend/coprs/views/backend_ns/backend_general.py b/coprs_frontend/coprs/views/backend_ns/backend_general.py
index 7be60a2..1f9c4f1 100644
--- a/coprs_frontend/coprs/views/backend_ns/backend_general.py
+++ b/coprs_frontend/coprs/views/backend_ns/backend_general.py
@@ -46,7 +46,7 @@ def update_builds():
def waiting_actions():
actions = models.Action.query.filter(models.Action.backend_result==helpers.BackendResultEnum('waiting')).all()
- return flask.jsonify({'actions': [action.to_dict(options={'__columns_except__': ['backend_result', 'backend_message']}) for action in actions]})
+ return flask.jsonify({'actions': [action.to_dict(options={'__columns_except__': ['backend_result', 'message', 'ended_on']}) for action in actions]})
# TODO: this is very similar to update_builds, we should pull out the common functionality into a single function
@backend_ns.route('/update_actions/', methods=['POST', 'PUT'])
@@ -67,7 +67,8 @@ def update_actions():
for i, action in existing.items():
existing[i].backend_result = to_update[i]['backend_result']
- existing[i].backend_message = to_update[i]['backend_message']
+ existing[i].message = to_update[i]['message']
+ existing[i].ended_on = to_update[i]['ended_on']
db.session.commit()
diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
index f5d84fe..cce699a 100644
--- a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
+++ b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
@@ -255,3 +255,8 @@ def copr_delete(username, coprname):
return flask.redirect(flask.url_for('coprs_ns.coprs_by_owner', username=username))
else:
return flask.render_template('coprs/detail/delete.html', form=form, copr=copr)
+
+(a)coprs_ns.route('/detail/<username>/<coprname>/legal_flag/')
+@login_required
+def copr_legal_flag(username, coprname):
+ pass
diff --git a/coprs_frontend/tests/test_views/test_backend_ns/test_backend_general.py b/coprs_frontend/tests/test_views/test_backend_ns/test_backend_general.py
index 5b021d8..2d3270c 100644
--- a/coprs_frontend/tests/test_views/test_backend_ns/test_backend_general.py
+++ b/coprs_frontend/tests/test_views/test_backend_ns/test_backend_general.py
@@ -143,7 +143,8 @@ class TestUpdateActions(CoprsTestCase):
{
"id": 1,
"backend_result": 1,
- "backend_message": "no problem"
+ "message": "no problem",
+ "ended_on": 123
}
]
}"""
@@ -153,17 +154,20 @@ class TestUpdateActions(CoprsTestCase):
{
"id": 1,
"backend_result": 1,
- "backend_message": null
+ "message": null,
+ "ended_on": 1234
},
{
"id": 2,
"backend_result": 2,
- "backend_message": "problem!"
+ "message": "problem!",
+ "ended_on": 12345
},
{
"id": 100,
"backend_result": 123,
- "backend_message": "wheeeee!"
+ "message": "wheeeee!",
+ "ended_on": 123456
}
]
}"""
@@ -178,7 +182,8 @@ class TestUpdateActions(CoprsTestCase):
updated = self.models.Action.query.filter(self.models.Action.id==1).first()
assert updated.backend_result == 1
- assert updated.backend_message == "no problem"
+ assert updated.message == "no problem"
+ assert updated.ended_on == 123
def test_update_more_existent_and_non_existent_builds(self, f_users, f_coprs, f_actions, f_db):
r = self.tc.post('/backend/update_actions/',
@@ -190,8 +195,10 @@ class TestUpdateActions(CoprsTestCase):
updated = self.models.Action.query.filter(self.models.Action.id==1).first()
assert updated.backend_result == 1
- assert updated.backend_message == None
+ assert updated.message == None
+ assert updated.ended_on == 1234
updated2 = self.models.Action.query.filter(self.models.Action.id==2).first()
assert updated2.backend_result == 2
- assert updated2.backend_message == "problem!"
+ assert updated2.message == "problem!"
+ assert updated2.ended_on == 12345
10 years, 6 months
[copr] master: Fix #60 - copr admin can now give up his permissions even if there are permissions for more users for the copr (8fb8188)
by bkabrda@fedoraproject.org
Repository : http://git.fedorahosted.org/cgit/copr.git
On branch : master
>---------------------------------------------------------------
commit 8fb8188a50af94855b0767f9bfddf63002c34efe
Author: Bohuslav Kabrda <bkabrda(a)redhat.com>
Date: Fri Mar 29 10:01:20 2013 +0100
Fix #60 - copr admin can now give up his permissions even if there are permissions for more users for the copr
- also added flexmock to requirements
>---------------------------------------------------------------
.../coprs/views/coprs_ns/coprs_general.py | 3 ++
.../test_views/test_coprs_ns/test_coprs_general.py | 25 ++++++++++++++++++++
requirements.txt | 1 +
3 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
index 1b8353b..f5d84fe 100644
--- a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
+++ b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
@@ -219,6 +219,9 @@ def copr_update_permissions(username, coprname):
if permissions_form.validate_on_submit():
# we don't change owner (yet)
try:
+ # if admin is changing his permissions, his must be changed last
+ # so that we don't get InsufficientRightsException
+ permissions.sort(cmp=lambda x, y: -1 if y.user_id == flask.g.user.id else 1)
for perm in permissions:
new_builder = permissions_form['copr_builder_{0}'.format(perm.user_id)].data
new_admin = permissions_form['copr_admin_{0}'.format(perm.user_id)].data
diff --git a/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py b/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py
index 63f3a06..6fa8e19 100644
--- a/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py
+++ b/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py
@@ -1,5 +1,7 @@
import flask
+from flexmock import flexmock
+
from coprs.signals import copr_created
from tests.coprs_test_case import CoprsTestCase
@@ -371,6 +373,29 @@ class TestCoprUpdatePermissions(CoprsTestCase):
follow_redirects = True)
assert 'Copr permissions were updated' in r.data
+ def test_copr_admin_can_give_up_his_permissions(self, f_users, f_coprs, f_copr_permissions, f_db):
+ # if admin is giving up his permission and there are more permissions for
+ # this copr, then if the admin is altered first, he won't be permitted
+ # to alter the other permissions and the whole update would fail
+ with self.tc as c:
+ with c.session_transaction() as s:
+ s['openid'] = self.u1.openid_name
+ self.db.session.add_all([self.u2, self.c3, self.cp2, self.cp3])
+ # mock out the order of CoprPermission objects, so that we are sure
+ # the admin is the first one and therefore this fails if
+ # the view doesn't reorder the permissions
+ flexmock(self.models.Copr, copr_permissions=[self.cp3, self.cp2])
+ r = c.post('/coprs/detail/{0}/{1}/update_permissions/'.format(self.u2.name, self.c3.name),
+ data = {'copr_admin_1': '1', 'copr_admin_3': '1'},
+ follow_redirects = True)
+ self.db.session.add_all([self.u1, self.c3])
+ perm = self.models.CoprPermission.query.filter(self.models.CoprPermission.user_id==self.u1.id).\
+ filter(self.models.CoprPermission.copr_id==self.c3.id).\
+ first()
+ assert perm.copr_admin == 1
+ assert 'Copr permissions were updated' in r.data
+
+
class TestCoprDelete(CoprsTestCase):
def test_delete(self, f_users, f_coprs, f_db):
with self.tc as c:
diff --git a/requirements.txt b/requirements.txt
index d9b8d1f..9094607 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -8,4 +8,5 @@ flask-openid
flask-script
flask-whooshee
flask-wtf
+flexmock
pytest
10 years, 6 months
[copr] master: Fix import and indentation in users_logic. (9c1f582)
by bkabrda@fedoraproject.org
Repository : http://git.fedorahosted.org/cgit/copr.git
On branch : master
>---------------------------------------------------------------
commit 9c1f582f1abb8ba6c5853a83382e093d2418313d
Author: Bohuslav Kabrda <bkabrda(a)redhat.com>
Date: Fri Mar 29 08:32:38 2013 +0100
Fix import and indentation in users_logic.
>---------------------------------------------------------------
coprs_frontend/coprs/logic/users_logic.py | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/coprs_frontend/coprs/logic/users_logic.py b/coprs_frontend/coprs/logic/users_logic.py
index 3f6d048..13289b6 100644
--- a/coprs_frontend/coprs/logic/users_logic.py
+++ b/coprs_frontend/coprs/logic/users_logic.py
@@ -1,4 +1,4 @@
-import exceptions
+from coprs import exceptions
class UsersLogic(object):
@classmethod
@@ -12,8 +12,8 @@ class UsersLogic(object):
@classmethod
def raise_if_cant_build_in_copr(cls, user, copr, message):
- """This method raises InsufficientRightsException if given user cant build in
- given copr. Returns None otherwise.
- """
- if not user.can_build_in(copr):
- raise exceptions.InsufficientRightsException(message)
+ """This method raises InsufficientRightsException if given user cant build in
+ given copr. Returns None otherwise.
+ """
+ if not user.can_build_in(copr):
+ raise exceptions.InsufficientRightsException(message)
10 years, 6 months
[copr] master: Start implementing admin section in frontend (604b0ff)
by bkabrda@fedoraproject.org
Repository : http://git.fedorahosted.org/cgit/copr.git
On branch : master
>---------------------------------------------------------------
commit 604b0ff75466a076bf9b8b5dcc20bde5fa1cd088
Author: Bohuslav Kabrda <bkabrda(a)redhat.com>
Date: Thu Mar 28 15:46:04 2013 +0100
Start implementing admin section in frontend
>---------------------------------------------------------------
coprs_frontend/coprs/__init__.py | 3 ++
coprs_frontend/coprs/helpers.py | 4 ++
coprs_frontend/coprs/templates/admin/index.html | 5 +++
coprs_frontend/coprs/templates/admin/layout.html | 8 +++++
coprs_frontend/coprs/views/admin_ns/__init__.py | 3 ++
.../coprs/views/admin_ns/admin_general.py | 11 +++++++
coprs_frontend/coprs/views/misc.py | 30 ++++++++++++++------
coprs_frontend/tests/coprs_test_case.py | 6 ++--
.../test_views/test_admin/test_admin_general.py | 20 +++++++++++++
9 files changed, 78 insertions(+), 12 deletions(-)
diff --git a/coprs_frontend/coprs/__init__.py b/coprs_frontend/coprs/__init__.py
index 6a9290c..103d7e1 100644
--- a/coprs_frontend/coprs/__init__.py
+++ b/coprs_frontend/coprs/__init__.py
@@ -25,6 +25,8 @@ import coprs.log
import coprs.models
import coprs.whoosheers
+from coprs.views import admin_ns
+from coprs.views.admin_ns import admin_general
from coprs.views import api_ns
from coprs.views.api_ns import api_general
from coprs.views import coprs_ns
@@ -35,6 +37,7 @@ from coprs.views.backend_ns import backend_general
from coprs.views import misc
app.register_blueprint(api_ns.api_ns)
+app.register_blueprint(admin_ns.admin_ns)
app.register_blueprint(coprs_ns.coprs_ns)
app.register_blueprint(misc.misc)
app.register_blueprint(backend_ns.backend_ns)
diff --git a/coprs_frontend/coprs/helpers.py b/coprs_frontend/coprs/helpers.py
index 15e942e..fdc87af 100644
--- a/coprs_frontend/coprs/helpers.py
+++ b/coprs_frontend/coprs/helpers.py
@@ -43,6 +43,10 @@ class BackendResultEnum(object):
__metaclass__ = EnumType
vals = {'waiting': 0, 'success': 1, 'failure': 2}
+class RoleEnum(object):
+ __metaclass__ = EnumType
+ vals = {'user': 0, 'admin': 1}
+
class Paginator(object):
def __init__(self, query, total_count, page = 1, per_page_override = None, urls_count_override = None):
self.query = query
diff --git a/coprs_frontend/coprs/templates/admin/index.html b/coprs_frontend/coprs/templates/admin/index.html
new file mode 100644
index 0000000..25562a2
--- /dev/null
+++ b/coprs_frontend/coprs/templates/admin/index.html
@@ -0,0 +1,5 @@
+{% extends "admin/layout.html" %}
+
+{% block admin_body %}
+Admin body
+{% endblock %}
diff --git a/coprs_frontend/coprs/templates/admin/layout.html b/coprs_frontend/coprs/templates/admin/layout.html
new file mode 100644
index 0000000..336e077
--- /dev/null
+++ b/coprs_frontend/coprs/templates/admin/layout.html
@@ -0,0 +1,8 @@
+{% extends "layout.html" %}
+
+{% block title %}Coprs - Admin{% endblock %}
+
+{% block body %}
+Admin menu
+{% block admin_body %}{% endblock %}
+{% endblock %}
diff --git a/coprs_frontend/coprs/views/admin_ns/__init__.py b/coprs_frontend/coprs/views/admin_ns/__init__.py
new file mode 100644
index 0000000..b7f5426
--- /dev/null
+++ b/coprs_frontend/coprs/views/admin_ns/__init__.py
@@ -0,0 +1,3 @@
+import flask
+
+admin_ns = flask.Blueprint('admin_ns', __name__, url_prefix = '/admin')
diff --git a/coprs_frontend/coprs/views/admin_ns/admin_general.py b/coprs_frontend/coprs/views/admin_ns/admin_general.py
new file mode 100644
index 0000000..8252c5e
--- /dev/null
+++ b/coprs_frontend/coprs/views/admin_ns/admin_general.py
@@ -0,0 +1,11 @@
+import flask
+
+from coprs import helpers
+
+from coprs.views.admin_ns import admin_ns
+from coprs.views.misc import login_required
+
+(a)admin_ns.route('/')
+(a)login_required(role=helpers.RoleEnum('admin'))
+def admin_index():
+ return flask.render_template('admin/index.html')
diff --git a/coprs_frontend/coprs/views/misc.py b/coprs_frontend/coprs/views/misc.py
index 1c5a47c..7ffad6b 100644
--- a/coprs_frontend/coprs/views/misc.py
+++ b/coprs_frontend/coprs/views/misc.py
@@ -107,15 +107,27 @@ def api_login_required(f):
return decorated_function
-def login_required(f):
- @functools.wraps(f)
- def decorated_function(*args, **kwargs):
- if flask.g.user is None:
- return flask.redirect(flask.url_for('misc.login',
- next = flask.request.url))
- return f(*args, **kwargs)
- return decorated_function
-
+def login_required(role=helpers.RoleEnum('user')):
+ def view_wrapper(f):
+ @functools.wraps(f)
+ def decorated_function(*args, **kwargs):
+ if flask.g.user is None:
+ return flask.redirect(flask.url_for('misc.login',
+ next = flask.request.url))
+ if role == helpers.RoleEnum('admin') and not flask.g.user.admin:
+ flask.flash('You are not allowed to access admin section.')
+ return flask.redirect(flask.url_for('coprs_ns.coprs_show'))
+ return f(*args, **kwargs)
+ return decorated_function
+ # hack: if login_required is used without params, the "role" parameter
+ # is in fact the decorated function, so we need to return
+ # the wrapped function, not the wrapper
+ # proper solution would be to use login_required() with parentheses
+ # everywhere, even if they're empty - TODO
+ if callable(role):
+ return view_wrapper(role)
+ else:
+ return view_wrapper
# backend authentication
def backend_authenticated(f):
diff --git a/coprs_frontend/tests/coprs_test_case.py b/coprs_frontend/tests/coprs_test_case.py
index 4f6043f..bea1f58 100644
--- a/coprs_frontend/tests/coprs_test_case.py
+++ b/coprs_frontend/tests/coprs_test_case.py
@@ -40,9 +40,9 @@ class CoprsTestCase(object):
@pytest.fixture
def f_users(self):
- self.u1 = models.User(openid_name = u'http://user1.id.fedoraproject.org/', proven = False, mail = 'user1(a)foo.bar')
- self.u2 = models.User(openid_name = u'http://user2.id.fedoraproject.org/', proven = False, mail = 'user2(a)spam.foo')
- self.u3 = models.User(openid_name = u'http://user3.id.fedoraproject.org/', proven = False, mail = 'baz(a)bar.bar')
+ self.u1 = models.User(openid_name=u'http://user1.id.fedoraproject.org/', proven=False, admin=True, mail='user1(a)foo.bar')
+ self.u2 = models.User(openid_name=u'http://user2.id.fedoraproject.org/', proven = False, mail='user2(a)spam.foo')
+ self.u3 = models.User(openid_name=u'http://user3.id.fedoraproject.org/', proven=False, mail='baz(a)bar.bar')
self.db.session.add_all([self.u1, self.u2, self.u3])
diff --git a/coprs_frontend/tests/test_views/test_admin/test_admin_general.py b/coprs_frontend/tests/test_views/test_admin/test_admin_general.py
new file mode 100644
index 0000000..1223934
--- /dev/null
+++ b/coprs_frontend/tests/test_views/test_admin/test_admin_general.py
@@ -0,0 +1,20 @@
+from tests.coprs_test_case import CoprsTestCase
+
+class TestAdminLogin(CoprsTestCase):
+ # TODO: test on something better then page title - maybe see rendered templates?
+ text_to_check = 'Coprs - Admin'
+ def test_nonadmin_cant_login(self, f_users, f_db):
+ with self.tc as c:
+ with c.session_transaction() as s:
+ s['openid'] = self.u2.openid_name
+
+ r = c.get('/admin/', follow_redirects=True)
+ assert self.text_to_check not in r.data
+
+ def test_admin_can_login(self, f_users, f_db):
+ with self.tc as c:
+ with c.session_transaction() as s:
+ s['openid'] = self.u1.openid_name
+
+ r = c.get('/admin/', follow_redirects=True)
+ assert self.text_to_check in r.data
10 years, 6 months
[copr] master: It seems that pytest does not like having files with same names... (57dc405)
by bkabrda@fedoraproject.org
Repository : http://git.fedorahosted.org/cgit/copr.git
On branch : master
>---------------------------------------------------------------
commit 57dc4058d4c6709bde5e95ddf67716f142482632
Author: Bohuslav Kabrda <bkabrda(a)redhat.com>
Date: Thu Mar 28 15:30:52 2013 +0100
It seems that pytest does not like having files with same names...
>---------------------------------------------------------------
.../{test_general.py => test_backend_general.py} | 0
.../{test_builds.py => test_coprs_builds.py} | 0
.../{test_general.py => test_coprs_general.py} | 0
3 files changed, 0 insertions(+), 0 deletions(-)
diff --git a/coprs_frontend/tests/test_views/test_backend_ns/test_general.py b/coprs_frontend/tests/test_views/test_backend_ns/test_backend_general.py
similarity index 100%
rename from coprs_frontend/tests/test_views/test_backend_ns/test_general.py
rename to coprs_frontend/tests/test_views/test_backend_ns/test_backend_general.py
diff --git a/coprs_frontend/tests/test_views/test_coprs_ns/test_builds.py b/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_builds.py
similarity index 100%
rename from coprs_frontend/tests/test_views/test_coprs_ns/test_builds.py
rename to coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_builds.py
diff --git a/coprs_frontend/tests/test_views/test_coprs_ns/test_general.py b/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py
similarity index 100%
rename from coprs_frontend/tests/test_views/test_coprs_ns/test_general.py
rename to coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py
10 years, 6 months
[copr] master: Move build creation authorization to logic. (6aacde2)
by bkabrda@fedoraproject.org
Repository : http://git.fedorahosted.org/cgit/copr.git
On branch : master
>---------------------------------------------------------------
commit 6aacde2a7f1f3e2651d0786277480d1d9727daab
Author: Bohuslav Kabrda <bkabrda(a)redhat.com>
Date: Thu Mar 28 13:10:52 2013 +0100
Move build creation authorization to logic.
>---------------------------------------------------------------
coprs_frontend/coprs/logic/builds_logic.py | 11 +++++------
coprs_frontend/coprs/logic/users_logic.py | 8 ++++++++
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/coprs_frontend/coprs/logic/builds_logic.py b/coprs_frontend/coprs/logic/builds_logic.py
index 2a91f95..834217d 100644
--- a/coprs_frontend/coprs/logic/builds_logic.py
+++ b/coprs_frontend/coprs/logic/builds_logic.py
@@ -6,6 +6,7 @@ from coprs import models
from coprs import signals
from coprs.logic import coprs_logic
+from coprs.logic import users_logic
class BuildsLogic(object):
@classmethod
@@ -59,6 +60,8 @@ class BuildsLogic(object):
def add(cls, user, pkgs, copr):
coprs_logic.CoprsLogic.raise_if_unfinished_action(user, copr,
'Can\'t build while there is an operation in progress: {action}')
+ users_logic.UsersLogic.raise_if_cant_build_in_copr(user, copr,
+ 'You don\'t have permissions to build in this copr.')
build = models.Build(
pkgs=pkgs,
copr=copr,
@@ -66,15 +69,11 @@ class BuildsLogic(object):
chroots=' '.join(map(lambda x: x.chroot_name, copr.active_mock_chroots)),
user=user,
submitted_on=int(time.time()))
- # no need to check for authorization here
- cls.new(user, build, copr, check_authorized=False)
+ cls.new(user, build, copr)
return build
@classmethod
- def new(cls, user, build, copr, check_authorized = True):
- if check_authorized:
- if not user.can_build_in(copr):
- raise exceptions.InsufficientRightsException('User {0} cannot build in copr {1}/{2}'.format(user.name, copr.owner.name, copr.name))
+ def new(cls, user, build, copr):
if not build.submitted_on:
build.submitted_on = int(time.time())
if not build.user:
diff --git a/coprs_frontend/coprs/logic/users_logic.py b/coprs_frontend/coprs/logic/users_logic.py
index 8ff0eb2..3f6d048 100644
--- a/coprs_frontend/coprs/logic/users_logic.py
+++ b/coprs_frontend/coprs/logic/users_logic.py
@@ -9,3 +9,11 @@ class UsersLogic(object):
# TODO: this is a bit inconsistent - shouldn't the user method be called can_update?
if not user.can_edit(copr):
raise exceptions.InsufficientRightsException(message)
+
+ @classmethod
+ def raise_if_cant_build_in_copr(cls, user, copr, message):
+ """This method raises InsufficientRightsException if given user cant build in
+ given copr. Returns None otherwise.
+ """
+ if not user.can_build_in(copr):
+ raise exceptions.InsufficientRightsException(message)
10 years, 6 months
[copr] master: Move authorization checking for deleting coprs into logic (971c737)
by bkabrda@fedoraproject.org
Repository : http://git.fedorahosted.org/cgit/copr.git
On branch : master
>---------------------------------------------------------------
commit 971c737b620cb4851bffddf3d234b6550ce7ced9
Author: Bohuslav Kabrda <bkabrda(a)redhat.com>
Date: Thu Mar 28 12:58:59 2013 +0100
Move authorization checking for deleting coprs into logic
>---------------------------------------------------------------
coprs_frontend/coprs/logic/coprs_logic.py | 3 ++-
.../coprs/views/coprs_ns/coprs_general.py | 6 +-----
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/coprs_frontend/coprs/logic/coprs_logic.py b/coprs_frontend/coprs/logic/coprs_logic.py
index 6922789..663c5cd 100644
--- a/coprs_frontend/coprs/logic/coprs_logic.py
+++ b/coprs_frontend/coprs/logic/coprs_logic.py
@@ -117,7 +117,8 @@ class CoprsLogic(object):
@classmethod
def delete(cls, user, copr, check_for_duplicates=True):
- # for the time being, we authorize user to do this in view...
+ if not copr.owner == user:
+ raise exceptions.InsufficientRightsException('Only owners may delete their Coprs.')
# TODO: do we want to dump the information somewhere, so that we can search it in future?
cls.raise_if_unfinished_action(user, copr,
'Can\'t delete this Copr, another operation is in progress: {action}')
diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
index 51bc7d1..1b8353b 100644
--- a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
+++ b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
@@ -238,15 +238,11 @@ def copr_update_permissions(username, coprname):
def copr_delete(username, coprname):
form = forms.CoprDeleteForm()
copr = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname).first()
- # only owner can delete a copr
- if flask.g.user != copr.owner:
- flask.flash('Only owners may delete their Coprs.')
- return flask.redirect(flask.url_for('coprs_ns.copr_detail', username=username, coprname=coprname))
if form.validate_on_submit():
try:
coprs_logic.CoprsLogic.delete(flask.g.user, copr)
- except exceptions.ActionInProgressException as e:
+ except (exceptions.ActionInProgressException, exceptions.InsufficientRightsException) as e:
db.session.rollback()
flask.flash(e)
return flask.redirect(flask.url_for('coprs_ns.copr_detail', username=username, coprname=coprname))
10 years, 6 months
[copr] master: Move authorization checking for updating permissions into logic (8fe7f0a)
by bkabrda@fedoraproject.org
Repository : http://git.fedorahosted.org/cgit/copr.git
On branch : master
>---------------------------------------------------------------
commit 8fe7f0ac8991df619754a60f253a690f7b129e1a
Author: Bohuslav Kabrda <bkabrda(a)redhat.com>
Date: Thu Mar 28 12:50:09 2013 +0100
Move authorization checking for updating permissions into logic
- refactor the raise_if_cant_update_copr method into newly introduced UsersLogic class
>---------------------------------------------------------------
coprs_frontend/coprs/logic/coprs_logic.py | 16 ++++---------
coprs_frontend/coprs/logic/users_logic.py | 11 +++++++++
.../coprs/views/coprs_ns/coprs_general.py | 24 ++++++++++----------
3 files changed, 28 insertions(+), 23 deletions(-)
diff --git a/coprs_frontend/coprs/logic/coprs_logic.py b/coprs_frontend/coprs/logic/coprs_logic.py
index 2b4b2d9..6922789 100644
--- a/coprs_frontend/coprs/logic/coprs_logic.py
+++ b/coprs_frontend/coprs/logic/coprs_logic.py
@@ -6,6 +6,7 @@ from coprs import helpers
from coprs import models
from coprs import signals
from coprs import whoosheers
+from coprs.logic import users_logic
class CoprsLogic(object):
"""Used for manipulating Coprs. All methods accept user object as a first argument, as this may be needed in future."""
@@ -94,7 +95,8 @@ class CoprsLogic(object):
def update(cls, user, copr, check_for_duplicates = True):
cls.raise_if_unfinished_action(user, copr,
'Can\'t change this Copr name, another operation is in progress: {action}')
- cls.raise_if_cant_update(user, copr, 'Only owners and admins may update their Coprs.')
+ users_logic.UsersLogic.raise_if_cant_update_copr(user, copr,
+ 'Only owners and admins may update their Coprs.')
existing = cls.exists_for_user(copr.owner, copr.name).first()
if existing:
@@ -160,16 +162,6 @@ class CoprsLogic(object):
if unfinished_actions:
raise exceptions.ActionInProgressException(message, unfinished_actions[0])
- @classmethod
- def raise_if_cant_update(cls, user, copr, message):
- """This method raises InsufficientRightsException if given user cant update
- given copr. Returns None otherwise.
- """
- # TODO: this is a bit inconsistent - shouldn't the user method be called can_update?
- if not user.can_edit(copr):
- raise exceptions.InsufficientRightsException(message)
-
-
class CoprPermissionsLogic(object):
@classmethod
def get(cls, user, copr, searched_user):
@@ -190,6 +182,8 @@ class CoprPermissionsLogic(object):
@classmethod
def update_permissions(cls, user, copr, copr_permission, new_builder, new_admin):
+ users_logic.UsersLogic.raise_if_cant_update_copr(user, copr,
+ 'Only owners and admins may update their Coprs permissions.')
models.CoprPermission.query.filter(models.CoprPermission.copr_id == copr.id).\
filter(models.CoprPermission.user_id == copr_permission.user_id).\
update({'copr_builder': new_builder,
diff --git a/coprs_frontend/coprs/logic/users_logic.py b/coprs_frontend/coprs/logic/users_logic.py
new file mode 100644
index 0000000..8ff0eb2
--- /dev/null
+++ b/coprs_frontend/coprs/logic/users_logic.py
@@ -0,0 +1,11 @@
+import exceptions
+
+class UsersLogic(object):
+ @classmethod
+ def raise_if_cant_update_copr(cls, user, copr, message):
+ """This method raises InsufficientRightsException if given user cant update
+ given copr. Returns None otherwise.
+ """
+ # TODO: this is a bit inconsistent - shouldn't the user method be called can_update?
+ if not user.can_edit(copr):
+ raise exceptions.InsufficientRightsException(message)
diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
index adba1d0..51bc7d1 100644
--- a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
+++ b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
@@ -216,20 +216,20 @@ def copr_update_permissions(username, coprname):
permissions = copr.copr_permissions
permissions_form = forms.PermissionsFormFactory.create_form_cls(permissions)()
- # only owner can update copr permissions
- if not flask.g.user.can_edit(copr):
- flask.flash('Only owners and admins may update their Coprs permissions.')
- return flask.redirect(flask.url_for('coprs_ns.copr_detail', username = copr.owner.name, coprname = copr.name))
-
if permissions_form.validate_on_submit():
# we don't change owner (yet)
- for perm in permissions:
- new_builder = permissions_form['copr_builder_{0}'.format(perm.user_id)].data
- new_admin = permissions_form['copr_admin_{0}'.format(perm.user_id)].data
- coprs_logic.CoprPermissionsLogic.update_permissions(flask.g.user, copr, perm, new_builder, new_admin)
-
- db.session.commit()
- flask.flash('Copr permissions were updated successfully.')
+ try:
+ for perm in permissions:
+ new_builder = permissions_form['copr_builder_{0}'.format(perm.user_id)].data
+ new_admin = permissions_form['copr_admin_{0}'.format(perm.user_id)].data
+ coprs_logic.CoprPermissionsLogic.update_permissions(flask.g.user, copr, perm, new_builder, new_admin)
+ # for now, we don't check for actions here, as permissions operation don't collide with any actions
+ except exceptions.InsufficientRightsException as e:
+ db.session.rollback()
+ flask.flash(e)
+ else:
+ db.session.commit()
+ flask.flash('Copr permissions were updated successfully.')
return flask.redirect(flask.url_for('coprs_ns.copr_detail', username = copr.owner.name, coprname = copr.name))
10 years, 6 months