-
Notifications
You must be signed in to change notification settings - Fork 338
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
Create standalone docker file & allow it to be used for retrieving credentials manually #72
base: main
Are you sure you want to change the base?
Changes from 4 commits
e32d6a5
83b1645
67f5e62
fb18dea
2ca5e52
53a58fa
688a72e
54d47ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
# Copyright 2016 Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"). You | ||
# may not use this file except in compliance with the License. A copy of | ||
# the License is located at | ||
# | ||
# http://aws.amazon.com/apache2.0/ | ||
# | ||
# or in the "license" file accompanying this file. This file is | ||
# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF | ||
# ANY KIND, either express or implied. See the License for the specific | ||
# language governing permissions and limitations under the License. | ||
|
||
FROM golang:1.9 | ||
|
||
WORKDIR /go/src/github.com/awslabs/amazon-ecr-credential-helper | ||
|
||
COPY . . | ||
|
||
CMD make |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,9 @@ | |
package main | ||
|
||
import ( | ||
"fmt" | ||
"os" | ||
|
||
ecr "github.com/awslabs/amazon-ecr-credential-helper/ecr-login" | ||
"github.com/awslabs/amazon-ecr-credential-helper/ecr-login/api" | ||
"github.com/awslabs/amazon-ecr-credential-helper/ecr-login/config" | ||
|
@@ -24,5 +27,39 @@ import ( | |
func main() { | ||
defer log.Flush() | ||
config.SetupLogger() | ||
credentials.Serve(ecr.ECRHelper{ClientFactory: api.DefaultClientFactory{}}) | ||
helper := ecr.ECRHelper{ClientFactory: api.DefaultClientFactory{}} | ||
if len(os.Args) > 1 && os.Args[1] == "eval" { | ||
evalCommand(helper) | ||
} else { | ||
credentials.Serve(helper) | ||
} | ||
} | ||
|
||
func evalCommand(helper credentials.Helper) { | ||
server, err := parseArgs(helper) | ||
if err == nil { | ||
var user, token string | ||
user, token, err = helper.Get(server) | ||
if err == nil { | ||
fmt.Printf("docker login -u %s -p %s %s\n", user, token, server) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a little leery of doing this. Docker has already changed the arguments that it accepts for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know; the latest version of docker is recommending to pass the arguments over stdin using My primary use case was running The only other alternative is to vendor the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think that using I think your use-case makes sense, but I'm still concerned as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can change It will help to silence warnings from docker. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The email flag for docker login has been optional since docker v1.4. So I do not think we need to worry about the email flag. I can add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know offhand, but I'm pretty sure email was still required as of Docker 1.9. I think I would prefer separate commands that correspond to the first version where the Docker CLI expected the invocation. |
||
} | ||
} | ||
if err != nil { | ||
fmt.Fprintf(os.Stdout, "%v\n", err) | ||
os.Exit(1) | ||
} | ||
} | ||
|
||
func parseArgs(helper credentials.Helper) (string, error) { | ||
if len(os.Args) > 2 { | ||
return os.Args[2], nil | ||
} | ||
servers, err := helper.List() | ||
if err == nil { | ||
for k := range servers { | ||
return k, nil | ||
} | ||
return "", fmt.Errorf("No default ECR servers found") | ||
} | ||
return "", err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any particular reason you need a full Alpine container instead of a scratch container? The executable is statically-linked, so the only thing that should be necessary at that point is a CA certificate bundle rather than the rest of the Linux userland.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Internally we used this container to run scripts within our CI so we found it useful to have basic UNIX utilities. Alpine overhead is around 4 MB which seemed reasonable to me. Also I am not sure how to just add the CA certificate bundle. If you have some pointers, I shall be happy to change this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think my preference would be for a scratch container so that we don't need to worry about updating the base layer. You can see an example that builds a scratch container with a CA certificate bundle here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK I shall update the Dockerfile to use scratch base.