-
Notifications
You must be signed in to change notification settings - Fork 428
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
feat(manifest): allow additional docker build overrides in manifest #1059
Changes from all commits
f740e81
add851a
637dccd
5ef0e0a
fb58231
ec1e5fd
0689c6a
c07791a
19f8380
c5c0956
d0799b2
1ad4212
597cab4
509b21c
2d910d0
a3c3862
185923f
22e3681
823c77e
ca0bf23
054f503
8a385f5
719e9cf
34d84db
3ed58cd
da4964c
ac2ec31
6e63e42
6cccd75
85bd6a0
ef7feb5
2139b32
f9cb57e
a247546
129beb8
7b38a38
f97d542
6bb3a15
79dc17e
1cb5473
f83de96
39befaf
b9b78ff
5d6e755
4d5861f
bcf19a8
951e132
d0ea656
b500b6c
80eaa2a
d6aaa98
5c35c8b
6424529
b22b186
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ package cli | |
import ( | ||
"errors" | ||
"fmt" | ||
"path/filepath" | ||
"strings" | ||
|
||
"github.com/aws/copilot-cli/internal/pkg/addon" | ||
|
@@ -50,8 +51,9 @@ type deploySvcOpts struct { | |
deploySvcVars | ||
|
||
store store | ||
ws wsSvcReader | ||
ws wsSvcDirReader | ||
imageBuilderPusher imageBuilderPusher | ||
unmarshal func(in []byte) (interface{}, error) | ||
s3 artifactUploader | ||
cmd runner | ||
addons templater | ||
|
@@ -78,12 +80,12 @@ func newSvcDeployOpts(vars deploySvcVars) (*deploySvcOpts, error) { | |
if err != nil { | ||
return nil, fmt.Errorf("new workspace: %w", err) | ||
} | ||
|
||
return &deploySvcOpts{ | ||
deploySvcVars: vars, | ||
|
||
store: store, | ||
ws: ws, | ||
unmarshal: manifest.UnmarshalService, | ||
spinner: termprogress.NewSpinner(), | ||
sel: selector.NewWorkspaceSelect(vars.prompt, store, ws), | ||
cmd: command.New(), | ||
|
@@ -286,38 +288,49 @@ func (o *deploySvcOpts) configureClients() error { | |
} | ||
|
||
func (o *deploySvcOpts) pushToECRRepo() error { | ||
path, err := o.getDockerfilePath() | ||
|
||
dockerBuildInput, err := o.getBuildArgs() | ||
if err != nil { | ||
return err | ||
} | ||
|
||
if err := o.imageBuilderPusher.BuildAndPush(docker.New(), path, o.ImageTag); err != nil { | ||
if err := o.imageBuilderPusher.BuildAndPush(docker.New(), dockerBuildInput); err != nil { | ||
return fmt.Errorf("build and push image: %w", err) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func (o *deploySvcOpts) getDockerfilePath() (string, error) { | ||
type dfPath interface { | ||
DockerfilePath() string | ||
func (o *deploySvcOpts) getBuildArgs() (*docker.BuildArguments, error) { | ||
type dfArgs interface { | ||
BuildArgs(rootDirectory string) *manifest.DockerBuildArgs | ||
} | ||
|
||
manifestBytes, err := o.ws.ReadServiceManifest(o.Name) | ||
if err != nil { | ||
return "", fmt.Errorf("read manifest file %s: %w", o.Name, err) | ||
return nil, fmt.Errorf("read manifest file %s: %w", o.Name, err) | ||
} | ||
|
||
svc, err := manifest.UnmarshalService(manifestBytes) | ||
svc, err := o.unmarshal(manifestBytes) | ||
if err != nil { | ||
return "", fmt.Errorf("unmarshal svc manifest: %w", err) | ||
return nil, fmt.Errorf("unmarshal service %s manifest: %w", o.Name, err) | ||
} | ||
|
||
mf, ok := svc.(dfPath) | ||
mf, ok := svc.(dfArgs) | ||
if !ok { | ||
return "", fmt.Errorf("service %s does not have a dockerfile path", o.Name) | ||
return nil, fmt.Errorf("service %s does not have required method Build()", o.Name) | ||
} | ||
return mf.DockerfilePath(), nil | ||
copilotDir, err := o.ws.CopilotDirPath() | ||
if err != nil { | ||
return nil, fmt.Errorf("get copilot directory: %w", err) | ||
} | ||
wsRoot := filepath.Dir(copilotDir) | ||
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? I believe 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, because we want the workspace root, not the Copilot dir. |
||
|
||
args := mf.BuildArgs(wsRoot) | ||
return &docker.BuildArguments{ | ||
Dockerfile: *args.Dockerfile, | ||
Context: *args.Context, | ||
Args: args.Args, | ||
ImageTag: o.ImageTag, | ||
}, nil | ||
} | ||
|
||
// pushAddonsTemplateToS3Bucket generates the addons template for the service and pushes it to S3. | ||
|
@@ -351,7 +364,7 @@ func (o *deploySvcOpts) manifest() (interface{}, error) { | |
if err != nil { | ||
return nil, fmt.Errorf("read service %s manifest from workspace: %w", o.Name, err) | ||
} | ||
mft, err := manifest.UnmarshalService(raw) | ||
mft, err := o.unmarshal(raw) | ||
if err != nil { | ||
return nil, fmt.Errorf("unmarshal service %s manifest: %w", o.Name, 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.
Maybe something our users could grok easier:
Or something like that?
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.
This is a weird error because it specifically has to do with the interface conversion to the dfArgs interface and making sure that the unmarshaled yaml fits into a
BackendService
orLoadBalancedWebService
struct with the right methods attached.