From b4b4f02d9931e590726cbb3eeea30f178e4c1164 Mon Sep 17 00:00:00 2001 From: Janice Date: Fri, 5 Mar 2021 09:57:07 -0800 Subject: [PATCH 1/9] build: enable CSC for GH --- internal/pkg/cli/pipeline_delete.go | 1 + internal/pkg/cli/pipeline_init.go | 54 ++++----- internal/pkg/cli/pipeline_init_test.go | 114 +++++++++--------- internal/pkg/cli/pipeline_update.go | 25 +++- .../deploy/cloudformation/pipeline_test.go | 5 +- .../cloudformation/stack/pipeline_test.go | 5 +- internal/pkg/deploy/pipeline.go | 67 +++++++++- internal/pkg/manifest/pipeline.go | 40 ++++-- templates/cicd/pipeline_cfn.yml | 28 ++++- 9 files changed, 222 insertions(+), 117 deletions(-) diff --git a/internal/pkg/cli/pipeline_delete.go b/internal/pkg/cli/pipeline_delete.go index 381476e69d5..54e0d9c9452 100644 --- a/internal/pkg/cli/pipeline_delete.go +++ b/internal/pkg/cli/pipeline_delete.go @@ -118,6 +118,7 @@ func (o *deletePipelineOpts) Ask() error { // Execute deletes the secret and pipeline stack. func (o *deletePipelineOpts) Execute() error { + // Pipelines created with GitHubV1 have secrets. if err := o.deleteSecret(); err != nil { return err } diff --git a/internal/pkg/cli/pipeline_init.go b/internal/pkg/cli/pipeline_init.go index 56c021bbf1a..9b3d0ddb3f5 100644 --- a/internal/pkg/cli/pipeline_init.go +++ b/internal/pkg/cli/pipeline_init.go @@ -45,14 +45,14 @@ Please enter full repository URL, e.g. "https://github.com/myCompany/myRepo", or const ( buildspecTemplatePath = "cicd/buildspec.yml" - + fmtPipelineName = "pipeline-%s-%s" // Ex: "pipeline-appName-repoName" // For a GitHub repository. - githubURL = "github.com" - ghProviderName = "GitHub" - defaultGHBranch = "main" - fmtPipelineName = "pipeline-%s-%s" // Ex: "pipeline-appName-repoName" - fmtGHRepoURL = "https://%s/%s/%s" // Ex: "https://github.com/repoOwner/repoName" - fmtSecretName = "github-token-%s-%s" // Ex: "github-token-appName-repoName" + githubURL = "github.com" + ghProviderName = "GitHub" + ghV1ProviderName = "GitHubV1" + defaultGHBranch = "main" + fmtGHRepoURL = "https://%s/%s/%s" // Ex: "https://github.com/repoOwner/repoName" + fmtSecretName = "github-token-%s-%s" // Ex: "github-token-appName-repoName" // For a CodeCommit repository. awsURL = "aws.amazon.com" ccIdentifier = "codecommit" @@ -190,7 +190,7 @@ func (o *initPipelineOpts) Ask() error { // Execute writes the pipeline manifest file. func (o *initPipelineOpts) Execute() error { - if o.provider == ghProviderName { + if o.provider == ghV1ProviderName { if err := o.storeGitHubAccessToken(); err != nil { return err } @@ -270,7 +270,13 @@ func (o *initPipelineOpts) askRepository() error { } func (o *initPipelineOpts) askGitHubRepoDetails() error { - o.provider = ghProviderName + // If the user uses a flag (now hidden) to specify a GitHub access token, + // GitHub version 1 (not CSC) is the provider. + if o.githubAccessToken != "" { + o.provider = ghV1ProviderName + } else { + o.provider = ghProviderName + } repoDetails, err := ghRepoURL(o.repoURL).parse() if err != nil { return err @@ -278,11 +284,6 @@ func (o *initPipelineOpts) askGitHubRepoDetails() error { o.repoName = repoDetails.name o.repoOwner = repoDetails.owner - if o.githubAccessToken == "" { - if err = o.getGitHubAccessToken(); err != nil { - return err - } - } if o.repoBranch == "" { o.repoBranch = defaultGHBranch } @@ -469,20 +470,6 @@ func (url bbRepoURL) parse() (bbRepoDetails, error) { }, nil } -func (o *initPipelineOpts) getGitHubAccessToken() error { - token, err := o.prompt.GetSecret( - fmt.Sprintf("Please enter your GitHub Personal Access Token for your repository %s:", color.HighlightUserInput(o.repoName)), - `The personal access token for the GitHub repository linked to your workspace. -For more information, please refer to: https://git.io/JfDFD.`, - ) - - if err != nil { - return fmt.Errorf("get GitHub access token: %w", err) - } - o.githubAccessToken = token - return nil -} - func (o *initPipelineOpts) storeGitHubAccessToken() error { secretName := o.secretName() _, err := o.secretsmanager.CreateSecret(secretName, o.githubAccessToken) @@ -608,12 +595,17 @@ func (o *initPipelineOpts) pipelineName() string { func (o *initPipelineOpts) pipelineProvider() (manifest.Provider, error) { var config interface{} switch o.provider { - case ghProviderName: - config = &manifest.GitHubProperties{ + case ghV1ProviderName: + config = &manifest.GitHubV1Properties{ RepositoryURL: fmt.Sprintf(fmtGHRepoURL, githubURL, o.repoOwner, o.repoName), Branch: o.repoBranch, GithubSecretIdKeyName: o.secret, } + case ghProviderName: + config = &manifest.GitHubProperties{ + RepositoryURL: fmt.Sprintf(fmtGHRepoURL, githubURL, o.repoOwner, o.repoName), + Branch: o.repoBranch, + } case ccProviderName: config = &manifest.CodeCommitProperties{ RepositoryURL: fmt.Sprintf(fmtCCRepoURL, o.ccRegion, awsURL, o.repoName), @@ -669,7 +661,6 @@ func buildPipelineInitCmd() *cobra.Command { Create a pipeline for the services in your workspace. /code $ copilot pipeline init \ /code --url https://github.com/gitHubUserName/myFrontendApp.git \ - /code --github-access-token file://myGitHubToken \ /code --environments "stage,prod"`, RunE: runCmdE(func(cmd *cobra.Command, args []string) error { opts, err := newInitPipelineOpts(vars) @@ -698,6 +689,7 @@ func buildPipelineInitCmd() *cobra.Command { _ = cmd.Flags().MarkHidden(githubURLFlag) cmd.Flags().StringVarP(&vars.repoURL, repoURLFlag, repoURLFlagShort, "", repoURLFlagDescription) cmd.Flags().StringVarP(&vars.githubAccessToken, githubAccessTokenFlag, githubAccessTokenFlagShort, "", githubAccessTokenFlagDescription) + _ = cmd.Flags().MarkHidden(githubAccessTokenFlag) cmd.Flags().StringVarP(&vars.repoBranch, gitBranchFlag, gitBranchFlagShort, "", gitBranchFlagDescription) cmd.Flags().StringSliceVarP(&vars.environments, envsFlag, envsFlagShort, []string{}, pipelineEnvsFlagDescription) diff --git a/internal/pkg/cli/pipeline_init_test.go b/internal/pkg/cli/pipeline_init_test.go index ec8bceafb15..a1629e5ea9a 100644 --- a/internal/pkg/cli/pipeline_init_test.go +++ b/internal/pkg/cli/pipeline_init_test.go @@ -140,11 +140,8 @@ func TestInitPipelineOpts_Validate(t *testing.T) { } func TestInitPipelineOpts_Ask(t *testing.T) { - githubOwner := "badGoose" - githubRepoName := "chaOS" - githubAnotherOwner := "goodGoose" + githubOwner := "goodGoose" githubAnotherRepoName := "bhaOS" - githubURL := "https://github.com/badGoose/chaOS" githubAnotherURL := "git@github.com:goodGoose/bhaOS.git" githubReallyBadURL := "reallybadGoosegithub.comNotEvenAURL" githubToken := "hunter2" @@ -181,7 +178,7 @@ func TestInitPipelineOpts_Ask(t *testing.T) { "no flags, prompts for all input, success case for GitHub": { inEnvironments: []string{}, inRepoURL: "", - inGitHubAccessToken: "", + inGitHubAccessToken: githubToken, inGitBranch: "", buffer: *bytes.NewBufferString("archer\tgit@github.com:goodGoose/bhaOS (fetch)\narcher\thttps://github.com/badGoose/chaOS (push)\narcher\tcodecommit::us-west-2://repo-man (fetch)\n"), @@ -203,12 +200,11 @@ func TestInitPipelineOpts_Ask(t *testing.T) { }, mockPrompt: func(m *mocks.Mockprompter) { m.EXPECT().SelectOne(pipelineSelectURLPrompt, gomock.Any(), gomock.Any()).Return(githubAnotherURL, nil).Times(1) - m.EXPECT().GetSecret(gomock.Eq("Please enter your GitHub Personal Access Token for your repository bhaOS:"), gomock.Any()).Return(githubToken, nil).Times(1) }, mockSessProvider: func(m *mocks.MocksessionProvider) {}, expectedRepoURL: githubAnotherURL, - expectedGitHubOwner: githubAnotherOwner, + expectedGitHubOwner: githubOwner, expectedRepoName: githubAnotherRepoName, expectedGitHubAccessToken: githubToken, expectedRepoBranch: "main", @@ -364,42 +360,6 @@ func TestInitPipelineOpts_Ask(t *testing.T) { expectedEnvironments: []string{"test", "prod"}, expectedError: fmt.Errorf("unable to parse the GitHub repository owner and name from reallybadGoosegithub.comNotEvenAURL: please pass the repository URL with the format `--url https://github.com/{owner}/{repositoryName}`"), }, - "returns error if fail to get GitHub access token": { - inEnvironments: []string{}, - inRepoURL: "", - inGitHubAccessToken: "", - inGitBranch: "", - buffer: *bytes.NewBufferString("archer\tgit@github.com:goodGoose/bhaOS (fetch)\narcher\thttps://github.com/badGoose/chaOS (push)\n"), - - mockSelector: func(m *mocks.MockpipelineSelector) { - m.EXPECT().Environments(pipelineSelectEnvPrompt, gomock.Any(), "my-app", gomock.Any()).Return([]string{"test", "prod"}, nil) - }, - mockStore: func(m *mocks.Mockstore) { - m.EXPECT().GetEnvironment("my-app", "test").Return(&config.Environment{ - Name: "test", - Region: "us-west-2", - }, nil) - m.EXPECT().GetEnvironment("my-app", "prod").Return(&config.Environment{ - Name: "prod", - Region: "us-west-2", - }, nil) - }, - mockRunner: func(m *mocks.Mockrunner) { - m.EXPECT().Run(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) - }, - mockPrompt: func(m *mocks.Mockprompter) { - m.EXPECT().SelectOne(pipelineSelectURLPrompt, gomock.Any(), gomock.Any()).Return(githubURL, nil).Times(1) - m.EXPECT().GetSecret(gomock.Eq("Please enter your GitHub Personal Access Token for your repository chaOS:"), gomock.Any()).Return("", errors.New("some error")).Times(1) - }, - mockSessProvider: func(m *mocks.MocksessionProvider) {}, - - expectedGitHubOwner: githubOwner, - expectedRepoName: githubRepoName, - expectedGitHubAccessToken: "", - expectedRepoBranch: "main", - expectedEnvironments: []string{"test", "prod"}, - expectedError: fmt.Errorf("get GitHub access token: some error"), - }, "returns error if fail to parse repo name out of CodeCommit URL": { inEnvironments: []string{}, inGitHubAccessToken: "", @@ -599,8 +559,8 @@ func TestInitPipelineOpts_Execute(t *testing.T) { expectedError error }{ - "creates secret and writes manifest and buildspecs for GH provider": { - inProvider: "GitHub", + "creates secret and writes manifest and buildspec for GHV1 provider": { + inProvider: "GitHubV1", inEnvConfigs: []*config.Environment{ { Name: "test", @@ -641,7 +601,7 @@ func TestInitPipelineOpts_Execute(t *testing.T) { }, expectedError: nil, }, - "creates secret and writes manifest and buildspecs for CC provider": { + "writes manifest and buildspec for GH(v2) provider": { inProvider: "CodeCommit", inEnvConfigs: []*config.Environment{ { @@ -649,10 +609,48 @@ func TestInitPipelineOpts_Execute(t *testing.T) { Prod: false, }, }, - inRepoName: "goose", - inBranch: "main", - inAppName: "badgoose", - inGitHubToken: "", + inRepoName: "goose", + inBranch: "main", + inAppName: "badgoose", + + mockSecretsManager: func(m *mocks.MocksecretsManager) {}, + mockWsWriter: func(m *mocks.MockwsPipelineWriter) { + m.EXPECT().WritePipelineManifest(gomock.Any()).Return("/pipeline.yml", nil) + m.EXPECT().WritePipelineBuildspec(gomock.Any()).Return("/buildspec.yml", nil) + }, + mockParser: func(m *templatemocks.MockParser) { + m.EXPECT().Parse(buildspecTemplatePath, gomock.Any()).Return(&template.Content{ + Buffer: bytes.NewBufferString("hello"), + }, nil) + }, + mockStoreSvc: func(m *mocks.Mockstore) { + m.EXPECT().GetApplication("badgoose").Return(&config.Application{ + Name: "badgoose", + }, nil) + }, + mockRegionalResourcesGetter: func(m *mocks.MockappResourcesGetter) { + m.EXPECT().GetRegionalAppResources(&config.Application{ + Name: "badgoose", + }).Return([]*stack.AppRegionalResources{ + { + Region: "us-west-2", + S3Bucket: "gooseBucket", + }, + }, nil) + }, + expectedError: nil, + }, + "writes manifest and buildspec for CC provider": { + inProvider: "CodeCommit", + inEnvConfigs: []*config.Environment{ + { + Name: "test", + Prod: false, + }, + }, + inRepoName: "goose", + inBranch: "main", + inAppName: "badgoose", mockSecretsManager: func(m *mocks.MocksecretsManager) {}, mockWsWriter: func(m *mocks.MockwsPipelineWriter) { @@ -681,7 +679,7 @@ func TestInitPipelineOpts_Execute(t *testing.T) { }, expectedError: nil, }, - "creates secret and writes manifest and buildspecs for BB provider": { + "writes manifest and buildspec for BB provider": { inProvider: "Bitbucket", inEnvConfigs: []*config.Environment{ { @@ -721,7 +719,7 @@ func TestInitPipelineOpts_Execute(t *testing.T) { expectedError: nil, }, "does not return an error if secret already exists": { - inProvider: "GitHub", + inProvider: "GitHubV1", inEnvConfigs: []*config.Environment{ { Name: "test", @@ -765,7 +763,7 @@ func TestInitPipelineOpts_Execute(t *testing.T) { expectedError: nil, }, "returns an error if can't write manifest": { - inProvider: "GitHub", + inProvider: "GitHubV1", inEnvConfigs: []*config.Environment{ { Name: "test", @@ -789,7 +787,7 @@ func TestInitPipelineOpts_Execute(t *testing.T) { expectedError: errors.New("write pipeline manifest to workspace: some error"), }, "returns an error if application cannot be retrieved": { - inProvider: "GitHub", + inProvider: "GitHubV1", inEnvConfigs: []*config.Environment{ { Name: "test", @@ -815,7 +813,7 @@ func TestInitPipelineOpts_Execute(t *testing.T) { expectedError: errors.New("get application badgoose: some error"), }, "returns an error if can't get regional application resources": { - inProvider: "GitHub", + inProvider: "GitHubV1", inEnvConfigs: []*config.Environment{ { Name: "test", @@ -847,7 +845,7 @@ func TestInitPipelineOpts_Execute(t *testing.T) { expectedError: fmt.Errorf("get regional application resources: some error"), }, "returns an error if buildspec cannot be parsed": { - inProvider: "GitHub", + inProvider: "GitHubV1", inEnvConfigs: []*config.Environment{ { Name: "test", @@ -887,7 +885,7 @@ func TestInitPipelineOpts_Execute(t *testing.T) { expectedError: errors.New("some error"), }, "does not return an error if buildspec and manifest already exists": { - inProvider: "GitHub", + inProvider: "GitHubV1", inEnvConfigs: []*config.Environment{ { Name: "test", @@ -929,7 +927,7 @@ func TestInitPipelineOpts_Execute(t *testing.T) { expectedError: nil, }, "returns an error if can't write buildspec": { - inProvider: "GitHub", + inProvider: "GitHubV1", inEnvConfigs: []*config.Environment{ { Name: "test", diff --git a/internal/pkg/cli/pipeline_update.go b/internal/pkg/cli/pipeline_update.go index 33b92b9877f..44f20a164eb 100644 --- a/internal/pkg/cli/pipeline_update.go +++ b/internal/pkg/cli/pipeline_update.go @@ -241,13 +241,32 @@ func (o *updatePipelineOpts) Execute() error { var source interface{} switch pipeline.Source.ProviderName { - case ghProviderName: - source = &deploy.GitHubSource{ - ProviderName: ghProviderName, + case ghV1ProviderName: + source = &deploy.GitHubV1Source{ + ProviderName: ghV1ProviderName, Branch: (pipeline.Source.Properties["branch"]).(string), RepositoryURL: (pipeline.Source.Properties["repository"]).(string), PersonalAccessTokenSecretID: (pipeline.Source.Properties["access_token_secret"]).(string), } + case ghProviderName: + // If the creation of the user's pipeline manifest predates Copilot's conversion to GHv2/CSC, the provider + // listed in the manifest will be "GitHub," not "GitHubV1." To differentiate it from the new default + // "GitHub," which refers to v2, we check for the presence of a secret, indicating a v1 GitHub connection. + if pipeline.Source.Properties["access_token_secret"] != nil { + source = &deploy.GitHubV1Source{ + ProviderName: ghV1ProviderName, + Branch: (pipeline.Source.Properties["branch"]).(string), + RepositoryURL: (pipeline.Source.Properties["repository"]).(string), + PersonalAccessTokenSecretID: (pipeline.Source.Properties["access_token_secret"]).(string), + } + } else { + source = &deploy.GitHubSource{ + ProviderName: ghProviderName, + Branch: (pipeline.Source.Properties["branch"]).(string), + RepositoryURL: (pipeline.Source.Properties["repository"]).(string), + } + o.shouldPromptUpdateConnection = true + } case ccProviderName: source = &deploy.CodeCommitSource{ ProviderName: ccProviderName, diff --git a/internal/pkg/deploy/cloudformation/pipeline_test.go b/internal/pkg/deploy/cloudformation/pipeline_test.go index 22978e052b1..639474c0ee3 100644 --- a/internal/pkg/deploy/cloudformation/pipeline_test.go +++ b/internal/pkg/deploy/cloudformation/pipeline_test.go @@ -236,9 +236,8 @@ func TestCloudFormation_UpdatePipeline(t *testing.T) { AppName: "kudos", Name: "cicd", Source: &deploy.GitHubSource{ - RepositoryURL: "aws/somethingCool", - PersonalAccessTokenSecretID: "github-token-badgoose-backend", - Branch: "main", + RepositoryURL: "aws/somethingCool", + Branch: "main", }, Stages: nil, ArtifactBuckets: nil, diff --git a/internal/pkg/deploy/cloudformation/stack/pipeline_test.go b/internal/pkg/deploy/cloudformation/stack/pipeline_test.go index 8a9a0ac9429..38b91de4e6a 100644 --- a/internal/pkg/deploy/cloudformation/stack/pipeline_test.go +++ b/internal/pkg/deploy/cloudformation/stack/pipeline_test.go @@ -126,9 +126,8 @@ func mockCreatePipelineInput() *deploy.CreatePipelineInput { AppName: projectName, Name: pipelineName, Source: &deploy.GitHubSource{ - RepositoryURL: "hencrice/amazon-ecs-cli-v2", - Branch: defaultBranch, - PersonalAccessTokenSecretID: "testGitHubSecret", + RepositoryURL: "hencrice/amazon-ecs-cli-v2", + Branch: defaultBranch, }, Stages: []deploy.PipelineStage{ { diff --git a/internal/pkg/deploy/pipeline.go b/internal/pkg/deploy/pipeline.go index 9c0cbdb57f7..14abdc49368 100644 --- a/internal/pkg/deploy/pipeline.go +++ b/internal/pkg/deploy/pipeline.go @@ -72,8 +72,14 @@ func (a *ArtifactBucket) Region() (string, error) { return parsedArn.Region, nil } -// GitHubSource defines the (GH) source of the artifacts to be built and deployed. type GitHubSource struct { + ProviderName string + Branch string + RepositoryURL string +} + +// GitHubSource defines the (GH) source of the artifacts to be built and deployed. +type GitHubV1Source struct { ProviderName string Branch string RepositoryURL string @@ -97,7 +103,7 @@ type BitbucketSource struct { // GitHubPersonalAccessTokenSecretID returns the ID of the secret in the // Secrets manager, which stores the GitHub Personal Access token if the // provider is "GitHub". Otherwise, it returns the detected provider. -func (s *GitHubSource) GitHubPersonalAccessTokenSecretID() (string, error) { +func (s *GitHubV1Source) GitHubPersonalAccessTokenSecretID() (string, error) { if s.PersonalAccessTokenSecretID == "" { return "", errors.New("the GitHub token secretID is not configured") } @@ -124,6 +130,26 @@ func (s *GitHubSource) parseOwnerAndRepo() (owner, repo string, err error) { return matches["owner"], matches["repo"], nil } +// parseOwnerAndRepo parses the owner and repo name from the GH repo URL, which was formatted and assigned in cli/pipeline_init.go. +func (s *GitHubV1Source) parseOwnerAndRepo() (owner, repo string, err error) { + if s.RepositoryURL == "" { + return "", "", fmt.Errorf("unable to locate the repository") + } + + match := ghRepoExp.FindStringSubmatch(s.RepositoryURL) + if len(match) == 0 { + return "", "", fmt.Errorf(fmtInvalidRepo, s.RepositoryURL) + } + + matches := make(map[string]string) + for i, name := range ghRepoExp.SubexpNames() { + if i != 0 && name != "" { + matches[name] = match[i] + } + } + return matches["owner"], matches["repo"], nil +} + // parseRepo parses the region (not returned) and repo name from the CC repo URL, which was formatted and assigned in cli/pipeline_init.go. func (s *CodeCommitSource) parseRepo() (string, error) { // NOTE: 'region' is not currently parsed out as a Source property, but this enables that possibility. @@ -190,9 +216,24 @@ func (s *BitbucketSource) ConnectionName() (string, error) { return fmt.Sprintf(fmtConnectionName, owner, repo), nil } +func (s *GitHubSource) ConnectionName() (string, error) { + owner, repo, err := s.parseOwnerAndRepo() + if err != nil { + return "", fmt.Errorf("parse owner and repo to generate connection name: %w", err) + } + + if len(owner) > maxOwnerLength { + owner = owner[:maxOwnerLength] + } + if len(repo) > maxRepoLength { + repo = repo[:maxRepoLength] + } + return fmt.Sprintf(fmtConnectionName, owner, repo), nil +} + // Repository returns the repository portion. For example, // given "aws/amazon-copilot", this function returns "amazon-copilot". -func (s *GitHubSource) Repository() (string, error) { +func (s *GitHubV1Source) Repository() (string, error) { _, repo, err := s.parseOwnerAndRepo() if err != nil { return "", err @@ -210,6 +251,16 @@ func (s *BitbucketSource) Repository() (string, error) { return fmt.Sprintf("%s/%s", owner, repo), nil } +// Repository returns the repository portion. For CodeStar Connections, +// this needs to be in the format "some-user/my-repo." +func (s *GitHubSource) Repository() (string, error) { + owner, repo, err := s.parseOwnerAndRepo() + if err != nil { + return "", err + } + return fmt.Sprintf("%s/%s", owner, repo), nil +} + // Repository returns the repository portion. For example, // given "aws/amazon-copilot", this function returns "amazon-copilot". func (s *CodeCommitSource) Repository() (string, error) { @@ -230,6 +281,16 @@ func (s *GitHubSource) Owner() (string, error) { return owner, nil } +// Owner returns the repository owner portion. For example, +// given "aws/amazon-copilot", this function returns "aws". +func (s *GitHubV1Source) Owner() (string, error) { + owner, _, err := s.parseOwnerAndRepo() + if err != nil { + return "", err + } + return owner, nil +} + // PipelineStage represents configuration for each deployment stage // of a workspace. A stage consists of the Config Environment the pipeline // is deploying to, the containerized services that will be deployed, and diff --git a/internal/pkg/manifest/pipeline.go b/internal/pkg/manifest/pipeline.go index 5008d7ca8b8..0969ad1ac14 100644 --- a/internal/pkg/manifest/pipeline.go +++ b/internal/pkg/manifest/pipeline.go @@ -14,6 +14,7 @@ import ( const ( GithubProviderName = "GitHub" + GithubV1ProviderName = "GitHubV1" CodeCommitProviderName = "CodeCommit" BitbucketProviderName = "Bitbucket" @@ -28,6 +29,20 @@ type Provider interface { Properties() map[string]interface{} } +type githubV1Provider struct { + properties *GitHubV1Properties +} + +func (p *githubV1Provider) Name() string { + return GithubV1ProviderName +} +func (p *githubV1Provider) String() string { + return GithubProviderName +} +func (p *githubV1Provider) Properties() map[string]interface{} { + return structs.Map(p.properties) +} + type githubProvider struct { properties *GitHubProperties } @@ -35,11 +50,9 @@ type githubProvider struct { func (p *githubProvider) Name() string { return GithubProviderName } - func (p *githubProvider) String() string { return GithubProviderName } - func (p *githubProvider) Properties() map[string]interface{} { return structs.Map(p.properties) } @@ -51,11 +64,9 @@ type codecommitProvider struct { func (p *codecommitProvider) Name() string { return CodeCommitProviderName } - func (p *codecommitProvider) String() string { return CodeCommitProviderName } - func (p *codecommitProvider) Properties() map[string]interface{} { return structs.Map(p.properties) } @@ -67,18 +78,16 @@ type bitbucketProvider struct { func (p *bitbucketProvider) Name() string { return BitbucketProviderName } - func (p *bitbucketProvider) String() string { return BitbucketProviderName } - func (p *bitbucketProvider) Properties() map[string]interface{} { return structs.Map(p.properties) } -// GitHubProperties contain information for configuring a Github +// GitHubV1Properties contain information for configuring a Githubv1 // source provider. -type GitHubProperties struct { +type GitHubV1Properties struct { // use tag from https://godoc.org/github.com/fatih/structs#example-Map--Tags // to specify the name of the field in the output properties RepositoryURL string `structs:"repository" yaml:"repository"` @@ -86,9 +95,9 @@ type GitHubProperties struct { GithubSecretIdKeyName string `structs:"access_token_secret" yaml:"access_token_secret"` } -// CodeCommitProperties contains information for configuring a CodeCommit +// GitHubProperties contains information for configuring a GitHubv2 // source provider. -type CodeCommitProperties struct { +type GitHubProperties struct { RepositoryURL string `structs:"repository" yaml:"repository"` Branch string `structs:"branch" yaml:"branch"` } @@ -100,10 +109,21 @@ type BitbucketProperties struct { Branch string `structs:"branch" yaml:"branch"` } +// CodeCommitProperties contains information for configuring a CodeCommit +// source provider. +type CodeCommitProperties struct { + RepositoryURL string `structs:"repository" yaml:"repository"` + Branch string `structs:"branch" yaml:"branch"` +} + // NewProvider creates a source provider based on the type of // the provided provider-specific configurations func NewProvider(configs interface{}) (Provider, error) { switch props := configs.(type) { + case *GitHubV1Properties: + return &githubV1Provider{ + properties: props, + }, nil case *GitHubProperties: return &githubProvider{ properties: props, diff --git a/templates/cicd/pipeline_cfn.yml b/templates/cicd/pipeline_cfn.yml index bed8595b350..cf094d037b1 100644 --- a/templates/cicd/pipeline_cfn.yml +++ b/templates/cicd/pipeline_cfn.yml @@ -3,12 +3,12 @@ AWSTemplateFormatVersion: '2010-09-09' Description: CodePipeline for {{$.AppName}} Resources: - {{- if eq .Source.ProviderName "Bitbucket"}} + {{- if or (eq .Source.ProviderName "Bitbucket") (eq .Source.ProviderName "GitHub")}} SourceConnection: Type: AWS::CodeStarConnections::Connection Properties: ConnectionName: {{.Source.ConnectionName}} - ProviderType: Bitbucket + ProviderType: {{.Source.ProviderName}} {{- end}} BuildProjectRole: Type: AWS::IAM::Role @@ -168,7 +168,7 @@ Resources: - s3:GetBucketLocation Resource: - "*" - {{- if eq .Source.ProviderName "Bitbucket"}} + {{- if or (eq .Source.ProviderName "Bitbucket") (eq .Source.ProviderName "GitHub")}} - Effect: Allow Action: - codestar-connections:CreateConnection @@ -203,7 +203,7 @@ Resources: - s3:GetBucketPolicy - s3:GetObject - s3:ListBucket - {{- if eq .Source.ProviderName "Bitbucket"}} + {{- if or (eq .Source.ProviderName "Bitbucket") (eq .Source.ProviderName "GitHub")}} - s3:PutObjectAcl - s3:GetObjectAcl {{- end}} @@ -259,7 +259,7 @@ Resources: RoleArn: !GetAtt PipelineRole.Arn Name: !Ref AWS::StackName Stages: - {{- if eq .Source.ProviderName "GitHub"}} + {{- if eq .Source.ProviderName "GitHubV1"}} - Name: Source Actions: - Name: SourceCodeFor-{{$.AppName}} @@ -280,6 +280,22 @@ Resources: OutputArtifacts: - Name: SCCheckoutArtifact RunOrder: 1 + {{- else if eq .Source.ProviderName "GitHub"}} + - Name: Source + Actions: + - Name: SourceCodeFor-{{$.AppName}} + ActionTypeId: + Category: Source + Owner: AWS + Version: 1 + Provider: CodeStarSourceConnection + Configuration: + ConnectionArn: !Ref SourceConnection + FullRepositoryId: {{$.Source.Repository}} + BranchName: {{$.Source.Branch}} + OutputArtifacts: + - Name: SCCheckoutArtifact + RunOrder: 1 {{- else if eq .Source.ProviderName "CodeCommit"}} - Name: Source Actions: @@ -374,7 +390,7 @@ Resources: RunOrder: 3 InputArtifacts: - Name: SCCheckoutArtifact{{end}}{{end}}{{end}}{{end}} -{{- if eq .Source.ProviderName "Bitbucket"}} +{{- if or (eq .Source.ProviderName "Bitbucket") (eq .Source.ProviderName "GitHub")}} Outputs: PipelineConnectionARN: Description: "ARN of CodeStar Connections connection" From 5c84b64ef1b3af04daa51f3214093bc4bcf17f30 Mon Sep 17 00:00:00 2001 From: Janice Date: Mon, 8 Mar 2021 10:52:08 -0800 Subject: [PATCH 2/9] refactor: add shared parsing method for URLs --- internal/pkg/cli/pipeline_update.go | 6 +-- internal/pkg/deploy/pipeline.go | 61 +++++++++------------------- internal/pkg/deploy/pipeline_test.go | 2 +- 3 files changed, 24 insertions(+), 45 deletions(-) diff --git a/internal/pkg/cli/pipeline_update.go b/internal/pkg/cli/pipeline_update.go index 44f20a164eb..54751f7f145 100644 --- a/internal/pkg/cli/pipeline_update.go +++ b/internal/pkg/cli/pipeline_update.go @@ -245,7 +245,7 @@ func (o *updatePipelineOpts) Execute() error { source = &deploy.GitHubV1Source{ ProviderName: ghV1ProviderName, Branch: (pipeline.Source.Properties["branch"]).(string), - RepositoryURL: (pipeline.Source.Properties["repository"]).(string), + RepositoryURL: deploy.GitHubURL((pipeline.Source.Properties["repository"]).(string)), PersonalAccessTokenSecretID: (pipeline.Source.Properties["access_token_secret"]).(string), } case ghProviderName: @@ -256,14 +256,14 @@ func (o *updatePipelineOpts) Execute() error { source = &deploy.GitHubV1Source{ ProviderName: ghV1ProviderName, Branch: (pipeline.Source.Properties["branch"]).(string), - RepositoryURL: (pipeline.Source.Properties["repository"]).(string), + RepositoryURL: deploy.GitHubURL((pipeline.Source.Properties["repository"]).(string)), PersonalAccessTokenSecretID: (pipeline.Source.Properties["access_token_secret"]).(string), } } else { source = &deploy.GitHubSource{ ProviderName: ghProviderName, Branch: (pipeline.Source.Properties["branch"]).(string), - RepositoryURL: (pipeline.Source.Properties["repository"]).(string), + RepositoryURL: deploy.GitHubURL((pipeline.Source.Properties["repository"]).(string)), } o.shouldPromptUpdateConnection = true } diff --git a/internal/pkg/deploy/pipeline.go b/internal/pkg/deploy/pipeline.go index 14abdc49368..978334c322e 100644 --- a/internal/pkg/deploy/pipeline.go +++ b/internal/pkg/deploy/pipeline.go @@ -75,17 +75,20 @@ func (a *ArtifactBucket) Region() (string, error) { type GitHubSource struct { ProviderName string Branch string - RepositoryURL string + RepositoryURL GitHubURL } // GitHubSource defines the (GH) source of the artifacts to be built and deployed. type GitHubV1Source struct { ProviderName string Branch string - RepositoryURL string + RepositoryURL GitHubURL PersonalAccessTokenSecretID string } +// GitHubURL is the common type for repo URLs for both GitHubSource versions +type GitHubURL string + // CodeCommitSource defines the (CC) source of the artifacts to be built and deployed. type CodeCommitSource struct { ProviderName string @@ -110,35 +113,15 @@ func (s *GitHubV1Source) GitHubPersonalAccessTokenSecretID() (string, error) { return s.PersonalAccessTokenSecretID, nil } -// parseOwnerAndRepo parses the owner and repo name from the GH repo URL, which was formatted and assigned in cli/pipeline_init.go. -func (s *GitHubSource) parseOwnerAndRepo() (owner, repo string, err error) { - if s.RepositoryURL == "" { +// parse parses the owner and repo name from the GH repo URL, which was formatted and assigned in cli/pipeline_init.go. +func (url GitHubURL) parse() (owner, repo string, err error) { + if url == "" { return "", "", fmt.Errorf("unable to locate the repository") } - match := ghRepoExp.FindStringSubmatch(s.RepositoryURL) + match := ghRepoExp.FindStringSubmatch(string(url)) if len(match) == 0 { - return "", "", fmt.Errorf(fmtInvalidRepo, s.RepositoryURL) - } - - matches := make(map[string]string) - for i, name := range ghRepoExp.SubexpNames() { - if i != 0 && name != "" { - matches[name] = match[i] - } - } - return matches["owner"], matches["repo"], nil -} - -// parseOwnerAndRepo parses the owner and repo name from the GH repo URL, which was formatted and assigned in cli/pipeline_init.go. -func (s *GitHubV1Source) parseOwnerAndRepo() (owner, repo string, err error) { - if s.RepositoryURL == "" { - return "", "", fmt.Errorf("unable to locate the repository") - } - - match := ghRepoExp.FindStringSubmatch(s.RepositoryURL) - if len(match) == 0 { - return "", "", fmt.Errorf(fmtInvalidRepo, s.RepositoryURL) + return "", "", fmt.Errorf(fmtInvalidRepo, url) } matches := make(map[string]string) @@ -206,35 +189,31 @@ func (s *BitbucketSource) ConnectionName() (string, error) { if err != nil { return "", fmt.Errorf("parse owner and repo to generate connection name: %w", err) } - - if len(owner) > maxOwnerLength { - owner = owner[:maxOwnerLength] - } - if len(repo) > maxRepoLength { - repo = repo[:maxRepoLength] - } - return fmt.Sprintf(fmtConnectionName, owner, repo), nil + return formatConnectionName(owner, repo), nil } func (s *GitHubSource) ConnectionName() (string, error) { - owner, repo, err := s.parseOwnerAndRepo() + owner, repo, err := s.RepositoryURL.parse() if err != nil { return "", fmt.Errorf("parse owner and repo to generate connection name: %w", err) } + return formatConnectionName(owner, repo), nil +} +func formatConnectionName(owner, repo string) string { if len(owner) > maxOwnerLength { owner = owner[:maxOwnerLength] } if len(repo) > maxRepoLength { repo = repo[:maxRepoLength] } - return fmt.Sprintf(fmtConnectionName, owner, repo), nil + return fmt.Sprintf(fmtConnectionName, owner, repo) } // Repository returns the repository portion. For example, // given "aws/amazon-copilot", this function returns "amazon-copilot". func (s *GitHubV1Source) Repository() (string, error) { - _, repo, err := s.parseOwnerAndRepo() + _, repo, err := s.RepositoryURL.parse() if err != nil { return "", err } @@ -254,7 +233,7 @@ func (s *BitbucketSource) Repository() (string, error) { // Repository returns the repository portion. For CodeStar Connections, // this needs to be in the format "some-user/my-repo." func (s *GitHubSource) Repository() (string, error) { - owner, repo, err := s.parseOwnerAndRepo() + owner, repo, err := s.RepositoryURL.parse() if err != nil { return "", err } @@ -274,7 +253,7 @@ func (s *CodeCommitSource) Repository() (string, error) { // Owner returns the repository owner portion. For example, // given "aws/amazon-copilot", this function returns "aws". func (s *GitHubSource) Owner() (string, error) { - owner, _, err := s.parseOwnerAndRepo() + owner, _, err := s.RepositoryURL.parse() if err != nil { return "", err } @@ -284,7 +263,7 @@ func (s *GitHubSource) Owner() (string, error) { // Owner returns the repository owner portion. For example, // given "aws/amazon-copilot", this function returns "aws". func (s *GitHubV1Source) Owner() (string, error) { - owner, _, err := s.parseOwnerAndRepo() + owner, _, err := s.RepositoryURL.parse() if err != nil { return "", err } diff --git a/internal/pkg/deploy/pipeline_test.go b/internal/pkg/deploy/pipeline_test.go index 2e7da0b7a82..e67c93db490 100644 --- a/internal/pkg/deploy/pipeline_test.go +++ b/internal/pkg/deploy/pipeline_test.go @@ -44,7 +44,7 @@ func TestParseOwnerAndRepo(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { - owner, repo, err := tc.src.parseOwnerAndRepo() + owner, repo, err := tc.src.RepositoryURL.parse() if tc.expectedErrMsg != nil { require.Contains(t, err.Error(), *tc.expectedErrMsg) } else { From bf2cfe6f20f16d2319269a8f3a41d8a5f3b6b39f Mon Sep 17 00:00:00 2001 From: Janice Date: Mon, 8 Mar 2021 16:40:20 -0800 Subject: [PATCH 3/9] fix: tweak wording of comment --- internal/pkg/deploy/pipeline.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/deploy/pipeline.go b/internal/pkg/deploy/pipeline.go index 978334c322e..d060bbbf933 100644 --- a/internal/pkg/deploy/pipeline.go +++ b/internal/pkg/deploy/pipeline.go @@ -105,7 +105,7 @@ type BitbucketSource struct { // GitHubPersonalAccessTokenSecretID returns the ID of the secret in the // Secrets manager, which stores the GitHub Personal Access token if the -// provider is "GitHub". Otherwise, it returns the detected provider. +// provider is "GitHubV1". func (s *GitHubV1Source) GitHubPersonalAccessTokenSecretID() (string, error) { if s.PersonalAccessTokenSecretID == "" { return "", errors.New("the GitHub token secretID is not configured") From f1502ade3e192cab09917dae516f8f865cc1596e Mon Sep 17 00:00:00 2001 From: Janice Date: Mon, 8 Mar 2021 18:37:02 -0800 Subject: [PATCH 4/9] chore: move pipeline source categorization into deploy package; fix odds and ends --- internal/pkg/cli/pipeline_delete.go | 2 +- internal/pkg/cli/pipeline_init.go | 6 +-- internal/pkg/cli/pipeline_update.go | 47 ++--------------- internal/pkg/cli/pipeline_update_test.go | 2 +- internal/pkg/deploy/pipeline.go | 67 +++++++++++++++++++++--- 5 files changed, 69 insertions(+), 55 deletions(-) diff --git a/internal/pkg/cli/pipeline_delete.go b/internal/pkg/cli/pipeline_delete.go index 54e0d9c9452..759f504c401 100644 --- a/internal/pkg/cli/pipeline_delete.go +++ b/internal/pkg/cli/pipeline_delete.go @@ -118,7 +118,6 @@ func (o *deletePipelineOpts) Ask() error { // Execute deletes the secret and pipeline stack. func (o *deletePipelineOpts) Execute() error { - // Pipelines created with GitHubV1 have secrets. if err := o.deleteSecret(); err != nil { return err } @@ -157,6 +156,7 @@ func (o *deletePipelineOpts) deleteSecret() error { if o.PipelineSecret == "" { return nil } + // Only pipelines created with GitHubV1 have personal access tokens saved as secrets. if !o.shouldDeleteSecret { confirmDeletion, err := o.prompt.Confirm( fmt.Sprintf(pipelineSecretDeleteConfirmPrompt, o.PipelineSecret, o.PipelineName), diff --git a/internal/pkg/cli/pipeline_init.go b/internal/pkg/cli/pipeline_init.go index 9b3d0ddb3f5..1b09d773d8e 100644 --- a/internal/pkg/cli/pipeline_init.go +++ b/internal/pkg/cli/pipeline_init.go @@ -270,13 +270,13 @@ func (o *initPipelineOpts) askRepository() error { } func (o *initPipelineOpts) askGitHubRepoDetails() error { - // If the user uses a flag (now hidden) to specify a GitHub access token, + // If the user uses a flag to specify a GitHub access token, // GitHub version 1 (not CSC) is the provider. + o.provider = ghProviderName if o.githubAccessToken != "" { o.provider = ghV1ProviderName - } else { - o.provider = ghProviderName } + repoDetails, err := ghRepoURL(o.repoURL).parse() if err != nil { return err diff --git a/internal/pkg/cli/pipeline_update.go b/internal/pkg/cli/pipeline_update.go index 54751f7f145..ffaa3e12af0 100644 --- a/internal/pkg/cli/pipeline_update.go +++ b/internal/pkg/cli/pipeline_update.go @@ -239,50 +239,11 @@ func (o *updatePipelineOpts) Execute() error { } o.pipelineName = pipeline.Name - var source interface{} - switch pipeline.Source.ProviderName { - case ghV1ProviderName: - source = &deploy.GitHubV1Source{ - ProviderName: ghV1ProviderName, - Branch: (pipeline.Source.Properties["branch"]).(string), - RepositoryURL: deploy.GitHubURL((pipeline.Source.Properties["repository"]).(string)), - PersonalAccessTokenSecretID: (pipeline.Source.Properties["access_token_secret"]).(string), - } - case ghProviderName: - // If the creation of the user's pipeline manifest predates Copilot's conversion to GHv2/CSC, the provider - // listed in the manifest will be "GitHub," not "GitHubV1." To differentiate it from the new default - // "GitHub," which refers to v2, we check for the presence of a secret, indicating a v1 GitHub connection. - if pipeline.Source.Properties["access_token_secret"] != nil { - source = &deploy.GitHubV1Source{ - ProviderName: ghV1ProviderName, - Branch: (pipeline.Source.Properties["branch"]).(string), - RepositoryURL: deploy.GitHubURL((pipeline.Source.Properties["repository"]).(string)), - PersonalAccessTokenSecretID: (pipeline.Source.Properties["access_token_secret"]).(string), - } - } else { - source = &deploy.GitHubSource{ - ProviderName: ghProviderName, - Branch: (pipeline.Source.Properties["branch"]).(string), - RepositoryURL: deploy.GitHubURL((pipeline.Source.Properties["repository"]).(string)), - } - o.shouldPromptUpdateConnection = true - } - case ccProviderName: - source = &deploy.CodeCommitSource{ - ProviderName: ccProviderName, - Branch: (pipeline.Source.Properties["branch"]).(string), - RepositoryURL: (pipeline.Source.Properties["repository"]).(string), - } - case bbProviderName: - source = &deploy.BitbucketSource{ - ProviderName: bbProviderName, - Branch: (pipeline.Source.Properties["branch"]).(string), - RepositoryURL: (pipeline.Source.Properties["repository"]).(string), - } - o.shouldPromptUpdateConnection = true - default: - return fmt.Errorf("invalid repo source provider: %s", pipeline.Source.ProviderName) + source, bool, err := deploy.PipelineSourceFromManifest(pipeline.Source) + if err != nil { + return fmt.Errorf("read source from manifest: %w", err) } + o.shouldPromptUpdateConnection = bool // convert environments to deployment stages stages, err := o.convertStages(pipeline.Stages) diff --git a/internal/pkg/cli/pipeline_update_test.go b/internal/pkg/cli/pipeline_update_test.go index 1f90dd87c97..51325d6bd11 100644 --- a/internal/pkg/cli/pipeline_update_test.go +++ b/internal/pkg/cli/pipeline_update_test.go @@ -492,7 +492,7 @@ source: m.ws.EXPECT().ReadPipelineManifest().Return([]byte(content), nil), ) }, - expectedError: fmt.Errorf("invalid repo source provider: NotGitHub"), + expectedError: fmt.Errorf("read source from manifest: invalid repo source provider: NotGitHub"), }, "returns an error if unable to convert environments to deployment stage": { inApp: &app, diff --git a/internal/pkg/deploy/pipeline.go b/internal/pkg/deploy/pipeline.go index d060bbbf933..0b54bd12e29 100644 --- a/internal/pkg/deploy/pipeline.go +++ b/internal/pkg/deploy/pipeline.go @@ -10,11 +10,18 @@ import ( "fmt" "regexp" + "github.com/aws/copilot-cli/internal/pkg/manifest" + "github.com/aws/aws-sdk-go/aws/arn" ) const ( fmtInvalidRepo = "unable to locate the repository URL from the properties: %+v" + // Redefining these consts from the cli package here so as to avoid an import cycle + ghProviderName = "GitHub" + ghV1ProviderName = "GitHubV1" + ccProviderName = "CodeCommit" + bbProviderName = "Bitbucket" ) var ( @@ -72,12 +79,6 @@ func (a *ArtifactBucket) Region() (string, error) { return parsedArn.Region, nil } -type GitHubSource struct { - ProviderName string - Branch string - RepositoryURL GitHubURL -} - // GitHubSource defines the (GH) source of the artifacts to be built and deployed. type GitHubV1Source struct { ProviderName string @@ -86,7 +87,7 @@ type GitHubV1Source struct { PersonalAccessTokenSecretID string } -// GitHubURL is the common type for repo URLs for both GitHubSource versions +// GitHubURL is the common type for repo URLs for both GitHubSource versions. type GitHubURL string // CodeCommitSource defines the (CC) source of the artifacts to be built and deployed. @@ -103,6 +104,58 @@ type BitbucketSource struct { RepositoryURL string } +// PipelineSourceFromManifest processes manifest info about the source based on provider type. +// The return boolean is true for CodeStar Connections sources that require a polling prompt. +func PipelineSourceFromManifest(source *manifest.Source) (interface{}, bool, error) { + switch source.ProviderName { + case ghV1ProviderName: + return GitHubV1Source{ + ProviderName: ghV1ProviderName, + Branch: (source.Properties["branch"]).(string), + RepositoryURL: GitHubURL((source.Properties["repository"]).(string)), + PersonalAccessTokenSecretID: (source.Properties["access_token_secret"]).(string), + }, false, nil + case ghProviderName: + // If the creation of the user's pipeline manifest predates Copilot's conversion to GHv2/CSC, the provider + // listed in the manifest will be "GitHub," not "GitHubV1." To differentiate it from the new default + // "GitHub," which refers to v2, we check for the presence of a secret, indicating a v1 GitHub connection. + if source.Properties["access_token_secret"] != nil { + return GitHubV1Source{ + ProviderName: ghV1ProviderName, + Branch: (source.Properties["branch"]).(string), + RepositoryURL: GitHubURL((source.Properties["repository"]).(string)), + PersonalAccessTokenSecretID: (source.Properties["access_token_secret"]).(string), + }, false, nil + } else { + return GitHubSource{ + ProviderName: ghProviderName, + Branch: (source.Properties["branch"]).(string), + RepositoryURL: GitHubURL((source.Properties["repository"]).(string)), + }, true, nil + } + case ccProviderName: + return CodeCommitSource{ + ProviderName: ccProviderName, + Branch: (source.Properties["branch"]).(string), + RepositoryURL: (source.Properties["repository"]).(string), + }, false, nil + case bbProviderName: + return BitbucketSource{ + ProviderName: bbProviderName, + Branch: (source.Properties["branch"]).(string), + RepositoryURL: (source.Properties["repository"]).(string), + }, true, nil + default: + return nil, false, fmt.Errorf("invalid repo source provider: %s", source.ProviderName) + } +} + +type GitHubSource struct { + ProviderName string + Branch string + RepositoryURL GitHubURL +} + // GitHubPersonalAccessTokenSecretID returns the ID of the secret in the // Secrets manager, which stores the GitHub Personal Access token if the // provider is "GitHubV1". From 7875da82685df6feba6ae4f64c712d5861e744fd Mon Sep 17 00:00:00 2001 From: Janice Date: Tue, 9 Mar 2021 12:54:22 -0800 Subject: [PATCH 5/9] fix: reword error msg --- internal/pkg/cli/pipeline_update.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/cli/pipeline_update.go b/internal/pkg/cli/pipeline_update.go index ffaa3e12af0..640e7edbe23 100644 --- a/internal/pkg/cli/pipeline_update.go +++ b/internal/pkg/cli/pipeline_update.go @@ -175,7 +175,7 @@ func (o *updatePipelineOpts) deployPipeline(in *deploy.CreatePipelineInput) erro if o.shouldPromptUpdateConnection { source, ok := in.Source.(codestar) if !ok { - return fmt.Errorf("source %v does not have a connection name", in.Source) + return fmt.Errorf("source %v does not have a connection", in.Source) } connectionName, err := source.ConnectionName() if err != nil { From 3787fa451087c6c1c82e591952af6be817afcf89 Mon Sep 17 00:00:00 2001 From: Janice Date: Tue, 9 Mar 2021 13:22:15 -0800 Subject: [PATCH 6/9] Revert "fix: reword error msg" This reverts commit 7875da82685df6feba6ae4f64c712d5861e744fd. --- internal/pkg/cli/pipeline_update.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/cli/pipeline_update.go b/internal/pkg/cli/pipeline_update.go index 640e7edbe23..ffaa3e12af0 100644 --- a/internal/pkg/cli/pipeline_update.go +++ b/internal/pkg/cli/pipeline_update.go @@ -175,7 +175,7 @@ func (o *updatePipelineOpts) deployPipeline(in *deploy.CreatePipelineInput) erro if o.shouldPromptUpdateConnection { source, ok := in.Source.(codestar) if !ok { - return fmt.Errorf("source %v does not have a connection", in.Source) + return fmt.Errorf("source %v does not have a connection name", in.Source) } connectionName, err := source.ConnectionName() if err != nil { From 2438ff6e4d305315260301bd9c107c7e36e74674 Mon Sep 17 00:00:00 2001 From: Janice Date: Tue, 9 Mar 2021 13:41:29 -0800 Subject: [PATCH 7/9] fix: add &s --- internal/pkg/deploy/pipeline.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/pkg/deploy/pipeline.go b/internal/pkg/deploy/pipeline.go index 0b54bd12e29..04d61c50357 100644 --- a/internal/pkg/deploy/pipeline.go +++ b/internal/pkg/deploy/pipeline.go @@ -109,7 +109,7 @@ type BitbucketSource struct { func PipelineSourceFromManifest(source *manifest.Source) (interface{}, bool, error) { switch source.ProviderName { case ghV1ProviderName: - return GitHubV1Source{ + return &GitHubV1Source{ ProviderName: ghV1ProviderName, Branch: (source.Properties["branch"]).(string), RepositoryURL: GitHubURL((source.Properties["repository"]).(string)), @@ -120,27 +120,27 @@ func PipelineSourceFromManifest(source *manifest.Source) (interface{}, bool, err // listed in the manifest will be "GitHub," not "GitHubV1." To differentiate it from the new default // "GitHub," which refers to v2, we check for the presence of a secret, indicating a v1 GitHub connection. if source.Properties["access_token_secret"] != nil { - return GitHubV1Source{ + return &GitHubV1Source{ ProviderName: ghV1ProviderName, Branch: (source.Properties["branch"]).(string), RepositoryURL: GitHubURL((source.Properties["repository"]).(string)), PersonalAccessTokenSecretID: (source.Properties["access_token_secret"]).(string), }, false, nil } else { - return GitHubSource{ + return &GitHubSource{ ProviderName: ghProviderName, Branch: (source.Properties["branch"]).(string), RepositoryURL: GitHubURL((source.Properties["repository"]).(string)), }, true, nil } case ccProviderName: - return CodeCommitSource{ + return &CodeCommitSource{ ProviderName: ccProviderName, Branch: (source.Properties["branch"]).(string), RepositoryURL: (source.Properties["repository"]).(string), }, false, nil case bbProviderName: - return BitbucketSource{ + return &BitbucketSource{ ProviderName: bbProviderName, Branch: (source.Properties["branch"]).(string), RepositoryURL: (source.Properties["repository"]).(string), From 8fad88a0a6aed637c1b0656b053fb53e3ed385e2 Mon Sep 17 00:00:00 2001 From: Janice Date: Tue, 9 Mar 2021 13:52:22 -0800 Subject: [PATCH 8/9] chore: use template function instead of 'or' --- .../deploy/cloudformation/stack/pipeline.go | 10 ++++++- templates/cicd/pipeline_cfn.yml | 26 ++++--------------- 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/pipeline.go b/internal/pkg/deploy/cloudformation/stack/pipeline.go index 30a26e7a3e3..10a35f692d7 100644 --- a/internal/pkg/deploy/cloudformation/stack/pipeline.go +++ b/internal/pkg/deploy/cloudformation/stack/pipeline.go @@ -31,7 +31,15 @@ func (p *pipelineStackConfig) StackName() string { } func (p *pipelineStackConfig) Template() (string, error) { - content, err := p.parser.Parse(pipelineCfnTemplatePath, p, template.WithFuncs(cfTemplateFunctions)) + content, err := p.parser.Parse(pipelineCfnTemplatePath, p, template.WithFuncs(cfTemplateFunctions), template.WithFuncs(map[string]interface{}{ + "isCodeStarConnection": func(source interface{}) bool { + type connectionName interface { + ConnectionName() (string, error) + } + _, ok := source.(connectionName) + return ok + }, + })) if err != nil { return "", err } diff --git a/templates/cicd/pipeline_cfn.yml b/templates/cicd/pipeline_cfn.yml index cf094d037b1..995073cd4a3 100644 --- a/templates/cicd/pipeline_cfn.yml +++ b/templates/cicd/pipeline_cfn.yml @@ -3,7 +3,7 @@ AWSTemplateFormatVersion: '2010-09-09' Description: CodePipeline for {{$.AppName}} Resources: - {{- if or (eq .Source.ProviderName "Bitbucket") (eq .Source.ProviderName "GitHub")}} + {{- if isCodeStarConnection .Source}} SourceConnection: Type: AWS::CodeStarConnections::Connection Properties: @@ -168,7 +168,7 @@ Resources: - s3:GetBucketLocation Resource: - "*" - {{- if or (eq .Source.ProviderName "Bitbucket") (eq .Source.ProviderName "GitHub")}} + {{- if isCodeStarConnection .Source}} - Effect: Allow Action: - codestar-connections:CreateConnection @@ -203,7 +203,7 @@ Resources: - s3:GetBucketPolicy - s3:GetObject - s3:ListBucket - {{- if or (eq .Source.ProviderName "Bitbucket") (eq .Source.ProviderName "GitHub")}} + {{- if isCodeStarConnection .Source}} - s3:PutObjectAcl - s3:GetObjectAcl {{- end}} @@ -280,7 +280,7 @@ Resources: OutputArtifacts: - Name: SCCheckoutArtifact RunOrder: 1 - {{- else if eq .Source.ProviderName "GitHub"}} + {{- else if isCodeStarConnection .Source}} - Name: Source Actions: - Name: SourceCodeFor-{{$.AppName}} @@ -311,22 +311,6 @@ Resources: OutputArtifacts: - Name: SCCheckoutArtifact RunOrder: 1 - {{- else if eq .Source.ProviderName "Bitbucket"}} - - Name: Source - Actions: - - Name: SourceCodeFor-{{$.AppName}} - ActionTypeId: - Category: Source - Owner: AWS - Version: 1 - Provider: CodeStarSourceConnection - Configuration: - ConnectionArn: !Ref SourceConnection - FullRepositoryId: {{$.Source.Repository}} - BranchName: {{$.Source.Branch}} - OutputArtifacts: - - Name: SCCheckoutArtifact - RunOrder: 1 {{- end }} - Name: Build Actions: @@ -390,7 +374,7 @@ Resources: RunOrder: 3 InputArtifacts: - Name: SCCheckoutArtifact{{end}}{{end}}{{end}}{{end}} -{{- if or (eq .Source.ProviderName "Bitbucket") (eq .Source.ProviderName "GitHub")}} +{{- if isCodeStarConnection .Source}} Outputs: PipelineConnectionARN: Description: "ARN of CodeStar Connections connection" From 3289228609c670a55a6d878c990fcfa653cef77a Mon Sep 17 00:00:00 2001 From: Janice Date: Tue, 9 Mar 2021 16:39:00 -0800 Subject: [PATCH 9/9] fix: use vars in manifest pkg, improve/add comments --- internal/pkg/cli/pipeline_init.go | 30 ++++++------ internal/pkg/deploy/pipeline.go | 77 ++++++++++++++++--------------- 2 files changed, 52 insertions(+), 55 deletions(-) diff --git a/internal/pkg/cli/pipeline_init.go b/internal/pkg/cli/pipeline_init.go index 1b09d773d8e..b119721da59 100644 --- a/internal/pkg/cli/pipeline_init.go +++ b/internal/pkg/cli/pipeline_init.go @@ -47,21 +47,17 @@ const ( buildspecTemplatePath = "cicd/buildspec.yml" fmtPipelineName = "pipeline-%s-%s" // Ex: "pipeline-appName-repoName" // For a GitHub repository. - githubURL = "github.com" - ghProviderName = "GitHub" - ghV1ProviderName = "GitHubV1" - defaultGHBranch = "main" - fmtGHRepoURL = "https://%s/%s/%s" // Ex: "https://github.com/repoOwner/repoName" - fmtSecretName = "github-token-%s-%s" // Ex: "github-token-appName-repoName" + githubURL = "github.com" + defaultGHBranch = "main" + fmtGHRepoURL = "https://%s/%s/%s" // Ex: "https://github.com/repoOwner/repoName" + fmtSecretName = "github-token-%s-%s" // Ex: "github-token-appName-repoName" // For a CodeCommit repository. awsURL = "aws.amazon.com" ccIdentifier = "codecommit" - ccProviderName = "CodeCommit" defaultCCBranch = "master" fmtCCRepoURL = "https://%s.console.%s/codesuite/codecommit/repositories/%s/browse" // Ex: "https://region.console.aws.amazon.com/codesuite/codecommit/repositories/repoName/browse" // For a Bitbucket repository. bbURL = "bitbucket.org" - bbProviderName = "Bitbucket" defaultBBBranch = "master" fmtBBRepoURL = "https://%s@%s/%s/%s" // Ex: "https://repoOwner@bitbucket.org/repoOwner/repoName ) @@ -190,7 +186,7 @@ func (o *initPipelineOpts) Ask() error { // Execute writes the pipeline manifest file. func (o *initPipelineOpts) Execute() error { - if o.provider == ghV1ProviderName { + if o.provider == manifest.GithubV1ProviderName { if err := o.storeGitHubAccessToken(); err != nil { return err } @@ -272,9 +268,9 @@ func (o *initPipelineOpts) askRepository() error { func (o *initPipelineOpts) askGitHubRepoDetails() error { // If the user uses a flag to specify a GitHub access token, // GitHub version 1 (not CSC) is the provider. - o.provider = ghProviderName + o.provider = manifest.GithubProviderName if o.githubAccessToken != "" { - o.provider = ghV1ProviderName + o.provider = manifest.GithubV1ProviderName } repoDetails, err := ghRepoURL(o.repoURL).parse() @@ -291,7 +287,7 @@ func (o *initPipelineOpts) askGitHubRepoDetails() error { } func (o *initPipelineOpts) parseCodeCommitRepoDetails() error { - o.provider = ccProviderName + o.provider = manifest.CodeCommitProviderName repoDetails, err := ccRepoURL(o.repoURL).parse() if err != nil { return err @@ -316,7 +312,7 @@ func (o *initPipelineOpts) parseCodeCommitRepoDetails() error { } func (o *initPipelineOpts) parseBitbucketRepoDetails() error { - o.provider = bbProviderName + o.provider = manifest.BitbucketProviderName repoDetails, err := bbRepoURL(o.repoURL).parse() if err != nil { return err @@ -595,23 +591,23 @@ func (o *initPipelineOpts) pipelineName() string { func (o *initPipelineOpts) pipelineProvider() (manifest.Provider, error) { var config interface{} switch o.provider { - case ghV1ProviderName: + case manifest.GithubV1ProviderName: config = &manifest.GitHubV1Properties{ RepositoryURL: fmt.Sprintf(fmtGHRepoURL, githubURL, o.repoOwner, o.repoName), Branch: o.repoBranch, GithubSecretIdKeyName: o.secret, } - case ghProviderName: + case manifest.GithubProviderName: config = &manifest.GitHubProperties{ RepositoryURL: fmt.Sprintf(fmtGHRepoURL, githubURL, o.repoOwner, o.repoName), Branch: o.repoBranch, } - case ccProviderName: + case manifest.CodeCommitProviderName: config = &manifest.CodeCommitProperties{ RepositoryURL: fmt.Sprintf(fmtCCRepoURL, o.ccRegion, awsURL, o.repoName), Branch: o.repoBranch, } - case bbProviderName: + case manifest.BitbucketProviderName: config = &manifest.BitbucketProperties{ RepositoryURL: fmt.Sprintf(fmtBBRepoURL, o.repoOwner, bbURL, o.repoOwner, o.repoName), Branch: o.repoBranch, diff --git a/internal/pkg/deploy/pipeline.go b/internal/pkg/deploy/pipeline.go index 04d61c50357..c4e8ac1b167 100644 --- a/internal/pkg/deploy/pipeline.go +++ b/internal/pkg/deploy/pipeline.go @@ -17,11 +17,6 @@ import ( const ( fmtInvalidRepo = "unable to locate the repository URL from the properties: %+v" - // Redefining these consts from the cli package here so as to avoid an import cycle - ghProviderName = "GitHub" - ghV1ProviderName = "GitHubV1" - ccProviderName = "CodeCommit" - bbProviderName = "Bitbucket" ) var ( @@ -79,7 +74,8 @@ func (a *ArtifactBucket) Region() (string, error) { return parsedArn.Region, nil } -// GitHubSource defines the (GH) source of the artifacts to be built and deployed. +// GitHubV1Source defines the source of the artifacts to be built and deployed. This version uses personal access tokens +// and is not recommended. https://docs.aws.amazon.com/codepipeline/latest/userguide/update-github-action-connections.html type GitHubV1Source struct { ProviderName string Branch string @@ -87,7 +83,16 @@ type GitHubV1Source struct { PersonalAccessTokenSecretID string } -// GitHubURL is the common type for repo URLs for both GitHubSource versions. +// GitHubSource (version 2) defines the source of the artifacts to be built and deployed. This version uses CodeStar +// Connections to authenticate access to the remote repo. +type GitHubSource struct { + ProviderName string + Branch string + RepositoryURL GitHubURL +} + +// GitHubURL is the common type for repo URLs for both GitHubSource versions: +// GitHubV1 (w/ access tokens) and GitHub (V2 w CodeStar Connections). type GitHubURL string // CodeCommitSource defines the (CC) source of the artifacts to be built and deployed. @@ -106,56 +111,50 @@ type BitbucketSource struct { // PipelineSourceFromManifest processes manifest info about the source based on provider type. // The return boolean is true for CodeStar Connections sources that require a polling prompt. -func PipelineSourceFromManifest(source *manifest.Source) (interface{}, bool, error) { - switch source.ProviderName { - case ghV1ProviderName: +func PipelineSourceFromManifest(mfSource *manifest.Source) (source interface{}, usesCodeStar bool, err error) { + switch mfSource.ProviderName { + case manifest.GithubV1ProviderName: return &GitHubV1Source{ - ProviderName: ghV1ProviderName, - Branch: (source.Properties["branch"]).(string), - RepositoryURL: GitHubURL((source.Properties["repository"]).(string)), - PersonalAccessTokenSecretID: (source.Properties["access_token_secret"]).(string), + ProviderName: manifest.GithubV1ProviderName, + Branch: (mfSource.Properties["branch"]).(string), + RepositoryURL: GitHubURL((mfSource.Properties["repository"]).(string)), + PersonalAccessTokenSecretID: (mfSource.Properties["access_token_secret"]).(string), }, false, nil - case ghProviderName: + case manifest.GithubProviderName: // If the creation of the user's pipeline manifest predates Copilot's conversion to GHv2/CSC, the provider // listed in the manifest will be "GitHub," not "GitHubV1." To differentiate it from the new default // "GitHub," which refers to v2, we check for the presence of a secret, indicating a v1 GitHub connection. - if source.Properties["access_token_secret"] != nil { + if mfSource.Properties["access_token_secret"] != nil { return &GitHubV1Source{ - ProviderName: ghV1ProviderName, - Branch: (source.Properties["branch"]).(string), - RepositoryURL: GitHubURL((source.Properties["repository"]).(string)), - PersonalAccessTokenSecretID: (source.Properties["access_token_secret"]).(string), + ProviderName: manifest.GithubV1ProviderName, + Branch: (mfSource.Properties["branch"]).(string), + RepositoryURL: GitHubURL((mfSource.Properties["repository"]).(string)), + PersonalAccessTokenSecretID: (mfSource.Properties["access_token_secret"]).(string), }, false, nil } else { return &GitHubSource{ - ProviderName: ghProviderName, - Branch: (source.Properties["branch"]).(string), - RepositoryURL: GitHubURL((source.Properties["repository"]).(string)), + ProviderName: manifest.GithubProviderName, + Branch: (mfSource.Properties["branch"]).(string), + RepositoryURL: GitHubURL((mfSource.Properties["repository"]).(string)), }, true, nil } - case ccProviderName: + case manifest.CodeCommitProviderName: return &CodeCommitSource{ - ProviderName: ccProviderName, - Branch: (source.Properties["branch"]).(string), - RepositoryURL: (source.Properties["repository"]).(string), + ProviderName: manifest.CodeCommitProviderName, + Branch: (mfSource.Properties["branch"]).(string), + RepositoryURL: (mfSource.Properties["repository"]).(string), }, false, nil - case bbProviderName: + case manifest.BitbucketProviderName: return &BitbucketSource{ - ProviderName: bbProviderName, - Branch: (source.Properties["branch"]).(string), - RepositoryURL: (source.Properties["repository"]).(string), + ProviderName: manifest.BitbucketProviderName, + Branch: (mfSource.Properties["branch"]).(string), + RepositoryURL: (mfSource.Properties["repository"]).(string), }, true, nil default: - return nil, false, fmt.Errorf("invalid repo source provider: %s", source.ProviderName) + return nil, false, fmt.Errorf("invalid repo source provider: %s", mfSource.ProviderName) } } -type GitHubSource struct { - ProviderName string - Branch string - RepositoryURL GitHubURL -} - // GitHubPersonalAccessTokenSecretID returns the ID of the secret in the // Secrets manager, which stores the GitHub Personal Access token if the // provider is "GitHubV1". @@ -237,6 +236,7 @@ const ( fmtConnectionName = "copilot-%s-%s" ) +// ConnectionName generates a recognizable string by which the connection may be identified. func (s *BitbucketSource) ConnectionName() (string, error) { owner, repo, err := s.parseOwnerAndRepo() if err != nil { @@ -245,6 +245,7 @@ func (s *BitbucketSource) ConnectionName() (string, error) { return formatConnectionName(owner, repo), nil } +// ConnectionName generates a recognizable string by which the connection may be identified. func (s *GitHubSource) ConnectionName() (string, error) { owner, repo, err := s.RepositoryURL.parse() if err != nil {