diff --git a/internal/acorn/config/customconfigint.go b/internal/acorn/config/customconfigint.go index b4f4a46..01704ea 100644 --- a/internal/acorn/config/customconfigint.go +++ b/internal/acorn/config/customconfigint.go @@ -71,14 +71,20 @@ type CustomConfiguration interface { CheckWarnMissingMainlineProtection() bool CheckExpectedRequiredConditions() []CheckedRequiredConditions + CheckedExpectedExemptions() []CheckedExpectedExemption } - type CheckedRequiredConditions struct { Name string `yaml:"name" json:"name"` AnnotationLevel string `yaml:"annotationLevel" json:"annotationLevel"` RefMatcher string `yaml:"refMatcher" json:"refMatcher"` } +type CheckedExpectedExemption struct { + Name string `yaml:"name" json:"name"` + RefMatcher string `yaml:"refMatcher" json:"refMatcher"` + Exemptions []string `yaml:"exemptions" json:"exemptions"` +} + type NotificationConsumerConfig struct { Subscribed map[types.NotificationPayloadType]map[types.NotificationEventType]struct{} ConsumerURL string @@ -131,4 +137,5 @@ const ( KeyFormattingActionCommitMsgPrefix = "FORMATTING_ACTION_COMMIT_MSG_PREFIX" KeyCheckWarnMissingMainlineProtection = "CHECK_WARN_MISSING_MAINLINE_PROTECTION" KeyCheckExpectedRequiredConditions = "CHECK_EXPECTED_REQUIRED_CONDITIONS" + KeyCheckExpectedExemptions = "CHECK_EXPECTED_EXEMPTIONS" ) diff --git a/internal/repository/config/accessors.go b/internal/repository/config/accessors.go index 5bb114d..bc51b75 100644 --- a/internal/repository/config/accessors.go +++ b/internal/repository/config/accessors.go @@ -201,3 +201,7 @@ func (c *CustomConfigImpl) CheckWarnMissingMainlineProtection() bool { func (c *CustomConfigImpl) CheckExpectedRequiredConditions() []config.CheckedRequiredConditions { return c.VCheckExpectedRequiredConditions } + +func (c *CustomConfigImpl) CheckedExpectedExemptions() []config.CheckedExpectedExemption { + return c.VCheckExpectedExemptions +} diff --git a/internal/repository/config/config.go b/internal/repository/config/config.go index 4caf932..9c310bf 100644 --- a/internal/repository/config/config.go +++ b/internal/repository/config/config.go @@ -301,6 +301,17 @@ var CustomConfigItems = []auconfigapi.ConfigItem{ return err }, }, + { + Key: config.KeyCheckExpectedExemptions, + EnvName: config.KeyCheckExpectedExemptions, + Description: "A JSON list defining all expected exemptions which will be checked for by the GitHub check for all defined repository.yaml files. Each entry contains the 'name' of the exemption, the expected 'refMatcher' and the 'exemptions'.", + Default: "[]", + Validate: func(key string) error { + value := auconfigenv.Get(key) + _, err := parseCheckExpectedExemptions(value) + return err + }, + }, } func ObtainPositiveInt64Validator() func(key string) error { diff --git a/internal/repository/config/plumbing.go b/internal/repository/config/plumbing.go index a6565c7..666a389 100644 --- a/internal/repository/config/plumbing.go +++ b/internal/repository/config/plumbing.go @@ -60,6 +60,7 @@ type CustomConfigImpl struct { VFormattingActionCommitMsgPrefix string VCheckWarnMissingMainlineProtection bool VCheckExpectedRequiredConditions []config.CheckedRequiredConditions + VCheckExpectedExemptions []config.CheckedExpectedExemption VKafkaConfig *kafka.Config GitUrlMatcher *regexp.Regexp @@ -123,6 +124,7 @@ func (c *CustomConfigImpl) Obtain(getter func(key string) string) { c.VYamlIndentation = toInt(getter(config.KeyYamlIndentation)) c.VFormattingActionCommitMsgPrefix = getter(config.KeyFormattingActionCommitMsgPrefix) c.VCheckExpectedRequiredConditions, _ = parseCheckExpectedRequiredConditions(getter(config.KeyCheckExpectedRequiredConditions)) + c.VCheckExpectedExemptions, _ = parseCheckExpectedExemptions(getter(config.KeyCheckExpectedExemptions)) c.VCheckWarnMissingMainlineProtection, _ = strconv.ParseBool(getter(config.KeyCheckWarnMissingMainlineProtection)) } @@ -252,3 +254,14 @@ func parseCheckExpectedRequiredConditions(rawJson string) ([]config.CheckedRequi } return result, nil } + +func parseCheckExpectedExemptions(rawJson string) ([]config.CheckedExpectedExemption, error) { + var parsed []config.CheckedExpectedExemption + if rawJson == "[]" { + return make([]config.CheckedExpectedExemption, 0), nil + } + if err := json.Unmarshal([]byte(rawJson), &parsed); err != nil { + return make([]config.CheckedExpectedExemption, 0), err + } + return parsed, nil +} diff --git a/internal/service/check/check.go b/internal/service/check/check.go index c7f61d4..6d10077 100644 --- a/internal/service/check/check.go +++ b/internal/service/check/check.go @@ -156,6 +156,7 @@ func (h *Impl) validateFiles(ctx context.Context, fs billy.Filesystem) (CheckRes johnnie := MetadataYamlFileWalker(fs, WithIndentation(h.CustomConfiguration.YamlIndentation()), WithExpectedRequiredConditions(h.CustomConfiguration.CheckExpectedRequiredConditions()), + WithExpectedExemptions(h.CustomConfiguration.CheckedExpectedExemptions()), WithMainlinePrProtection(h.CustomConfiguration.CheckWarnMissingMainlineProtection()), ) err := johnnie.ValidateMetadata() @@ -172,6 +173,7 @@ func (h *Impl) validateFiles(ctx context.Context, fs billy.Filesystem) (CheckRes func walkerToCheckRunOutput(johnnie *MetadataWalker) CheckResult { result := CheckResult{ conclusion: repository.CheckRunSuccess, + actions: make([]*github.CheckRunAction, 0), } title := SuccessValidationTitle @@ -201,12 +203,18 @@ func walkerToCheckRunOutput(johnnie *MetadataWalker) CheckResult { Text: details, } - if johnnie.hasFormatErrors { + if johnnie.hasFormatErrors || len(johnnie.hasMissingRequiredConditionExemptions) > 0 { + actionLabel := "Fix formatting" + description := "Adds a new commit with fixed formatting." + if len(johnnie.hasMissingRequiredConditionExemptions) > 0 { + actionLabel = "Fix missing exemptions" + description = "Adds a new commit with missing exemptions." + } result.actions = []*github.CheckRunAction{ { - Label: "Fix formatting", - Description: "Adds a new commit with fixed formatting.", - Identifier: FixFormattingAction, + Label: actionLabel, + Description: description, + Identifier: FixAction, }, } } diff --git a/internal/service/check/check_actions.go b/internal/service/check/check_actions.go index d06dc0e..ef73367 100644 --- a/internal/service/check/check_actions.go +++ b/internal/service/check/check_actions.go @@ -14,8 +14,8 @@ import ( ) const ( - FixFormattingAction = "fix-formatting" - ActionTimeout = 1 * time.Minute + FixAction = "fix-all" + ActionTimeout = 1 * time.Minute ) func (h *Impl) PerformRequestedAction(ctx context.Context, requestedAction string, checkRun *github.CheckRun, requestingUser *github.User) error { @@ -24,19 +24,33 @@ func (h *Impl) PerformRequestedAction(ctx context.Context, requestedAction strin defer cancel() switch requestedAction { - case FixFormattingAction: - return h.commitFormatFixes(independentCtx, checkRun.GetCheckSuite().GetHeadBranch(), requestingUser.GetLogin()) + case FixAction: + msg := "formatting files/adding missing exemptions" + fixFunc := func(branchName string, worktree *git.Worktree) error { + aulogging.Logger.Ctx(ctx).Debug().Printf("%s on branch %s", msg, branchName) + err := MetadataYamlFileWalker(worktree.Filesystem, + WithIndentation(h.CustomConfiguration.YamlIndentation()), + ).FormatMetadata() + if err != nil { + return err + } + err = MetadataYamlFileWalker(worktree.Filesystem, + WithExpectedExemptions(h.CustomConfiguration.CheckedExpectedExemptions()), + ).FixExemptions() + return err + } + return h.commitFixes(independentCtx, fixFunc, checkRun.GetCheckSuite().GetHeadBranch(), requestingUser.GetLogin(), msg) } aulogging.Logger.Ctx(independentCtx).Info().Printf("successfully processed webhook for requested_action %s (suite: %d|run: %d)", requestedAction, checkRun.CheckSuite.GetID(), checkRun.GetID()) return nil } -func (h *Impl) commitFormatFixes(ctx context.Context, branchName string, user string) error { +func (h *Impl) commitFixes(ctx context.Context, fixFunc func(branchName string, worktree *git.Worktree) error, branchName string, user string, msg string) error { if branchName == "" { - return fmt.Errorf("missing branch name for formatting files") + return fmt.Errorf("missing branch name for fixing %s", msg) } - aulogging.Logger.Ctx(ctx).Info().Printf("start fixing file format on branch %s", branchName) + aulogging.Logger.Ctx(ctx).Info().Printf("start fixing '%s' on branch %s", msg, branchName) author, err := h.Github.GetUser(ctx, user) if err != nil { return err @@ -48,21 +62,18 @@ func (h *Impl) commitFormatFixes(ctx context.Context, branchName string, user st return err } - aulogging.Logger.Ctx(ctx).Debug().Printf("formatting files on branch %s", branchName) - err = MetadataYamlFileWalker(worktree.Filesystem, - WithIndentation(h.CustomConfiguration.YamlIndentation()), - ).FormatMetadata() + err = fixFunc(branchName, worktree) if err != nil { return err } - aulogging.Logger.Ctx(ctx).Debug().Printf("committing formatted files onto branch %s", branchName) - err = h.commit(worktree, author) + aulogging.Logger.Ctx(ctx).Debug().Printf("committing %s onto branch %s", msg, branchName) + err = h.commit(worktree, author, msg) if err != nil { return err } - aulogging.Logger.Ctx(ctx).Debug().Printf("pushing formatted files onto branch %s", branchName) + aulogging.Logger.Ctx(ctx).Debug().Printf("pushing %s onto branch %s", msg, branchName) err = repo.PushContext(ctx, &git.PushOptions{ Auth: h.AuthProvider.ProvideAuth(ctx), RemoteName: "origin", @@ -70,7 +81,7 @@ func (h *Impl) commitFormatFixes(ctx context.Context, branchName string, user st if err != nil { return err } - aulogging.Logger.Ctx(ctx).Info().Printf("finished fixing file format on branch %s", branchName) + aulogging.Logger.Ctx(ctx).Info().Printf("finished fixing '%s' on branch %s", msg, branchName) return nil } @@ -92,9 +103,9 @@ func (h *Impl) clone(ctx context.Context, branchName string) (*git.Repository, * return repo, worktree, nil } -func (h *Impl) commit(worktree *git.Worktree, author *github.User) error { +func (h *Impl) commit(worktree *git.Worktree, author *github.User, msg string) error { commitTimestamp := h.timestamp.Now() - commitMsg := fmt.Sprintf("%sFormat files", h.CustomConfiguration.FormattingActionCommitMsgPrefix()) + commitMsg := fmt.Sprintf("%s%s", h.CustomConfiguration.FormattingActionCommitMsgPrefix(), msg) _, err := worktree.Commit(commitMsg, &git.CommitOptions{ All: true, Author: &object.Signature{ diff --git a/internal/service/check/yamlwalker.go b/internal/service/check/yamlwalker.go index 4ca5f7c..85a3490 100644 --- a/internal/service/check/yamlwalker.go +++ b/internal/service/check/yamlwalker.go @@ -18,14 +18,21 @@ type walkedRepos struct { keyToPath map[string]string } type MetadataWalker struct { - fs billy.Filesystem - Annotations []*github.CheckRunAnnotation - Errors map[string]error - IgnoredWithReason map[string]string - walkedRepos walkedRepos - fmtEngine yamlfmt.Engine - hasFormatErrors bool - config Config + fs billy.Filesystem + Annotations []*github.CheckRunAnnotation + Errors map[string]error + IgnoredWithReason map[string]string + walkedRepos walkedRepos + fmtEngine yamlfmt.Engine + hasFormatErrors bool + hasMissingRequiredConditionExemptions []MissingRequiredConditionExemption + config Config +} + +type MissingRequiredConditionExemption struct { + Name string + RefMatcher string + Exemptions []string } type Config struct { @@ -33,6 +40,7 @@ type Config struct { indentation int requireMainlinePrProtection bool expectedRequiredConditions []config.CheckedRequiredConditions + expectedExemptions []config.CheckedExpectedExemption } type Option = func(config *Config) @@ -61,6 +69,12 @@ func WithExpectedRequiredConditions(expectedReqConditions []config.CheckedRequir } } +func WithExpectedExemptions(expectedExemptions []config.CheckedExpectedExemption) Option { + return func(config *Config) { + config.expectedExemptions = expectedExemptions + } +} + const lineBreakStyle = yamlfmt.LineBreakStyleLF const lineSeparatorCharacter = "\n" diff --git a/internal/service/check/yamlwalker_exemptions.go b/internal/service/check/yamlwalker_exemptions.go new file mode 100644 index 0000000..36dfa8b --- /dev/null +++ b/internal/service/check/yamlwalker_exemptions.go @@ -0,0 +1,115 @@ +package check + +import ( + "fmt" + openapi "github.com/Interhyp/metadata-service/api" + "github.com/Interhyp/metadata-service/internal/acorn/config" + "github.com/go-git/go-billy/v5/util" + "gopkg.in/yaml.v3" + "io/fs" + "strings" +) + +func (v *MetadataWalker) FixExemptions() error { + return util.Walk(v.fs, v.config.rootDir, v.fixExemptionsFunc) +} + +func (v *MetadataWalker) fixExemptionsFunc(path string, info fs.FileInfo, err error) error { + return v.walkFunc( + func(fileContents []byte) error { + return v.fixExemptionsInFile(fileContents, path) + })(path, info, err) +} + +func (v *MetadataWalker) fixExemptionsInFile(fileContents []byte, path string) error { + if strings.Contains(path, "/repositories/") { + v.validateRepositoryFile(path, string(fileContents)) + if len(v.hasMissingRequiredConditionExemptions) > 0 { + dto := &openapi.RepositoryDto{} + _ = parseStrict(path, string(fileContents), dto) + for _, missingExemption := range v.hasMissingRequiredConditionExemptions { + switch missingExemption.Name { + case fmt.Sprintf("%s.requirePR", refProtectionBranchIdentifier): + var newRefProtection []openapi.ProtectedRef + for _, refProtection := range dto.Configuration.RefProtections.Branches.RequirePR { + newRefProtection = addMissingExemption(refProtection, missingExemption, newRefProtection) + } + dto.Configuration.RefProtections.Branches.RequirePR = newRefProtection + case fmt.Sprintf("%s.preventAllChanges", refProtectionBranchIdentifier): + var newRefProtection []openapi.ProtectedRef + for _, refProtection := range dto.Configuration.RefProtections.Branches.PreventAllChanges { + newRefProtection = addMissingExemption(refProtection, missingExemption, newRefProtection) + } + dto.Configuration.RefProtections.Branches.PreventAllChanges = newRefProtection + case fmt.Sprintf("%s.preventCreation", refProtectionBranchIdentifier): + var newRefProtection []openapi.ProtectedRef + for _, refProtection := range dto.Configuration.RefProtections.Branches.PreventCreation { + newRefProtection = addMissingExemption(refProtection, missingExemption, newRefProtection) + } + dto.Configuration.RefProtections.Branches.PreventCreation = newRefProtection + case fmt.Sprintf("%s.preventDeletion", refProtectionBranchIdentifier): + var newRefProtection []openapi.ProtectedRef + for _, refProtection := range dto.Configuration.RefProtections.Branches.PreventDeletion { + newRefProtection = addMissingExemption(refProtection, missingExemption, newRefProtection) + } + dto.Configuration.RefProtections.Branches.PreventDeletion = newRefProtection + case fmt.Sprintf("%s.preventPush", refProtectionBranchIdentifier): + var newRefProtection []openapi.ProtectedRef + for _, refProtection := range dto.Configuration.RefProtections.Branches.PreventPush { + newRefProtection = addMissingExemption(refProtection, missingExemption, newRefProtection) + } + dto.Configuration.RefProtections.Branches.PreventPush = newRefProtection + case fmt.Sprintf("%s.preventForcePush", refProtectionBranchIdentifier): + var newRefProtection []openapi.ProtectedRef + for _, refProtection := range dto.Configuration.RefProtections.Branches.PreventForcePush { + newRefProtection = addMissingExemption(refProtection, missingExemption, newRefProtection) + } + dto.Configuration.RefProtections.Branches.PreventForcePush = newRefProtection + case fmt.Sprintf("%s.preventAllChanges", refProtectionTagIdentifier): + var newRefProtection []openapi.ProtectedRef + for _, refProtection := range dto.Configuration.RefProtections.Tags.PreventAllChanges { + newRefProtection = addMissingExemption(refProtection, missingExemption, newRefProtection) + } + dto.Configuration.RefProtections.Tags.PreventAllChanges = newRefProtection + case fmt.Sprintf("%s.preventCreation", refProtectionTagIdentifier): + var newRefProtection []openapi.ProtectedRef + for _, refProtection := range dto.Configuration.RefProtections.Tags.PreventCreation { + newRefProtection = addMissingExemption(refProtection, missingExemption, newRefProtection) + } + dto.Configuration.RefProtections.Tags.PreventCreation = newRefProtection + case fmt.Sprintf("%s.preventDeletion", refProtectionTagIdentifier): + var newRefProtection []openapi.ProtectedRef + for _, refProtection := range dto.Configuration.RefProtections.Tags.PreventDeletion { + newRefProtection = addMissingExemption(refProtection, missingExemption, newRefProtection) + } + dto.Configuration.RefProtections.Tags.PreventDeletion = newRefProtection + case fmt.Sprintf("%s.preventForcePush", refProtectionTagIdentifier): + var newRefProtection []openapi.ProtectedRef + for _, refProtection := range dto.Configuration.RefProtections.Tags.PreventForcePush { + newRefProtection = addMissingExemption(refProtection, missingExemption, newRefProtection) + } + dto.Configuration.RefProtections.Tags.PreventForcePush = newRefProtection + } + if isExpectedExemptionCondition(config.CheckedExpectedExemption{Name: missingExemption.Name}) { + if current, ok := dto.Configuration.RequireConditions[missingExemption.Name]; ok && current.RefMatcher == missingExemption.RefMatcher { + current.Exemptions = append(current.Exemptions, missingExemption.Exemptions...) + dto.Configuration.RequireConditions[missingExemption.Name] = current + } + } + } + fixed, err := yaml.Marshal(dto) + if err != nil { + return err + } + return v.formatSingleYamlFile(fixed, path) + } + } + return nil +} + +func addMissingExemption(refProtection openapi.ProtectedRef, missingExemption MissingRequiredConditionExemption, newRefProtection []openapi.ProtectedRef) []openapi.ProtectedRef { + if refProtection.Pattern == missingExemption.RefMatcher { + refProtection.Exemptions = append(refProtection.Exemptions, missingExemption.Exemptions...) + } + return append(newRefProtection, refProtection) +} diff --git a/internal/service/check/yamlwalker_exemptions_test.go b/internal/service/check/yamlwalker_exemptions_test.go new file mode 100644 index 0000000..29d33b6 --- /dev/null +++ b/internal/service/check/yamlwalker_exemptions_test.go @@ -0,0 +1,235 @@ +package check + +import ( + "github.com/Interhyp/metadata-service/internal/acorn/config" + "github.com/go-git/go-billy/v5/memfs" + "github.com/stretchr/testify/assert" + "io" + "reflect" + "testing" +) + +func TestMetadataYamlFileWalker_fixExemptionsInFile(t *testing.T) { + type args struct { + path string + fileContents []byte + } + type want struct { + result any + } + type mock struct { + config *Config + } + hasMock := func(m mock) bool { + return !reflect.DeepEqual(m, mock{config: &Config{}}) + } + tests := []struct { + name string + args args + want want + mock mock + }{ + { + name: "not in /repository", + args: args{ + path: "some/other/path.yaml", + fileContents: []byte("attribute: value\notherAttribute: otherValue"), + }, + want: want{ + result: nil, + }, + }, + { + name: "repositoryType not matching", + mock: mock{ + config: &Config{ + expectedExemptions: []config.CheckedExpectedExemption{ + { + Name: "some-condition", + RefMatcher: "some-ref-matcher", + Exemptions: []string{"existing-exemption-one", "existing-exemption-two"}, + }, + }, + }, + }, + args: args{ + path: "owners/some-owner/repositories/repository.other-repository-type.yaml", + fileContents: []byte(`url: existing-repo-url +mainline: master +configuration: + requireConditions: + some-condition: + refMatcher: some-ref-matcher + exemptions: + - existing-exemption-one + - existing-exemption-two +`), + }, + want: want{ + result: nil, + }, + }, + { + name: "no missing exemptions", + mock: mock{ + config: &Config{ + expectedExemptions: []config.CheckedExpectedExemption{ + { + Name: "some-condition", + RefMatcher: "some-ref-matcher", + Exemptions: []string{"existing-exemption-one", "existing-exemption-two"}, + }, + }, + }, + }, + args: args{ + path: "owners/some-owner/repositories/repository.helm-deployment.yaml", + fileContents: []byte(`url: existing-repo-url +mainline: master +configuration: + requireConditions: + some-condition: + refMatcher: some-ref-matcher + exemptions: + - existing-exemption-one + - existing-exemption-two +`), + }, + want: want{ + result: nil, + }, + }, + { + name: "refMatcher not matching", + mock: mock{ + config: &Config{ + expectedExemptions: []config.CheckedExpectedExemption{ + { + Name: "some-condition", + RefMatcher: "some-ref-matcher", + Exemptions: []string{"existing-exemption-one", "existing-exemption-two"}, + }, + }, + }, + }, + args: args{ + path: "owners/some-owner/repositories/repository.helm-deployment.yaml", + fileContents: []byte(`url: existing-repo-url +mainline: master +configuration: + requireConditions: + some-condition: + refMatcher: other-ref-matcher + exemptions: + - existing-exemption-one + - existing-exemption-two +`), + }, + want: want{ + result: nil, + }, + }, + { + name: "fix exemptions in requireConditions", + mock: mock{ + config: &Config{ + expectedExemptions: []config.CheckedExpectedExemption{ + { + Name: "some-condition", + RefMatcher: "some-ref-matcher", + Exemptions: []string{"existing-exemption", "missing-exemption"}, + }, + }, + }, + }, + args: args{ + path: "owners/some-owner/repositories/repository.helm-deployment.yaml", + fileContents: []byte(`url: existing-repo-url +mainline: master +configuration: + requireConditions: + some-condition: + refMatcher: some-ref-matcher + exemptions: + - existing-exemption +`), + }, + want: want{ + result: `url: existing-repo-url +mainline: master +configuration: + requireConditions: + some-condition: + refMatcher: some-ref-matcher + exemptions: + - existing-exemption + - missing-exemption +`, + }, + }, + { + name: "fix exemptions in refProtections", + mock: mock{ + config: &Config{ + expectedExemptions: []config.CheckedExpectedExemption{ + { + Name: "tags.preventCreation", + RefMatcher: "some-ref-matcher", + Exemptions: []string{"existing-exemption", "missing-exemption"}, + }, + }, + }, + }, + args: args{ + path: "owners/some-owner/repositories/repository.helm-deployment.yaml", + fileContents: []byte(`url: existing-repo-url +mainline: master +configuration: + refProtections: + tags: + preventCreation: + - pattern: some-ref-matcher + exemptions: + - existing-exemption +`), + }, + want: want{ + result: `url: existing-repo-url +mainline: master +configuration: + refProtections: + tags: + preventCreation: + - pattern: some-ref-matcher + exemptions: + - existing-exemption + - missing-exemption +`, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + v := MetadataYamlFileWalker(nil) + if hasMock(tt.mock) { + v.fs = memfs.New() + _, _ = v.fs.Create(tt.args.path) + if tt.mock.config != nil { + v.config = *tt.mock.config + } + } + if got := v.fixExemptionsInFile(tt.args.fileContents, tt.args.path); got == nil { + file, err := v.fs.Open(tt.args.path) + assert.NoError(t, err) + defer file.Close() + content, err := io.ReadAll(file) + assert.NoError(t, err) + if tt.want.result != nil { + assert.Equal(t, tt.want.result, string(content)) + } else { + assert.Equal(t, "", string(content)) + } + } + }) + } +} diff --git a/internal/service/check/yamlwalker_validate.go b/internal/service/check/yamlwalker_validate.go index 9006967..1bbb8e1 100644 --- a/internal/service/check/yamlwalker_validate.go +++ b/internal/service/check/yamlwalker_validate.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "github.com/Interhyp/metadata-service/api" + "github.com/Interhyp/metadata-service/internal/acorn/config" "github.com/go-git/go-billy/v5/util" "github.com/google/go-github/v70/github" "github.com/google/yamlfmt" @@ -14,6 +15,13 @@ import ( "strings" ) +const ( + annotationLevelWarning = "warning" + deploymentRepositoryIdentifier = "helm-deployment" + refProtectionBranchIdentifier = "branches" + refProtectionTagIdentifier = "tags" +) + func (v *MetadataWalker) ValidateMetadata() error { return util.Walk(v.fs, v.config.rootDir, v.validateWalkFunc) } @@ -199,23 +207,39 @@ func (v *MetadataWalker) checkMainlineProtection(path string, dto *openapi.Repos } func (v *MetadataWalker) checkRequiredConditions(path string, dto *openapi.RepositoryDto) []*github.CheckRunAnnotation { - if len(v.config.expectedRequiredConditions) == 0 { + if len(v.config.expectedExemptions) == 0 { return nil } - if dto == nil || dto.Configuration == nil || dto.Configuration.RequireConditions == nil { + if dto == nil || dto.Configuration == nil && (dto.Configuration.RequireConditions == nil && dto.Configuration.RefProtections == nil) { return nil } annotations := make([]*github.CheckRunAnnotation, 0) - for _, expected := range v.config.expectedRequiredConditions { - c, ok := dto.Configuration.RequireConditions[expected.Name] - if !ok || c.RefMatcher != expected.RefMatcher { + for _, expected := range v.config.expectedExemptions { + if !strings.Contains(path, deploymentRepositoryIdentifier) { + continue + } + conditionExists, missingConditionExemptions := v.checkExpectedExemptionOnRequiredConditions(expected, dto) + refProtectionExists, missingRefProtectionExemptions := v.checkExpectedExemptionOnRefProtections(expected, dto) + if !conditionExists && !refProtectionExists { annotations = append(annotations, &github.CheckRunAnnotation{ Path: github.Ptr(path), StartLine: github.Ptr(1), EndLine: github.Ptr(1), - AnnotationLevel: github.Ptr(expected.AnnotationLevel), - Message: github.Ptr(fmt.Sprintf("This file does not contain the required condition %s with the refMatcher %s.", expected.Name, expected.RefMatcher)), - Title: github.Ptr("missing expected required condition"), + AnnotationLevel: github.Ptr(annotationLevelWarning), + Message: github.Ptr(fmt.Sprintf("This file does not contain the required condition/refProtection %s with the refMatcher %s.", expected.Name, expected.RefMatcher)), + Title: github.Ptr("missing expected condition/refProtection"), + }) + } + if len(missingConditionExemptions) > 0 || len(missingRefProtectionExemptions) > 0 { + missing := append(missingConditionExemptions, missingRefProtectionExemptions...) + v.hasMissingRequiredConditionExemptions = missing + annotations = append(annotations, &github.CheckRunAnnotation{ + Path: github.Ptr(path), + StartLine: github.Ptr(1), + EndLine: github.Ptr(1), + AnnotationLevel: github.Ptr(annotationLevelWarning), + Message: github.Ptr(fmt.Sprintf("This file does not contain all required exemptions %s for condition %s with the refMatcher %s.", strings.Join(expected.Exemptions, ", "), expected.Name, expected.RefMatcher)), + Title: github.Ptr("missing expected required exemptions"), }) } } @@ -223,6 +247,117 @@ func (v *MetadataWalker) checkRequiredConditions(path string, dto *openapi.Repos return annotations } +func (v *MetadataWalker) checkExpectedExemptionOnRequiredConditions(expected config.CheckedExpectedExemption, dto *openapi.RepositoryDto) (bool, []MissingRequiredConditionExemption) { + missingExemptions := make([]MissingRequiredConditionExemption, 0) + // requiredCondition missing false, exists true + okResult := false + if dto.Configuration.RequireConditions == nil && isExpectedExemptionCondition(expected) { + return okResult, missingExemptions + } + if condition, ok := dto.Configuration.RequireConditions[expected.Name]; ok && condition.RefMatcher == expected.RefMatcher { + okResult = ok + if missing := allEntriesExist(expected.Exemptions, condition.Exemptions); len(missing) > 0 { + missingExemptions = append(missingExemptions, MissingRequiredConditionExemption{ + Name: expected.Name, + RefMatcher: expected.RefMatcher, + Exemptions: missing, + }) + } + } + + return okResult, missingExemptions +} + +func (v *MetadataWalker) checkExpectedExemptionOnRefProtections(expected config.CheckedExpectedExemption, dto *openapi.RepositoryDto) (bool, []MissingRequiredConditionExemption) { + missingExemptions := make([]MissingRequiredConditionExemption, 0) + // requiredCondition missing false, exists true + okResult := false + if dto.Configuration.RefProtections == nil && !isExpectedExemptionCondition(expected) { + return okResult, missingExemptions + } + var protectedRefs []openapi.ProtectedRef + switch expected.Name { + case fmt.Sprintf("%s.requirePR", refProtectionBranchIdentifier): + if dto.Configuration.RefProtections.Branches != nil && dto.Configuration.RefProtections.Branches.RequirePR != nil { + protectedRefs = dto.Configuration.RefProtections.Branches.RequirePR + } + case fmt.Sprintf("%s.preventAllChanges", refProtectionBranchIdentifier): + if dto.Configuration.RefProtections.Branches != nil && dto.Configuration.RefProtections.Branches.PreventAllChanges != nil { + + protectedRefs = dto.Configuration.RefProtections.Branches.PreventAllChanges + } + case fmt.Sprintf("%s.preventCreation", refProtectionBranchIdentifier): + if dto.Configuration.RefProtections.Branches != nil && dto.Configuration.RefProtections.Branches.PreventCreation != nil { + protectedRefs = dto.Configuration.RefProtections.Branches.PreventCreation + } + case fmt.Sprintf("%s.preventDeletion", refProtectionBranchIdentifier): + if dto.Configuration.RefProtections.Branches != nil && dto.Configuration.RefProtections.Branches.PreventDeletion != nil { + protectedRefs = dto.Configuration.RefProtections.Branches.PreventDeletion + } + case fmt.Sprintf("%s.preventPush", refProtectionBranchIdentifier): + if dto.Configuration.RefProtections.Branches != nil && dto.Configuration.RefProtections.Branches.PreventPush != nil { + protectedRefs = dto.Configuration.RefProtections.Branches.PreventPush + } + case fmt.Sprintf("%s.preventForcePush", refProtectionBranchIdentifier): + if dto.Configuration.RefProtections.Branches != nil && dto.Configuration.RefProtections.Branches.PreventForcePush != nil { + protectedRefs = dto.Configuration.RefProtections.Branches.PreventForcePush + } + case fmt.Sprintf("%s.preventAllChanges", refProtectionTagIdentifier): + if dto.Configuration.RefProtections.Tags != nil && dto.Configuration.RefProtections.Tags.PreventAllChanges != nil { + protectedRefs = dto.Configuration.RefProtections.Tags.PreventAllChanges + } + case fmt.Sprintf("%s.preventCreation", refProtectionTagIdentifier): + if dto.Configuration.RefProtections.Tags != nil && dto.Configuration.RefProtections.Tags.PreventCreation != nil { + protectedRefs = dto.Configuration.RefProtections.Tags.PreventCreation + } + case fmt.Sprintf("%s.preventDeletion", refProtectionTagIdentifier): + if dto.Configuration.RefProtections.Tags != nil && dto.Configuration.RefProtections.Tags.PreventDeletion != nil { + protectedRefs = dto.Configuration.RefProtections.Tags.PreventDeletion + } + case fmt.Sprintf("%s.preventForcePush", refProtectionTagIdentifier): + if dto.Configuration.RefProtections.Tags != nil && dto.Configuration.RefProtections.Tags.PreventForcePush != nil { + protectedRefs = dto.Configuration.RefProtections.Tags.PreventForcePush + } + } + + for _, protectedRef := range protectedRefs { + if protectedRef.Pattern == expected.RefMatcher { + okResult = true + if missing := allEntriesExist(expected.Exemptions, protectedRef.Exemptions); len(missing) > 0 { + okResult = true + missingExemptions = append(missingExemptions, MissingRequiredConditionExemption{ + Name: expected.Name, + RefMatcher: expected.RefMatcher, + Exemptions: missing, + }) + } + } + } + + return okResult, missingExemptions +} + +func isExpectedExemptionCondition(expected config.CheckedExpectedExemption) bool { + return !strings.Contains(expected.Name, "branches") || !strings.Contains(expected.Name, "tags") +} + +func allEntriesExist(arr1, arr2 []string) []string { + var missing []string + for _, str1 := range arr1 { + found := false + for _, str2 := range arr2 { + if str1 == str2 { + found = true + break + } + } + if !found { + missing = append(missing, str1) + } + } + return missing +} + func (v *MetadataWalker) checkFormatting(path string, content string) *github.CheckRunAnnotation { formatted, err := v.fmtEngine.FormatContent([]byte(content)) if err != nil { diff --git a/internal/service/check/yamlwalker_validate_test.go b/internal/service/check/yamlwalker_validate_test.go index 3b28cb4..2ed917c 100644 --- a/internal/service/check/yamlwalker_validate_test.go +++ b/internal/service/check/yamlwalker_validate_test.go @@ -2,6 +2,7 @@ package check import ( "fmt" + "github.com/Interhyp/metadata-service/internal/acorn/config" "github.com/google/go-github/v70/github" "github.com/stretchr/testify/require" "reflect" @@ -21,6 +22,7 @@ func TestMetadataYamlFileWalker_validateSingleYamlFile(t *testing.T) { } type mock struct { walkedRepos walkedRepos + config *Config } hasMock := func(m mock) bool { return !reflect.DeepEqual(m, mock{walkedRepos: walkedRepos{}}) @@ -348,12 +350,387 @@ configuration: errors: make(map[string]error), }, }, + { + name: "invalid repo - missing condition in requireConditions", + mock: mock{ + walkedRepos: walkedRepos{ + urlToPath: make(map[string]string), + keyToPath: map[string]string{}, + }, + config: &Config{ + expectedExemptions: []config.CheckedExpectedExemption{ + { + Name: "some-condition", + RefMatcher: "some-ref-matcher", + Exemptions: []string{"missing-exemption-one", "missing-exemption-two"}, + }, + }, + }, + }, + args: args{ + path: "owners/some-owner/repositories/repository.helm-deployment.yaml", + contents: `url: existing-repo-url +mainline: master +configuration: + requireConditions: + other-condition: + exemptions: + - some-exemption +`, + }, + want: want{ + result: []*github.CheckRunAnnotation{ + { + Path: github.Ptr("owners/some-owner/repositories/repository.helm-deployment.yaml"), + StartLine: github.Ptr(1), + EndLine: github.Ptr(1), + AnnotationLevel: github.Ptr("warning"), + Message: github.Ptr("This file does not contain the required condition/refProtection some-condition with the refMatcher some-ref-matcher."), + Title: github.Ptr("missing expected condition/refProtection"), + }, + }, + ignored: make(map[string]string), + errors: make(map[string]error), + }, + }, + { + name: "invalid repo - ref not matching in requireConditions", + mock: mock{ + walkedRepos: walkedRepos{ + urlToPath: make(map[string]string), + keyToPath: map[string]string{}, + }, + config: &Config{ + expectedExemptions: []config.CheckedExpectedExemption{ + { + Name: "some-condition", + RefMatcher: "some-ref-matcher", + Exemptions: []string{"missing-exemption-one", "missing-exemption-two"}, + }, + }, + }, + }, + args: args{ + path: "owners/some-owner/repositories/repository.helm-deployment.yaml", + contents: `url: existing-repo-url +mainline: master +configuration: + requireConditions: + some-condition: + refMatcher: "other-ref-matcher" + exemptions: + - some-exemption +`, + }, + want: want{ + result: []*github.CheckRunAnnotation{ + { + Path: github.Ptr("owners/some-owner/repositories/repository.helm-deployment.yaml"), + StartLine: github.Ptr(1), + EndLine: github.Ptr(1), + AnnotationLevel: github.Ptr("warning"), + Message: github.Ptr("This file does not contain the required condition/refProtection some-condition with the refMatcher some-ref-matcher."), + Title: github.Ptr("missing expected condition/refProtection"), + }, + }, + ignored: make(map[string]string), + errors: make(map[string]error), + }, + }, + { + name: "invalid repo - missing exemption in requireConditions", + mock: mock{ + walkedRepos: walkedRepos{ + urlToPath: make(map[string]string), + keyToPath: map[string]string{}, + }, + config: &Config{ + expectedExemptions: []config.CheckedExpectedExemption{ + { + Name: "some-condition", + RefMatcher: "some-ref-matcher", + Exemptions: []string{"missing-exemption-one", "some-exemption"}, + }, + }, + }, + }, + args: args{ + path: "owners/some-owner/repositories/repository.helm-deployment.yaml", + contents: `url: existing-repo-url +mainline: master +configuration: + requireConditions: + some-condition: + refMatcher: "some-ref-matcher" + exemptions: + - some-exemption +`, + }, + want: want{ + result: []*github.CheckRunAnnotation{ + { + Path: github.Ptr("owners/some-owner/repositories/repository.helm-deployment.yaml"), + StartLine: github.Ptr(1), + EndLine: github.Ptr(1), + AnnotationLevel: github.Ptr("warning"), + Message: github.Ptr("This file does not contain all required exemptions missing-exemption-one, some-exemption for condition some-condition with the refMatcher some-ref-matcher."), + Title: github.Ptr("missing expected required exemptions"), + }, + }, + ignored: make(map[string]string), + errors: make(map[string]error), + }, + }, + { + name: "invalid repo - missing condition in refProtections", + mock: mock{ + walkedRepos: walkedRepos{ + urlToPath: make(map[string]string), + keyToPath: map[string]string{}, + }, + config: &Config{ + expectedExemptions: []config.CheckedExpectedExemption{ + { + Name: "branches.preventDeletion", + RefMatcher: "some-ref-matcher", + Exemptions: []string{"missing-exemption-one", "some-exemption"}, + }, + }, + }, + }, + args: args{ + path: "owners/some-owner/repositories/repository.helm-deployment.yaml", + contents: `url: existing-repo-url +mainline: master +configuration: + refProtections: + branches: + requirePR: + - exemptions: + - some-exemption +`, + }, + want: want{ + result: []*github.CheckRunAnnotation{ + { + Path: github.Ptr("owners/some-owner/repositories/repository.helm-deployment.yaml"), + StartLine: github.Ptr(1), + EndLine: github.Ptr(1), + AnnotationLevel: github.Ptr("warning"), + Message: github.Ptr("This file does not contain the required condition/refProtection branches.preventDeletion with the refMatcher some-ref-matcher."), + Title: github.Ptr("missing expected condition/refProtection"), + }, + }, + ignored: make(map[string]string), + errors: make(map[string]error), + }, + }, + { + name: "invalid repo - ref not matching in refProtections", + mock: mock{ + walkedRepos: walkedRepos{ + urlToPath: make(map[string]string), + keyToPath: map[string]string{}, + }, + config: &Config{ + expectedExemptions: []config.CheckedExpectedExemption{ + { + Name: "branches.preventDeletion", + RefMatcher: "some-ref-matcher", + Exemptions: []string{"missing-exemption-one", "some-exemption"}, + }, + }, + }, + }, + args: args{ + path: "owners/some-owner/repositories/repository.helm-deployment.yaml", + contents: `url: existing-repo-url +mainline: master +configuration: + refProtections: + branches: + preventDeletion: + - pattern: "other-ref-matcher" + exemptions: + - some-exemption +`, + }, + want: want{ + result: []*github.CheckRunAnnotation{ + { + Path: github.Ptr("owners/some-owner/repositories/repository.helm-deployment.yaml"), + StartLine: github.Ptr(1), + EndLine: github.Ptr(1), + AnnotationLevel: github.Ptr("warning"), + Message: github.Ptr("This file does not contain the required condition/refProtection branches.preventDeletion with the refMatcher some-ref-matcher."), + Title: github.Ptr("missing expected condition/refProtection"), + }, + }, + ignored: make(map[string]string), + errors: make(map[string]error), + }, + }, + { + name: "invalid repo - missing exemption in refProtections", + mock: mock{ + walkedRepos: walkedRepos{ + urlToPath: make(map[string]string), + keyToPath: map[string]string{}, + }, + config: &Config{ + expectedExemptions: []config.CheckedExpectedExemption{ + { + Name: "branches.preventDeletion", + RefMatcher: "some-ref-matcher", + Exemptions: []string{"missing-exemption-one", "some-exemption"}, + }, + }, + }, + }, + args: args{ + path: "owners/some-owner/repositories/repository.helm-deployment.yaml", + contents: `url: existing-repo-url +mainline: master +configuration: + refProtections: + branches: + preventDeletion: + - pattern: "some-ref-matcher" + exemptions: + - some-exemption +`, + }, + want: want{ + result: []*github.CheckRunAnnotation{ + { + Path: github.Ptr("owners/some-owner/repositories/repository.helm-deployment.yaml"), + StartLine: github.Ptr(1), + EndLine: github.Ptr(1), + AnnotationLevel: github.Ptr("warning"), + Message: github.Ptr("This file does not contain all required exemptions missing-exemption-one, some-exemption for condition branches.preventDeletion with the refMatcher some-ref-matcher."), + Title: github.Ptr("missing expected required exemptions"), + }, + }, + ignored: make(map[string]string), + errors: make(map[string]error), + }, + }, + { + name: "invalid repo - missing exemption in requiredConditions and refProtections", + mock: mock{ + walkedRepos: walkedRepos{ + urlToPath: make(map[string]string), + keyToPath: map[string]string{}, + }, + config: &Config{ + expectedExemptions: []config.CheckedExpectedExemption{ + { + Name: "branches.preventDeletion", + RefMatcher: "some-ref-matcher", + Exemptions: []string{"missing-exemption-one", "some-exemption"}, + }, + { + Name: "some-name", + RefMatcher: "some-ref-matcher", + Exemptions: []string{"missing-exemption-one", "some-exemption"}, + }, + }, + }, + }, + args: args{ + path: "owners/some-owner/repositories/repository.helm-deployment.yaml", + contents: `url: existing-repo-url +mainline: master +configuration: + requireConditions: + other-condition: + exemptions: + - some-exemption + refProtections: + branches: + requirePR: + - pattern: "some-ref-matcher" + exemptions: + - some-exemption +`, + }, + want: want{ + result: []*github.CheckRunAnnotation{ + { + Path: github.Ptr("owners/some-owner/repositories/repository.helm-deployment.yaml"), + StartLine: github.Ptr(1), + EndLine: github.Ptr(1), + AnnotationLevel: github.Ptr("warning"), + Message: github.Ptr("This file does not contain the required condition/refProtection branches.preventDeletion with the refMatcher some-ref-matcher."), + Title: github.Ptr("missing expected condition/refProtection"), + }, { + Path: github.Ptr("owners/some-owner/repositories/repository.helm-deployment.yaml"), + StartLine: github.Ptr(1), + EndLine: github.Ptr(1), + AnnotationLevel: github.Ptr("warning"), + Message: github.Ptr("This file does not contain the required condition/refProtection some-name with the refMatcher some-ref-matcher."), + Title: github.Ptr("missing expected condition/refProtection"), + }, + }, + ignored: make(map[string]string), + errors: make(map[string]error), + }, + }, + { + name: "valid repo checkRequiredConditions", + mock: mock{ + walkedRepos: walkedRepos{ + urlToPath: make(map[string]string), + keyToPath: map[string]string{}, + }, + config: &Config{ + expectedExemptions: []config.CheckedExpectedExemption{ + { + Name: "branches.preventDeletion", + RefMatcher: "some-ref-matcher", + Exemptions: []string{"some-exemption"}, + }, + { + Name: "some-name", + RefMatcher: "some-ref-matcher", + Exemptions: []string{"some-exemption"}, + }, + }, + }, + }, + args: args{ + path: "owners/some-owner/repositories/repository.helm-deployment.yaml", + contents: `url: existing-repo-url +mainline: master +configuration: + requireConditions: + some-name: + refMatcher: "some-ref-matcher" + exemptions: + - some-exemption + refProtections: + branches: + preventDeletion: + - pattern: "some-ref-matcher" + exemptions: + - some-exemption +`, + }, + want: want{ + result: nil, + ignored: make(map[string]string), + errors: make(map[string]error), + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { v := MetadataYamlFileWalker(nil) if hasMock(tt.mock) { v.walkedRepos = tt.mock.walkedRepos + if tt.mock.config != nil { + v.config = *tt.mock.config + } } if got := v.validateSingleYamlFile(tt.args.path, tt.args.contents); !reflect.DeepEqual(got, tt.want.result) { t.Errorf("validateSingleYamlFile() = %v, want %v", printAnnotations(got), printAnnotations(tt.want.result)) diff --git a/test/mock/configmock/configmock.go b/test/mock/configmock/configmock.go index 95a957f..fad80ea 100644 --- a/test/mock/configmock/configmock.go +++ b/test/mock/configmock/configmock.go @@ -303,6 +303,11 @@ func (c *MockConfig) FormattingActionCommitMsgPrefix() string { func (c *MockConfig) CheckWarnMissingMainlineProtection() bool { return false } + func (c *MockConfig) CheckExpectedRequiredConditions() []config.CheckedRequiredConditions { return make([]config.CheckedRequiredConditions, 0) } + +func (c *MockConfig) CheckedExpectedExemptions() []config.CheckedExpectedExemption { + return make([]config.CheckedExpectedExemption, 0) +} diff --git a/test/mock/githubmock/githubmock.go b/test/mock/githubmock/githubmock.go index 635cbee..587ae51 100644 --- a/test/mock/githubmock/githubmock.go +++ b/test/mock/githubmock/githubmock.go @@ -12,10 +12,17 @@ func (this *GitHubMock) StartCheckRun(ctx context.Context, owner, repoName, chec return 0, nil } -func (this *GitHubMock) ConcludeCheckRun(ctx context.Context, owner, repoName, checkName string, checkRunId int64, conclusion repository.CheckRunConclusion, details github.CheckRunOutput) error { +func (this *GitHubMock) ConcludeCheckRun(ctx context.Context, owner, repoName, checkName string, checkRunId int64, conclusion repository.CheckRunConclusion, details github.CheckRunOutput, actions ...*github.CheckRunAction) error { return nil } +func (this *GitHubMock) GetUser(ctx context.Context, username string) (*github.User, error) { + return &github.User{ + Email: github.Ptr("some-email"), + Name: github.Ptr("some-name"), + }, nil +} + func (this *GitHubMock) CreateInstallationToken(ctx context.Context, installationId int64) (*github.InstallationToken, *github.Response, error) { return &github.InstallationToken{}, nil, nil } diff --git a/test/resources/recordings/github/request_patch_repos-er-metadata-check-runs-123456_241554d6.json b/test/resources/recordings/github/request_patch_repos-er-metadata-check-runs-123456_9b798edf.json similarity index 100% rename from test/resources/recordings/github/request_patch_repos-er-metadata-check-runs-123456_241554d6.json rename to test/resources/recordings/github/request_patch_repos-er-metadata-check-runs-123456_9b798edf.json