PR - Ticket 50151 - lib389 support cli add/replace/delete on objects

[389-ds-bot]

Cloned from Pagure Pull-Request: https://pagure.io/389-ds-base/pull-request/50158

• Created at 2019-01-15 03:10:40 by firstyear (@Firstyear)
• Merged at 2019-01-28 23:41:35

---

Bug Description: We need a generic way to add/replace/delete on
objects, that is not ldif. Ldif is wildly inaccessible and hard
to use.

Fix Description: Add a "modify" generic to cli_base, that is
used by user. It supports a syntax of:

modify <add|replace|delete>::

An example is:

... user modify demo_user add:objectclass:nsMemberOf

These can have many modifications in a single transaction:

user modify demo_user add:objectclass:nsMemberOf add:description:test

Resolves: #3210

Author: William Brown william@blackhats.net.au

Review by: ???

[389-ds-bot]

Comment from lkrispen (@elkris) at 2019-01-15 10:36:56

It would be nice to have this possibilty to use ldapmodify easily, but as far as I see the patch only covers a subset of what is possible with ldapmodify.

ldapmodify allows multiple values in one ADD/DEL/REPLACE operation, it also allows DEL/REPLACE without a value to completely remove an attribute.
I think the error handling based on the number of args shoul be done after determining the type and allow a list of values

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-01-15 23:45:37

So if I'm reading this, I think you are suggesting something like:

"add:attribute:value:value:value"? Is that what you mean here?

The del/replace with no value to remove an attribute however, is an oversight, so I can fix this to allow "delete:attr:" to remove an attribute completely.

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-01-18 00:16:46

@elkris What do you think of the above suggestion for the value list? :)

[389-ds-bot]

Comment from tbordaz (@tbordaz) at 2019-01-18 08:50:38

I will let @elkris comment but he may had in mind something like

dn: dc=foo19,dc=example,dc=com
changetype: modify
add: sn
sn: x3
sn: x4
-
delete: sn
sn: x1
-
replace: cn
cn: foo

Do you want the generic update to cover these kind of update ?

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-01-18 09:58:18

@tbordaz I'd ready like to avoid that syntax-style because it's basically "reinventing ldif", and it's a bit awkward. This tool is intended for quick single attribute or a few attribute changes, not large bulk updates like this. Part of the motivation is that asking people to provide a file, and then apply it, is a barrier to entry because the syntax of that file is foreign. But having a cli tool we get something easier and quicker to use, and then admins can "step up" to ldif if they require the complete ability for ldap to express complex changes like that.

Does this help explain a bit of my motivation?

[389-ds-bot]

Comment from tbordaz (@tbordaz) at 2019-01-18 10:16:59

@Firstyear, thanks for clarification. Supporting only 'simple' updates looks fine to me as it is likely the vast majority of admins need.

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-01-21 00:18:49

rebased onto 90d27f9ac3dd8d45dabdd4c1cec51f0c87848292

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-01-21 00:19:43

Delete now allows deleting all values with "delete:attr:". I think that we can do the multiple value replace with "delete:attr:" "add:attr:value" "add:attr:value2" instead rather than writing a multi-value parser.

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-01-23 15:39:09

I haven't tested but the code looks good.

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-01-23 23:56:30

@droideck Great thanks! I'll give @elkris another day to comment, and then I'll merge tomorrow if that's okay :)

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-01-24 00:53:28

@droideck Great thanks! I'll give @elkris another day to comment, and then I'll merge tomorrow if that's okay :)

I'd like for @mreynolds389 to check it too and leave his opinion...

It's a new functionality (which I agree that we'll need someday) but we will have it in master after the merge. And the master is going to the soonest RHEL release.
To me, it looks like it makes more sense to branch it (or postpone the merge) for now and to write a design doc for the new syntax.

But maybe I am overthinking the thing and Mark will correct me.

P.S. sorry if it's too much of a resistance. I really like the thing but we should be careful about what we deliver.

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-01-24 02:14:47

I'm on the fence about when to merge this. It does make sense to wait for 1.4.1 branch (which I plan to do soon). @Firstyear any objections waiting for 1.4.1?

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-01-24 03:13:03

@mreynolds389 Maybe this is a good time to test feature gating by version? We could have a flag for "if defaults. version >= 1.4.1" then to add the arg parse lines? This way we could merge now, and it would only unblock on the future version bump? I think we should consider this gating strategy now since we are running into it.

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-01-28 23:38:27

As @mreynolds389 has branched, I'll commit this now.

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-01-28 23:41:02

rebased onto 341eeab

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-01-28 23:41:36

Pull-Request has been merged by Firstyear

[389-ds-bot]

Patch
50158.patch