Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PoC: sync: add sudo support #305

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

marco-m
Copy link

@marco-m marco-m commented Nov 17, 2021

NOTE

  • This is a PoC just to start a discussion on wether such feature could be interesting. It doesn't have tests and might require refactoring.
  • I understand that "Mutagen is currently in a state of high code churn and I simply don't have the time to review pull requests". I implemented this in my fork because I need it and I can understand you might not want it :-)

DESCRIPTION

Sometimes you need to sync files that the SSH user cannot modify, and the beta
endpoint doesn't have root SSH access.

If the SSH user has passwordless sudo, adding to mutagen.yml

sync:
  defaults:
    permissions:
      sudo: true

will invoke the mutagen agent with sudo.

Signed-off-by: Marco Molteni [email protected]

NOTE This is a PoC just to start a discussion on wether such feature could be
     interesting. It doesn't have tests and might require refactoring.

Sometimes you need to sync files that the SSH user cannot modify, and the beta
endpoint doesn't have root SSH access.

If the SSH user has passwordless sudo, adding to mutagen.yml

    sync:
      defaults:
        permissions:
          sudo: true

will invoke the mutagen agent with sudo.

Signed-off-by: Marco Molteni <[email protected]>
@xenoscopic
Copy link
Member

Thanks for the pull request. I'll leave it open for now as a discussion point about how elevated permissions could be acquired, because I think it might possibly be important for fanotify support and possibly the larger discussion of sudo-based operations.

Not really a full review, but my initial thoughts are:

  1. For your own needs/maintenance (i.e. to avoid breaks in a rebase or even the need for a fork), it might be easier to accomplish this with ssh/scp wrapper scripts (injected via MUTAGEN_SSH_PATH) that understand something like username+sudo@host to indicate that sudo should be used. Mutagen doesn't parse the username component of SSH URLs, it just looks for an @ to split on, so there's some flexibility on the exact format.
  2. In that same spirit, it might make sense to solve this as part of a "custom transport" solution, which is something that's been discussed in the past. This particular case is close enough to normal SSH that it can be solved with wrappers, but that's obviously not the case for everything.
  3. I don't know how/if agent installation should be affected by the use of sudo.
  4. In general, if Mutagen were to support this natively, it would probably be easier to store a "use sudo" flag as a URL parameter since this is more transport-related than behavior-related, perhaps specified by the user in a similar manner to that mentioned in 1. The master branch adds a parameters field to URL to encode transport-specific behaviors. This is currently used for things like Docker CLI configuration, but eventually I'd like to add things like SSH configuration paths and SSH options to this map, so sudo configuration might be easier to store there since it would require less threading of arguments through the protocol/transport stack.

I don't see merging this into mainline Mutagen at the moment, mostly just due to the added maintenance burden and the support requirements (since sudo will likely open up a number of support questions). I'm also worried that people will use it as a bit of a sledgehammer for permissions issues (though I certainly concede that there are genuine use cases for it).

If this could be done via ssh/scp wrappers in a fairly robust way, it would probably make a good sister project (especially since users could easily opt-in to using the functionality by setting MUTAGEN_SSH_PATH and it wouldn't require any modification to Mutagen itself).

@marco-m
Copy link
Author

marco-m commented Nov 20, 2021

@xenoscopic I appreciate your thorough reasonings and I have to agree on all aspects, unfortunately also on the "sledgehammer" worry, I was worried myself. I will reflect on how to go for the next step, thanks to your suggestions.

Where would you prefer to continue the discussion? Here on in another place? I was a bit worried about Slack, because it is unsearchable and messy, while in a PR at least the focus stays on one thing. Or should I close this and open an issue?

@xenoscopic
Copy link
Member

Let's keep the discussion here for now, especially since it's a good discussion to preserve for posterity.

Just to clarify: this only works with passwordless sudo enabled, correct?

@marco-m
Copy link
Author

marco-m commented Nov 23, 2021

Let's keep the discussion here for now, especially since it's a good discussion to preserve for posterity.

Ok

Just to clarify: this only works with passwordless sudo enabled, correct?

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants