all lists on 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal