From fdb75767db5a1c2d2c900bb7a6041474e1a3e085 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Wed, 8 Jan 2020 11:58:36 -0500 Subject: [PATCH] fix: Creating dev env repo should come from GitHub, even with different target providers The default upstream boot config repo that's used as the basis for creating the dev env repo for a new cluster is, at least as of now, always on github.com, but `DuplicateGitRepoFromCommitish` only uses a single `GitProvider`, meaning that if you're using GitHub Enterprise, or Gitlab, etc, the attempt to get information needed about the upstream repo in order to create the dev env repo tries to look for the upstream repo's org/name in the provider that will have the dev env repo in it. Which is bad. So instead, let's check if the provider we'll be creating in is not github.com and the upstream boot config repo we're going to use as the basis for the dev env repo *is* on github.com, and if so, use a different provider to get the required information. fixes #6315 Signed-off-by: Andrew Bayer --- .../step/verify/step_verify_environments.go | 28 +++- pkg/config/install_requirements.go | 7 +- pkg/gits/github.go | 21 ++- pkg/gits/helpers.go | 28 +++- pkg/gits/helpers_test.go | 122 ++++++++++++++++-- 5 files changed, 187 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/step/verify/step_verify_environments.go b/pkg/cmd/step/verify/step_verify_environments.go index af0426ec80..58837c3b20 100644 --- a/pkg/cmd/step/verify/step_verify_environments.go +++ b/pkg/cmd/step/verify/step_verify_environments.go @@ -402,7 +402,11 @@ func (o *StepVerifyEnvironmentsOptions) handleDevEnvironmentRepository(envGitInf } func (o *StepVerifyEnvironmentsOptions) createDevEnvironmentRepository(gitInfo *gits.GitRepository, localRepoDir string, fromGitURL string, fromGitRef string, privateRepo bool, requirements *config.RequirementsConfig, provider gits.GitProvider, gitter gits.Gitter) (*gits.GitRepository, error) { - if fromGitURL == config.DefaultBootRepository && fromGitRef == "master" { + isDefaultBootURL, err := gits.IsDefaultBootConfigURL(fromGitURL) + if err != nil { + return nil, errors.Wrapf(err, "failed to verify whether %s is the default boot config repository", fromGitURL) + } + if isDefaultBootURL && fromGitRef == "master" { // If the GitURL is not overridden and the GitRef is set to it's default value then look up the version number resolver, err := o.CreateVersionResolver(requirements.VersionStream.URL, requirements.VersionStream.Ref) if err != nil { @@ -428,7 +432,27 @@ func (o *StepVerifyEnvironmentsOptions) createDevEnvironmentRepository(gitInfo * log.Logger().Debugf("set commitish to '%s'", commitish) } - duplicateInfo, err := gits.DuplicateGitRepoFromCommitish(gitInfo.Organisation, gitInfo.Name, fromGitURL, commitish, "master", privateRepo, provider, gitter) + var fromProvider gits.GitProvider + // If the to URL isn't github.com, and the fromGitURL is the default boot repository, use a non-authenticated github.com provider for the from provider. + if !gitInfo.IsGitHub() && isDefaultBootURL { + fromGitInfo, err := gits.ParseGitURL(fromGitURL) + if err != nil { + return nil, errors.Wrapf(err, "failed to parse upstream boot config URL %s", fromGitURL) + } + if fromGitInfo.IsGitHub() { + ghServer := &auth.AuthServer{ + URL: "https://github.com", + Name: "gh", + Kind: "github", + } + fromProvider, err = gits.NewAnonymousGitHubProvider(ghServer, gitter) + if err != nil { + return nil, errors.Wrapf(err, "failed to configure anonymous GitHub provider for upstream boot config URL %s", fromGitURL) + } + } + } + + duplicateInfo, err := gits.DuplicateGitRepoFromCommitish(gitInfo.Organisation, gitInfo.Name, fromGitURL, commitish, "master", privateRepo, provider, gitter, fromProvider) if err != nil { return nil, errors.Wrapf(err, "duplicating %s to %s/%s", fromGitURL, gitInfo.Organisation, gitInfo.Name) } diff --git a/pkg/config/install_requirements.go b/pkg/config/install_requirements.go index 6507e116c8..3349ee899c 100644 --- a/pkg/config/install_requirements.go +++ b/pkg/config/install_requirements.go @@ -3,7 +3,10 @@ package config import ( "encoding/json" "fmt" + "io/ioutil" "os" + "path/filepath" + "reflect" "strings" "github.com/vrischmann/envconfig" @@ -17,10 +20,6 @@ import ( "github.com/jenkins-x/jx/pkg/log" "github.com/pkg/errors" - "io/ioutil" - "path/filepath" - "reflect" - "github.com/jenkins-x/jx/pkg/util" ) diff --git a/pkg/gits/github.go b/pkg/gits/github.go index 84afaf4b2e..c8cd5b8663 100644 --- a/pkg/gits/github.go +++ b/pkg/gits/github.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "io/ioutil" + "net/http" "os" "strconv" "strings" @@ -48,8 +49,26 @@ func NewGitHubProvider(server *auth.AuthServer, user *auth.UserAuth, git Gitter) ) tc := oauth2.NewClient(ctx, ts) + return newGitHubProviderFromOauthClient(tc, provider) +} + +// NewAnonymousGitHubProvider returns a new GitHubProvider without any authentication +func NewAnonymousGitHubProvider(server *auth.AuthServer, git Gitter) (GitProvider, error) { + ctx := context.Background() + + provider := GitHubProvider{ + Server: *server, + User: auth.UserAuth{}, + Context: ctx, + Git: git, + } + + return newGitHubProviderFromOauthClient(nil, provider) +} + +func newGitHubProviderFromOauthClient(tc *http.Client, provider GitHubProvider) (GitProvider, error) { var err error - u := server.URL + u := provider.Server.URL if IsGitHubServerURL(u) { provider.Client = github.NewClient(tc) } else { diff --git a/pkg/gits/helpers.go b/pkg/gits/helpers.go index 5ffa86b489..1135a9c2f1 100644 --- a/pkg/gits/helpers.go +++ b/pkg/gits/helpers.go @@ -8,6 +8,7 @@ import ( "os/user" "path" "path/filepath" + "reflect" "sort" "strings" @@ -15,9 +16,9 @@ import ( uuid "github.com/satori/go.uuid" - "github.com/jenkins-x/jx/pkg/util" - + jxconfig "github.com/jenkins-x/jx/pkg/config" "github.com/jenkins-x/jx/pkg/log" + "github.com/jenkins-x/jx/pkg/util" "github.com/pkg/errors" ) @@ -776,7 +777,7 @@ func FindTagForVersion(dir string, version string, gitter Gitter) (string, error // DuplicateGitRepoFromCommitish will duplicate branches (but not tags) from fromGitURL to toOrg/toName. It will reset the // head of the toBranch on the duplicated repo to fromCommitish. It returns the GitRepository for the duplicated repo. // If the repository already exist and error is returned. -func DuplicateGitRepoFromCommitish(toOrg string, toName string, fromGitURL string, fromCommitish string, toBranch string, private bool, provider GitProvider, gitter Gitter) (*GitRepository, error) { +func DuplicateGitRepoFromCommitish(toOrg string, toName string, fromGitURL string, fromCommitish string, toBranch string, private bool, provider GitProvider, gitter Gitter, fromProvider GitProvider) (*GitRepository, error) { log.Logger().Debugf("getting repo %s/%s", toOrg, toName) _, err := provider.GetRepository(toOrg, toName) if err == nil { @@ -789,10 +790,16 @@ func DuplicateGitRepoFromCommitish(toOrg string, toName string, fromGitURL strin if err != nil { return nil, errors.Wrapf(err, "parsing %s", fromGitURL) } - fromInfo, err = provider.GetRepository(fromInfo.Organisation, fromInfo.Name) + + // If no separate fromProvider is specified, use the same provider. + if fromProvider == nil || reflect.ValueOf(fromProvider).IsNil() { + fromProvider = provider + } + fromInfo, err = fromProvider.GetRepository(fromInfo.Organisation, fromInfo.Name) if err != nil { return nil, errors.Wrapf(err, "getting repo for %s", fromGitURL) } + duplicateInfo, err := provider.CreateRepository(toOrg, toName, private) if err != nil { return nil, errors.Wrapf(err, "failed to create GitHub repo %s/%s", toOrg, toName) @@ -945,3 +952,16 @@ func RefIsBranch(dir string, ref string, gitter Gitter) (bool, error) { } return false, nil } + +// IsDefaultBootConfigURL checks if the given URL corresponds to the default boot config URL +func IsDefaultBootConfigURL(url string) (bool, error) { + gitInfo, err := ParseGitURL(url) + if err != nil { + return false, errors.Wrap(err, "couldn't parse provided repo URL") + } + defaultInfo, err := ParseGitURL(jxconfig.DefaultBootRepository) + if err != nil { + return false, errors.Wrap(err, "couldn't parse default boot config URL") + } + return gitInfo.HttpsURL() == defaultInfo.HttpsURL(), nil +} diff --git a/pkg/gits/helpers_test.go b/pkg/gits/helpers_test.go index 6b622a9dfe..9f9fad485d 100644 --- a/pkg/gits/helpers_test.go +++ b/pkg/gits/helpers_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + "github.com/jenkins-x/jx/pkg/config" "github.com/jenkins-x/jx/pkg/util" "github.com/jenkins-x/jx/pkg/tests" @@ -1376,6 +1377,14 @@ func TestDuplicateGitRepoFromCommitish(t *testing.T) { return nil }, gitter) assert.NoError(t, err) + otherProviderRepo, err := gits.NewFakeRepository("foo", "bar", func(dir string) error { + err := ioutil.WriteFile(filepath.Join(dir, "README"), []byte("Goodbye!"), 0655) + if err != nil { + return errors.Wrapf(err, "writing README") + } + return nil + }, gitter) + assert.NoError(t, err) dir, err := ioutil.TempDir("", "") assert.NoError(t, err) @@ -1432,12 +1441,13 @@ func TestDuplicateGitRepoFromCommitish(t *testing.T) { gitter gits.Gitter } tests := []struct { - provider *gits.FakeProvider - name string - args args - want *gits.GitRepository - wantFiles map[string][]byte - wantErr string + provider *gits.FakeProvider + useOtherProvider bool + name string + args args + want *gits.GitRepository + wantFiles map[string][]byte + wantErr string }{ { name: "sameOrg", @@ -1499,6 +1509,37 @@ func TestDuplicateGitRepoFromCommitish(t *testing.T) { "README": []byte("Hello!"), }, }, + { + name: "differentProvider", + args: args{ + toOrg: "coyote", + toName: "wile", + fromGitURL: "https://fake.git/foo/bar.git", + fromCommitish: "master", + toBranch: "master", + gitter: gitter, + }, + useOtherProvider: true, + want: &gits.GitRepository{ + Name: "wile", + AllowMergeCommit: false, + HTMLURL: "https://fake.git/coyote/wile", + CloneURL: "", + SSHURL: "", + Language: "", + Fork: false, + Stars: 0, + URL: "https://fake.git/coyote/wile.git", + Scheme: "https", + Host: "fake.git", + Organisation: "coyote", + Project: "", + Private: false, + }, + wantFiles: map[string][]byte{ + "README": []byte("Goodbye!"), + }, + }, { name: "tag", args: args{ @@ -1621,6 +1662,38 @@ func TestDuplicateGitRepoFromCommitish(t *testing.T) { "LICENSE": []byte("TODO"), }, }, + { + name: "wrongDifferentProvider", + args: args{ + toOrg: "coyote", + toName: "wile", + fromGitURL: "https://fake.git/acme/roadrunner.git", + fromCommitish: "master", + toBranch: "master", + gitter: gitter, + }, + useOtherProvider: true, + want: &gits.GitRepository{ + Name: "wile", + AllowMergeCommit: false, + HTMLURL: "https://fake.git/coyote/wile", + CloneURL: "", + SSHURL: "", + Language: "", + Fork: false, + Stars: 0, + URL: "https://fake.git/coyote/wile.git", + Scheme: "https", + Host: "fake.git", + Organisation: "coyote", + Project: "", + Private: false, + }, + wantErr: "organization 'acme' not found", + wantFiles: map[string][]byte{ + "README": []byte("Hello!"), + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1635,7 +1708,13 @@ func TestDuplicateGitRepoFromCommitish(t *testing.T) { } tt.provider = provider - got, err := gits.DuplicateGitRepoFromCommitish(tt.args.toOrg, tt.args.toName, tt.args.fromGitURL, tt.args.fromCommitish, tt.args.toBranch, false, tt.provider, tt.args.gitter) + var fromProvider *gits.FakeProvider + if tt.useOtherProvider { + fromProvider = gits.NewFakeProvider(otherProviderRepo) + fromProvider.Gitter = gitter + } + + got, err := gits.DuplicateGitRepoFromCommitish(tt.args.toOrg, tt.args.toName, tt.args.fromGitURL, tt.args.fromCommitish, tt.args.toBranch, false, tt.provider, tt.args.gitter, fromProvider) if tt.wantErr != "" { assert.Error(t, err) assert.Contains(t, err.Error(), tt.wantErr) @@ -1701,7 +1780,7 @@ func Test_DuplicateGitRepoFromCommitish_returns_error_if_target_repo_exists(t *t provider := gits.NewFakeProvider(originalRepo, targetRepo) provider.Gitter = gitter - repo, err := gits.DuplicateGitRepoFromCommitish(targetRepo.GitRepo.Organisation, targetRepo.GitRepo.Name, originalRepo.GitRepo.CloneURL, "origin/foo", "bar", false, provider, gitter) + repo, err := gits.DuplicateGitRepoFromCommitish(targetRepo.GitRepo.Organisation, targetRepo.GitRepo.Name, originalRepo.GitRepo.CloneURL, "origin/foo", "bar", false, provider, gitter, nil) assert.Error(t, err) assert.Equal(t, "repository acme/coyote already exists", err.Error()) assert.Nil(t, repo) @@ -2395,3 +2474,30 @@ func TestIsCouldntFindRemoteRefErrorHandlesUppercaseRef(t *testing.T) { ref := "add-app-your-app-0.0.0-SNAPSHOT-PR-1234-1" assert.True(t, gits.IsCouldntFindRemoteRefError(error, ref)) } + +func TestIsDefaultBootConfigURL(t *testing.T) { + wrongURL := "https://github.com/something-else/jenkins-x-boot-config.git" + + var rightURLWithDotGit string + var rightURLWithoutDotGit string + + if strings.HasSuffix(config.DefaultBootRepository, ".git") { + rightURLWithDotGit = config.DefaultBootRepository + rightURLWithoutDotGit = strings.TrimSuffix(config.DefaultBootRepository, ".git") + } else { + rightURLWithoutDotGit = config.DefaultBootRepository + rightURLWithDotGit = config.DefaultBootRepository + ".git" + } + + withWrongURL, err := gits.IsDefaultBootConfigURL(wrongURL) + assert.NoError(t, err) + assert.False(t, withWrongURL) + + withRightURLDotGit, err := gits.IsDefaultBootConfigURL(rightURLWithDotGit) + assert.NoError(t, err) + assert.True(t, withRightURLDotGit) + + withRightURLNoDotGit, err := gits.IsDefaultBootConfigURL(rightURLWithoutDotGit) + assert.NoError(t, err) + assert.True(t, withRightURLNoDotGit) +}