Skip to content

Experimental: Use zfs permissions instead of zfs shell and sudo#32

Open
MichaelHierweck wants to merge 3 commits intoRudd-O:masterfrom
MichaelHierweck:experimental-zfs-permissions-instead-of-zfs-shell
Open

Experimental: Use zfs permissions instead of zfs shell and sudo#32
MichaelHierweck wants to merge 3 commits intoRudd-O:masterfrom
MichaelHierweck:experimental-zfs-permissions-instead-of-zfs-shell

Conversation

@MichaelHierweck
Copy link
Contributor

Experimental: Use zfs permissions instead of zfs shell and sudo
Note: pr23 and pr24 have to be merged before.

@Rudd-O
Copy link
Owner

Rudd-O commented Dec 28, 2020

Can you look at the merge conflict fix and see if the code is adequate for merging? Thanks.

if self._dirty:
properties = [ 'creation' ] + self._properties
stdout2 = subprocess.check_output(self.command + ["zfs", "list", "-Hpr", "-o", ",".join( ['name'] + properties ), "-t", "all"])
stdout2 = subprocess.check_output(self.command + ["/sbin/zfs", "list", "-Hpr", "-o", ",".join( ['name'] + properties ), "-t", "all"])
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should not encode the path to zfs. Let the PATH resolution find it.

| <img width="164" height="164" title="" alt="" src="doc/bitcoin.png" /> |
| [1Cw9nZu9ygknussPofMWCzmSMveusTbQvN](bitcoin:1Cw9nZu9ygknussPofMWCzmSMveusTbQvN) |

## Experimental version of zfs-tools:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to maintain support for zfs-shell since not everyone is ready to migrate to ZFS permissions, and people deploy from this repo.

If needed, we can add to zfs-shell some sort of canary command that the client can then query, and if that fails, revert back to using zfs-shell instead of ZFS permissions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all, sorry, the last weeks I was too busy with other things .

I wonder how we could make zfs-shell optional.

Case "zfs-shell":
zfs-shell should be installed and registered as a shell. We might even need the sudo rules.
ztools should invoke zfs-shell as a helper script.

Case "zfs-permissions":
zfs-shell should not be installed (or at least) not registered. Sudo rules should not be installed or at least not activated.
ztools should invoke zpool and zfs directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about creating two Debian packages out of a single source package, e.g. "zfs-tools" and optional "zfs-shell" (providing zfs-shell and sudo rules). zfs-tools could suggest or even recommend zfs-shell.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Rudd-O
Copy link
Owner

Rudd-O commented Apr 29, 2021

Happy to take a look at a second round!

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