[MIR][SRU] new package - joyent-mdata-client

Bug #1248000 reported by Ben Howard on 2013-11-05
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Debian
New
Unknown
joyent-mdata-client (Ubuntu)
Medium
Ben Howard
Precise
Medium
Ben Howard
Quantal
Medium
Ben Howard
Raring
Medium
Ben Howard
Saucy
Medium
Ben Howard
Trusty
Medium
Ben Howard

Bug Description

[Background information]: Joyent is an IaaS vendor offering compute instances and object storage. Joyent is the sponsor of the SmartOS project and uses SmartOS for their compute offering. Each compute instance uses a serial console for a meta-data service. The joyent-mdata-client tools are required to enable two-way communication with the meta-data service.

[Availability]: This is a new package for cloud enablement. In order for a package to be included on the official cloud images, it must be in Main, as Universe is disallowed by policy.

[Rationale]: This package enables two-way communication with the meta-data service on Joyent/SmartOS instances.

[Security]: The source package was introduced early Octoboer 2013.

[Quality assurance]: No obvious issues with the MIR requirements

[UI standard]: Not applicable

[Dependencies]: All dependencies are already in main.

[Standards compliance]: No issues identified

[Maintenance]: This package will be jointly maintained by the Ubuntu Server EcoSystems team in conjunction with Joyent through a business relationship.

[Source Location]: Currently working on landing this package into Debian. (http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=728749)

Ben Howard (darkmuggle) on 2013-11-05
Changed in ubuntu:
milestone: none → ubuntu-14.04
Ben Howard (darkmuggle) on 2013-11-05
description: updated
summary: - Need joyent-mdata-get packaged
+ [MIR] new package - joyent-mdata-client
Ben Howard (darkmuggle) on 2013-11-05
Changed in ubuntu:
importance: Wishlist → Medium
Ben Howard (darkmuggle) on 2013-11-05
summary: - [MIR] new package - joyent-mdata-client
+ [MIR][SRU] new package - joyent-mdata-client
Robie Basak (racb) wrote :

I've reviewed lp:~utlemming/+junk/mdata_tools/ for an upload to universe.

In general the packaging is of a high quality. Thank you Ben! A few minor issues that I've patched (attached):

* Fix LP bug reference number in debian/changelog.
* Convert debian/copyright to machine-readable dep5 format (current best practice).
* Rename get-packaged-orig-source debian/rules target to get-orig-source (Debian policy section 4.9).
* Fix get-orig-source target to produce orig tarball with directory prefixed.
* Fix get-orig-source target to clean up after itself.

Some items not fixed; I don't think these are major enough to block upload. I suggest we create bugs for these after the package is accepted into NEW:

* get-orig-source target should work from any directory according to Debian policy; but it's an optional target and the spirit of the policy is to make it possible for anybody to see and reconstruct the orig tarball, which is fulfilled here.

* /lib/smartdc/mdata-get is non-standard, and I think deserves some documentation in the packaging. I understand from Ben that it is a legacy which some people might expect, and that it is used for boot when cloud-init isn't working. Do we need to explain this in a README.Debian, perhaps?

Recommendations to upstream:

Please drop "All rights reserved" from the copyright notice; this is not required any more and contradicts the licence statement. Please also explicitly add the licence to each source file. This would make the licensing situation completely unambiguous.

Ideally each source file should have copyright notices, a licence statement, and no "all rights reserved" statement. Right now, they all have a copyright and "all rights reserved" but no licence statement. reqid.c has nothing.

Debian appear to accept the contradiction in general because the intent to licence MIT is clear, so I think this is OK for Ubuntu too. However, it's common practice to annotate source files with a copyright notice and licence; the upstream source files in this project do state copyright and "All rights reserved" but do not state licences in each file. I think this is still acceptable, but more contradictory than what Debian have accepted previously. See http://lists.debian.org/debian-mentors/2013/07/msg00149.html for discussion.

One issue blocking upload (at least for me):

crc32.c contains a copyright statement from Gary S. Brown but does not include licensing information. Thus it is not clear if this file is re-distributable. I found http://lists.debian.org/debian-legal/2003/02/msg00100.html which suggests that Gary S. Brown has never explicitly licensed this code for redistribution. Do we know if it is permitted for this file to be released under the MIT licence? Given the triviality of the code, I guess it is probably easiest to replace crc32.c with a DFSG-compliant implementation? These appear to be available, but it might need some API adaptation.

Robie Basak (racb) wrote :

PS. I noticed that there was no debian/watch file, but upstream don't have releases (yet, at least), so I don't think it makes sense to have one right now.

Ben Howard (darkmuggle) wrote :

Thanks Robie for the feedback.

