Samba4 Patch: newuseradv and newgroupadv scripts for net cmd utlity

[Lukasz Zalewski]

On 13/06/2010 11:14, Lukasz Zalewski wrote:
> On 13/06/2010 05:56, Andrew Bartlett wrote:
>> On Sun, 2010-06-13 at 01:39 +0100, Lukasz Zalewski wrote:
>>> On 12/06/2010 13:25, Andrew Bartlett wrote:
>>>> On Tue, 2010-06-08 at 20:47 +0100, Lukasz Zalewski wrote:
>>>>> Hi Jelmer,
>>>>>
>>>>> On 08/06/2010 19:39, Jelmer Vernooij wrote:
>>>>>> Hi Lukasz,
>>>>>>
>>>>>> On Mon, 2010-06-07 at 17:29 +0100, Lukasz Zalewski wrote:
>>>>>>> Attached is a new patch which is an extension to the previous
>>>>>>> newuser
>>>>>>> functionality. It also adds the set of group commands (operating
>>>>>>> on the ldb)
>>>>>>>
>>>>>>> Now, new user is created with CN='Firstname Initials.
>>>>>>> Lastname',<> if
>>>>>>> any of them exists and --use-username-as-cn is not set (this is
>>>>>>> the AD
>>>>>>> way) otherwise CN=username,<> is used
>>>>>>> If --use-username-as-cn is defined then the user is created with
>>>>>>> CN=username,<> even if any of (Firstname, Initials,Lastname) exist
>>>>>>>
>>>>>>> I also took liberty to add group set of subcommands, i.e.
>>>>>>> /usr/local/samba/bin/net group
>>>>>>> Available subcommands:
>>>>>>> addmembers
>>>>>>> add
>>>>>>> removemembers
>>>>>>> delete
>>>>>>>
>>>>>>> These only operate on ldb at the moment.
>>>>>> Thanks! This looks very nice overall. Some minor comments:
>>>>> Thx for all of the comments. I have attached modified patches
>>>>>>
>>>>>> * PEP8 - please use two empty lines between top-level elements
>>>>>> (such as
>>>>>> classes).
>>>>> I hope this is fixed - I'm quite new to python
>>>>>>
>>>>>> * When dealing with transactions it's usually better to delay
>>>>>> starting
>>>>>> the transaction as long as possible (where it makes sense, of
>>>>>> course).
>>>>>> This makes the code more readable (less indentation). E.g. at the
>>>>>> bottom
>>>>>> of your patch you create a ldbmessage after starting a transaction
>>>>>> rather than just for the self.add call. There's no need to go
>>>>>> through a
>>>>>> lot of trouble to do this but if it's easy it's preferable.
>>>>> newuser and newgroup functions corrected.
>>>>>>
>>>>>> * Any reason for using cmp() rather than == when comparing two
>>>>>> strings ?
>>>>>> The latter is more readable and more commonly used.
>>>>> No reason, except old habbit of using strcmp i guess ;)
>>>>>>
>>>>>> Cheers,
>>>>>>
>>>>>> Jelmer
>>>>> Let me know if i need to make any more changes
>>>>
>>>> There is one more thing that would be really useful. If you look at
>>>> setup/tests/blackbox_newuser.sh, we have a test for the old version of
>>>> this script. If you can extend it to cover all the new options you have
>>>> here, that would help ensure it keeps working.
>>>>
>>>> Also make sure that the tree, with this patch in, passes 'make test'
>>>> just as much as without it.
>>>>
>>>> Thanks,
>>>>
>>>> Andrew Bartlett
>>>>
>>> Hi Andrew,
>>>
>>> I have almost finished the test suites, however i have a question - how
>>> does one pass switches with spaces to testit function, e.g.
>>> --my-switch="My value with spaces"
>>> I have tried escaping adding quotes to no success, i.e. its never
>>> treated as a single parameter
>>> for example test script fails with the following (i have added an echo
>>> line to testit function):
>>> test: group addmembers
>>> ./bin/net group addmembers --configfile=sh/simple-dc/etc/smb.conf dsg
>>> "User UT. Tester,User1 UT. Tester"
>>> failure: group addmembers [
>>> Usage: net group addmembers<groupname> <listofmembers>
>>> ]
>>>
>>> but when run on the cmd line
>>> ./bin/net group addmembers --configfile=sh/simple-dc/etc/smb.conf dsg
>>> "User UT. Tester,User1 UT. Tester"
>>> all is fine
>>
>> Yeah, shell quoting sucks - we really need to redo all these scripts in
>> python.
>>
>> Have you tried using ' or \' rather than "?
>>
>> Otherwise, look at all the blackbox scripts in the tree and see if
>> another already solves this.
>>
>> Andrew Bartlett
>>
>
> Hi Andrew,
> Tried various combinations of ", \" and ' with no success :( As a
> workaround i have made all the parameters that i could without spaces. I
> had to comment our two tests (add/remove group members through their cn)
> as they could not be shortened. I have attached new patch.
> Please let me know your comments
>
> Regards
>
> Luk

Andrew, Jelmer
Did you get a chance to look at the added test? are they adequate?

Regards

Luk