PR - Issue 50545 - Port dbgen.pl to dsctl

[389-ds-bot]

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

• Created at 2020-04-09 23:47:33 by mreynolds (@mreynolds389)
• Merged at 2020-04-24 16:14:38

---

Description: Ported the main features to lib389 and added some other useful features:

          Now there are several LDIFs that can be created:

          - User LDIFs (different types)
          - Group LDIFs
          - COS LDIFs
          - Role LDIFs
          - Modification LDIFs
          - Nested LDIFs

    There is also an interactive mode which makes some of this much easier to understand.  The interactive mode is initiated by not providing any arguments to the subcommand,  See the design doc for more info.

Design Doc: https://www.port389.org/docs/389ds/design/dbgen-design.html

fixes: Resolves: #3601

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2020-04-14 07:55:40

maybe for the cli we call it ldifgen not dbgen?

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2020-04-14 07:56:10

Besides that one naming comment, it otherwise looks good to me. :) Great to see this nearly done!

[389-ds-bot]

Comment from spichugi (@droideck) at 2020-04-14 11:06:09

Could you please take a look at the tests that use lib389.dbgen? They are failing now...

tests/suites/mapping_tree/referral_during_tot_init_test.py:17:from lib389.dbgen import dbgen
tests/suites/basic/basic_test.py:21:from lib389.dbgen import dbgen
tests/suites/syntax/mr_test.py:5:from lib389.dbgen import dbgen
tests/suites/import/regression_test.py:13:from lib389.dbgen import dbgen
tests/stress/search/simple.py:11:from lib389.dbgen import dbgen

[389-ds-bot]

Comment from spichugi (@droideck) at 2020-04-14 11:16:15

Why not get the props from get_input() here too? I think it shouldn't hurt... Or do I miss something?

[389-ds-bot]

Comment from spichugi (@droideck) at 2020-04-14 11:23:05

It gives Error: [Errno 13] Permission denied: '/tmp/dbgen.ldif' error when /tmp/dbgen.ldif already exists.
Maybe the tool should ask something like '/tmp/dbgen.ldif' already exists. Do you want to overwrite it? Or something like this...

[389-ds-bot]

Comment from spichugi (@droideck) at 2020-04-14 11:27:29

Another a bit odd behaviour that I found is when we specify optional arguments like

dsctl localhost dbgen groups --number 10 --suffix dc=example,dc=com my_group

then dbgen just silently assumes that other arguments should be default ones.

It may be intended but then we shouldn't do that silently, in my opinion...
Or, as an option, we should ask about the non-specified arguments.

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2020-04-14 14:17:49

maybe for the cli we call it ldifgen not dbgen?

Hahaha, my tool I ported most of this from is call "ldif-gen.py", so yes I will gladly make that change :-)

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2020-04-14 15:35:51

Another a bit odd behaviour that I found is when we specify optional arguments like
dsctl localhost dbgen groups --number 10 --suffix dc=example,dc=com my_group

then dbgen just silently assumes that other arguments should be default ones.
It may be intended but then we shouldn't do that silently, in my opinion...
Or, as an option, we should ask about the non-specified arguments.

Not sure how I feel about this request. You should not have to specify 10 options if you just want to quickly create a basic group with commonly used attributes. I'm trying to make the tool really easy/simple and fast. So, if you want to tweak the default LDIF you can use the other options. Or, if you want that fine grained control you could use the interactive mode which asks for everything.

[389-ds-bot]

Comment from spichugi (@droideck) at 2020-04-14 16:12:44

Not sure how I feel about this request. You should not have to specify 10 options if you just want to quickly create a basic group with commonly used attributes. I'm trying to make the tool really easy/simple and fast. So, if you want to tweak the default LDIF you can use the other options. Or, if you want that fine grained control you could use the interactive mode which asks for everything.

Maybe we can print the defaults that were used? Just one simple line.
It will be more explicit for the user. Which never hurts.

And if the user doesn't like the options he can regenerate the thing right away (basically, in the end, it is for speeding up the process)

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2020-04-14 16:44:15

Not sure how I feel about this request. You should not have to specify 10 options if you just want to quickly create a basic group with commonly used attributes. I'm trying to make the tool really easy/simple and fast. So, if you want to tweak the default LDIF you can use the other options. Or, if you want that fine grained control you could use the interactive mode which asks for everything.

Maybe we can print the defaults that were used? Just one simple line.
It will be more explicit for the user. Which never hurts.

Isn't that what "--help" is for? :-)

And if the user doesn't like the options he can regenerate the thing right away (basically, in the end, it is for speeding up the process)

Again isn't this what the interactive mode is for? The non-interactive mode should just run and assume you know what you want to do. I do not want to make the non-interactive mode interactive.

So... I'm not rejecting what you are saying, but I don't think I am visualizing what you want it to look like. Maybe you could write up some mock terminal output, or something, and show me how you think it should it behave? Thanks!!

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2020-04-14 17:45:51

rebased onto dfd34337bdff1889fb4bb3a031107fb37ff7147b

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2020-04-14 17:47:05

Fixed all of other issues in the meantime:

• Improved LDIF file validation
• Fixed CI tests
• Fixed nested LDIF entries (nsContainer -> organizationalUnit)

[389-ds-bot]

Comment from spichugi (@droideck) at 2020-04-15 01:27:00

