Repository: fluxcd/notification-controller
Pull request #27 (2020-07-29 20:23:46)

Implement GitHub commit status notifier
Implements #26 

phillebaba: This is the resulting commit status of a successful deployment. 

![image](https://user-images.githubusercontent.com/10550243/88906355-a4f92300-d257-11ea-8fae-8dc290eff16b.png)

stefanprodan: @phillebaba can we set the icon to the [FluxCD one](https://github.com/fluxcd/toolkit/blob/master/docs/_files/flux-icon%402x.png) the public URL is `https://raw.githubusercontent.com/fluxcd/toolkit/master/docs/_files/flux-icon%402x.png`.
phillebaba: @stefanprodan It seems like it is not possible to change the icon when using a PAT to authenticate. Github will fetch the avatar of the user who owns the PAT and use that to authenticate. After reading a bit, it seems like it is only possible to use custom avatars through Github applications. In that case each end user would have to create their own application instead.
phillebaba: We still need to figure out the context so that it is not overriden every time. If you look att the e2e test the context is "e2e / kind (pull_request)" without a message. This allows Github Actions to post the status from multiple actions on a single sha. 

In my mind, for each Kustomization and HelmRelease has to events, that reconcile and the health check. Either we merge these two together or they become a separate event. Either way each event needs its own unique context. My suggestion is something similar to "cd/flux/{{ .Kind }}/{{ .Name }}" maybe even remove the "cd/flux" prefix.
stefanprodan: @phillebaba let's go with `{{ .Kind }}/{{ .Name }}` and remove cd/flux as the label shouldn't be too long.
stefanprodan: @phillebaba please rebase with master and reinstall the toolkit, to clean your cluster do `tk uninstall --crds`. The reason should work now with the latest toolkit release.
phillebaba: I will try to get it done later today. The CRD changes should not affect this functionality too much so it should be a quick job.
stefanprodan: @phillebaba can you please post here a print screen with new format?
phillebaba: I added the namespace of the resource as part of the context, and as you can see the text it overflowing.

![image](https://user-images.githubusercontent.com/10550243/89518742-a2f50e00-d7db-11ea-9ff7-b99f07bab321.png)

Its sucks that there is no way of getting a detailed view of the status, but its fine as I use the information through API calls anyways.

stefanprodan: I don't think we should add the namespace, all kustomization work if they are in the same namespace as the control plane.
phillebaba: Ok so this would only be an issue for user using the same repo for multiple control planes.

Additionally this will be an issue for remote ref of Kustomization resources. I created my setup according to the tutorial, so I have a fleet-infra repo, that contains Kustomization resources for a podinfo app. These resource are deployed in the cluster.
```
apiVersion: kustomize.toolkit.fluxcd.io/v1alpha1
kind: Kustomization
metadata:
  creationTimestamp: null
  name: webapp-backend
  namespace: gitops-system
spec:
  dependsOn:
  - webapp-common
  healthChecks:
  - kind: Deployment
    name: backend
    namespace: webapp
  interval: 10m0s
  path: ./deploy/webapp/backend
  prune: true
  sourceRef:
    kind: GitRepository
    name: webapp
  timeout: 2m0s
  validation: client
status: {}
---
apiVersion: source.toolkit.fluxcd.io/v1alpha1
kind: GitRepository
metadata:
  creationTimestamp: null
  name: webapp
  namespace: gitops-system
spec:
  interval: 30s
  ref:
    branch: master
  url: https://github.com/stefanprodan/podinfo
status: {}
```

The event received will contain the commit id for the `podinfo` repo and not the `fleet-infra` repo. In my case this is `d3865f924736fe5d4c52bb9f8db86e46cfd1fe52` [which is this commit](https://github.com/stefanprodan/podinfo/commit/d3865f924736fe5d4c52bb9f8db86e46cfd1fe52). If we follow this notification behavior multiple instances would update the same commit sha. 

Maybe we should remove the namespace and add a instance field (dev, qa, prod)? `{{ .Instance }}/{{ .Kind }}/{{ .Name }}` to differentiate between toolkit instances.

stefanprodan: To differentiate between toolkit instances users could use a prefix/suffix for Kustomization naming, e.g. `webapp-prod`, `webapp-staging`. Should we change the get started guide and use such names?
phillebaba: I guess all of this depends on the multi tenancy discussion.
https://github.com/fluxcd/toolkit/discussions/93

If the guidelines would be to prefix/suffix with environment name even when you have separate clusters for each environment, then I am fine with it. Then I would just assume uniqueness in the name in my implementation, so that the controller does not override statuses.

I am still a bit confused with the metadata though. My understanding, which turns out is wrong, was that the event was given the commit id for the origin of the `HelmRelease` or `Kustomization`. It turns out it the commit id refers to where the Kustomization points towards.
![Untitled Diagram(1)](https://user-images.githubusercontent.com/10550243/89529917-e0fb2d80-d7ed-11ea-8656-5643080e7a6a.png)

I want to get the CommitID 1 to the notification controller and set the status in that repo, which is not happening now. This was my original idea for how it would work, do you agree with this?
stefanprodan: The feel-infra repo contains GitRepositories and Kustomizations. When you create an alert it should not be for the fleet-infra but for the repository that's referenced in GitRepository. The only exception to this is the `gitops-system` repo+kustomization that synchronises all the repos and kustomizations.
phillebaba: Ok i guees i didnt understand the future workflow. It is a bit different from current flux which is more single repo centered. I am currently on vaccation but will look into this later when i get back.

Patch:
diff --git a/api/v1alpha1/provider_types.go b/api/v1alpha1/provider_types.go
index 32e9b3c..6e08c3d 100644
--- a/api/v1alpha1/provider_types.go
+++ b/api/v1alpha1/provider_types.go
@@ -24,7 +24,7 @@ import (
 // ProviderSpec defines the desired state of Provider
 type ProviderSpec struct {
 	// Type of provider
-	// +kubebuilder:validation:Enum=slack;discord;msteams;rocket;generic
+	// +kubebuilder:validation:Enum=slack;discord;msteams;rocket;generic;github
 	// +required
 	Type string `json:"type"`
 
@@ -51,6 +51,7 @@ const (
 	DiscordProvider string = "discord"
 	MSTeamsProvider string = "msteams"
 	RocketProvider  string = "rocket"
+	GitHubProvider  string = "github"
 )
 
 // ProviderStatus defines the observed state of Provider
diff --git a/config/crd/bases/notification.toolkit.fluxcd.io_providers.yaml b/config/crd/bases/notification.toolkit.fluxcd.io_providers.yaml
index dceb8d0..c523d23 100644
--- a/config/crd/bases/notification.toolkit.fluxcd.io_providers.yaml
+++ b/config/crd/bases/notification.toolkit.fluxcd.io_providers.yaml
@@ -68,6 +68,7 @@ spec:
                 - msteams
                 - rocket
                 - generic
+                - github
                 type: string
               username:
                 description: Bot username for this provider
diff --git a/docs/spec/v1alpha1/provider.md b/docs/spec/v1alpha1/provider.md
index 02db8be..cd860e6 100644
--- a/docs/spec/v1alpha1/provider.md
+++ b/docs/spec/v1alpha1/provider.md
@@ -37,6 +37,7 @@ Notification providers:
 * Discord
 * Microsoft Teams
 * Rocket
+* GitHub
 * Generic webhook
 
 Status:
@@ -86,7 +87,7 @@ kubectl -n gitops-system create secret generic webhook-url \
 
 Note that the secret must contain an `address` field.
 
-The provider type can be: `slack`, `msteams`, `rocket`, `discord` or `generic`. 
+The provider type can be: `slack`, `msteams`, `rocket`, `discord`, `github` or `generic`.
 
 When type `generic` is specified, the notification controller will post the
-incoming [event](event.md) in JSON format to the webhook address. 
+incoming [event](event.md) in JSON format to the webhook address.
diff --git a/go.mod b/go.mod
index 3edf0fd..9e8d7ef 100644
--- a/go.mod
+++ b/go.mod
@@ -12,6 +12,7 @@ require (
 	github.com/onsi/ginkgo v1.12.1
 	github.com/onsi/gomega v1.10.1
 	github.com/stretchr/testify v1.6.1
+	golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45
 	k8s.io/api v0.18.8
 	k8s.io/apimachinery v0.18.8
 	k8s.io/client-go v0.18.8
diff --git a/internal/notifier/camelcase.go b/internal/notifier/camelcase.go
new file mode 100644
index 0000000..e396c77
--- /dev/null
+++ b/internal/notifier/camelcase.go
@@ -0,0 +1,51 @@
+package notifier
+
+import (
+	"unicode"
+	"unicode/utf8"
+)
+
+func split(src string) (entries []string) {
+	// don't split invalid utf8
+	if !utf8.ValidString(src) {
+		return []string{src}
+	}
+	entries = []string{}
+	var runes [][]rune
+	lastClass := 0
+	class := 0
+	// split into fields based on class of unicode character
+	for _, r := range src {
+		switch true {
+		case unicode.IsLower(r):
+			class = 1
+		case unicode.IsUpper(r):
+			class = 2
+		case unicode.IsDigit(r):
+			class = 3
+		default:
+			class = 4
+		}
+		if class == lastClass {
+			runes[len(runes)-1] = append(runes[len(runes)-1], r)
+		} else {
+			runes = append(runes, []rune{r})
+		}
+		lastClass = class
+	}
+	// handle upper case -> lower case sequences, e.g.
+	// "PDFL", "oader" -> "PDF", "Loader"
+	for i := 0; i < len(runes)-1; i++ {
+		if unicode.IsUpper(runes[i][0]) && unicode.IsLower(runes[i+1][0]) {
+			runes[i+1] = append([]rune{runes[i][len(runes[i])-1]}, runes[i+1]...)
+			runes[i] = runes[i][:len(runes[i])-1]
+		}
+	}
+	// construct []string from results
+	for _, s := range runes {
+		if len(s) > 0 {
+			entries = append(entries, string(s))
+		}
+	}
+	return
+}
diff --git a/internal/notifier/factory.go b/internal/notifier/factory.go
index 06e4e6c..8d059a5 100644
--- a/internal/notifier/factory.go
+++ b/internal/notifier/factory.go
@@ -26,13 +26,15 @@ type Factory struct {
 	URL      string
 	Username string
 	Channel  string
+	Token    string
 }
 
-func NewFactory(url string, username string, channel string) *Factory {
+func NewFactory(url string, username string, channel string, token string) *Factory {
 	return &Factory{
 		URL:      url,
 		Channel:  channel,
 		Username: username,
+		Token:    token,
 	}
 }
 
@@ -54,6 +56,8 @@ func (f Factory) Notifier(provider string) (Interface, error) {
 		n, err = NewRocket(f.URL, f.Username, f.Channel)
 	case v1alpha1.MSTeamsProvider:
 		n, err = NewMSTeams(f.URL)
+	case v1alpha1.GitHubProvider:
+		n, err = NewGitHub(f.URL, f.Token)
 	default:
 		err = fmt.Errorf("provider %s not supported", provider)
 	}
diff --git a/internal/notifier/github.go b/internal/notifier/github.go
new file mode 100644
index 0000000..eeabed6
--- /dev/null
+++ b/internal/notifier/github.go
@@ -0,0 +1,123 @@
+/*
+Copyright 2020 The Flux CD contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package notifier
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"net/url"
+	"strings"
+	"time"
+
+	"github.com/fluxcd/pkg/recorder"
+	"github.com/google/go-github/v32/github"
+	"golang.org/x/oauth2"
+)
+
+type GitHub struct {
+	Owner  string
+	Repo   string
+	Client *github.Client
+}
+
+func NewGitHub(addr string, token string) (*GitHub, error) {
+	owner, repo, err := parseGithubAddress(addr)
+	if err != nil {
+		return nil, err
+	}
+
+	ctx := context.Background()
+	ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token})
+	tc := oauth2.NewClient(ctx, ts)
+	client := github.NewClient(tc)
+
+	return &GitHub{
+		Owner:  owner,
+		Repo:   repo,
+		Client: client,
+	}, nil
+}
+
+// Post Github commit status
+func (g *GitHub) Post(event recorder.Event) error {
+	revString, ok := event.Metadata["revision"]
+	if !ok {
+		return errors.New("Missing revision metadata")
+	}
+	rev, err := parseRevision(revString)
+	if err != nil {
+		return err
+	}
+	state, err := toGitHubState(event.Severity)
+	if err != nil {
+		return err
+	}
+
+	githubCtx := fmt.Sprintf("%v/%v", event.InvolvedObject.Kind, event.InvolvedObject.Name)
+	githubCtx = strings.ToLower(githubCtx)
+	desc := strings.Join(split(event.Reason), " ")
+	desc = strings.ToLower(desc)
+	status := &github.RepoStatus{
+		State:       &state,
+		Context:     &githubCtx,
+		Description: &desc,
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+	defer cancel()
+	_, _, err = g.Client.Repositories.CreateStatus(ctx, g.Owner, g.Repo, rev, status)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func parseGithubAddress(addr string) (string, string, error) {
+	u, err := url.Parse(addr)
+	if err != nil {
+		return "", "", nil
+	}
+
+	comp := strings.Split(u.Path, "/")
+	if len(comp) < 3 {
+		return "", "", fmt.Errorf("Not enough components in path %v", u.Path)
+	}
+
+	return comp[1], comp[2], nil
+}
+
+func parseRevision(rev string) (string, error) {
+	comp := strings.Split(rev, "/")
+	if len(comp) < 2 {
+		return "", fmt.Errorf("Revision string format incorrect: %v", rev)
+	}
+
+	return comp[1], nil
+}
+
+func toGitHubState(severity string) (string, error) {
+	switch severity {
+	case "info":
+		return "success", nil
+	case "error":
+		return "failure", nil
+	default:
+		return "", errors.New("Can't convert to GitHub state")
+	}
+}
diff --git a/internal/server/event_handlers.go b/internal/server/event_handlers.go
index 0f47412..5990502 100644
--- a/internal/server/event_handlers.go
+++ b/internal/server/event_handlers.go
@@ -109,6 +109,7 @@ func (s *EventServer) handleEvent() func(w http.ResponseWriter, r *http.Request)
 			}
 
 			webhook := provider.Spec.Address
+			token := ""
 			if provider.Spec.SecretRef != nil {
 				var secret corev1.Secret
 				secretName := types.NamespacedName{Namespace: alert.Namespace, Name: provider.Spec.SecretRef.Name}
@@ -124,12 +125,10 @@ func (s *EventServer) handleEvent() func(w http.ResponseWriter, r *http.Request)
 
 				if address, ok := secret.Data["address"]; ok {
 					webhook = string(address)
-				} else {
-					s.logger.Error(err, "secret does not contain an address",
-						"provider", providerName,
-						"secret", secretName.Name)
-					w.WriteHeader(http.StatusBadRequest)
-					return
+				}
+
+				if t, ok := secret.Data["token"]; ok {
+					token = string(t)
 				}
 			}
 
@@ -140,7 +139,7 @@ func (s *EventServer) handleEvent() func(w http.ResponseWriter, r *http.Request)
 				return
 			}
 
-			factory := notifier.NewFactory(webhook, provider.Spec.Username, provider.Spec.Channel)
+			factory := notifier.NewFactory(webhook, provider.Spec.Username, provider.Spec.Channel, token)
 			sender, err := factory.Notifier(provider.Spec.Type)
 			if err != nil {
 				s.logger.Error(err, "failed to initialise provider",


Test patch:
diff --git a/internal/notifier/github_test.go b/internal/notifier/github_test.go
new file mode 100644
index 0000000..cf43151
--- /dev/null
+++ b/internal/notifier/github_test.go
@@ -0,0 +1,31 @@
+/*
+Copyright 2020 The Flux CD contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package notifier
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestGithub_ParseAddress(t *testing.T) {
+	addr := "https://github.com/stefanprodan/podinfo"
+	owner, repo, err := parseGithubAddress(addr)
+	require.NoError(t, err)
+	require.Equal(t, "stefanprodan", owner)
+	require.Equal(t, "podinfo", repo)
+}