diff --git a/pkg/config/repo.go b/pkg/config/repo.go index 4ee8cc05a..92c9d1b80 100644 --- a/pkg/config/repo.go +++ b/pkg/config/repo.go @@ -25,6 +25,7 @@ import ( "sigs.k8s.io/yaml" "opendev.org/airship/airshipctl/pkg/errors" + "opendev.org/airship/airshipctl/pkg/log" ) // Constants for possible repo authentication types @@ -123,7 +124,7 @@ func (repo *Repository) String() string { // Validate check possible values for repository and // returns Error when incorrect value is given -// retruns nill when there are no errors +// returns nil when there are no errors func (repo *Repository) Validate() error { if repo.URLString == "" { return ErrRepoSpecRequiresURL{} @@ -141,6 +142,8 @@ func (repo *Repository) Validate() error { if err != nil { return err } + } else { + log.Debugf("Checkout options not defined, cloning from master") } return nil @@ -171,14 +174,15 @@ func (repo *Repository) ToCheckoutOptions(force bool) *git.CheckoutOptions { co := &git.CheckoutOptions{ Force: force, } - switch { - case repo.CheckoutOptions == nil: - case repo.CheckoutOptions.Branch != "": - co.Branch = plumbing.NewBranchReferenceName(repo.CheckoutOptions.Branch) - case repo.CheckoutOptions.Tag != "": - co.Branch = plumbing.NewTagReferenceName(repo.CheckoutOptions.Tag) - case repo.CheckoutOptions.CommitHash != "": - co.Hash = plumbing.NewHash(repo.CheckoutOptions.CommitHash) + if repo.CheckoutOptions != nil { + switch { + case repo.CheckoutOptions.Branch != "": + co.Branch = plumbing.NewBranchReferenceName(repo.CheckoutOptions.Branch) + case repo.CheckoutOptions.Tag != "": + co.Branch = plumbing.NewTagReferenceName(repo.CheckoutOptions.Tag) + case repo.CheckoutOptions.CommitHash != "": + co.Hash = plumbing.NewHash(repo.CheckoutOptions.CommitHash) + } } return co } @@ -187,10 +191,19 @@ func (repo *Repository) ToCheckoutOptions(force bool) *git.CheckoutOptions { // authentication and URL set // CloneOptions describes how a clone should be performed func (repo *Repository) ToCloneOptions(auth transport.AuthMethod) *git.CloneOptions { - return &git.CloneOptions{ + cl := &git.CloneOptions{ Auth: auth, URL: repo.URLString, } + if repo.CheckoutOptions != nil { + switch { + case repo.CheckoutOptions.Branch != "": + cl.ReferenceName = plumbing.NewBranchReferenceName(repo.CheckoutOptions.Branch) + case repo.CheckoutOptions.Tag != "": + cl.ReferenceName = plumbing.NewTagReferenceName(repo.CheckoutOptions.Tag) + } + } + return cl } // ToFetchOptions returns an instance of git.FetchOptions for given authentication diff --git a/pkg/config/repo_test.go b/pkg/config/repo_test.go index 29939ff3c..a82eaa0f2 100644 --- a/pkg/config/repo_test.go +++ b/pkg/config/repo_test.go @@ -31,6 +31,7 @@ const ( toAuthTestName = "ToAuth" toAuthNilTestName = "ToAuthNil" ToFetchOptionsTestName = "ToFetchOptions" + ToCloneOptionsTestName = "ToCloneOptions" toAuthNilError = "toAuthNilError" URLTestName = "URLTest" StringTestData = `test-data: @@ -147,6 +148,11 @@ var ( dataMapEntry: []string{"no-auth"}, expectedNil: false, }, + ToCloneOptionsTestName: { + expectError: false, + dataMapEntry: []string{"http-basic-auth", "ssh-key-auth", "no-auth", "empty-checkout"}, + expectedNil: false, + }, URLTestName: { expectError: false, expectedNil: false, @@ -253,12 +259,18 @@ func TestToCloneOptions(t *testing.T) { err := yaml.Unmarshal([]byte(StringTestData), data) require.NoError(t, err) - testCase := TestCaseMap[ToFetchOptionsTestName] + testCase := TestCaseMap[ToCloneOptionsTestName] for _, name := range testCase.dataMapEntry { repo := data.TestData[name] require.NotNil(t, repo) - assert.NotNil(t, repo.ToCloneOptions(nil)) + cl := repo.ToCloneOptions(nil) + if testCase.expectedNil { + assert.Nil(t, cl) + } else { + assert.NotNil(t, cl) + assert.NoError(t, cl.Validate()) + } } } diff --git a/pkg/document/pull/pull.go b/pkg/document/pull/pull.go index a09f436ac..e3ecb2542 100644 --- a/pkg/document/pull/pull.go +++ b/pkg/document/pull/pull.go @@ -44,6 +44,10 @@ func (s *Settings) cloneRepositories() error { // Clone repositories for _, extraRepoConfig := range currentManifest.Repositories { + err := extraRepoConfig.Validate() + if err != nil { + return err + } repository, err := repo.NewRepository(currentManifest.TargetPath, extraRepoConfig) if err != nil { return err diff --git a/pkg/document/pull/pull_test.go b/pkg/document/pull/pull_test.go index e8f018076..dea563a4e 100644 --- a/pkg/document/pull/pull_test.go +++ b/pkg/document/pull/pull_test.go @@ -21,13 +21,11 @@ import ( "testing" fixtures "github.com/go-git/go-git-fixtures/v4" - - repo2 "opendev.org/airship/airshipctl/pkg/document/repo" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "opendev.org/airship/airshipctl/pkg/config" + "opendev.org/airship/airshipctl/pkg/document/repo" "opendev.org/airship/airshipctl/pkg/environment" "opendev.org/airship/airshipctl/pkg/util" "opendev.org/airship/airshipctl/testutil" @@ -45,82 +43,97 @@ func TestPull(t *testing.T) { require := require.New(t) assert := assert.New(t) - t.Run("cloneRepositories", func(t *testing.T) { - dummyPullSettings := getDummyPullSettings() - currentManifest, err := dummyPullSettings.Config.CurrentContextManifest() - require.NoError(err) - - fx := fixtures.Basic().One() - - dummyGitDir := fx.DotGit().Root() - currentManifest.Repositories = map[string]*config.Repository{currentManifest.PrimaryRepositoryName: { - URLString: dummyGitDir, - CheckoutOptions: &config.RepoCheckout{ + tests := []struct { + name string + url string + checkoutOpts *config.RepoCheckout + error error + }{ + { + name: "TestCloneRepositoriesValidOpts", + checkoutOpts: &config.RepoCheckout{ Branch: "master", ForceCheckout: false, }, - Auth: &config.RepoAuth{ - Type: "http-basic", - }, + error: nil, }, - } - - tmpDir, cleanup := testutil.TempDir(t, "airshipctlPullTest-") - defer cleanup(t) - - currentManifest.TargetPath = tmpDir - - _, err = repo2.NewRepository(".", currentManifest.Repositories[currentManifest.PrimaryRepositoryName]) - require.NoError(err) - - err = dummyPullSettings.cloneRepositories() - - require.NoError(err) - dummyRepoDirName := util.GitDirNameFromURL(dummyGitDir) - assert.FileExists(path.Join(tmpDir, dummyRepoDirName, "go/example.go")) - assert.FileExists(path.Join(tmpDir, dummyRepoDirName, ".git/HEAD")) - contents, err := ioutil.ReadFile(path.Join(tmpDir, dummyRepoDirName, ".git/HEAD")) - require.NoError(err) - assert.Equal("ref: refs/heads/master", strings.TrimRight(string(contents), "\t \n")) - }) - - t.Run("Pull", func(t *testing.T) { - dummyPullSettings := getDummyPullSettings() - conf := dummyPullSettings.AirshipCTLSettings.Config - - fx := fixtures.Basic().One() - - mfst := conf.Manifests["dummy_manifest"] - dummyGitDir := fx.DotGit().Root() - mfst.Repositories = map[string]*config.Repository{ - mfst.PrimaryRepositoryName: { - URLString: dummyGitDir, - CheckoutOptions: &config.RepoCheckout{ - Branch: "master", - ForceCheckout: false, - }, - Auth: &config.RepoAuth{ - Type: "http-basic", - }, + { + name: "TestCloneRepositoriesMissingCheckoutOptions", + error: nil, + }, + { + name: "TestCloneRepositoriesNonMasterBranch", + checkoutOpts: &config.RepoCheckout{ + Branch: "branch", + ForceCheckout: false, }, - } - dummyPullSettings.Config = conf + error: nil, + }, + { + name: "TestCloneRepositoriesInvalidOpts", + checkoutOpts: &config.RepoCheckout{ + Branch: "master", + Tag: "someTag", + ForceCheckout: false, + }, + error: config.ErrMutuallyExclusiveCheckout{}, + }, + } + dummyPullSettings := getDummyPullSettings() + currentManifest, err := dummyPullSettings.Config.CurrentContextManifest() + require.NoError(err) - tmpDir, cleanup := testutil.TempDir(t, "airshipctlPullTest-") - defer cleanup(t) + testGitURL := fixtures.Basic().One().URL + dirNameFromURL := util.GitDirNameFromURL(testGitURL) + globalTmpDir, cleanup := testutil.TempDir(t, "airshipctlCloneTest-") + defer cleanup(t) - mfst.TargetPath = tmpDir + for _, tt := range tests { + tmpDir := path.Join(globalTmpDir, tt.name) + expectedErr := tt.error + chkOutOpts := tt.checkoutOpts + t.Run(tt.name, func(t *testing.T) { + currentManifest.Repositories = map[string]*config.Repository{ + currentManifest.PrimaryRepositoryName: { + URLString: testGitURL, + CheckoutOptions: chkOutOpts, + Auth: &config.RepoAuth{ + Type: "http-basic", + }, + }, + } - err := dummyPullSettings.Pull() - require.NoError(err) + currentManifest.TargetPath = tmpDir - dummyRepoDirName := util.GitDirNameFromURL(dummyGitDir) - assert.FileExists(path.Join(tmpDir, dummyRepoDirName, "go/example.go")) - assert.FileExists(path.Join(tmpDir, dummyRepoDirName, ".git/HEAD")) - contents, err := ioutil.ReadFile(path.Join(tmpDir, dummyRepoDirName, ".git/HEAD")) - require.NoError(err) - assert.Equal("ref: refs/heads/master", strings.TrimRight(string(contents), "\t \n")) - }) + _, err = repo.NewRepository( + ".", + currentManifest.Repositories[currentManifest.PrimaryRepositoryName], + ) + require.NoError(err) + err = dummyPullSettings.Pull() + if expectedErr != nil { + assert.NotNil(err) + assert.Equal(expectedErr, err) + } else { + require.NoError(err) + assert.FileExists(path.Join(currentManifest.TargetPath, dirNameFromURL, "go/example.go")) + assert.FileExists(path.Join(currentManifest.TargetPath, dirNameFromURL, ".git/HEAD")) + contents, err := ioutil.ReadFile(path.Join(currentManifest.TargetPath, dirNameFromURL, ".git/HEAD")) + require.NoError(err) + if chkOutOpts == nil { + assert.Equal( + "ref: refs/heads/master", + strings.TrimRight(string(contents), "\t \n"), + ) + } else { + assert.Equal( + "ref: refs/heads/"+chkOutOpts.Branch, + strings.TrimRight(string(contents), "\t \n"), + ) + } + } + }) + } testutil.CleanUpGitFixtures(t) }