Not sure how I feel about this request. You should not have to specify 10 options if you just want to quickly create a basic group with commonly used attributes. I'm trying to make the tool really easy/simple and fast. So, if you want to tweak the default LDIF you can use the other options. Or, if you want that fine grained control you could use the interactive mode which asks for everything.
Maybe we can print the defaults that were used? Just one simple line.
It will be more explicit for the user. Which never hurts.

Isn't that what "--help" is for? :-)

Okay:) I was thinking more and I think it's okay but we should make sure that the --help has all of the defaults in the text.
For example, we can run like this:

 dsctl localhost ldifgen groups --suffix dc=example,dc=com my_group  
 Writing LDIF file /tmp/dbgen.ldif ...
 Successfully created LDIF file: /tmp/dbgen.ldif

But the --help doesn't have a default for the number (and there are more cases like that)

 dsctl localhost ldifgen groups --suffix dc=example,dc=com my_group -h
 ...
 --number NUMBER       The number of groups to create.
 ...

---

Also, I am a bit confused here:

dsctl localhost ldifgen users --suffix dc=example,dc=com
Missing required parameters, switching to Interactive mode ...
Enter the suffix [dc=example,dc=com]:

Because --help doesn't have any mentioning about these required parameters. So I don't know what I missed (if I want a non-interactive mode)

dsctl localhost ldifgen users -h
usage: dsctl [instance] ldifgen users [-h] [--number NUMBER] [--suffix SUFFIX]
                                  [--parent PARENT] [--generic]
                                  [--start-idx START_IDX] [--rdn-cn]
                                  [--localize] [--ldif-file LDIF_FILE]

optional arguments:
-h, --help            show this help message and exit
--number NUMBER       The number of users to create.
--suffix SUFFIX       The database suffix where the entries will be created.
...

And if the user doesn't like the options he can regenerate the thing right away (basically, in the end, it is for speeding up the process)

So... I'm not rejecting what you are saying, but I don't think I am visualizing what you want it to look like. Maybe you could write up some mock terminal output, or something, and show me how you think it should it behave? Thanks!!

I thought about something like this:

dsctl localhost ldifgen users  --suffix dc=example,dc=com --number=10
Non-interactive mode is used.
Setting the defaults: parent='ou=people,dc=example,dc=com', rdn-cn='yes', etc.
Writing LDIF file /tmp/dbgen.ldif ...
Successfully created LDIF file: /tmp/dbgen.ldif

[389-ds-bot]

Comment from spichugi (@droideck) at 2020-04-15 01:40:43

If I understand correctly, get_ldif_file_input returns NoneType so args.ldif_file becomes corrupted if we go with the interactive option

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2020-04-15 02:09:59

It gives Error: [Errno 13] Permission denied: '/tmp/dbgen.ldif' error when /tmp/dbgen.ldif already exists.
Maybe the tool should ask something like '/tmp/dbgen.ldif' already exists. Do you want to overwrite it? Or something like this...

Simple option here, why not write to the ldif dir of the related instance? If we plan to import it to that instance, then this would make the most sense because it then isolates between instances, and means the instance can actually read the ldif dir (a common issue with the import from say /root or /tmp is the dirsrv user of the instance can't read the directory the ldif is in ...)

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2020-04-15 02:10:56

Sorry to follow up, imagine this case:

instanceA = dbgen
instanceB= dbgen
instanceA.import_from_ldif
instanceb.import_from_ldif

The second dbgen would clobber the first. So if we had parallel tests or anything else it could destroy evidence or import the wrong data ....

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2020-04-15 15:36:59

It gives Error: [Errno 13] Permission denied: '/tmp/dbgen.ldif' error when /tmp/dbgen.ldif already exists.
Maybe the tool should ask something like '/tmp/dbgen.ldif' already exists. Do you want to overwrite it? Or something like this...

Simple option here, why not write to the ldif dir of the related instance? If we plan to import it to that instance, then this would make the most sense because it then isolates between instances, and means the instance can actually read the ldif dir (a common issue with the import from say /root or /tmp is the dirsrv user of the instance can't read the directory the ldif is in ...)

Well not all of these LDIFs are designed to be imported. Groups, COS, Roles and modification ldifs are design to be used by ldapmodify, not ldif2db. And smaller user LDIF's could also be added using ldapmodify. Now I did think about using the server's LDIF directory since that information is available, but I also want all LDIF options to be consistent and forcing all LDIFs into the server's LDIF directory is not something I want to do, especially since this is all for testing and not for production, and most of the LDIF options are not meant for ldif2db.

Now in the interactive mode I added checks to see if the file can be written, if it can't it keeps prompting for a valid path/name. So permission issues and invalid paths are covered in the interactive mode.

Anyway let me work on all of this and I'll try and come up with something that works for all the cases. :-)

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2020-04-23 19:22:19

rebased onto 2d9263020be4b14953bd26419293ac750b35a892

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2020-04-23 19:23:24

All changes applied @droideck and @Firstyear, please review...

[389-ds-bot]

Comment from spichugi (@droideck) at 2020-04-24 00:51:14

Looks really good! You have my ack, at least. :)

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2020-04-24 02:56:57

It's a yay from me.

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2020-04-24 16:13:32

rebased onto 326be2c

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2020-04-24 16:14:38

Pull-Request has been merged by mreynolds389

[389-ds-bot]

Patch
51022.patch