Qpid
  1. Qpid
  2. QPID-2393

Qpid C++ broker: request for feature to limit number of queues per user

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.19
    • Component/s: C++ Broker
    • Labels:
      None
    • Environment:

      Red Hat Enterprise MRG 1.2

      Description

      With issue QPID-2108 (Red Hat service request #1950278) a new feature has been introduced which allows to control via ACL the size of queues and their limit policy on user level.

      The original request contained also the requirement to gain control over the number of queues a user may create.
      ACL should be enhanced to allow specifying a maximum number of queues for a single user.

      Altogether these features shall enable the operator of a Qpid broker to keep better control over the resources.

      We will prepare a draft implementation and provide it asap.

      This request has also been reported as Red Hat service request #1992776.

        Activity

        Hide
        Chuck Rolke added a comment -

        Resolved with merge 1376961 from branch branches/qpid-2393

        Show
        Chuck Rolke added a comment - Resolved with merge 1376961 from branch branches/qpid-2393
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/5015/
        -----------------------------------------------------------

        (Updated 2012-05-15 13:41:01.807339)

        Review request for qpid, Alan Conway, Kim van der Riet, and Ted Ross.

        Changes
        -------

        1. User name against whom quotas are taken is stored in queue.settings so that it persists in store and across replication.
        2. Queue provides user name accessor methods for QueueRegistry.
        3. Queue limits exceed by 'recovered' events are allowed. There's no proper mechanism to dispose/disallow creation of these queues and doing so would result in data loss and other issues.
        4. This is untested against a store or cluster replication but is ready to go for either.

        Summary
        -------

        This patch fulfills a long-standing request to keep users from abusing broker queue resources. If a user is allowed to create one queue he then can create them by the thousdands.

        The code is more of a quota than an access control but it fits naturally in the current ACL module. The implementation here is queue-centric but could be generalized to support limiting exchanges as well.

        A few concerns arise:

        1. This code counts/protects live requests coming in to single node. This code does not protect queues that are presisting. The concern is that a user creates his quota of persistent queues and then upon system restart the same user can create another batch of queues since the persisted queues aren't tracked. Is this a vaild concern?

        2. The patch provides only a single setting for all users.

        3. The patch makes no effort to replicate the queue count state across a cluster. Surely this is a problem for clusters.

        This addresses bug QPID-2393.
        https://issues.apache.org/jira/browse/QPID-2393

        Diffs (updated)


        trunk/qpid/cpp/src/qpid/acl/Acl.h 1336822
        trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1336822
        trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp 1336822
        trunk/qpid/cpp/src/qpid/acl/management-schema.xml 1336822
        trunk/qpid/cpp/src/qpid/broker/AclModule.h 1336822
        trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1336822
        trunk/qpid/cpp/src/qpid/broker/Queue.h 1336822
        trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1336822
        trunk/qpid/cpp/src/qpid/broker/QueueRegistry.h 1336822
        trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1336822
        trunk/qpid/cpp/src/tests/acl.py 1336822
        trunk/qpid/cpp/src/tests/run_acl_tests 1336822

        Diff: https://reviews.apache.org/r/5015/diff

        Testing
        -------

        Unit tests included.

        Thanks,

        Chug

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5015/ ----------------------------------------------------------- (Updated 2012-05-15 13:41:01.807339) Review request for qpid, Alan Conway, Kim van der Riet, and Ted Ross. Changes ------- 1. User name against whom quotas are taken is stored in queue.settings so that it persists in store and across replication. 2. Queue provides user name accessor methods for QueueRegistry. 3. Queue limits exceed by 'recovered' events are allowed. There's no proper mechanism to dispose/disallow creation of these queues and doing so would result in data loss and other issues. 4. This is untested against a store or cluster replication but is ready to go for either. Summary ------- This patch fulfills a long-standing request to keep users from abusing broker queue resources. If a user is allowed to create one queue he then can create them by the thousdands. The code is more of a quota than an access control but it fits naturally in the current ACL module. The implementation here is queue-centric but could be generalized to support limiting exchanges as well. A few concerns arise: 1. This code counts/protects live requests coming in to single node. This code does not protect queues that are presisting. The concern is that a user creates his quota of persistent queues and then upon system restart the same user can create another batch of queues since the persisted queues aren't tracked. Is this a vaild concern? 2. The patch provides only a single setting for all users. 3. The patch makes no effort to replicate the queue count state across a cluster. Surely this is a problem for clusters. This addresses bug QPID-2393 . https://issues.apache.org/jira/browse/QPID-2393 Diffs (updated) trunk/qpid/cpp/src/qpid/acl/Acl.h 1336822 trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1336822 trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp 1336822 trunk/qpid/cpp/src/qpid/acl/management-schema.xml 1336822 trunk/qpid/cpp/src/qpid/broker/AclModule.h 1336822 trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1336822 trunk/qpid/cpp/src/qpid/broker/Queue.h 1336822 trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1336822 trunk/qpid/cpp/src/qpid/broker/QueueRegistry.h 1336822 trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1336822 trunk/qpid/cpp/src/tests/acl.py 1336822 trunk/qpid/cpp/src/tests/run_acl_tests 1336822 Diff: https://reviews.apache.org/r/5015/diff Testing ------- Unit tests included. Thanks, Chug
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-05-07 15:43:12, Alan Conway wrote:

        > Definitely needs to replicate state in a cluster. Shout if you need pointers.

        rajith attapattu wrote:

        This more of a general problem where ACL doesn't play well with the clustered setup.

        Perhaps we could work on a case by case for the time being to get certain functionality like this working.

        However longer term we need to find a way to ensure the ACL in-memory-model is replicated so any change done in one broker is relected on it's members.

        That would be the first step in allowing dynamic provisioning of rules.

        AFAIK all the existing ACL model is replicated in a cluster, so it's just a matter of keeping it up to date as we add new functionality. There might be a case for some refactoring to make that easier.

        • Alan

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/5015/#review7640
        -----------------------------------------------------------

        On 2012-05-04 19:41:45, Chug Rolke wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/5015/

        -----------------------------------------------------------

        (Updated 2012-05-04 19:41:45)

        Review request for qpid, Alan Conway, Kim van der Riet, and Ted Ross.

        Summary

        -------

        This patch fulfills a long-standing request to keep users from abusing broker queue resources. If a user is allowed to create one queue he then can create them by the thousdands.

        The code is more of a quota than an access control but it fits naturally in the current ACL module. The implementation here is queue-centric but could be generalized to support limiting exchanges as well.

        A few concerns arise:

        1. This code counts/protects live requests coming in to single node. This code does not protect queues that are presisting. The concern is that a user creates his quota of persistent queues and then upon system restart the same user can create another batch of queues since the persisted queues aren't tracked. Is this a vaild concern?

        2. The patch provides only a single setting for all users.

        3. The patch makes no effort to replicate the queue count state across a cluster. Surely this is a problem for clusters.

        This addresses bug QPID-2393.

        https://issues.apache.org/jira/browse/QPID-2393

        Diffs

        -----

        trunk/qpid/cpp/src/qpid/acl/Acl.h 1334118

        trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1334118

        trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp 1334118

        trunk/qpid/cpp/src/qpid/acl/management-schema.xml 1334118

        trunk/qpid/cpp/src/qpid/broker/AclModule.h 1334118

        trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1334118

        trunk/qpid/cpp/src/tests/acl.py 1334118

        trunk/qpid/cpp/src/tests/run_acl_tests 1334118

        Diff: https://reviews.apache.org/r/5015/diff

        Testing

        -------

        Unit tests included.

        Thanks,

        Chug

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-05-07 15:43:12, Alan Conway wrote: > Definitely needs to replicate state in a cluster. Shout if you need pointers. rajith attapattu wrote: This more of a general problem where ACL doesn't play well with the clustered setup. Perhaps we could work on a case by case for the time being to get certain functionality like this working. However longer term we need to find a way to ensure the ACL in-memory-model is replicated so any change done in one broker is relected on it's members. That would be the first step in allowing dynamic provisioning of rules. AFAIK all the existing ACL model is replicated in a cluster, so it's just a matter of keeping it up to date as we add new functionality. There might be a case for some refactoring to make that easier. Alan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5015/#review7640 ----------------------------------------------------------- On 2012-05-04 19:41:45, Chug Rolke wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5015/ ----------------------------------------------------------- (Updated 2012-05-04 19:41:45) Review request for qpid, Alan Conway, Kim van der Riet, and Ted Ross. Summary ------- This patch fulfills a long-standing request to keep users from abusing broker queue resources. If a user is allowed to create one queue he then can create them by the thousdands. The code is more of a quota than an access control but it fits naturally in the current ACL module. The implementation here is queue-centric but could be generalized to support limiting exchanges as well. A few concerns arise: 1. This code counts/protects live requests coming in to single node. This code does not protect queues that are presisting. The concern is that a user creates his quota of persistent queues and then upon system restart the same user can create another batch of queues since the persisted queues aren't tracked. Is this a vaild concern? 2. The patch provides only a single setting for all users. 3. The patch makes no effort to replicate the queue count state across a cluster. Surely this is a problem for clusters. This addresses bug QPID-2393 . https://issues.apache.org/jira/browse/QPID-2393 Diffs ----- trunk/qpid/cpp/src/qpid/acl/Acl.h 1334118 trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1334118 trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp 1334118 trunk/qpid/cpp/src/qpid/acl/management-schema.xml 1334118 trunk/qpid/cpp/src/qpid/broker/AclModule.h 1334118 trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1334118 trunk/qpid/cpp/src/tests/acl.py 1334118 trunk/qpid/cpp/src/tests/run_acl_tests 1334118 Diff: https://reviews.apache.org/r/5015/diff Testing ------- Unit tests included. Thanks, Chug
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/5015/#review7644
        -----------------------------------------------------------

        From a functionality pov, this is a very welcome addition. The max queue-size was really useless as a malicious user could create 1000s of queues still within the max size but creating havoc in the system!

        • rajith

        On 2012-05-04 19:41:45, Chug Rolke wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/5015/

        -----------------------------------------------------------

        (Updated 2012-05-04 19:41:45)

        Review request for qpid, Alan Conway, Kim van der Riet, and Ted Ross.

        Summary

        -------

        This patch fulfills a long-standing request to keep users from abusing broker queue resources. If a user is allowed to create one queue he then can create them by the thousdands.

        The code is more of a quota than an access control but it fits naturally in the current ACL module. The implementation here is queue-centric but could be generalized to support limiting exchanges as well.

        A few concerns arise:

        1. This code counts/protects live requests coming in to single node. This code does not protect queues that are presisting. The concern is that a user creates his quota of persistent queues and then upon system restart the same user can create another batch of queues since the persisted queues aren't tracked. Is this a vaild concern?

        2. The patch provides only a single setting for all users.

        3. The patch makes no effort to replicate the queue count state across a cluster. Surely this is a problem for clusters.

        This addresses bug QPID-2393.

        https://issues.apache.org/jira/browse/QPID-2393

        Diffs

        -----

        trunk/qpid/cpp/src/qpid/acl/Acl.h 1334118

        trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1334118

        trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp 1334118

        trunk/qpid/cpp/src/qpid/acl/management-schema.xml 1334118

        trunk/qpid/cpp/src/qpid/broker/AclModule.h 1334118

        trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1334118

        trunk/qpid/cpp/src/tests/acl.py 1334118

        trunk/qpid/cpp/src/tests/run_acl_tests 1334118

        Diff: https://reviews.apache.org/r/5015/diff

        Testing

        -------

        Unit tests included.

        Thanks,

        Chug

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5015/#review7644 ----------------------------------------------------------- From a functionality pov, this is a very welcome addition. The max queue-size was really useless as a malicious user could create 1000s of queues still within the max size but creating havoc in the system! rajith On 2012-05-04 19:41:45, Chug Rolke wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5015/ ----------------------------------------------------------- (Updated 2012-05-04 19:41:45) Review request for qpid, Alan Conway, Kim van der Riet, and Ted Ross. Summary ------- This patch fulfills a long-standing request to keep users from abusing broker queue resources. If a user is allowed to create one queue he then can create them by the thousdands. The code is more of a quota than an access control but it fits naturally in the current ACL module. The implementation here is queue-centric but could be generalized to support limiting exchanges as well. A few concerns arise: 1. This code counts/protects live requests coming in to single node. This code does not protect queues that are presisting. The concern is that a user creates his quota of persistent queues and then upon system restart the same user can create another batch of queues since the persisted queues aren't tracked. Is this a vaild concern? 2. The patch provides only a single setting for all users. 3. The patch makes no effort to replicate the queue count state across a cluster. Surely this is a problem for clusters. This addresses bug QPID-2393 . https://issues.apache.org/jira/browse/QPID-2393 Diffs ----- trunk/qpid/cpp/src/qpid/acl/Acl.h 1334118 trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1334118 trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp 1334118 trunk/qpid/cpp/src/qpid/acl/management-schema.xml 1334118 trunk/qpid/cpp/src/qpid/broker/AclModule.h 1334118 trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1334118 trunk/qpid/cpp/src/tests/acl.py 1334118 trunk/qpid/cpp/src/tests/run_acl_tests 1334118 Diff: https://reviews.apache.org/r/5015/diff Testing ------- Unit tests included. Thanks, Chug
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-05-07 15:43:12, Alan Conway wrote:

        > Definitely needs to replicate state in a cluster. Shout if you need pointers.

        This more of a general problem where ACL doesn't play well with the clustered setup.
        Perhaps we could work on a case by case for the time being to get certain functionality like this working.

        However longer term we need to find a way to ensure the ACL in-memory-model is replicated so any change done in one broker is relected on it's members.
        That would be the first step in allowing dynamic provisioning of rules.

        • rajith

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/5015/#review7640
        -----------------------------------------------------------

        On 2012-05-04 19:41:45, Chug Rolke wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/5015/

        -----------------------------------------------------------

        (Updated 2012-05-04 19:41:45)

        Review request for qpid, Alan Conway, Kim van der Riet, and Ted Ross.

        Summary

        -------

        This patch fulfills a long-standing request to keep users from abusing broker queue resources. If a user is allowed to create one queue he then can create them by the thousdands.

        The code is more of a quota than an access control but it fits naturally in the current ACL module. The implementation here is queue-centric but could be generalized to support limiting exchanges as well.

        A few concerns arise:

        1. This code counts/protects live requests coming in to single node. This code does not protect queues that are presisting. The concern is that a user creates his quota of persistent queues and then upon system restart the same user can create another batch of queues since the persisted queues aren't tracked. Is this a vaild concern?

        2. The patch provides only a single setting for all users.

        3. The patch makes no effort to replicate the queue count state across a cluster. Surely this is a problem for clusters.

        This addresses bug QPID-2393.

        https://issues.apache.org/jira/browse/QPID-2393

        Diffs

        -----

        trunk/qpid/cpp/src/qpid/acl/Acl.h 1334118

        trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1334118

        trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp 1334118

        trunk/qpid/cpp/src/qpid/acl/management-schema.xml 1334118

        trunk/qpid/cpp/src/qpid/broker/AclModule.h 1334118

        trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1334118

        trunk/qpid/cpp/src/tests/acl.py 1334118

        trunk/qpid/cpp/src/tests/run_acl_tests 1334118

        Diff: https://reviews.apache.org/r/5015/diff

        Testing

        -------

        Unit tests included.

        Thanks,

        Chug

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-05-07 15:43:12, Alan Conway wrote: > Definitely needs to replicate state in a cluster. Shout if you need pointers. This more of a general problem where ACL doesn't play well with the clustered setup. Perhaps we could work on a case by case for the time being to get certain functionality like this working. However longer term we need to find a way to ensure the ACL in-memory-model is replicated so any change done in one broker is relected on it's members. That would be the first step in allowing dynamic provisioning of rules. rajith ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5015/#review7640 ----------------------------------------------------------- On 2012-05-04 19:41:45, Chug Rolke wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5015/ ----------------------------------------------------------- (Updated 2012-05-04 19:41:45) Review request for qpid, Alan Conway, Kim van der Riet, and Ted Ross. Summary ------- This patch fulfills a long-standing request to keep users from abusing broker queue resources. If a user is allowed to create one queue he then can create them by the thousdands. The code is more of a quota than an access control but it fits naturally in the current ACL module. The implementation here is queue-centric but could be generalized to support limiting exchanges as well. A few concerns arise: 1. This code counts/protects live requests coming in to single node. This code does not protect queues that are presisting. The concern is that a user creates his quota of persistent queues and then upon system restart the same user can create another batch of queues since the persisted queues aren't tracked. Is this a vaild concern? 2. The patch provides only a single setting for all users. 3. The patch makes no effort to replicate the queue count state across a cluster. Surely this is a problem for clusters. This addresses bug QPID-2393 . https://issues.apache.org/jira/browse/QPID-2393 Diffs ----- trunk/qpid/cpp/src/qpid/acl/Acl.h 1334118 trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1334118 trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp 1334118 trunk/qpid/cpp/src/qpid/acl/management-schema.xml 1334118 trunk/qpid/cpp/src/qpid/broker/AclModule.h 1334118 trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1334118 trunk/qpid/cpp/src/tests/acl.py 1334118 trunk/qpid/cpp/src/tests/run_acl_tests 1334118 Diff: https://reviews.apache.org/r/5015/diff Testing ------- Unit tests included. Thanks, Chug
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/5015/#review7640
        -----------------------------------------------------------

        Definitely needs to replicate state in a cluster. Shout if you need pointers.

        trunk/qpid/cpp/src/qpid/broker/Broker.cpp
        <https://reviews.apache.org/r/5015/#comment16850>

        This doesn't count deletions of auto-delete queues. Perhaps better to put this check in Queue::destroyed

        trunk/qpid/cpp/src/tests/acl.py
        <https://reviews.apache.org/r/5015/#comment16851>

        No need to catch an exception just to fail the test as a result. Allow the exception to cause the test to fail directly, that gives you a deeper stack trace to debug.

        • Alan

        On 2012-05-04 19:41:45, Chug Rolke wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/5015/

        -----------------------------------------------------------

        (Updated 2012-05-04 19:41:45)

        Review request for qpid, Alan Conway, Kim van der Riet, and Ted Ross.

        Summary

        -------

        This patch fulfills a long-standing request to keep users from abusing broker queue resources. If a user is allowed to create one queue he then can create them by the thousdands.

        The code is more of a quota than an access control but it fits naturally in the current ACL module. The implementation here is queue-centric but could be generalized to support limiting exchanges as well.

        A few concerns arise:

        1. This code counts/protects live requests coming in to single node. This code does not protect queues that are presisting. The concern is that a user creates his quota of persistent queues and then upon system restart the same user can create another batch of queues since the persisted queues aren't tracked. Is this a vaild concern?

        2. The patch provides only a single setting for all users.

        3. The patch makes no effort to replicate the queue count state across a cluster. Surely this is a problem for clusters.

        This addresses bug QPID-2393.

        https://issues.apache.org/jira/browse/QPID-2393

        Diffs

        -----

        trunk/qpid/cpp/src/qpid/acl/Acl.h 1334118

        trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1334118

        trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp 1334118

        trunk/qpid/cpp/src/qpid/acl/management-schema.xml 1334118

        trunk/qpid/cpp/src/qpid/broker/AclModule.h 1334118

        trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1334118

        trunk/qpid/cpp/src/tests/acl.py 1334118

        trunk/qpid/cpp/src/tests/run_acl_tests 1334118

        Diff: https://reviews.apache.org/r/5015/diff

        Testing

        -------

        Unit tests included.

        Thanks,

        Chug

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5015/#review7640 ----------------------------------------------------------- Definitely needs to replicate state in a cluster. Shout if you need pointers. trunk/qpid/cpp/src/qpid/broker/Broker.cpp < https://reviews.apache.org/r/5015/#comment16850 > This doesn't count deletions of auto-delete queues. Perhaps better to put this check in Queue::destroyed trunk/qpid/cpp/src/tests/acl.py < https://reviews.apache.org/r/5015/#comment16851 > No need to catch an exception just to fail the test as a result. Allow the exception to cause the test to fail directly, that gives you a deeper stack trace to debug. Alan On 2012-05-04 19:41:45, Chug Rolke wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5015/ ----------------------------------------------------------- (Updated 2012-05-04 19:41:45) Review request for qpid, Alan Conway, Kim van der Riet, and Ted Ross. Summary ------- This patch fulfills a long-standing request to keep users from abusing broker queue resources. If a user is allowed to create one queue he then can create them by the thousdands. The code is more of a quota than an access control but it fits naturally in the current ACL module. The implementation here is queue-centric but could be generalized to support limiting exchanges as well. A few concerns arise: 1. This code counts/protects live requests coming in to single node. This code does not protect queues that are presisting. The concern is that a user creates his quota of persistent queues and then upon system restart the same user can create another batch of queues since the persisted queues aren't tracked. Is this a vaild concern? 2. The patch provides only a single setting for all users. 3. The patch makes no effort to replicate the queue count state across a cluster. Surely this is a problem for clusters. This addresses bug QPID-2393 . https://issues.apache.org/jira/browse/QPID-2393 Diffs ----- trunk/qpid/cpp/src/qpid/acl/Acl.h 1334118 trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1334118 trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp 1334118 trunk/qpid/cpp/src/qpid/acl/management-schema.xml 1334118 trunk/qpid/cpp/src/qpid/broker/AclModule.h 1334118 trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1334118 trunk/qpid/cpp/src/tests/acl.py 1334118 trunk/qpid/cpp/src/tests/run_acl_tests 1334118 Diff: https://reviews.apache.org/r/5015/diff Testing ------- Unit tests included. Thanks, Chug
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/5015/
        -----------------------------------------------------------

        Review request for qpid, Alan Conway, Kim van der Riet, and Ted Ross.

        Summary
        -------

        This patch fulfills a long-standing request to keep users from abusing broker queue resources. If a user is allowed to create one queue he then can create them by the thousdands.

        The code is more of a quota than an access control but it fits naturally in the current ACL module. The implementation here is queue-centric but could be generalized to support limiting exchanges as well.

        A few concerns arise:

        1. This code counts/protects live requests coming in to single node. This code does not protect queues that are presisting. The concern is that a user creates his quota of persistent queues and then upon system restart the same user can create another batch of queues since the persisted queues aren't tracked. Is this a vaild concern?

        2. The patch provides only a single setting for all users.

        3. The patch makes no effort to replicate the queue count state across a cluster. Surely this is a problem for clusters.

        This addresses bug QPID-2393.
        https://issues.apache.org/jira/browse/QPID-2393

        Diffs


        trunk/qpid/cpp/src/qpid/acl/Acl.h 1334118
        trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1334118
        trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp 1334118
        trunk/qpid/cpp/src/qpid/acl/management-schema.xml 1334118
        trunk/qpid/cpp/src/qpid/broker/AclModule.h 1334118
        trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1334118
        trunk/qpid/cpp/src/tests/acl.py 1334118
        trunk/qpid/cpp/src/tests/run_acl_tests 1334118

        Diff: https://reviews.apache.org/r/5015/diff

        Testing
        -------

        Unit tests included.

        Thanks,

        Chug

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5015/ ----------------------------------------------------------- Review request for qpid, Alan Conway, Kim van der Riet, and Ted Ross. Summary ------- This patch fulfills a long-standing request to keep users from abusing broker queue resources. If a user is allowed to create one queue he then can create them by the thousdands. The code is more of a quota than an access control but it fits naturally in the current ACL module. The implementation here is queue-centric but could be generalized to support limiting exchanges as well. A few concerns arise: 1. This code counts/protects live requests coming in to single node. This code does not protect queues that are presisting. The concern is that a user creates his quota of persistent queues and then upon system restart the same user can create another batch of queues since the persisted queues aren't tracked. Is this a vaild concern? 2. The patch provides only a single setting for all users. 3. The patch makes no effort to replicate the queue count state across a cluster. Surely this is a problem for clusters. This addresses bug QPID-2393 . https://issues.apache.org/jira/browse/QPID-2393 Diffs trunk/qpid/cpp/src/qpid/acl/Acl.h 1334118 trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1334118 trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp 1334118 trunk/qpid/cpp/src/qpid/acl/management-schema.xml 1334118 trunk/qpid/cpp/src/qpid/broker/AclModule.h 1334118 trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1334118 trunk/qpid/cpp/src/tests/acl.py 1334118 trunk/qpid/cpp/src/tests/run_acl_tests 1334118 Diff: https://reviews.apache.org/r/5015/diff Testing ------- Unit tests included. Thanks, Chug

          People

          • Assignee:
            Chuck Rolke
            Reporter:
            Armin Noll
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development