Thrift
  1. Thrift
  2. THRIFT-183

let non-root issues run fb303 status commands

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.1
    • Fix Version/s: 0.1
    • Component/s: None
    • Labels:
      None
    • Environment:

      all

    • Patch Info:
      Patch Available

      Description

      this patch allows non-root users to issue the following commands:
      counters
      name
      version
      alive
      status

      leaving 'stop' and 'reload' as root operations.
      It also provides a basic help/usage

      1. fb303.patch
        2 kB
        Ian Holsman

        Activity

        Hide
        Ian Holsman added a comment -

        patch to allow non-root users to run commands

        Show
        Ian Holsman added a comment - patch to allow non-root users to run commands
        Hide
        Mark Slee added a comment -

        I don't think this patch really belongs in the codebase. This is only a client-side script that you're protecting. This isn't affecting who actually has any privileges to make this type of call into the server. There's no real reason why root should be required to run fb303 management scripts. You may have your network set up this way, but it's completely reasonable for people to run Thrift services in non-root environments and want to administer them as such.

        So, this is a nice gatekeeper to block mistakes in some configurations, but an unnecessary limitation for non-root environments..

        Show
        Mark Slee added a comment - I don't think this patch really belongs in the codebase. This is only a client-side script that you're protecting. This isn't affecting who actually has any privileges to make this type of call into the server. There's no real reason why root should be required to run fb303 management scripts. You may have your network set up this way, but it's completely reasonable for people to run Thrift services in non-root environments and want to administer them as such. So, this is a nice gatekeeper to block mistakes in some configurations, but an unnecessary limitation for non-root environments..
        Hide
        Ian Holsman added a comment -

        Hi Mark.

        your mistaking the patch.

        as-is ALL commands are being blocked for non-root users, so this is relaxing that, not restricting it.

        I left the stop/reload commands as root-required as they could cause damage.

        Show
        Ian Holsman added a comment - Hi Mark. your mistaking the patch. as-is ALL commands are being blocked for non-root users, so this is relaxing that, not restricting it. I left the stop/reload commands as root-required as they could cause damage.
        Hide
        Mark Slee added a comment -

        Ack, yes. You're right I read this totally backwards. This change is fine then. And I would support a further change to remove the root requirement altogether if other people working on non-root environments would find that useful. But I guess there's no rush until someone specifically needs that.

        Show
        Mark Slee added a comment - Ack, yes. You're right I read this totally backwards. This change is fine then. And I would support a further change to remove the root requirement altogether if other people working on non-root environments would find that useful. But I guess there's no rush until someone specifically needs that.
        Hide
        Mark Slee added a comment -

        Committed revision 708364.

        Show
        Mark Slee added a comment - Committed revision 708364.

          People

          • Assignee:
            Unassigned
            Reporter:
            Ian Holsman
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development