Samba4 Patch: newuseradv and newgroupadv scripts for net cmd utlity

[Jelmer Vernooij]

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?
Yep, thanks. My apologies for the delay, this is now merged into Git.

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/17517631/attachment.pgp>