Skip to content

Commit

Permalink
fix plugins' config propagation of trigger plugin
Browse files Browse the repository at this point in the history
  • Loading branch information
Osher De Paz committed Feb 14, 2022
1 parent b27b363 commit 140be8e
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 2 deletions.
15 changes: 13 additions & 2 deletions prow/plugins/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -1875,7 +1875,7 @@ type Override struct {

func (c *Configuration) mergeFrom(other *Configuration) error {
var errs []error
if diff := cmp.Diff(other, &Configuration{Approve: other.Approve, Bugzilla: other.Bugzilla, ExternalPlugins: other.ExternalPlugins, Label: Label{RestrictedLabels: other.Label.RestrictedLabels}, Lgtm: other.Lgtm, Plugins: other.Plugins}); diff != "" {
if diff := cmp.Diff(other, &Configuration{Approve: other.Approve, Bugzilla: other.Bugzilla, ExternalPlugins: other.ExternalPlugins, Label: Label{RestrictedLabels: other.Label.RestrictedLabels}, Lgtm: other.Lgtm, Plugins: other.Plugins, Triggers: other.Triggers}); diff != "" {
errs = append(errs, fmt.Errorf("supplemental plugin configuration has config that doesn't support merging: %s", diff))
}

Expand All @@ -1892,6 +1892,7 @@ func (c *Configuration) mergeFrom(other *Configuration) error {

c.Approve = append(c.Approve, other.Approve...)
c.Lgtm = append(c.Lgtm, other.Lgtm...)
c.Triggers = append(c.Triggers, other.Triggers...)

if err := c.mergeExternalPluginsFrom(other.ExternalPlugins); err != nil {
errs = append(errs, fmt.Errorf("failed to merge .external-plugins from supplemental config: %w", err))
Expand Down Expand Up @@ -2020,7 +2021,7 @@ func getLabelConfigFromRestrictedLabelsSlice(s []RestrictedLabel, label string)
}

func (c *Configuration) HasConfigFor() (global bool, orgs sets.String, repos sets.String) {
if !reflect.DeepEqual(c, &Configuration{Approve: c.Approve, Bugzilla: c.Bugzilla, ExternalPlugins: c.ExternalPlugins, Label: Label{RestrictedLabels: c.Label.RestrictedLabels}, Lgtm: c.Lgtm, Plugins: c.Plugins}) || c.Bugzilla.Default != nil {
if !reflect.DeepEqual(c, &Configuration{Approve: c.Approve, Bugzilla: c.Bugzilla, ExternalPlugins: c.ExternalPlugins, Label: Label{RestrictedLabels: c.Label.RestrictedLabels}, Lgtm: c.Lgtm, Plugins: c.Plugins, Triggers: c.Triggers}) || c.Bugzilla.Default != nil {
global = true
}
orgs = sets.String{}
Expand Down Expand Up @@ -2075,6 +2076,16 @@ func (c *Configuration) HasConfigFor() (global bool, orgs sets.String, repos set
}
}

for _, trigger := range c.Triggers {
for _, orgOrRepo := range trigger.Repos {
if strings.Contains(orgOrRepo, "/") {
repos.Insert(orgOrRepo)
} else {
orgs.Insert(orgOrRepo)
}
}
}

for orgOrRepo := range c.ExternalPlugins {
if strings.Contains(orgOrRepo, "/") {
repos.Insert(orgOrRepo)
Expand Down
29 changes: 29 additions & 0 deletions prow/plugins/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1998,6 +1998,7 @@ func TestHasConfigFor(t *testing.T) {
fuzzedConfig.Approve = nil
fuzzedConfig.Label.RestrictedLabels = nil
fuzzedConfig.Lgtm = nil
fuzzedConfig.Triggers = nil
fuzzedConfig.ExternalPlugins = nil
return fuzzedConfig, !reflect.DeepEqual(fuzzedConfig, &Configuration{}), nil, nil
},
Expand Down Expand Up @@ -2073,6 +2074,25 @@ func TestHasConfigFor(t *testing.T) {
return fuzzedConfig, false, expectOrgs, expectRepos
},
},
{
name: "Any config with triggers is considered to be for the orgs and repos references there",
resultGenerator: func(fuzzedConfig *Configuration) (toCheck *Configuration, expectGlobal bool, expectOrgs sets.String, expectRepos sets.String) {
fuzzedConfig = &Configuration{Triggers: fuzzedConfig.Triggers}
expectOrgs, expectRepos = sets.String{}, sets.String{}

for _, trigger := range fuzzedConfig.Triggers {
for _, orgOrRepo := range trigger.Repos {
if strings.Contains(orgOrRepo, "/") {
expectRepos.Insert(orgOrRepo)
} else {
expectOrgs.Insert(orgOrRepo)
}
}
}

return fuzzedConfig, false, expectOrgs, expectRepos
},
},
{
name: "Any config with external-plugins is considered to be for the orgs and repos references there",
resultGenerator: func(fuzzedConfig *Configuration) (toCheck *Configuration, expectGlobal bool, expectOrgs sets.String, expectRepos sets.String) {
Expand Down Expand Up @@ -2170,6 +2190,15 @@ func TestMergeFrom(t *testing.T) {
{Repos: []string{"foo/baz"}},
}},
},
{
name: "Triggers config gets merged",
in: Configuration{Triggers: []Trigger{{Repos: []string{"foo/bar"}}}},
supplementalConfigs: []Configuration{{Triggers: []Trigger{{Repos: []string{"foo/baz"}}}}},
expected: Configuration{Triggers: []Trigger{
{Repos: []string{"foo/bar"}},
{Repos: []string{"foo/baz"}},
}},
},
{
name: "ExternalPlugins get merged",
in: Configuration{
Expand Down

0 comments on commit 140be8e

Please sign in to comment.