Samba4 Patch: newuseradv and newgroupadv scripts for net cmd utlity

[Lukasz Zalewski]

On 20/06/2010 10:28, Jelmer Vernooij wrote:
> On Sun, 2010-06-20 at 13:31 +1000, Andrew Bartlett wrote:
>> On Sat, 2010-06-19 at 22:12 +0100, Lukasz Zalewski wrote:
>>> 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?
>>
>> I've looked over the tests, and I agree, they are the best that can be
>> managed.  I've not run it, but assuming it passes 'make test', I'm very
>> happy for this to be merged.
>>
>> Matthias, Can you handle this for me?
> I've pushed it yesterday evening.
>
> Cheers,
>
> Jelmer

Thank you all :)

Regards

Luk