public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Dominik Csapak <d.csapak@proxmox.com>
Subject: Re: [pve-devel] [PATCH access-control/manager v2] fix #3668: improving realm sync
Date: Tue, 22 Mar 2022 14:44:05 +0100	[thread overview]
Message-ID: <50f39747-1685-92d4-ae3e-5cfe5c288776@proxmox.com> (raw)
In-Reply-To: <d2fa2293-72d8-e9c8-96e6-b85664557f93@proxmox.com>

On 22.03.22 07:11, Thomas Lamprecht wrote:
> On 04.02.22 15:24, Dominik Csapak wrote:
>> this deprecates the 'full' sync option and replaces it with
>> a 'mode' option, where we add a third one that updates
>> the current users (while retaining their custom set attributes not
>> exisiting in the source) and removing users that don't exist anymore
>> in the source
>>
> I'm not yet 100% sure about the specific mode names, as sync normally means
> 100% sync, I'll see if I find some other tool (rsync?) with similar option naming
> problems. Independent from the specific names, this really needs a docs patch,
> ideally with a table listing the modi as rows and having the various "user added",
> "user removed", "properties added/updated", "properties removed" as columns, for a
> better understanding of the effects..
> 
A thought (train): what we decide with this isn't what gets added/updated, that's
always the same, only what gets removed if vanished on the source, so maybe:

remove-vanished: < none | user | user-and-properties >

Or if we can actually also remove either user *or* group then: s/user/entity/ ?

ps. the web interface should probably do a s/Purge/Purge ACLs/ too; or with that
in mind we could actually drop that do and have:

remove-vanished: < none | user | user-and-properties | user-and-properties-and-acl >

And with that, we could go the separate semicolon-endcoded-flag-list like we do for
some CT features (or mount options) IIRC:

remove-vanished: [<user>];[<properties>];[acls]

I.e., those three flags would replace your new mode + purge like:

+--------+--------+---------------------+
|  Mode  | Purge  | -> removed-vanished |
+--------+--------+---------------------+
| update |      0 | "" (none)           |
| sync   |      0 | user                |
| full   |      0 | user;properties     |
| update |      1 | acl                 |
| sync   |      1 | acl;user            |
| full   |      1 | acl;user;properties |
+--------+--------+---------------------+

The selector for them could be either three check boxes on one line (similar to the
privilege level radio buttons from CT restore) or even a full blown combobox with all
the options spelled out.

It's only slightly weird for acl, as there the "remove-vanished" somewhat implies that
we import acl's in the first place, if we really don't want that we could keep
"Purge ACLs" as separate option that is only enabled if "remove-vanished" "user" flag
is set, put IMO not _that_ of a big problem to understand compared to the status quo.

Does (any of) this make sense to you?




  reply	other threads:[~2022-03-22 13:44 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-04 14:24 Dominik Csapak
2022-02-04 14:24 ` [pve-devel] [PATCH access-control v2 1/2] realm-sync: replace 'full' option with 'mode' Dominik Csapak
2022-02-04 14:25 ` [pve-devel] [PATCH access-control v2 2/2] fix #3668: realm-sync: add mode 'sync' Dominik Csapak
2022-02-04 14:25 ` [pve-devel] [PATCH manager v2 1/1] ui: realm sync: replace 'full' with 'mode' Dominik Csapak
2022-03-22  6:11 ` [pve-devel] [PATCH access-control/manager v2] fix #3668: improving realm sync Thomas Lamprecht
2022-03-22 13:44   ` Thomas Lamprecht [this message]
2022-03-22 15:23     ` Dominik Csapak
2022-03-23  7:33       ` Thomas Lamprecht
2022-03-23  8:21         ` Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50f39747-1685-92d4-ae3e-5cfe5c288776@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal