From 79f792f09f0d5af61234e64581c78d8c5c574fb2 Mon Sep 17 00:00:00 2001 From: Nicolas Zin Date: Sun, 17 Sep 2023 01:56:10 -0400 Subject: [PATCH] bugfix: fixing usersync code --- internal/engine/githubsaml.go | 9 +++++++ internal/engine/goliac_reconciliator_test.go | 2 +- internal/engine/local.go | 25 +++++++++++++++----- internal/engine/local_test.go | 4 ++-- internal/entity/user.go | 2 +- internal/goliac.go | 6 ++--- 6 files changed, 35 insertions(+), 13 deletions(-) diff --git a/internal/engine/githubsaml.go b/internal/engine/githubsaml.go index db3a06b..26244f8 100644 --- a/internal/engine/githubsaml.go +++ b/internal/engine/githubsaml.go @@ -7,6 +7,7 @@ import ( "github.com/Alayacare/goliac/internal/config" "github.com/Alayacare/goliac/internal/entity" "github.com/Alayacare/goliac/internal/github" + "github.com/sirupsen/logrus" ) const listUsersFromGithubOrgSaml = ` @@ -102,6 +103,14 @@ func LoadUsersFromGithubOrgSaml(client github.GitHubClient) (map[string]*entity. } for _, c := range gResult.Data.Organization.SamlIdentityProvider.ExternalIdentities.Edges { + if c.Node.SamlIdentity.NameId == "" { + logrus.Debugf("Skipping user with empty NameId: %s", c.Node.User.Login) + continue + } + if c.Node.User.Login == "" { + logrus.Debugf("Skipping user with empty login: %s", c.Node.SamlIdentity.NameId) + continue + } user := &entity.User{} user.ApiVersion = "v1" user.Kind = "User" diff --git a/internal/engine/goliac_reconciliator_test.go b/internal/engine/goliac_reconciliator_test.go index 1577339..1fd2548 100644 --- a/internal/engine/goliac_reconciliator_test.go +++ b/internal/engine/goliac_reconciliator_test.go @@ -64,7 +64,7 @@ func (m *GoliacLocalMock) RuleSets() map[string]*entity.RuleSet { func (m *GoliacLocalMock) UpdateAndCommitCodeOwners(repoconfig *config.RepositoryConfig, dryrun bool, accesstoken string, branch string, tagname string) error { return nil } -func (m *GoliacLocalMock) SyncUsersAndTeams(repoconfig *config.RepositoryConfig, plugin UserSyncPlugin, dryrun bool) error { +func (m *GoliacLocalMock) SyncUsersAndTeams(repoconfig *config.RepositoryConfig, plugin UserSyncPlugin, accesstoken string, dryrun bool) error { return nil } func (m *GoliacLocalMock) Close() { diff --git a/internal/engine/local.go b/internal/engine/local.go index 67fb558..9beeb4b 100644 --- a/internal/engine/local.go +++ b/internal/engine/local.go @@ -51,7 +51,7 @@ type GoliacLocalGit interface { // whenever someone create/delete a team, we must update the github CODEOWNERS UpdateAndCommitCodeOwners(repoconfig *config.RepositoryConfig, dryrun bool, accesstoken string, branch string, tagname string) error // whenever the users list is changing, reload users and teams, and commit them - SyncUsersAndTeams(repoconfig *config.RepositoryConfig, plugin UserSyncPlugin, dryrun bool) error + SyncUsersAndTeams(repoconfig *config.RepositoryConfig, plugin UserSyncPlugin, accesstoken string, dryrun bool) error Close() // Load and Validate from a local directory @@ -439,7 +439,7 @@ func syncUsersViaUserPlugin(repoconfig *config.RepositoryConfig, fs afero.Fs, us for username, user := range orgUsers { if newuser, ok := newOrgUsers[username]; !ok { // deleted user - deletedusers = append(deletedusers, filepath.Join(rootDir, "users", "org", fmt.Sprintf("%s.yaml", username))) + deletedusers = append(deletedusers, filepath.Join("users", "org", fmt.Sprintf("%s.yaml", username))) fs.Remove(filepath.Join(rootDir, "users", "org", fmt.Sprintf("%s.yaml", username))) } else { // check if user changed @@ -457,7 +457,7 @@ func syncUsersViaUserPlugin(repoconfig *config.RepositoryConfig, fs afero.Fs, us if err != nil { return nil, nil, err } - updatedusers = append(updatedusers, filepath.Join(rootDir, "users", "org", fmt.Sprintf("%s.yaml", username))) + updatedusers = append(updatedusers, filepath.Join("users", "org", fmt.Sprintf("%s.yaml", username))) } delete(newOrgUsers, username) @@ -477,12 +477,12 @@ func syncUsersViaUserPlugin(repoconfig *config.RepositoryConfig, fs afero.Fs, us if err != nil { return nil, nil, err } - updatedusers = append(updatedusers, filepath.Join(rootDir, "users", "org", fmt.Sprintf("%s.yaml", username))) + updatedusers = append(updatedusers, filepath.Join("users", "org", fmt.Sprintf("%s.yaml", username))) } return deletedusers, updatedusers, nil } -func (g *GoliacLocalImpl) SyncUsersAndTeams(repoconfig *config.RepositoryConfig, userplugin UserSyncPlugin, dryrun bool) error { +func (g *GoliacLocalImpl) SyncUsersAndTeams(repoconfig *config.RepositoryConfig, userplugin UserSyncPlugin, accesstoken string, dryrun bool) error { if g.repo == nil { return fmt.Errorf("git repository not cloned") } @@ -519,6 +519,11 @@ func (g *GoliacLocalImpl) SyncUsersAndTeams(repoconfig *config.RepositoryConfig, return err } + // check if we have too many changesets + if len(teamschanged)+len(deletedusers)+len(addedusers) > repoconfig.MaxChangesets { + return fmt.Errorf("too many changesets (%d) to commit. Please increase max_changesets in goliac.yaml", len(teamschanged)+len(deletedusers)+len(addedusers)) + } + // // let's commit // @@ -570,7 +575,15 @@ func (g *GoliacLocalImpl) SyncUsersAndTeams(repoconfig *config.RepositoryConfig, return err } - err = g.repo.Push(&git.PushOptions{}) + // Now push the tag to the remote repository + auth := &http.BasicAuth{ + Username: "x-access-token", // This can be anything except an empty string + Password: accesstoken, + } + + err = g.repo.Push(&git.PushOptions{ + Auth: auth, + }) return err } diff --git a/internal/engine/local_test.go b/internal/engine/local_test.go index b352d59..b0fb66a 100644 --- a/internal/engine/local_test.go +++ b/internal/engine/local_test.go @@ -218,8 +218,8 @@ func TestSyncUsersViaUserPlugin(t *testing.T) { assert.Nil(t, err) assert.Equal(t, 1, len(removed)) assert.Equal(t, 2, len(added)) - assert.Equal(t, "/tmp/goliac/users/org/user1.yaml", added[0]) - assert.Equal(t, "/tmp/goliac/users/org/foobar.yaml", added[1]) + assert.Equal(t, "users/org/user1.yaml", added[0]) + assert.Equal(t, "users/org/foobar.yaml", added[1]) }) t.Run("not happy path: dealing with usersync error", func(t *testing.T) { fs := afero.NewMemMapFs() diff --git a/internal/entity/user.go b/internal/entity/user.go index 0d6e178..0512a98 100644 --- a/internal/entity/user.go +++ b/internal/entity/user.go @@ -109,7 +109,7 @@ func (u *User) Validate(filename string) error { } if u.Spec.GithubID == "" { - return fmt.Errorf("data.githubID is empty for user filename %s", filename) + return fmt.Errorf("spec.githubID is empty for user filename %s", filename) } return nil diff --git a/internal/goliac.go b/internal/goliac.go index 6e4c0fd..2ec96ab 100644 --- a/internal/goliac.go +++ b/internal/goliac.go @@ -279,11 +279,11 @@ func (g *GoliacImpl) UsersUpdate(repositoryUrl, branch string) error { return fmt.Errorf("unable to read goliac.yaml config file: %v", err) } - userplugin, found := engine.GetUserSyncPlugin(g.repoconfig.UserSync.Plugin) + userplugin, found := engine.GetUserSyncPlugin(repoconfig.UserSync.Plugin) if !found { - return fmt.Errorf("User Sync Plugin %s not found", g.repoconfig.UserSync.Plugin) + return fmt.Errorf("User Sync Plugin %s not found", repoconfig.UserSync.Plugin) } - err = g.local.SyncUsersAndTeams(repoconfig, userplugin, false) + err = g.local.SyncUsersAndTeams(repoconfig, userplugin, accessToken, false) return err }