From 54eb8d4dba542a3c949e91a5bdf2940d547916bd Mon Sep 17 00:00:00 2001 From: Iulian Mandache <25257851+iul1an@users.noreply.github.com> Date: Mon, 16 Sep 2024 21:05:17 +0300 Subject: [PATCH] fix reconciliation loop for repository branch protection rules, GitHub apps, users and teams are case-insensitive (#28) --- internal/controller/repository/repository.go | 36 +++---- .../controller/repository/repository_test.go | 99 ++++++++++++++++++- internal/util/util.go | 9 ++ 3 files changed, 122 insertions(+), 22 deletions(-) diff --git a/internal/controller/repository/repository.go b/internal/controller/repository/repository.go index bc7b909..325b1a7 100644 --- a/internal/controller/repository/repository.go +++ b/internal/controller/repository/repository.go @@ -623,13 +623,13 @@ func getBPRMapFromCr(rules []v1alpha1.BranchProtectionRule) map[string]v1alpha1. if restr != nil { restr.BlockCreations = util.BoolDerefToPointer(restr.BlockCreations, false) if restr.Users != nil { - restr.Users = util.SortAndReturn(restr.Users) + restr.Users = util.SortAndReturn(util.ToLowerSlice(restr.Users)) } if restr.Teams != nil { - restr.Teams = util.SortAndReturn(restr.Teams) + restr.Teams = util.SortAndReturn(util.ToLowerSlice(restr.Teams)) } if restr.Apps != nil { - restr.Apps = util.SortAndReturn(restr.Apps) + restr.Apps = util.SortAndReturn(util.ToLowerSlice(restr.Apps)) } } @@ -641,25 +641,25 @@ func getBPRMapFromCr(rules []v1alpha1.BranchProtectionRule) map[string]v1alpha1. allowances := rPRs.BypassPullRequestAllowances if allowances != nil { if allowances.Users != nil { - allowances.Users = util.SortAndReturn(allowances.Users) + allowances.Users = util.SortAndReturn(util.ToLowerSlice(allowances.Users)) } if allowances.Teams != nil { - allowances.Teams = util.SortAndReturn(allowances.Teams) + allowances.Teams = util.SortAndReturn(util.ToLowerSlice(allowances.Teams)) } if allowances.Apps != nil { - allowances.Apps = util.SortAndReturn(allowances.Apps) + allowances.Apps = util.SortAndReturn(util.ToLowerSlice(allowances.Apps)) } } dismissal := rPRs.DismissalRestrictions if dismissal != nil { if dismissal.Users != nil { - dismissal.Users = util.SortAndReturnPointer(*dismissal.Users) + dismissal.Users = util.SortAndReturnPointer(util.ToLowerSlice(*dismissal.Users)) } if dismissal.Teams != nil { - dismissal.Teams = util.SortAndReturnPointer(*dismissal.Teams) + dismissal.Teams = util.SortAndReturnPointer(util.ToLowerSlice(*dismissal.Teams)) } if dismissal.Apps != nil { - dismissal.Apps = util.SortAndReturnPointer(*dismissal.Apps) + dismissal.Apps = util.SortAndReturnPointer(util.ToLowerSlice(*dismissal.Apps)) } } } @@ -731,21 +731,21 @@ func getBPRWithConfig(ctx context.Context, gh *ghclient.Client, owner, repo stri for i, user := range dismissal.Users { users[i] = user.GetLogin() } - bpr.RequiredPullRequestReviews.DismissalRestrictions.Users = util.SortAndReturnPointer(users) + bpr.RequiredPullRequestReviews.DismissalRestrictions.Users = util.SortAndReturnPointer(util.ToLowerSlice(users)) } if len(dismissal.Teams) > 0 { teams := make([]string, len(dismissal.Teams)) for i, team := range dismissal.Teams { teams[i] = team.GetSlug() } - bpr.RequiredPullRequestReviews.DismissalRestrictions.Teams = util.SortAndReturnPointer(teams) + bpr.RequiredPullRequestReviews.DismissalRestrictions.Teams = util.SortAndReturnPointer(util.ToLowerSlice(teams)) } if len(dismissal.Apps) > 0 { apps := make([]string, len(dismissal.Apps)) for i, app := range dismissal.Apps { apps[i] = app.GetSlug() } - bpr.RequiredPullRequestReviews.DismissalRestrictions.Apps = util.SortAndReturnPointer(apps) + bpr.RequiredPullRequestReviews.DismissalRestrictions.Apps = util.SortAndReturnPointer(util.ToLowerSlice(apps)) } } @@ -757,21 +757,21 @@ func getBPRWithConfig(ctx context.Context, gh *ghclient.Client, owner, repo stri for i, user := range allowances.Users { users[i] = user.GetLogin() } - bpr.RequiredPullRequestReviews.BypassPullRequestAllowances.Users = util.SortAndReturn(users) + bpr.RequiredPullRequestReviews.BypassPullRequestAllowances.Users = util.SortAndReturn(util.ToLowerSlice(users)) } if len(allowances.Teams) > 0 { teams := make([]string, len(allowances.Teams)) for i, team := range allowances.Teams { teams[i] = team.GetSlug() } - bpr.RequiredPullRequestReviews.BypassPullRequestAllowances.Teams = util.SortAndReturn(teams) + bpr.RequiredPullRequestReviews.BypassPullRequestAllowances.Teams = util.SortAndReturn(util.ToLowerSlice(teams)) } if len(allowances.Apps) > 0 { apps := make([]string, len(allowances.Apps)) for i, app := range allowances.Apps { apps[i] = app.GetSlug() } - bpr.RequiredPullRequestReviews.BypassPullRequestAllowances.Apps = util.SortAndReturn(apps) + bpr.RequiredPullRequestReviews.BypassPullRequestAllowances.Apps = util.SortAndReturn(util.ToLowerSlice(apps)) } } } @@ -785,21 +785,21 @@ func getBPRWithConfig(ctx context.Context, gh *ghclient.Client, owner, repo stri for i, user := range restr.Users { users[i] = user.GetLogin() } - bpr.BranchProtectionRestrictions.Users = util.SortAndReturn(users) + bpr.BranchProtectionRestrictions.Users = util.SortAndReturn(util.ToLowerSlice(users)) } if len(restr.Teams) > 0 { teams := make([]string, len(restr.Teams)) for i, team := range restr.Teams { teams[i] = team.GetSlug() } - bpr.BranchProtectionRestrictions.Teams = util.SortAndReturn(teams) + bpr.BranchProtectionRestrictions.Teams = util.SortAndReturn(util.ToLowerSlice(teams)) } if len(restr.Apps) > 0 { apps := make([]string, len(restr.Apps)) for i, app := range restr.Apps { apps[i] = app.GetSlug() } - bpr.BranchProtectionRestrictions.Apps = util.SortAndReturn(apps) + bpr.BranchProtectionRestrictions.Apps = util.SortAndReturn(util.ToLowerSlice(apps)) } } diff --git a/internal/controller/repository/repository_test.go b/internal/controller/repository/repository_test.go index 3cd13c7..cbe687f 100644 --- a/internal/controller/repository/repository_test.go +++ b/internal/controller/repository/repository_test.go @@ -18,6 +18,7 @@ package repository import ( "context" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -60,6 +61,8 @@ var ( team2 = "test-team-2" team2Role = "pull" + githubApp1 = "my-awesome-app" + webhook1url = "https://example.org/webhook" webhook1active = true webhook1InsecureSsl = false @@ -107,21 +110,21 @@ func repository(m ...repositoryModifier) *v1alpha1.Repository { cr.Spec.ForProvider.Permissions = v1alpha1.RepositoryPermissions{ Users: []v1alpha1.RepositoryUser{ { - User: user1, + User: strings.ToUpper(user1), Role: user1Role, }, { - User: user2, + User: strings.ToUpper(user2), Role: user2Role, }, }, Teams: []v1alpha1.RepositoryTeam{ { - Team: team1, + Team: strings.ToUpper(team1), Role: team1Role, }, { - Team: team2, + Team: strings.ToUpper(team2), Role: team2Role, }, }, @@ -156,6 +159,41 @@ func repository(m ...repositoryModifier) *v1alpha1.Repository { }, }, }, + BranchProtectionRestrictions: &v1alpha1.BranchProtectionRestrictions{ + Users: []string{ + strings.ToUpper(user1), + }, + Teams: []string{ + strings.ToUpper(team1), + }, + Apps: []string{ + strings.ToUpper(githubApp1), + }, + }, + RequiredPullRequestReviews: &v1alpha1.RequiredPullRequestReviews{ + BypassPullRequestAllowances: &v1alpha1.BypassPullRequestAllowancesRequest{ + Users: []string{ + strings.ToUpper(user1), + }, + Teams: []string{ + strings.ToUpper(team1), + }, + Apps: []string{ + strings.ToUpper(githubApp1), + }, + }, + DismissalRestrictions: &v1alpha1.DismissalRestrictionsRequest{ + Users: &[]string{ + strings.ToUpper(user1), + }, + Teams: &[]string{ + strings.ToUpper(team1), + }, + Apps: &[]string{ + strings.ToUpper(githubApp1), + }, + }, + }, }, } cr.Spec.ForProvider.RepositoryRules = []v1alpha1.RepositoryRuleset{ @@ -254,6 +292,59 @@ func githubProtectedBranch() *github.Protection { RequiredSignatures: &github.SignaturesProtectedBranch{ Enabled: &bpr1requireSignedCommits, }, + Restrictions: &github.BranchRestrictions{ + Users: []*github.User{ + { + Login: &user1, + }, + }, + Teams: []*github.Team{ + { + Slug: &team1, + }, + }, + Apps: []*github.App{ + { + Slug: &githubApp1, + }, + }, + }, + RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{ + BypassPullRequestAllowances: &github.BypassPullRequestAllowances{ + Users: []*github.User{ + { + Login: &user1, + }, + }, + Teams: []*github.Team{ + { + Slug: &team1, + }, + }, + Apps: []*github.App{ + { + Slug: &githubApp1, + }, + }, + }, + DismissalRestrictions: &github.DismissalRestrictions{ + Users: []*github.User{ + { + Login: &user1, + }, + }, + Teams: []*github.Team{ + { + Slug: &team1, + }, + }, + Apps: []*github.App{ + { + Slug: &githubApp1, + }, + }, + }, + }, } } diff --git a/internal/util/util.go b/internal/util/util.go index f3d67a5..834b8a6 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -22,6 +22,7 @@ import ( "encoding/hex" "reflect" "sort" + "strings" "github.com/crossplane/provider-github/apis/organizations/v1alpha1" "github.com/google/go-cmp/cmp" @@ -293,3 +294,11 @@ func GenerateSHA1Hash(s string) string { return hashedString } + +// ToLowerSlice converts all the strings in the slice to lowercase. +func ToLowerSlice(input []string) []string { + for i, v := range input { + input[i] = strings.ToLower(v) + } + return input +}