I've fixed in the packaging branch.
1. LP Bug reference number in debian/changelog
2. Implemented dep5 format debian/copyright
3. Used get-orig-source instead of get-packaged-orig-sourc
4. Added directory prefix for get-orig-source (based on Robie's patch)
5. Implemented cleanup.

I've also asked for Joyent to address the Copyright and CRC32 issues. Once I have a new branch to work with from Joyent, I'll rebase and get a new review.

tags: added: patch
Seth Arnold (seth-arnold) wrote :

I reviewed mdata-tools as checked into lp:~utlemming/+junk/mdata_tools on
Tuesday 5 November 2013, a temporary staging area while being packaged for
trusty.

- No CVE history visible at the time of the audit
- The package provides several command-line utilities and a corresponding
  serial-line protocol to manage the client-side of metadata in
  SmartOS-managed KVM (and perhaps Zones?) instances.
- No cryptography
- Some Unix-domain sockets
- No 'networking' but relies upon serial communication with a hypervisor
- No daemons
- I didn't see pre/post inst/rm scripts
- I didn't see init scripts
- I didn't see dbus services
- I didn't see setuid executables
- I didn't see sudo fragments
- I didn't see udev rules
- Provides mdata-get, mdata-put, mdata-delete, mdata-list binaries.
- No test suite

- No subprocesses spawned
- Nearly all memory management is checked and safe
- Most file operations have hard-coded pathnames
- No environment use
- Very nice safer-strings helper routines
- Some privileged serial-port handling, looked safe, they even included
  helpful hints to proper documentation
- Some unix-domain networking, name handling looked safe
- Command stream is parsed with a simple and careful hand-written scanner
- No WebKit
- Since hypervisor will handle serial, MITM attacks are unlikely
- No PolicyKit
- No Javascript

mdata-tools is high-quality code programmed in a professional manner.

Some unit testing code would be nice to have, but the actual function of
the utilities would be quite difficult to test in any automated fashion.

I found several minor issues while reviewing the code that I thought I
should call out in the hopes that it is useful to someone:

- plat_init() unchecked calloc() call
- open_md(), unix_open_serial() O_EXCL used without O_CREAT; O_EXCL
  doesn't influence character device opens. Undefined behavior, but
  probably harmless.

(Issues have been filed at https://github.com/joyent/mdata-client/issues)

Security team ACK for promoting to main.

Thanks

Ben Howard (darkmuggle) wrote :

Worked with upstream to fix copyright headers. Packaging now uses a watch file as well.

We're ready for the SRU team to review.

Changed in ubuntu:
assignee: Ben Howard (utlemming) → nobody
Changed in debian:
status: Unknown → New
Ben Howard (darkmuggle) wrote :

Regarding the crc32.c copyright issue, the new branch uses a new implementation of crc32.c that is _different_ from the the Gary S. Brown. implementation.

Adam Conrad (adconrad) wrote :

So, I'm accepting this directly to main with a few notes to be fixed in the next upload:

1) EXPAT is spelled "Expat" in dep-5
2) Licensing your debian/* (which may contain patches) with a non-upstream-compatible license probably isn't clever, I'd recommend switching from GPL-2 to Expat for the packaging too, so you don't have to explicitly call out patches later as being safe for upstream consumption.
3) Please make sure there's a team bug subscription on the package as soon as it's accepted and LP knows it exists.
4) Please toss it in some supported seed so it doesn't fall out of main.
5) Please talk to upstream about that weird non-FHS legacy path. If they can live without it, we really don't want stuff like that setting a precedent for other people thinking it's hip and cool to litter all over standards and the filesystem.

Adam Conrad (adconrad) wrote :

6) kill off the last lintian warning on your next upload too:

I: joyent-mdata-client source: binary-control-field-duplicates-source field "homepage" in package joyent-mdata-client

On Fri, Nov 08, 2013 at 03:57:12AM -0000, Adam Conrad wrote:
> 5) Please talk to upstream about that weird non-FHS legacy path. If
> they can live without it, we really don't want stuff like that setting
> a precedent for other people thinking it's hip and cool to litter all
> over standards and the filesystem.

Could we, for example, use cloud-init's user data vendor merging thing to
have a runcmd create the symlink only when it's needed, to keep it out
of the packaging? Just a thought.

What is the use case for needing the symlink?

Adam Conrad (adconrad) on 2013-11-08
affects: Ubuntu Trusty → joyent-mdata-client (Ubuntu Trusty)
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package joyent-mdata-client - 0.0.1-0ubuntu1

---------------
joyent-mdata-client (0.0.1-0ubuntu1) trusty; urgency=low

  * Initial release (LP: #1248000).
 -- Ben Howard <email address hidden> Thu, 07 Nov 2013 08:14:52 -0700

Changed in joyent-mdata-client (Ubuntu Trusty):
status: In Progress → Fix Released
Adam Conrad (adconrad) wrote :

Having the symlink outside the packaging is even worse than shipping it, as removing the package would then leave a dangling link. Ideally, we shouldn't be shipping (or creating) non-FHS clutter at all, unless it's absolutely, undeniably REQUIRED by this platform. In which case, we should convince them to fix their platform, and grudgingly ship the link for now.

Ben Howard (darkmuggle) wrote :

joyent-mdata-client replaces a bash script that has traditionally on SmartOS instances resided in /lib/smartdc. On non-Cloud-init booted systems, this file is required to get the meta-data. The inclusion of the link in /lib/smartdc is for historical reasons and to support users that have written scripts against it.

Ben Howard (darkmuggle) on 2013-11-08
Changed in joyent-mdata-client (Ubuntu Precise):
assignee: nobody → Ben Howard (utlemming)
Changed in joyent-mdata-client (Ubuntu Quantal):
assignee: nobody → Ben Howard (utlemming)
Changed in joyent-mdata-client (Ubuntu Raring):
assignee: nobody → Ben Howard (utlemming)
Changed in joyent-mdata-client (Ubuntu Saucy):
assignee: nobody → Ben Howard (utlemming)
Changed in joyent-mdata-client (Ubuntu Trusty):
assignee: nobody → Ben Howard (utlemming)
Ben Howard (darkmuggle) wrote :

Building packages into PPA for SRU, and validating builds.

Ben Howard (darkmuggle) wrote :

Confirmed PPA packages, pending upload to -proposed.

Ben Howard (darkmuggle) wrote :

Packages are now waiting in the new queue for Precise, Quantal, Raring and Saucy.

We are now ready for the SRU processing.

Changed in joyent-mdata-client (Ubuntu Saucy):
importance: Undecided → Medium
Changed in joyent-mdata-client (Ubuntu Raring):
importance: Undecided → Medium
Changed in joyent-mdata-client (Ubuntu Precise):
importance: Undecided → Medium
Changed in joyent-mdata-client (Ubuntu Quantal):
milestone: none → quantal-updates
Changed in joyent-mdata-client (Ubuntu Precise):
milestone: none → precise-updates
Changed in joyent-mdata-client (Ubuntu Quantal):
importance: Undecided → Medium
Changed in joyent-mdata-client (Ubuntu Raring):
milestone: none → raring-updates
Changed in joyent-mdata-client (Ubuntu Saucy):
milestone: none → saucy-updates

Hello Ben, or anyone else affected,

Accepted joyent-mdata-client into precise-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/joyent-mdata-client/0.0.1-0ubuntu2~12.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in joyent-mdata-client (Ubuntu Precise):
status: New → Fix Committed
tags: added: verification-needed
Changed in joyent-mdata-client (Ubuntu Quantal):
status: New → Fix Committed
Adam Conrad (adconrad) wrote :

Hello Ben, or anyone else affected,

Accepted joyent-mdata-client into quantal-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/joyent-mdata-client/0.0.1-0ubuntu2~12.10.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in joyent-mdata-client (Ubuntu Raring):
status: New → Fix Committed
Adam Conrad (adconrad) wrote :

Hello Ben, or anyone else affected,

Accepted joyent-mdata-client into raring-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/joyent-mdata-client/0.0.1-0ubuntu2~13.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in joyent-mdata-client (Ubuntu Saucy):
status: New → Fix Committed
Adam Conrad (adconrad) wrote :

Hello Ben, or anyone else affected,

Accepted joyent-mdata-client into saucy-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/joyent-mdata-client/0.0.1-0ubuntu2~13.10.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Ben Howard (darkmuggle) on 2013-11-25
tags: added: verification-done
removed: verification-needed

The verification of this Stable Release Update has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regresssions.

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package joyent-mdata-client - 0.0.1-0ubuntu2~13.10.1

---------------
joyent-mdata-client (0.0.1-0ubuntu2~13.10.1) saucy; urgency=low

  * Backport from trusty to saucy for Joyent IaaS (LP: #1248000).
 -- Ben Howard <email address hidden> Fri, 22 Nov 2013 12:14:36 -0700

Changed in joyent-mdata-client (Ubuntu Saucy):
status: Fix Committed → Fix Released
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package joyent-mdata-client - 0.0.1-0ubuntu2~13.04.1

---------------
joyent-mdata-client (0.0.1-0ubuntu2~13.04.1) raring; urgency=low

  * Backport from trusty to raring for Joyent IaaS (LP: #1248000).
 -- Ben Howard <email address hidden> Fri, 22 Nov 2013 12:14:36 -0700

Changed in joyent-mdata-client (Ubuntu Raring):
status: Fix Committed → Fix Released
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package joyent-mdata-client - 0.0.1-0ubuntu2~12.10.1

---------------
joyent-mdata-client (0.0.1-0ubuntu2~12.10.1) quantal; urgency=low

  * Backport from trusty to quantal for Joyent IaaS (LP: #1248000).
 -- Ben Howard <email address hidden> Fri, 22 Nov 2013 12:14:36 -0700

Changed in joyent-mdata-client (Ubuntu Quantal):
status: Fix Committed → Fix Released
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package joyent-mdata-client - 0.0.1-0ubuntu2~12.04.1

---------------
joyent-mdata-client (0.0.1-0ubuntu2~12.04.1) precise; urgency=low

  * Backport from trusty to precise for Joyent IaaS (LP: #1248000).
 -- Ben Howard <email address hidden> Fri, 22 Nov 2013 12:14:36 -0700

Changed in joyent-mdata-client (Ubuntu Precise):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Patches

Remote bug watches

Bug watches keep track of this bug in other bug trackers.