-
Notifications
You must be signed in to change notification settings - Fork 14
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
Use Docker credentials file to log in to registry #461
Changes from 2 commits
37a4904
d2d0088
89483e3
723b44b
19d3f0a
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 |
---|---|---|
|
@@ -3161,9 +3161,7 @@ | |
}, | ||
"type": "object", | ||
"required": [ | ||
"server", | ||
"username", | ||
"password" | ||
"server" | ||
] | ||
} | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -175,15 +175,22 @@ func (p *dockerNativeProvider) dockerBuild(ctx context.Context, | |
if err != nil { | ||
return "", nil, err | ||
} | ||
// Quick and dirty auth; we can also preconfigure the client itself I believe | ||
// TODO: use auth pattern as above to use default auth | ||
var authConfig = types.AuthConfig{ | ||
Username: img.Registry.Username, | ||
Password: img.Registry.Password, | ||
ServerAddress: img.Registry.Server, | ||
|
||
// authentication for registry push | ||
// we check if the user set creds in the Pulumi program, and use those preferentially | ||
var pushAuthConfig types.AuthConfig | ||
|
||
if img.Registry.Username != "" && img.Registry.Password != "" { | ||
pushAuthConfig.Username = img.Registry.Username | ||
pushAuthConfig.Password = img.Registry.Password | ||
pushAuthConfig.ServerAddress = img.Registry.Server | ||
} else { | ||
// we push to the server declared in the program, using our auth configs from image build. | ||
// if the program does not have a server declared, we will let the docker client error | ||
pushAuthConfig = authConfigs[img.Registry.Server] | ||
} | ||
|
||
authConfigBytes, err := json.Marshal(authConfig) | ||
authConfigBytes, err := json.Marshal(pushAuthConfig) | ||
|
||
if err != nil { | ||
return "", nil, errors.Wrap(err, "Error parsing authConfig") | ||
|
@@ -208,7 +215,7 @@ func (p *dockerNativeProvider) dockerBuild(ctx context.Context, | |
if err != nil { | ||
return "", nil, err | ||
} | ||
err = p.host.LogStatus(ctx, "info", urn, info) | ||
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. Is this needed? 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. yes - thank you for catching! |
||
err = p.host.Log(ctx, "info", urn, info) | ||
if err != nil { | ||
return "", nil, err | ||
} | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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 it valid to pass a username without a password or vice versa? If not, do we warn?
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.
either is valid - neither field is required per the schema. If either field is empty, we default to the host credentials. 🤔 warning might make sense because using host credentials isn't as obvious. 👍
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.
"valid" was perhaps the wrong word to use. What I should have said, and what I meant, was "does it ever make sense for the user to set a username without a password or vice versa."
My goal is to make sure that if the user does something that doesn't make sense, we will let them know.