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

feat(manifest): support codebuild environment image config #2125

Merged
merged 9 commits into from
Apr 14, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions internal/pkg/cli/pipeline_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,8 @@ func (o *updatePipelineOpts) Execute() error {
}
o.shouldPromptUpdateConnection = bool

build := deploy.PipelineBuildFromManifest(pipeline.Build)

// convert environments to deployment stages
stages, err := o.convertStages(pipeline.Stages)
if err != nil {
Expand All @@ -272,6 +274,7 @@ func (o *updatePipelineOpts) Execute() error {
AppName: o.appName,
Name: pipeline.Name,
Source: source,
Build: build,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: to remove the extra variable

Suggested change
Build: build,
Build: deploy.PipelineBuildFromManifest(pipeline.Build),

Stages: stages,
ArtifactBuckets: artifactBuckets,
AdditionalTags: o.app.Tags,
Expand Down
55 changes: 55 additions & 0 deletions internal/pkg/cli/pipeline_update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,61 @@ source:

expectedError: fmt.Errorf("update pipeline: some error"),
},
"update and deploy pipeline with specifying build property": {
inApp: &app,
inAppName: appName,
inRegion: region,
callMocks: func(m updatePipelineMocks) {
content := `
name: pipepiper
version: 1

source:
provider: GitHub
properties:
repository: aws/somethingCool
access_token_secret: "github-token-badgoose-backend"
branch: main

build:
image: aws/codebuild/standard:3.0

stages:
-
name: chicken
test_commands:
- make test
- echo "made test"
-
name: wings
test_commands:
- echo "bok bok bok"
`
gomock.InOrder(
m.prog.EXPECT().Start(fmt.Sprintf(fmtPipelineUpdateResourcesStart, appName)).Times(1),
m.deployer.EXPECT().AddPipelineResourcesToApp(&app, region).Return(nil),
m.prog.EXPECT().Stop(log.Ssuccessf(fmtPipelineUpdateResourcesComplete, appName)).Times(1),

m.ws.EXPECT().ReadPipelineManifest().Return([]byte(content), nil),
m.ws.EXPECT().WorkloadNames().Return([]string{"frontend", "backend"}, nil).Times(1),

// convertStages
m.envStore.EXPECT().GetEnvironment(appName, "chicken").Return(mockEnv, nil).Times(1),
m.envStore.EXPECT().GetEnvironment(appName, "wings").Return(mockEnv, nil).Times(1),

// getArtifactBuckets
m.deployer.EXPECT().GetRegionalAppResources(gomock.Any()).Return(mockResources, nil),

// deployPipeline
m.deployer.EXPECT().PipelineExists(gomock.Any()).Return(true, nil),
m.prompt.EXPECT().Confirm(fmt.Sprintf(fmtPipelineUpdateExistPrompt, pipelineName), "").Return(true, nil),
m.prog.EXPECT().Start(fmt.Sprintf(fmtPipelineUpdateProposalStart, pipelineName)).Times(1),
m.deployer.EXPECT().UpdatePipeline(gomock.Any()).Return(nil),
m.prog.EXPECT().Stop(log.Ssuccessf(fmtPipelineUpdateProposalComplete, pipelineName)).Times(1),
)
},
expectedError: nil,
},
}
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
Expand Down
8 changes: 8 additions & 0 deletions internal/pkg/deploy/cloudformation/pipeline_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (
"github.com/stretchr/testify/require"
)

const defaultImage = "aws/codebuild/amazonlinux2-x86_64-standard:3.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we reuse defaultPipelineBuildImage?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I want to clarify. Do you mean rename defaultImage into defaultPipelineBuildImage? Or, using defaultPipelineBuildImage in internal/pkg/deploy/pipeline.go? This constant is in deploy pkg, but this test file is in cloudformation pkg. Do you intend to fix not internal/pkg/deploy/cloudformation/pipeline_test.go but l.152 in internal/pkg/deploy/pipeline_test.go?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I see sorry that's my bad! you can ignore the comment then, I thought they were in the same pkg :)


