From 66cd311eec53c64e10d22786755ac7e208a68a9b Mon Sep 17 00:00:00 2001 From: "Stollin, Thomas" <thomas.stollin@interhyp.de> Date: Mon, 9 Dec 2024 10:40:39 +0100 Subject: [PATCH] feat(repository): patch now patches all fields --- ...odel_repository_configuration_patch_dto.go | 12 ++- api/openapi-v3-spec.yaml | 25 +++-- internal/service/repositories/repositories.go | 98 +++++++++++++++---- test/acceptance/repositoryctl_acc_test.go | 34 ------- test/acceptance/util_dtos_test.go | 27 ++++- .../acceptance-expected/repository-patch.json | 12 ++- 6 files changed, 142 insertions(+), 66 deletions(-) diff --git a/api/generated_model_repository_configuration_patch_dto.go b/api/generated_model_repository_configuration_patch_dto.go index cbeeb83..945a320 100644 --- a/api/generated_model_repository_configuration_patch_dto.go +++ b/api/generated_model_repository_configuration_patch_dto.go @@ -14,8 +14,9 @@ package openapi // RepositoryConfigurationPatchDto Attributes to configure the repository. If a configuration exists there are also some configured defaults for the repository. type RepositoryConfigurationPatchDto struct { // Ssh-Keys configured on the repository. - AccessKeys []RepositoryConfigurationAccessKeyDto `yaml:"accessKeys,omitempty" json:"accessKeys,omitempty"` - MergeConfig *RepositoryConfigurationDtoMergeConfig `yaml:"mergeConfig,omitempty" json:"mergeConfig,omitempty"` + AccessKeys []RepositoryConfigurationAccessKeyDto `yaml:"accessKeys,omitempty" json:"accessKeys,omitempty"` + MergeConfig *RepositoryConfigurationDtoMergeConfig `yaml:"mergeConfig,omitempty" json:"mergeConfig,omitempty"` + DefaultTasks []RepositoryConfigurationDefaultTaskDto `yaml:"defaultTasks,omitempty" json:"defaultTasks,omitempty"` // Use an explicit branch name regex. BranchNameRegex *string `yaml:"branchNameRegex,omitempty" json:"branchNameRegex,omitempty"` // Use an explicit commit message regex. @@ -38,7 +39,12 @@ type RepositoryConfigurationPatchDto struct { // Moves the repository into the archive. Archived *bool `yaml:"archived,omitempty" json:"archived,omitempty"` // Repository will not be configured, also not archived. - Unmanaged *bool `yaml:"unmanaged,omitempty" json:"unmanaged,omitempty"` + Unmanaged *bool `yaml:"unmanaged,omitempty" json:"unmanaged,omitempty"` + RefProtections *RefProtections `yaml:"refProtections,omitempty" json:"refProtections,omitempty"` + // Configures JQL matcher with query: issuetype in (Story, Bug) AND 'Risk Level' is not EMPTY + RequireIssue *bool `yaml:"requireIssue,omitempty" json:"requireIssue,omitempty"` + // Configuration of conditional builds as map of structs (key name e.g. some-key) of target references. + RequireConditions map[string]ConditionReferenceDto `yaml:"requireConditions,omitempty" json:"requireConditions,omitempty"` // Control how the repository is used by GitHub Actions workflows in other repositories ActionsAccess *string `yaml:"actionsAccess,omitempty" json:"actionsAccess,omitempty"` // Configuration of conditional builds as map of structs (key name e.g. some-key) of target references. diff --git a/api/openapi-v3-spec.yaml b/api/openapi-v3-spec.yaml index d02ba3a..485323d 100644 --- a/api/openapi-v3-spec.yaml +++ b/api/openapi-v3-spec.yaml @@ -2127,6 +2127,10 @@ components: type: array items: $ref: '#/components/schemas/MergeStrategy' + defaultTasks: + type: array + items: + $ref: '#/components/schemas/RepositoryConfigurationDefaultTaskDto' branchNameRegex: description: Use an explicit branch name regex. type: string @@ -2179,14 +2183,11 @@ components: unmanaged: description: 'Repository will not be configured, also not archived.' type: boolean - actionsAccess: - description: Control how the repository is used by GitHub Actions workflows in other repositories - type: string - enum: - - NONE - - ORGANIZATION - - ENTERPRISE - example: NOT_ACCESSIBLE + refProtections: + $ref: '#/components/schemas/RefProtections' + requireIssue: + description: 'Configures JQL matcher with query: issuetype in (Story, Bug) AND ''Risk Level'' is not EMPTY' + type: boolean requireConditions: description: Configuration of conditional builds as map of structs (key name e.g. some-key) of target references. type: object @@ -2195,6 +2196,14 @@ components: refMatcher: main additionalProperties: $ref: '#/components/schemas/ConditionReferenceDto' + actionsAccess: + description: Control how the repository is used by GitHub Actions workflows in other repositories + type: string + enum: + - NONE + - ORGANIZATION + - ENTERPRISE + example: NOT_ACCESSIBLE MergeStrategy: type: object properties: diff --git a/internal/service/repositories/repositories.go b/internal/service/repositories/repositories.go index 55a341b..2f8533b 100644 --- a/internal/service/repositories/repositories.go +++ b/internal/service/repositories/repositories.go @@ -447,27 +447,100 @@ func patchConfiguration(patch *openapi.RepositoryConfigurationPatchDto, original } return &openapi.RepositoryConfigurationDto{ AccessKeys: patchAccessKeys(patch.AccessKeys, original.AccessKeys), + MergeConfig: patchMergeConfig(patch.MergeConfig, original.MergeConfig), + DefaultTasks: patchSlice(patch.DefaultTasks, original.DefaultTasks), BranchNameRegex: patchStringPtr(patch.BranchNameRegex, original.BranchNameRegex), CommitMessageRegex: patchStringPtr(patch.CommitMessageRegex, original.CommitMessageRegex), CommitMessageType: patchStringPtr(patch.CommitMessageType, original.CommitMessageType), RequireSuccessfulBuilds: patchPtr[int32](patch.RequireSuccessfulBuilds, original.RequireSuccessfulBuilds), + RequireApprovals: patchPtr[int32](patch.RequireApprovals, original.RequireApprovals), ExcludeMergeCommits: patchPtr[bool](patch.ExcludeMergeCommits, original.ExcludeMergeCommits), ExcludeMergeCheckUsers: patchExcludeMergeCheckUsers(patch.ExcludeMergeCheckUsers, original.ExcludeMergeCheckUsers), Webhooks: patchWebhooks(patch.Webhooks, original.Webhooks), Approvers: patchApprovers(patch.Approvers, original.Approvers), - Watchers: patchStringSlice(patch.Watchers, original.Watchers), + Watchers: patchSlice(patch.Watchers, original.Watchers), Archived: patchPtr[bool](patch.Archived, original.Archived), - ActionsAccess: patchStringPtr(patch.ActionsAccess, original.ActionsAccess), + Unmanaged: patchPtr[bool](patch.Unmanaged, original.Unmanaged), + RefProtections: patchRefProtections(patch.RefProtections, original.RefProtections), + RequireIssue: patchPtr[bool](patch.RequireIssue, original.RequireIssue), RequireConditions: patchRequireConditions(patch.RequireConditions, original.RequireConditions), - // fields not allowed for patching carry over from original - RequireIssue: original.RequireIssue, - RefProtections: original.RefProtections, + ActionsAccess: patchStringPtr(patch.ActionsAccess, original.ActionsAccess), + } + } else { + return original + } +} + +func patchMergeConfig(patch *openapi.RepositoryConfigurationDtoMergeConfig, original *openapi.RepositoryConfigurationDtoMergeConfig) *openapi.RepositoryConfigurationDtoMergeConfig { + if patch != nil { + if original == nil { + return patch + } else { + return &openapi.RepositoryConfigurationDtoMergeConfig{ + DefaultStrategy: patchPtr(patch.DefaultStrategy, original.DefaultStrategy), + Strategies: patchSlice(patch.Strategies, original.Strategies), + } + } + } else { + return original + } +} + +func patchRefProtections(patch *openapi.RefProtections, original *openapi.RefProtections) *openapi.RefProtections { + if patch != nil { + if original == nil { + return patch + } else { + return &openapi.RefProtections{ + Branches: patchRefProtectionsBranches(patch.Branches, original.Branches), + Tags: patchRefProtectionsTags(patch.Tags, original.Tags), + } } } else { return original } } +func patchRefProtectionsBranches(patch *openapi.RefProtectionsBranches, original *openapi.RefProtectionsBranches) *openapi.RefProtectionsBranches { + if patch != nil { + return &openapi.RefProtectionsBranches{ + RequirePR: patchSlice(patch.RequirePR, original.RequirePR), + PreventAllChanges: patchSlice(patch.PreventAllChanges, original.PreventAllChanges), + PreventCreation: patchSlice(patch.PreventCreation, original.PreventCreation), + PreventDeletion: patchSlice(patch.PreventDeletion, original.PreventDeletion), + PreventPush: patchSlice(patch.PreventPush, original.PreventPush), + PreventForcePush: patchSlice(patch.PreventForcePush, original.PreventForcePush), + } + } else { + return original + } +} + +func patchRefProtectionsTags(patch *openapi.RefProtectionsTags, original *openapi.RefProtectionsTags) *openapi.RefProtectionsTags { + if patch != nil { + return &openapi.RefProtectionsTags{ + PreventAllChanges: patchSlice(patch.PreventAllChanges, original.PreventAllChanges), + PreventCreation: patchSlice(patch.PreventCreation, original.PreventCreation), + PreventDeletion: patchSlice(patch.PreventDeletion, original.PreventDeletion), + PreventForcePush: patchSlice(patch.PreventForcePush, original.PreventForcePush), + } + } else { + return original + } +} + +func patchRequireConditions(patch map[string]openapi.ConditionReferenceDto, original map[string]openapi.ConditionReferenceDto) map[string]openapi.ConditionReferenceDto { + if patch != nil { + if len(patch) == 0 { + // remove + return nil + } + return patch + } else { + return original + } +} + func patchApprovers(patch map[string][]string, original map[string][]string) map[string][]string { return patchMapStringListString(patch, original) } @@ -549,20 +622,7 @@ func patchAccessKeys(patch []openapi.RepositoryConfigurationAccessKeyDto, origin } } -func patchRequireConditions(patch map[string]openapi.ConditionReferenceDto, original map[string]openapi.ConditionReferenceDto) map[string]openapi.ConditionReferenceDto { - if patch != nil { - if len(patch) == 0 { - // remove - return nil - } else { - return patch - } - } else { - return original - } -} - -func patchStringSlice(patch []string, original []string) []string { +func patchSlice[E any](patch []E, original []E) []E { if patch != nil { if len(patch) == 0 { // remove diff --git a/test/acceptance/repositoryctl_acc_test.go b/test/acceptance/repositoryctl_acc_test.go index 07d742a..3ace0a6 100644 --- a/test/acceptance/repositoryctl_acc_test.go +++ b/test/acceptance/repositoryctl_acc_test.go @@ -683,40 +683,6 @@ func TestPATCHRepository_Success(t *testing.T) { hasSentNotification(t, "receivesRepository", "karma-wrapper.helm-chart", types.ModifiedEvent, types.RepositoryPayload, &payload) } -func TestPATCHRepository_UnsupportedConfigurationFieldsAreIgnored(t *testing.T) { - tstReset() - - docs.Given("Given an authenticated admin user") - token := tstValidAdminToken() - - docs.When("When they perform a patch changing unsupported fields in the configuration of an existing repository") - body := tstRepositoryPatchWithIgnoredConfigurationFields() - response, err := tstPerformPatch("/rest/api/v1/repositories/karma-wrapper.helm-chart", token, &body) - - docs.Then("Then the request is successful and the response is as expected") - tstAssert(t, response, err, http.StatusOK, "repository-patch.json") - - docs.Then("And the repository has been correctly written, committed and pushed") - filename := "owners/some-owner/repositories/karma-wrapper.helm-chart.yaml" - require.Equal(t, tstRepositoryExpectedYamlKarmaWrapper(), metadataImpl.ReadContents(filename)) - require.True(t, metadataImpl.FilesCommitted[filename]) - require.True(t, metadataImpl.Pushed) - - docs.Then("And the repository has been cached and can be read again") - readAgain, err := tstPerformGet("/rest/api/v1/repositories/karma-wrapper.helm-chart", tstUnauthenticated()) - tstAssert(t, readAgain, err, http.StatusOK, "repository-patch.json") - - docs.Then("And a kafka message notifying other instances of the update has been sent") - require.Equal(t, 1, len(kafkaImpl.Recording)) - actual, _ := json.Marshal(kafkaImpl.Recording[0]) - require.Equal(t, tstRepositoryExpectedKafka("karma-wrapper.helm-chart"), string(actual)) - - docs.Then("And a notification has been sent to all matching owners") - payload := tstUpdatedRepositoryPayload() - hasSentNotification(t, "receivesModified", "karma-wrapper.helm-chart", types.ModifiedEvent, types.RepositoryPayload, &payload) - hasSentNotification(t, "receivesRepository", "karma-wrapper.helm-chart", types.ModifiedEvent, types.RepositoryPayload, &payload) -} - func TestPATCHRepository_NoChangeSuccess(t *testing.T) { tstReset() diff --git a/test/acceptance/util_dtos_test.go b/test/acceptance/util_dtos_test.go index 6f3e1ab..f106d14 100644 --- a/test/acceptance/util_dtos_test.go +++ b/test/acceptance/util_dtos_test.go @@ -3,6 +3,7 @@ package acceptance import ( "github.com/Interhyp/metadata-service/api" "github.com/Interhyp/metadata-service/internal/repository/notifier" + "github.com/Interhyp/metadata-service/internal/util" ) func ptr[T interface{}](v T) *T { @@ -281,7 +282,18 @@ func tstRepositoryPatch() openapi.RepositoryPatchDto { return openapi.RepositoryPatchDto{ Mainline: ptr("main"), Configuration: &openapi.RepositoryConfigurationPatchDto{ - BranchNameRegex: ptr("testing_.*"), + BranchNameRegex: ptr("testing_.*"), + RequireIssue: ptr(true), + RequireConditions: make(map[string]openapi.ConditionReferenceDto), + RefProtections: &openapi.RefProtections{ + Branches: &openapi.RefProtectionsBranches{ + RequirePR: []openapi.ProtectedRef{ + { + Pattern: ".*", + }, + }, + }, + }, }, TimeStamp: "2022-11-06T18:14:10Z", CommitHash: "6c8ac2c35791edf9979623c717a243fc53400000", @@ -357,6 +369,11 @@ mainline: main unittest: false configuration: branchNameRegex: testing_.* + refProtections: + branches: + requirePR: + - pattern: .* + requireIssue: true ` } @@ -393,6 +410,14 @@ func tstUpdatedRepositoryPayload() openapi.NotificationPayload { repo.CommitHash = "6c8ac2c35791edf9979623c717a2430000000000" repo.Configuration = &openapi.RepositoryConfigurationDto{ BranchNameRegex: ptr("testing_.*"), + RequireIssue: util.Ptr(true), + RefProtections: &openapi.RefProtections{ + Branches: &openapi.RefProtectionsBranches{ + RequirePR: []openapi.ProtectedRef{ + {Pattern: ".*"}, + }, + }, + }, } return notifier.AsPayload(repo) } diff --git a/test/resources/acceptance-expected/repository-patch.json b/test/resources/acceptance-expected/repository-patch.json index 70f2906..5118685 100644 --- a/test/resources/acceptance-expected/repository-patch.json +++ b/test/resources/acceptance-expected/repository-patch.json @@ -1,7 +1,17 @@ { "commitHash": "6c8ac2c35791edf9979623c717a2430000000000", "configuration": { - "branchNameRegex": "testing_.*" + "branchNameRegex": "testing_.*", + "refProtections": { + "branches": { + "requirePR": [ + { + "pattern": ".*" + } + ] + } + }, + "requireIssue": true }, "jiraIssue": "ISSUE-2345", "mainline": "main",