Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

snap: initial packaging #138

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

snap: initial packaging #138

wants to merge 1 commit into from

Conversation

samuelkarp
Copy link
Contributor

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
Copy link
Contributor Author

Opened #139 for failing CI, which is unrelated to the changes on this branch.

@stilvoid
Copy link
Member

stilvoid commented Apr 5, 2019

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

@samuelkarp
Copy link
Contributor Author

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. 😄 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.

@stilvoid
Copy link
Member

stilvoid commented Apr 8, 2019

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?

@samuelkarp
Copy link
Contributor Author

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.

@stilvoid
Copy link
Member

stilvoid commented Apr 9, 2019

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

@samuelkarp
Copy link
Contributor Author

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
Copy link
Contributor Author

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

Base automatically changed from master to main February 18, 2021 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants