[pkg-eucalyptus-maintainers] your mail

[Rudy Godoy]

Hi,

On Thu, Jul 26, 2012 at 08:05:29PM -0400, Brian Thomason wrote:
> Hi Luca,
> 
> First off, thanks to Steffen for forwarding this as I was not subscribed to
> the pkg-eucalyptus-maintainers list, just pkg-eucalyptus O_O.
> 
> Second, big thanks to you Luca (and the unnamed trainee!)
> 
> I will comment inline below:
> 
> 
> source:
> =======
>  * libhamcrest1.2-java | libhamcrest-java (>=1.2)
>    The former is Ubuntu-only, Debian has libhamcrest-java only. This
>    would result in autobuilders failing to build the source if they do
>    not resolve alternatives in build-deps. Would you mind switching the
>    order of the two build-dependencies?
> 
> 
> 
> Yes, this was done for Ubuntu - not a problem.  I will change this straight
> away.
> 
> 
> copyright file
> ==============
>   debian/copyright has a License: Apache section, where the license
>   itelf is Apache-2.0. The short license abbreviationshould be
>   Apache-2.0 too, according to my reading of the machine readable
>   copyright format.
> 
> 
> Rudy added this - I'll ping him about it.
> 
> 
> eucalyptus-cc
> =============
> * Depends on dhcp3-server, which is a transitional package, which was
>   aiding in the lenny->squeeze upgrade.
>   The correct dependency would be isc-dhcp-server.
> 
> 
> Yes, this another holdover for Lucid compat; Will change it.
> 
> 
> eucalyptus-admin-tools
> ======================
> * Lots of missing man pages. Are there plans to provide some?
> 
> 
> At the moment, no :-(
> 
> 
> eucalyptus-cloud
> ================
> 
> Recommends postfix | m-t-a. While minor, I believe it's good manners
> to recommend the default MTA first. But that's arguable, and most
> definitely a minor thing.
> 
> 
> Worksforme, will change it.
> 
> 
> eucalyptus-java-common
> ======================
> 
> Depends on libhamcrest1.2-java | libhamcrest-java (>= 1.2), probably
> for Ubuntu compatibility. The first is not satisfiable by debian,
> though. While it will work, as apt will resolve alternatives, it's
> still nicer to list the only valid candidate first.
> 
> 
> Same ordering as the build-dep above - will change it.
> 

Those above are already fixed and available on the pkg-eucalyptus repo.


> 
> Looking at this snippet:
> 
>   if dpkg --compare-versions "$2" lt "3.0~"; then
>       if [ -f /tmp/eucaback.dir ]; then
>  BACKDIR=`cat /tmp/eucaback.dir`
>  if [ -d "$BACKDIR" ]; then
>               if [ -f "$BACKDIR/etc/eucalyptus/eucalyptus-version" -a -f
> "/etc/eucalyptus/eucalyptus-version" ]; then
>  export OLDVERSION=`cat $BACKDIR/etc/eucalyptus/eucalyptus-version`
>  export NEWVERSION=`cat /etc/eucalyptus/eucalyptus-version`
>  if [ "$OLDVERSION" != "$NEWVERSION" ]; then
>                       rm -f
> /usr/share/eucalyptus/eucalyptus-*$OLDVERSION*.jar
>  fi
>               fi
>  fi
>       fi
>   fi
> 
> A bigger problem is that *any* user can write to /tmp/eucaback.dir,
> and using the tempfile this way might open up the postinst to
> attack. So, it is an unsafe use, although differently so than the
> common case, I believe.
> 
> 
> I had worried about this section :-)  This is hairy upgrade code that
> precedes my time here.  It isn't safe, as you mention, but not a true race
> condition threat either as we simply read the file created there. Even if a
> bogus file is inserted there, our upgrade procedure would simply abort.  I
> hesitated to remove this only because, despite it being hairy, has been
> tested for quite some time and I didn't want to introduce the risk of
> breaking the upgrade process by improving the packaging.  Is this a must?
>  If it is, I already have untested code to fix it.
> 

Seconded. The code doesn't look complex, we could just sort of switch dirs, 
but it's hairy indeed. 