func TestCloudFormation_PipelineExists(t *testing.T) {
in := &deploy.CreatePipelineInput{
AppName: "kudos",
Expand Down Expand Up @@ -79,6 +81,9 @@ func TestCloudFormation_CreatePipeline(t *testing.T) {
ProviderName: "Bitbucket",
Branch: "main",
},
Build: &deploy.Build{
Image: defaultImage,
},
Stages: nil,
ArtifactBuckets: nil,
}
Expand Down Expand Up @@ -239,6 +244,9 @@ func TestCloudFormation_UpdatePipeline(t *testing.T) {
RepositoryURL: "aws/somethingCool",
Branch: "main",
},
Build: &deploy.Build{
Image: defaultImage,
},
Stages: nil,
ArtifactBuckets: nil,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ func TestGHv1Pipeline_Template(t *testing.T) {
Branch: "mainline",
PersonalAccessTokenSecretID: "my secret",
},
Build: &deploy.Build{
Image: "aws/codebuild/amazonlinux2-x86_64-standard:3.0",
},
Stages: []deploy.PipelineStage{
{
AssociatedEnvironment: &deploy.AssociatedEnvironment{
Expand Down
23 changes: 23 additions & 0 deletions internal/pkg/deploy/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ type CreatePipelineInput struct {
// The source code provider for this pipeline
Source interface{}

// The build project settings for this pipeline
Build *Build

// The stages of the pipeline. The order of stages in this list
// will be the order we deploy to.
Stages []PipelineStage
Expand All @@ -52,6 +55,13 @@ type CreatePipelineInput struct {
AdditionalTags map[string]string
}

// Build represents CodeBuild project used in the CodePipeline
// to build and test Docker image.
type Build struct {
// The URI that identifies the Docker image to use for this build project.
Image string
}

// ArtifactBucket represents an S3 bucket used by the CodePipeline to store
// intermediate artifacts produced by the pipeline.
type ArtifactBucket struct {
Expand Down Expand Up @@ -171,6 +181,19 @@ func PipelineSourceFromManifest(mfSource *manifest.Source) (source interface{},
}
}

// PipelineBuildFromManifest processes manifest info about the build project settings.
func PipelineBuildFromManifest(mfBuild *manifest.Build) (build *Build) {
var imageURI string
if (mfBuild == nil || mfBuild.Image == "") {
imageURI = manifest.DefaultImage
} else {
imageURI = mfBuild.Image
}
return &Build{
Image: imageURI,
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var imageURI string
if (mfBuild == nil || mfBuild.Image == "") {
imageURI = manifest.DefaultImage
} else {
imageURI = mfBuild.Image
}
return &Build{
Image: imageURI,
}
image := defaultPipelineBuildImage
if mfBuild != nil && mfBuild.Image != "" {
image = mfBuild.Image
}
return &Build{
Image: image,
}

}

// GitHubPersonalAccessTokenSecretID returns the ID of the secret in the
// Secrets manager, which stores the GitHub Personal Access token if the
// provider is "GitHubV1".
Expand Down
30 changes: 30 additions & 0 deletions internal/pkg/deploy/pipeline_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,36 @@ func TestPipelineSourceFromManifest(t *testing.T) {
}
}

func TestPipelineBuildFromManifest(t *testing.T) {
const defaultImage = "aws/codebuild/amazonlinux2-x86_64-standard:3.0"

testCases := map[string]struct {
mfBuild *manifest.Build
expectedBuild *Build
}{
"set default image if not be specified in manifest": {
mfBuild: nil,
expectedBuild: &Build{
Image: defaultImage,
},
},
"set image according to manifest": {
mfBuild: &manifest.Build{
Image: "aws/codebuild/standard:3.0",
},
expectedBuild: &Build{
Image: "aws/codebuild/standard:3.0",
},
},
}
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
build := PipelineBuildFromManifest(tc.mfBuild)
require.Equal(t, tc.expectedBuild, build, "mismatched build")
})
}
}

func TestParseOwnerAndRepo(t *testing.T) {
testCases := map[string]struct {
src *GitHubSource
Expand Down
10 changes: 10 additions & 0 deletions internal/pkg/manifest/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const (
GithubV1ProviderName = "GitHubV1"
CodeCommitProviderName = "CodeCommit"
BitbucketProviderName = "Bitbucket"
DefaultImage = "aws/codebuild/amazonlinux2-x86_64-standard:3.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we define this constant in thedeploy pkg instead? this would allow us to keep it private


pipelineManifestPath = "cicd/pipeline.yml"
)
Expand Down Expand Up @@ -157,6 +158,7 @@ type PipelineManifest struct {
Name string `yaml:"name"`
Version PipelineSchemaMajorVersion `yaml:"version"`
Source *Source `yaml:"source"`
Build *Build `yaml:"build"`
Stages []PipelineStage `yaml:"stages"`

parser template.Parser
Expand All @@ -168,6 +170,11 @@ type Source struct {
Properties map[string]interface{} `yaml:"properties"`
}

// Build defines the build project to build and test image.
type Build struct {
Image string `yaml:"image"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: is this formatted correctly with gofmt?

}

// PipelineStage represents a stage in the pipeline manifest
type PipelineStage struct {
Name string `yaml:"name"`
Expand All @@ -190,6 +197,9 @@ func NewPipelineManifest(pipelineName string, provider Provider, stages []Pipeli
ProviderName: provider.Name(),
Properties: provider.Properties(),
},
Build: &Build{
Image: DefaultImage,
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If leave the default to be nil, this allow us to move the constant to the deploy pkg and the translation logic to deploy.PipelineBuildFromManifest

Stages: stages,

parser: template.New(),
Expand Down
48 changes: 47 additions & 1 deletion internal/pkg/manifest/pipeline_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
const (
defaultGHBranch = "main"
defaultCCBranch = "master"
defaultImage = "aws/codebuild/amazonlinux2-x86_64-standard:3.0"
)

func TestNewProvider(t *testing.T) {
Expand Down Expand Up @@ -106,6 +107,9 @@ func TestNewPipelineManifest(t *testing.T) {
Branch: defaultGHBranch,
}),
},
Build: &Build{
Image: defaultImage,
},
Stages: []PipelineStage{
{
Name: "chicken",
Expand Down Expand Up @@ -218,7 +222,7 @@ stages:
inContent: `corrupted yaml`,
expectedErr: errors.New("yaml: unmarshal errors:\n line 1: cannot unmarshal !!str `corrupt...` into manifest.PipelineManifest"),
},
"valid pipeline.yml": {
"valid pipeline.yml without build": {
inContent: `
name: pipepiper
version: 1
Expand Down Expand Up @@ -261,6 +265,48 @@ stages:
},
},
},
"valid pipeline.yml with build": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for writing all these tests! They are really helpful and comprehensive.

inContent: `
name: pipepiper
version: 1

source:
provider: GitHub
properties:
repository: aws/somethingCool
access_token_secret: "github-token-badgoose-backend"
branch: main

build:
image: aws/codebuild/standard:3.0

stages:
-
name: chicken
test_commands: []
`,
expectedManifest: &PipelineManifest{
Name: "pipepiper",
Version: Ver1,
Source: &Source{
ProviderName: "GitHub",
Properties: map[string]interface{}{
"access_token_secret": "github-token-badgoose-backend",
"repository": "aws/somethingCool",
"branch": defaultGHBranch,
},
},
Build: &Build{
Image: "aws/codebuild/standard:3.0",
},
Stages: []PipelineStage{
{
Name: "chicken",
TestCommands: []string{},
},
},
},
},
}

for name, tc := range testCases {
Expand Down
6 changes: 6 additions & 0 deletions templates/cicd/pipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ source:
# Optional: specify the name of an existing CodeStar Connections connection.
# connection_name: a-connection
{{- end}}

# This section defines the build project.
build:
# The URI that identifies the Docker image to use for this build project.
image: {{.Build.Image}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should not show this setting by default as it is mostly for advanced users. I expect most folks are satisfied with the default image that we provide.

Can we instead add it as a field in our documentation here? https://github.com/aws/copilot-cli/blob/mainline/site/content/docs/manifest/pipeline.md 🙏


{{$length := len .Stages}}{{if gt $length 0}}
# The deployment section defines the order the pipeline will deploy
# to your environments.
Expand Down
2 changes: 1 addition & 1 deletion templates/cicd/pipeline_cfn.yml
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ Resources:
Type: LINUX_CONTAINER
ComputeType: BUILD_GENERAL1_SMALL
PrivilegedMode: true
Image: aws/codebuild/amazonlinux2-x86_64-standard:3.0
Image: {{.Build.Image}}
EnvironmentVariables:
- Name: AWS_ACCOUNT_ID
Value: !Sub '${AWS::AccountId}'
Expand Down