Crier/Slack: Allow to overwrite the channel on the job

prow/ANNOUNCEMENTS.md

@@ -1,6 +1,8 @@
 # Announcements
 
 New features added to each component:
+ - *June 17, 2019* It is now possible to configure the channel for the Slack reporter
+   directly on jobs via the `.reporter_config.slack.channel` config option
  - *May 13, 2019* New `plank` config `pod_running_timeout` is added and
    defaulted to two days to allow plank abort pods stuck in running state.
  - *April 25, 2019* `--job-config` in `peribolos` has never been used; it is

prow/apis/prowjobs/v1/types.go

@@ -158,6 +158,17 @@ type ProwJobSpec struct {
 	// DecorationConfig holds configuration options for
 	// decorating PodSpecs that users provide
 	DecorationConfig *DecorationConfig `json:"decoration_config,omitempty"`
+
+	// ReporterConfig holds reporter-specific configuration
+	ReporterConfig *ReporterConfig `json:"reporter_config,omitempty"`
+}
+
+type ReporterConfig struct {
+	Slack *SlackReporterConfig `json:"slack,omitempty"`
+}
+
+type SlackReporterConfig struct {
+	Channel string `json:"channel"`
 }
 
 // Duration is a wrapper around time.Duration that parses times in either

prow/config/config.go

@@ -373,7 +373,8 @@ type GitHubOptions struct {
 	LinkURL *url.URL
 }
 
-// SlackReporter represents the config for the Slack reporter
+// SlackReporter represents the config for the Slack reporter. The channel can be overridden
+// on the job via the .reporter_config.slack.channel property
 type SlackReporter struct {
 	JobTypesToReport  []prowapi.ProwJobType  `json:"job_types_to_report"`
 	JobStatesToReport []prowapi.ProwJobState `json:"job_states_to_report"`

prow/config/jobs.go

@@ -109,6 +109,8 @@ type JobBase struct {
 	PipelineRunSpec *pipelinev1alpha1.PipelineRunSpec `json:"pipeline_run_spec,omitempty"`
 	// Annotations are unused by prow itself, but provide a space to configure other automation.
 	Annotations map[string]string `json:"annotations,omitempty"`
+	// ReporterConfig provides the option to configure reporting on job level
+	ReporterConfig *prowapi.ReporterConfig `json:"reporter_config,omitempty"`
 
 	UtilityConfig
 }

prow/pjutil/pjutil.go

@@ -175,6 +175,8 @@ func specFromJobBase(jb config.JobBase) prowapi.ProwJobSpec {
 		PodSpec:         jb.Spec,
 		BuildSpec:       jb.BuildSpec,
 		PipelineRunSpec: jb.PipelineRunSpec,
+
+		ReporterConfig: jb.ReporterConfig,
 	}
 }
 

prow/pjutil/pjutil_test.go

@@ -17,6 +17,8 @@ limitations under the License.
 package pjutil
 
 import (
+	"errors"
+	"fmt"
 	"reflect"
 	"testing"
 	"text/template"
@@ -700,3 +702,44 @@ func TestCreateRefs(t *testing.T) {
 		t.Errorf("diff between expected and actual refs:%s", diff.ObjectReflectDiff(expected, actual))
 	}
 }
+
+func TestSpecFromJobBase(t *testing.T) {
+	testCases := []struct {
+		name    string
+		jobBase config.JobBase
+		verify  func(prowapi.ProwJobSpec) error
+	}{
+		{
+			name: "Verify reporter config gets copied",
+			jobBase: config.JobBase{
+				ReporterConfig: &prowapi.ReporterConfig{
+					Slack: &prowapi.SlackReporterConfig{
+						Channel: "my-channel",
+					},
+				},
+			},
+			verify: func(pj prowapi.ProwJobSpec) error {
+				if pj.ReporterConfig == nil {
+					return errors.New("Expected ReporterConfig to be non-nil")
+				}
+				if pj.ReporterConfig.Slack == nil {
+					return errors.New("Expected ReporterConfig.Slack to be non-nil")
+				}
+				if pj.ReporterConfig.Slack.Channel != "my-channel" {
+					return fmt.Errorf("Expected pj.ReporterConfig.Slack.Channel to be \"my-channel\", was %q",
+						pj.ReporterConfig.Slack.Channel)
+				}
+				return nil
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			pj := specFromJobBase(tc.jobBase)
+			if err := tc.verify(pj); err != nil {
+				t.Fatalf("Verification failed: %v", err)
+			}
+		})
+	}
+}

prow/slack/reporter/reporter.go

@@ -38,8 +38,16 @@ type slackReporter struct {
 	dryRun bool
 }
 
+func channel(cfg *config.SlackReporter, pj *v1.ProwJob) string {
+	if pj.Spec.ReporterConfig != nil && pj.Spec.ReporterConfig.Slack != nil && pj.Spec.ReporterConfig.Slack.Channel != "" {
+		return pj.Spec.ReporterConfig.Slack.Channel
+	}
+	return cfg.Channel
+}
+
 func (sr *slackReporter) Report(pj *v1.ProwJob) ([]*v1.ProwJob, error) {
 	config := sr.config()
+	channel := channel(config, pj)
 	b := &bytes.Buffer{}
 	tmpl, err := template.New("").Parse(config.ReportTemplate)
 	if err != nil {
@@ -57,7 +65,7 @@ func (sr *slackReporter) Report(pj *v1.ProwJob) ([]*v1.ProwJob, error) {
 			Debug("Skipping reporting because dry-run is enabled")
 		return []*v1.ProwJob{pj}, nil
 	}
-	if err := sr.client.WriteMessage(b.String(), config.Channel); err != nil {
+	if err := sr.client.WriteMessage(b.String(), channel); err != nil {
 		sr.logger.WithError(err).Error("failed to write Slack message")
 		return nil, fmt.Errorf("failed to write Slack message: %v", err)
 	}

prow/slack/reporter/reporter_test.go

@@ -145,3 +145,45 @@ func TestReloadsConfig(t *testing.T) {
 		t.Error("Did expect shouldReport to be true after config change")
 	}
 }
+
+func TestUsesChannelOverrideFromJob(t *testing.T) {
+	testCases := []struct {
+		name     string
+		cfg      *config.SlackReporter
+		pj       *v1.ProwJob
+		expected string
+	}{
+		{
+			name: "No job-level config, use global default",
+			cfg: &config.SlackReporter{
+				Channel: "global-default",
+			},
+			pj:       &v1.ProwJob{},
+			expected: "global-default",
+		},
+		{
+			name: "Job-level config present, use it",
+			cfg: &config.SlackReporter{
+				Channel: "global-default",
+			},
+			pj: &v1.ProwJob{
+				Spec: v1.ProwJobSpec{
+					ReporterConfig: &v1.ReporterConfig{
+						Slack: &v1.SlackReporterConfig{
+							Channel: "team-a",
+						},
+					},
+				},
+			},
+			expected: "team-a",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			if result := channel(tc.cfg, tc.pj); result != tc.expected {
+				t.Fatalf("Expected result to be %q, was %q", tc.expected, result)
+			}
+		})
+	}
+}