amazon-ecr-credential-helper icon indicating copy to clipboard operation
amazon-ecr-credential-helper copied to clipboard

snap: initial packaging

Open samuelkarp opened this issue 5 years ago • 8 comments

Description of changes: Initial packaging as a snap package.

This needs review in the following areas:

  • Is it possible for us to get away from needing classic confinement? The home plug does not allow reading hidden directories, and we need to read ~/.aws (and write to ~/.ecr, but that could really be confined inside the snap's $HOME instead of the user's).
  • The snapcore/snapcraft Docker image is based on Ubuntu 16.04, which does not have a new enough golang-go package for this project to build. Consequently, I wrote a Dockerfile to build from Ubuntu 18.04 which has Go 1.10 available.
  • The snap name is docker-credential-ecr-login as that seems to be the only way for a binary named docker-credential-ecr-login to be added to the user's $PATH, and Docker requires that naming scheme. Is it possible to name the snap something more meaningful (like amazon-ecr-credential-helper while still exposing a binary that appears as docker-credential-ecr-login in $PATH?
  • I used override-build to control the build process and actually use our Makefile, but it does then mean manually handling copying files to the appropriate location. Is there a way to either still use our Makefile or to inject the appropriate Go options (including injecting the project version and commit into the binary)?

/cc @davdunc

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

samuelkarp avatar Jan 02 '19 08:01 samuelkarp

Opened https://github.com/awslabs/amazon-ecr-credential-helper/issues/139 for failing CI, which is unrelated to the changes on this branch.

samuelkarp avatar Jan 02 '19 08:01 samuelkarp

Hi @samuelkarp, I'm looking into this. To address your questions:

  • Yes, we should be able to do this without classic by using the personal-files interface.

  • I've got an updated recipe using core18 that seems to be working ok

  • It's possible to ask for an auto-alias for a snap package but for this one I'm not sure it's worth it. snap search checks description text as well as the package name. It's possible though if you think it's worth fighting for :)

  • Removing the need for a complex override-build is what I'm looking at right now. I can get the package to build ok without using the makefile at all so I might just be able to get away with setting a few build-time environment variables. I'll let you know when I make progress.

My branch is here: https://github.com/stilvoid/amazon-ecr-credential-helper/tree/snap/snap

stilvoid avatar Apr 05 '19 14:04 stilvoid

Hi @stilvoid, thank you for taking a look!

Yes, we should be able to do this without classic by using the personal-files interface.

Is personal-files new? I hadn't seen it before and it addresses one of my main complaints with snaps. :smile: I think it'll work perfectly for this.

I've got an updated recipe using core18 that seems to be working ok

Does core18 require building or running on Ubuntu 18.04? 16.04 is still an LTS until 2021.

It's possible to ask for an auto-alias for a snap package but for this one I'm not sure it's worth it. snap search checks description text as well as the package name. It's possible though if you think it's worth fighting for :)

I'd like to have the auto-alias. I was talking with @davdunc about this (as he helps manage the Snap store account for AWS) and he did end up creating the snap listing for the name we want, but it ended up falling off both of our radars for now. I'll restart that discussion.

Removing the need for a complex override-build is what I'm looking at right now. I can get the package to build ok without using the makefile at all so I might just be able to get away with setting a few build-time environment variables. I'll let you know when I make progress.

That might work, though I still want to make sure that the snap includes the manual page and the licensing files.

samuelkarp avatar Apr 05 '19 18:04 samuelkarp

Does core18 require building or running on Ubuntu 18.04? 16.04 is still an LTS until 2021.

I'm building and running from Arch Linux so... nope ;)

I still want to make sure that the snap includes the manual page and the licensing files.

Can snaps even do that? The man page wouldn't be able to live under /usr/share/man so I'm not sure how man would index them. Have you seen an example of a package that does this?

stilvoid avatar Apr 08 '19 11:04 stilvoid

Can snaps even do that? The man page wouldn't be able to live under /usr/share/man so I'm not sure how man would index them. Have you seen an example of a package that does this?

I'm hoping that they will in the future :)

I'm really more concerned about the licensing files (LICENSE, NOTICE, and THIRD-PARTY-LICENSES) than I am about the man page, though I would like the man page to be included as well.

samuelkarp avatar Apr 08 '19 16:04 samuelkarp

I'm really more concerned about the licensing files (LICENSE, NOTICE, and THIRD-PARTY-LICENSES) than I am about the man page, though I would like the man page to be included as well.

So we can certainly include those in the snap but they'll just live in /var/lib/snapd/snap/whatever so I'm not certain how useful it would be.

I've included the license info in my snapcraft.yaml so this is what you get when you run snap info sam-cli:

name:      sam-cli
summary:   The AWS SAM CLI tool for the AWS Serverless Application Model
publisher: –
license:   Apache-2.0
description: |
  `sam` is the AWS CLI tool for managing Serverless applications written with the AWS Serverless
  Application Model (SAM). SAM CLI can be used to test functions locally, start a local API Gateway
  from a SAM template, validate a SAM template, fetch logs, generate sample payloads for various
  event sources, and generate a SAM project in your favorite Lambda Runtime.
commands:
  - sam-cli
refresh-date: 11 days ago, at 10:15 GMT
installed:    0+git.3c26c2c (x1) 19MB devmode

stilvoid avatar Apr 09 '19 13:04 stilvoid

So we can certainly include those in the snap but they'll just live in /var/lib/snapd/snap/whatever so I'm not certain how useful it would be.

That's fine, I'd rather have them included in the package than not included.

Want to send me a PR for the confinement, personal-files, and license changes? Otherwise I can update this branch with a new commit and credit you as the author.

samuelkarp avatar Apr 09 '19 17:04 samuelkarp

Looks like https://github.com/docker-snap/docker-snap/issues/10 covers some complications that would make it hard to use both the Docker snap and a snap of this credential helper together.

samuelkarp avatar Feb 17 '21 01:02 samuelkarp