[Summary]
It seems mostly to me packaging wise, but I think there are a bunch of things
needed to be doen to complete this. We need:
- an ack by the cloud-init Team that this does not conflict with our usual
  services provided through cloud init
  - I'm subscribing the cloud-init Team to give it a look
- Security review as "getting a key that then is allowed for local login" has
  one of the biggest "unintended backdoor" potential I ever seen
  - assigning security to the bug to evaluate this further
- I really would like upstream to pass and fix most of the shellcheck findings
  - @cyphermox do you have a relation with them, could you ask them to do that?
- a few improvements for better maintenance e.g. changes in postinst described
  in detail later on in this review
  - @cyphermox would you do those or who do we need to ask for that?
- the service should not run as root, use PrivateTmp and maybe a few other
  systemd service isolations
  - @cyphermox - this is part of the upstream, will you ask them to improve
    this?
- need to add package subscribing team
- it is not strictly required, but would be great to have some test on this
  Not sure if one can set up a compatible backend on the expected static IP
  in an autopkgtest.
  But if one could do so that would be a great (albeit optional) addition.
- please add d/watch and/or at least upstream VCS references

Status: incomplete until the issues above are resolved.
I subscribed ubuntu-security as they can already take a look / put it in their queue.
Once the findings are resolved AND security AND cloud-init acks as well this would be complete.

[Duplication]
Ok:
- to some extend you'd think that cloud-init would do that.
- But I know that the new sevrice isn't in there yet.
  So the MIR is not blocked for being a duplicate, but we should ask the
  cloud-init team so that this will not conflict.

[Embedded sources and static linking]
Ok:
- no embedeed libs/sources
- no static linking
- no golang

[Security]
Ok:
- no history of CVEs
- does not use webkit1,2
- does not use lib*v8 directly
- does not open a port
- does not processes arbitrary web content
- does not integrate arbitrary javascript into the desktop

Although it (needs security ack):
- parses data formats
- uses centralized online accounts
- deals with system authentication (eg, pam), etc)

Further (should be improved):
- The package runs a service as root wihch only does processing of remote data
  This script/service gets data from remote endpoints, I see no need to run
  it as root
  The package creates a custom user, why not use this
  Furthermore privateTmp should maybe used as well as some other service
  lockdowns if they apply.

[Common blockers]
Ok:
- does not FTBFS currently?
- not a python package

not perfect but ok:
- code has output in logs, no translations
- it has neither an upstream not a autopkgtest testsuite

needs to be fixed:
- There is no package subscriber yet, I assume Foundations is doing that?

[Packaging red flags]
Ok:
- no Ubuntu delta atm
- no library/symbols concerns
- it isn't very old, from the bit I see it seems upstream updates are ok
- no MOTU relation
- all dependencies are already in main
- no massive Lintian warnings
- d/rules is super clean
- no debian/control use of Built-Using
- no golang specifics needed

Not so great, needs some fixes/adaptions:
- it lacks all references to get the code
  - no upstream VCS, no debian/watch file, ...
    Please add something so that a drive by maintainer can still help if all
    people that did it before are gone.
  - due to that it is for example hard to ensure right now if the current
    release is packaged
- One of the red-flags usually is "modifying the config of another package".
  This is done, but also the purpose of this package, so we can't "not do it".
  Although I'd have a suggestion, to make this even safer.
  The postinst already checks for conflicting configs and overrides (great)
  But we all know users are not always perfect, I'd like to also address:
  1. users modified /lib/systemd/system/ssh.service instead (as they should)
     add an override. You might store expected content of the original
     ExecStart and bail out if the current one differs.
  This would also provide some protection from e.g. a security fix that adds
  an option in that line being lost when ec2-instance-connect is installed.
- There are more minor things like the preinst reporting "Created system user
  ec2-instance-connect" even when it already existed.

[Upstream red flags]
Ok:
- no Errors/warnings during the build (isn't a real build)
  - but see shellcheck and other things I mentioned already for the same purpose
- bash only, no malloc/sprintf
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH
- no use of User nobody
- no use of setuid
- no known Important bugs (crashers, etc) in Debian or Ubuntu
- no dependency on webkit, qtwebkit, seed or libgoa-*
- not part of Unity Dash / UI in regadr to privacy settings?