From b36ccf95f2f40d76b54397327035ebb8fe0fd8a2 Mon Sep 17 00:00:00 2001 From: Lado Golijashvili Date: Mon, 27 Nov 2023 17:50:32 +0400 Subject: [PATCH 1/2] skip actions enablement on org lvl, if enabledRepo is not set in CRD --- .../controller/organization/organization.go | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/internal/controller/organization/organization.go b/internal/controller/organization/organization.go index 4fb23f0..0b77889 100644 --- a/internal/controller/organization/organization.go +++ b/internal/controller/organization/organization.go @@ -219,11 +219,12 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext if err != nil { return managed.ExternalUpdate{}, err } - - for _, missingRepo := range missingReposIds { - _, err := gh.Actions.AddEnabledReposInOrg(ctx, name, missingRepo) - if err != nil { - return managed.ExternalUpdate{}, err + if cr.Spec.ForProvider.Actions.EnabledRepos != nil { + for _, missingRepo := range missingReposIds { + _, err := gh.Actions.AddEnabledReposInOrg(ctx, name, missingRepo) + if err != nil { + return managed.ExternalUpdate{}, err + } } } @@ -233,10 +234,12 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext } // Disable actions for missing repositories - for _, toDeleteRepo := range toDeleteReposIds { - _, err := gh.Actions.RemoveEnabledRepoInOrg(ctx, name, toDeleteRepo) - if err != nil { - return managed.ExternalUpdate{}, err + if cr.Spec.ForProvider.Actions.EnabledRepos != nil { + for _, toDeleteRepo := range toDeleteReposIds { + _, err := gh.Actions.RemoveEnabledRepoInOrg(ctx, name, toDeleteRepo) + if err != nil { + return managed.ExternalUpdate{}, err + } } } From 17cbee7e3623010bf71270c97b6f6486e55d02ed Mon Sep 17 00:00:00 2001 From: Lado Golijashvili Date: Mon, 27 Nov 2023 18:19:10 +0400 Subject: [PATCH 2/2] refactor to reduce cyclomatic complexity in update function --- .../controller/organization/organization.go | 84 +++++++++++-------- 1 file changed, 51 insertions(+), 33 deletions(-) diff --git a/internal/controller/organization/organization.go b/internal/controller/organization/organization.go index 0b77889..3e280f7 100644 --- a/internal/controller/organization/organization.go +++ b/internal/controller/organization/organization.go @@ -204,45 +204,16 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext return managed.ExternalUpdate{}, err } - crARepos := getSortedEnabledReposFromCr(cr.Spec.ForProvider.Actions.EnabledRepos) - - // To use this function, the organization permission policy for enabled_repositories must be configured to selected, otherwise you get error 409 Conflict - aResp, _, err := gh.Actions.ListEnabledReposInOrg(ctx, name, &github.ListOptions{PerPage: 100}) - if err != nil { - return managed.ExternalUpdate{}, err - } - - // Extract repository names from the list - aRepos := getSortedRepoNames(aResp.Repositories) - - missingReposIds, err := getUpdateRepoIds(ctx, gh, name, crARepos, aRepos) - if err != nil { - return managed.ExternalUpdate{}, err - } - if cr.Spec.ForProvider.Actions.EnabledRepos != nil { - for _, missingRepo := range missingReposIds { - _, err := gh.Actions.AddEnabledReposInOrg(ctx, name, missingRepo) - if err != nil { - return managed.ExternalUpdate{}, err - } - } - } - - toDeleteReposIds, err := getUpdateRepoIds(ctx, gh, name, aRepos, crARepos) + missingReposIds, toDeleteReposIds, err := getMissingAndToDeleteRepos(ctx, gh, name, cr) if err != nil { return managed.ExternalUpdate{}, err } - - // Disable actions for missing repositories if cr.Spec.ForProvider.Actions.EnabledRepos != nil { - for _, toDeleteRepo := range toDeleteReposIds { - _, err := gh.Actions.RemoveEnabledRepoInOrg(ctx, name, toDeleteRepo) - if err != nil { - return managed.ExternalUpdate{}, err - } + err = updateRepos(ctx, gh, name, missingReposIds, toDeleteReposIds) + if err != nil { + return managed.ExternalUpdate{}, err } } - return managed.ExternalUpdate{}, nil } @@ -293,3 +264,50 @@ func getUpdateRepoIds(ctx context.Context, gh *ghclient.Client, org string, crRe } return reposIds, nil } + +func getMissingAndToDeleteRepos(ctx context.Context, gh *ghclient.Client, name string, cr *v1alpha1.Organization) ([]int64, []int64, error) { + crARepos := getSortedEnabledReposFromCr(cr.Spec.ForProvider.Actions.EnabledRepos) + + // To use this function, the organization permission policy for enabled_repositories must be configured to selected, otherwise you get error 409 Conflict + aResp, _, err := gh.Actions.ListEnabledReposInOrg(ctx, name, &github.ListOptions{PerPage: 100}) + if err != nil { + return nil, nil, err + } + + // Extract repository names from the list + aRepos := getSortedRepoNames(aResp.Repositories) + + missingReposIds, err := getUpdateRepoIds(ctx, gh, name, crARepos, aRepos) + if err != nil { + return nil, nil, err + } + + toDeleteReposIds, err := getUpdateRepoIds(ctx, gh, name, aRepos, crARepos) + if err != nil { + return nil, nil, err + } + + return missingReposIds, toDeleteReposIds, nil +} + +func updateRepos(ctx context.Context, gh *ghclient.Client, name string, missingReposIds []int64, toDeleteReposIds []int64) error { + if len(missingReposIds) > 0 { + for _, missingRepo := range missingReposIds { + _, err := gh.Actions.AddEnabledReposInOrg(ctx, name, missingRepo) + if err != nil { + return err + } + } + } + + if len(toDeleteReposIds) > 0 { + for _, toDeleteRepo := range toDeleteReposIds { + _, err := gh.Actions.RemoveEnabledRepoInOrg(ctx, name, toDeleteRepo) + if err != nil { + return err + } + } + } + + return nil +}