Repository: fluxcd/notification-controller
Pull request #82 (2020-11-17 12:51:11)

Verify repository id in git notifiers
The different git notifiers need repository ids in different formats.
On top of that some git providers support repositories in subgroups
while others do not support nesting. Each notifier needs test to verify
that they support their specific functionality.


Patch:
diff --git a/internal/notifier/bitbucket.go b/internal/notifier/bitbucket.go
index c368e67..e0123c8 100644
--- a/internal/notifier/bitbucket.go
+++ b/internal/notifier/bitbucket.go
@@ -18,6 +18,7 @@ package notifier
 
 import (
 	"errors"
+	"fmt"
 	"strings"
 
 	"github.com/fluxcd/pkg/recorder"
@@ -32,7 +33,7 @@ type Bitbucket struct {
 
 func NewBitbucket(addr string, token string) (*Bitbucket, error) {
 	if len(token) == 0 {
-		return nil, errors.New("Bitbucket token cannot be empty")
+		return nil, errors.New("bitbucket token cannot be empty")
 	}
 
 	_, id, err := parseGitAddress(addr)
@@ -42,14 +43,14 @@ func NewBitbucket(addr string, token string) (*Bitbucket, error) {
 
 	comp := strings.Split(token, ":")
 	if len(comp) != 2 {
-		return nil, errors.New("Invalid token format, expected to be <user>:<password>")
+		return nil, errors.New("invalid token format, expected to be <user>:<password>")
 	}
 	username := comp[0]
 	password := comp[1]
 
 	comp = strings.Split(id, "/")
 	if len(comp) != 2 {
-		return nil, errors.New("Invalid bitbucket repository id")
+		return nil, fmt.Errorf("invalid repository id %q", id)
 	}
 	owner := comp[0]
 	repo := comp[1]
@@ -70,7 +71,7 @@ func (b Bitbucket) Post(event recorder.Event) error {
 
 	revString, ok := event.Metadata["revision"]
 	if !ok {
-		return errors.New("Missing revision metadata")
+		return errors.New("missing revision metadata")
 	}
 	rev, err := parseRevision(revString)
 	if err != nil {
@@ -110,6 +111,6 @@ func toBitbucketState(severity string) (string, error) {
 	case recorder.EventSeverityError:
 		return "FAILED", nil
 	default:
-		return "", errors.New("Can't convert to GitHub state")
+		return "", errors.New("can't convert to bitbucket state")
 	}
 }
diff --git a/internal/notifier/github.go b/internal/notifier/github.go
index a37afa1..f5755cc 100644
--- a/internal/notifier/github.go
+++ b/internal/notifier/github.go
@@ -19,6 +19,7 @@ package notifier
 import (
 	"context"
 	"errors"
+	"fmt"
 	"strings"
 	"time"
 
@@ -35,7 +36,7 @@ type GitHub struct {
 
 func NewGitHub(addr string, token string) (*GitHub, error) {
 	if len(token) == 0 {
-		return nil, errors.New("GitHub token  cannot be empty")
+		return nil, errors.New("github token cannot be empty")
 	}
 
 	_, id, err := parseGitAddress(addr)
@@ -43,12 +44,16 @@ func NewGitHub(addr string, token string) (*GitHub, error) {
 		return nil, err
 	}
 
+	comp := strings.Split(id, "/")
+	if len(comp) != 2 {
+		return nil, fmt.Errorf("invalid repository id %q", id)
+	}
+
 	ctx := context.Background()
 	ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token})
 	tc := oauth2.NewClient(ctx, ts)
 	client := github.NewClient(tc)
 
-	comp := strings.Split(id, "/")
 	return &GitHub{
 		Owner:  comp[0],
 		Repo:   comp[1],
@@ -65,7 +70,7 @@ func (g *GitHub) Post(event recorder.Event) error {
 
 	revString, ok := event.Metadata["revision"]
 	if !ok {
-		return errors.New("Missing revision metadata")
+		return errors.New("missing revision metadata")
 	}
 	rev, err := parseRevision(revString)
 	if err != nil {
@@ -100,6 +105,6 @@ func toGitHubState(severity string) (string, error) {
 	case recorder.EventSeverityError:
 		return "failure", nil
 	default:
-		return "", errors.New("Can't convert to GitHub state")
+		return "", errors.New("can't convert to github state")
 	}
 }
diff --git a/internal/notifier/gitlab.go b/internal/notifier/gitlab.go
index f4d01be..ff0b04d 100644
--- a/internal/notifier/gitlab.go
+++ b/internal/notifier/gitlab.go
@@ -30,7 +30,7 @@ type GitLab struct {
 
 func NewGitLab(addr string, token string) (*GitLab, error) {
 	if len(token) == 0 {
-		return nil, errors.New("GitLab token  cannot be empty")
+		return nil, errors.New("gitlab token cannot be empty")
 	}
 
 	host, id, err := parseGitAddress(addr)
@@ -61,7 +61,7 @@ func (g *GitLab) Post(event recorder.Event) error {
 
 	revString, ok := event.Metadata["revision"]
 	if !ok {
-		return errors.New("Missing revision metadata")
+		return errors.New("missing revision metadata")
 	}
 	rev, err := parseRevision(revString)
 	if err != nil {
@@ -94,6 +94,6 @@ func toGitLabState(severity string) (gitlab.BuildStateValue, error) {
 	case recorder.EventSeverityError:
 		return gitlab.Failed, nil
 	default:
-		return "", errors.New("Can't convert to GitLab state")
+		return "", errors.New("can't convert to gitlab state")
 	}
 }


Test patch:
diff --git a/internal/notifier/bitbucket_test.go b/internal/notifier/bitbucket_test.go
index 74f44a1..6108516 100644
--- a/internal/notifier/bitbucket_test.go
+++ b/internal/notifier/bitbucket_test.go
@@ -24,7 +24,6 @@ import (
 
 func TestNewBitbucketBasic(t *testing.T) {
 	b, err := NewBitbucket("https://bitbucket.org/foo/bar", "foo:bar")
-
 	assert.Nil(t, err)
 	assert.Equal(t, b.Owner, "foo")
 	assert.Equal(t, b.Repo, "bar")
diff --git a/internal/notifier/github_test.go b/internal/notifier/github_test.go
new file mode 100644
index 0000000..cd9bc25
--- /dev/null
+++ b/internal/notifier/github_test.go
@@ -0,0 +1,40 @@
+/*
+Copyright 2020 The Flux authors
+
+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/assert"
+)
+
+func TestNewGitHubBasic(t *testing.T) {
+	g, err := NewGitHub("https://github.com/foo/bar", "foobar")
+	assert.Nil(t, err)
+	assert.Equal(t, g.Owner, "foo")
+	assert.Equal(t, g.Repo, "bar")
+}
+
+func TestNewGitHubInvalidUrl(t *testing.T) {
+	_, err := NewGitHub("https://github.com/foo/bar/baz", "foobar")
+	assert.NotNil(t, err)
+}
+
+func TestNewGitHubEmptyToken(t *testing.T) {
+	_, err := NewGitHub("https://github.com/foo/bar", "")
+	assert.NotNil(t, err)
+}
diff --git a/internal/notifier/gitlab_test.go b/internal/notifier/gitlab_test.go
new file mode 100644
index 0000000..24a58c7
--- /dev/null
+++ b/internal/notifier/gitlab_test.go
@@ -0,0 +1,47 @@
+/*
+Copyright 2020 The Flux authors
+
+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/assert"
+)
+
+func TestNewGitLabBasic(t *testing.T) {
+	g, err := NewGitLab("https://gitlab.com/foo/bar", "foobar")
+	assert.Nil(t, err)
+	assert.Equal(t, g.Id, "foo/bar")
+}
+
+func TestNewGitLabSubgroups(t *testing.T) {
+	g, err := NewGitLab("https://gitlab.com/foo/bar/baz", "foobar")
+	assert.Nil(t, err)
+	assert.Equal(t, g.Id, "foo/bar/baz")
+}
+
+func TestNewGitLabSelfHosted(t *testing.T) {
+	g, err := NewGitLab("https://example.com/foo/bar", "foo:bar")
+	assert.Nil(t, err)
+	assert.Equal(t, g.Id, "foo/bar")
+	assert.Equal(t, g.Client.BaseURL().Host, "example.com")
+}
+
+func TestNewGitLabEmptyToken(t *testing.T) {
+	_, err := NewGitLab("https://gitlab.com/foo/bar", "")
+	assert.NotNil(t, err)
+}