> 
> Why is /usr/sbin/eucalyptus-cloud in this package, and not in
> eucalyptus-cloud?
> 
> 
> This is an unfortunate side effect of the fact that the init script
> eucalyptus-cloud not only controls the cloud process, but also the nc, cc,
> etc...  It could have been placed in -common, but was placed in -java as it
> depends on JARs located there.  Unfortunately, this can't be mitigated
> easily :-(
> 
> 
> Also, could you comment about the following lintian warnings?
> 
> W: eucalyptus-java-common: codeless-jar
> usr/share/eucalyptus/eucalyptus-bootstrap-3.1.0.jar
> 
> 
> I'll have to check with engineering on this one.
> 
> 
> W: eucalyptus-java-common: missing-classpath libpostgresql-jdbc-java,
> libjboss-common-java, libhibernate-commons-annotations-java,
> libactivemq-java, libasm3-java, libavalon-framework-java, libaxiom-java,
> libbackport-util-concurrent-java, libbatik-java, libbcel-java,
> libbcprov-java, libbsf-java, libcommons-beanutils-java,
> libcommons-cli-java, libcommons-codec-java, libcommons-collections3-java,
> libcommons-digester-java, libcommons-discovery-java,
> libcommons-fileupload-java, libcommons-httpclient-java, libcommons-io-java,
> libcommons-jxpath-java, libcommons-lang-java, libcommons-logging-java,
> libcommons-pool-java, libdnsjava-java, libdom4j-java, libehcache-java,
> libexcalibur-logkit-java, libezmorph-java,
> libgeronimo-activation-1.1-spec-java, libgeronimo-ejb-3.0-spec-java,
> libantlr-java, libgeronimo-javamail-1.4-provider-java,
> libgeronimo-javamail-1.4-spec-java, libgeronimo-jms-1.1-spec-java,
> libgeronimo-jpa-2.0-spec-java, libgeronimo-jta-1.1-spec-java,
> libgeronimo-stax-1.2-spec-java, libguava-java, libhamcrest1.2-java,
> libhamcrest-java, libhsqldb-java, libitext-java, libjavassist-java,
> libjaxen-java, libjaxp1.3-java, libjboss-marshalling-java,
> libjcip-annotations-java, libjettison-java, libjetty-extra-java,
> libjetty-java, libjgroups-java, libjibx1.2-java, libjna-java, libjsch-java,
> libjson-java, libjug-java, liblog4j1.2-java, libmule-java,
> libnetty3.1-java, libnetty-java, libproxool-java, libquartz-java,
> libregexp-java, libservlet2.5-java, libslf4j-java, libspring-beans-java,
> libspring-context-java, libspring-context-support-java,
> libspring-core-java, libspring-web-java, libstax2-api-java, libwsdl4j-java,
> libwss4j-java, libxalan2-java, libxerces2-java, libxml-security-java,
> libxom-java, libxpp3-java, libaxis-java, libhibernate3-java,
> libcommons-compress-java, libbtm-java, libjasperreports3.7-java,
> libwoodstox-java, libcglib-java, libclean-crypto-java,
> libgeronimo-j2ee-connector-1.5-spec-java, libjboss-cache3-java,
> libha-jdbc-java, libgroovy1.7.2-java, libgroovy1.7-java,
> libhibernate-jbosscache-java, libhibernate-validator-java,
> libspring-expression-java
> 
> 
> I'm showing my Java ignorance here as I'm not sure how to remedy this or I
> would have.  Is this a showstopper?  Rudy, any ideas here?
> 

The package only holds few jar files and a war for the web admin
interface. The issue here is related to those files not having a
classpath in MANIFEST. However, since they are called by euca
programs classpath is set on that call (AFAIK). Not big issue
from Debian's POV.

> 
> W: eucalyptus-java-common:
> possibly-insecure-handling-of-tmp-files-in-maintainer-script postinst:14
> 
> 
> Same issue here - part of the hairy upgrade process.
> 
> 
> eucalyptus-walrus
> =================
> 
> This has a heavy set of dependencies (on lvm2 and drbd8-utils), but
> the effective contents are a drbd config example. Are there some missing
> files from this binary?
> 
> 
> No, by design (I'm not saying it's a good one) all JARs are thrown into
> java-common as even though Walrus may not be installed on a given machine
> (i.e.e the lvm2 and drbd8-utils packages aren't needed there) the JARs are
> still needed by other components to communicate with Walrus.  Again, this
> is not one that will be easily mitigated :-(
> 
> 
> eucalpytus-nc
> =============
> 
> N: helper, will never have manpage
> O: eucalyptus-nc: binary-without-manpage usr/sbin/euca_test_nc
> 
> If it's a helper, why is it in usr/sbin, and not under
> /usr/lib/eucalyptus or some other private directory?
> 
> 
> Another that I will have to check with engineering on as it was added
> before my time.  I see no reason in it being there either though as we
> sorely lack manpages and it is something we need to address at some point.
> 
> 
> eucalyptus-common
> =================
> 
> It is a little bit uncommon to have a -common package to be arch:any, this
> is
> probably because of the /usr/lib/eucalyptus/euca_* binaries.
> 
> W: eucalyptus-common:
> possibly-insecure-handling-of-tmp-files-in-maintainer-script postrm:4
> W: eucalyptus-common:
> possibly-insecure-handling-of-tmp-files-in-maintainer-script postinst:27
> W: eucalyptus-common:
> possibly-insecure-handling-of-tmp-files-in-maintainer-script preinst:11
> 
> If one wants to back up the data, back it up under /var seems a much
> better option than the one implemented here.
> 
> 
> Correct, it is marked any (should technically probably be amd64 as we don't
> certify on any other platform, even i386) due to the binaries that are
> needed by the various processes.  As for the tmp stuff, this was addressed
> above.
> 
> 
> Misc
> ====
> 
> By the way, someone else filed an ITP: eucalyptus bug recently:
>  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=680589
> 
> 
> Apologies here.  Rudy thought I had filed one but I hadn't, so he filed one
> after the fact just to ensure one existed.  Better communication on our
> team is definitely needed.  Please comment on the upgrade scenario response
> I provided above.
> 

That was me indeed. Steffen already commented on that. 

Regards

-- 
Rudy Godoy
http://stone-head.org