Samba4 Patch: newuseradv and newgroupadv scripts for net cmd utlity

[Jelmer Vernooij]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20100620/c552a751/attachment.pgp>