← Back to team overview

cloud-init-dev team mailing list archive

Re: [Merge] lp:~juergh/cloud-init/add-sles-support into lp:cloud-init

 

Review: Needs Fixing

A few comments / requested fixes:
 - using 'passwd -l' is ok globally I think (rather than copying that)
   just put a comment in the parent class that the long format is not
   supported on suse. 'chpasswd -e' is ok too.
 
   I do prefer the long command names (as just now I had to look up
   what '-e' meant), but it reduces code duplication here.  Just add
   some comments.

   also, please leave in the 'lock_passwd' as that makes sense to
   be its own method i think.

 - "Copyright (C) 2013 SUSE LLC"
   We'll need the contributors agreement signed by the authoring party.
   http://www.canonical.com/contributors

 - '--no-gpg-checks'
   If 'zypper -t' is analagus to 'apt-get update', then you ignoring gpg
   checks is dangerous, as the content is pulled over non-secure transport
   so without a gpg check you're vulnerable to man in the middle.

   If the image creator has added additional repositories that they trust
   then they most certainly should have imported the gpg keys for those
   repositories as well.

   Did I miss something?


 - can we get packages/rpmb to work on suse? or should would it be better to
   do packages/suse-rpmb ?

Those are moderately small things.
The big thing is thank you for your contribution.

Scott


-- 
https://code.launchpad.net/~juergh/cloud-init/add-sles-support/+merge/171223
Your team cloud init development team is requested to review the proposed merge of lp:~juergh/cloud-init/add-sles-support into lp:cloud-init.


Follow ups