Merge pull request #320 from keel-hq/feature/296_issue

Feature/296 issue

internal/policy/semver.go

@@ -1,7 +1,9 @@
 package policy
 
 import (
+	"errors"
 	"fmt"
+	"strings"
 
 	"github.com/Masterminds/semver"
 )
@@ -9,6 +11,10 @@ import (
 // SemverPolicyType - policy type
 type SemverPolicyType int
 
+var (
+	ErrNoMajorMinorPatchElementsFound = errors.New("No Major.Minor.Patch elements found")
+)
+
 // available policies
 const (
 	SemverPolicyTypeNone SemverPolicyType = iota
@@ -60,6 +66,11 @@ func shouldUpdate(spt SemverPolicyType, current, new string) (bool, error) {
 		return true, nil
 	}
 
+	parts := strings.SplitN(new, ".", 3)
+	if len(parts) != 3 {
+		return false, ErrNoMajorMinorPatchElementsFound
+	}
+
 	currentVersion, err := semver.NewVersion(current)
 	if err != nil {
 		return false, fmt.Errorf("failed to parse current version: %s", err)

internal/policy/semver_test.go

@@ -176,6 +176,16 @@ func Test_shouldUpdate(t *testing.T) {
 			want:    false,
 			wantErr: false,
 		},
+		{
+			name: "number",
+			args: args{
+				current: "1.4.5",
+				new:     "3050",
+				spt:     SemverPolicyTypeAll,
+			},
+			want:    false,
+			wantErr: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

provider/kubernetes/kubernetes_test.go

@@ -726,8 +726,8 @@ func TestProcessEventBuildNumber(t *testing.T) {
 		t.Errorf("got error while processing event: %s", err)
 	}
 
-	if fp.updated.Containers()[0].Image != repo.Name+":"+repo.Tag {
+	if fp.updated != nil {
-		t.Errorf("expected to find a deployment with updated image but found: %s", fp.updated.Containers()[0].Image)
+		t.Errorf("didn't expect to get updated containers, bot got: %s", fp.updated.Identifier)
 	}
 }
 
@@ -757,7 +757,7 @@ func TestEventSent(t *testing.T) {
 					Spec: v1.PodSpec{
 						Containers: []v1.Container{
 							v1.Container{
-								Image: "gcr.io/v2-namespace/hello-world:10",
+								Image: "gcr.io/v2-namespace/hello-world:10.0.0",
 							},
 						},
 					},
@@ -779,7 +779,7 @@ func TestEventSent(t *testing.T) {
 
 	repo := types.Repository{
 		Name: "gcr.io/v2-namespace/hello-world",
-		Tag:  "11",
+		Tag:  "11.0.0",
 	}
 
 	event := &types.Event{Repository: repo}
@@ -792,8 +792,8 @@ func TestEventSent(t *testing.T) {
 		t.Errorf("expected to find a deployment with updated image but found: %s", fp.updated.Containers()[0].Image)
 	}
 
-	if fs.sentEvent.Message != "Successfully updated deployment xxxx/deployment-1 10->11 (gcr.io/v2-namespace/hello-world:11)" {
+	if fs.sentEvent.Message != "Successfully updated deployment xxxx/deployment-1 10.0.0->11.0.0 (gcr.io/v2-namespace/hello-world:11.0.0)" {
-		t.Errorf("expected 'Successfully updated deployment xxxx/deployment-1 10->11 (gcr.io/v2-namespace/hello-world:11)' sent message, got: %s", fs.sentEvent.Message)
+		t.Errorf("expected 'Successfully updated deployment xxxx/deployment-1 10.0.0->11.0.0 (gcr.io/v2-namespace/hello-world:11.0.0)' sent message, got: %s", fs.sentEvent.Message)
 	}
 }
 
@@ -823,7 +823,7 @@ func TestEventSentWithReleaseNotes(t *testing.T) {
 					Spec: v1.PodSpec{
 						Containers: []v1.Container{
 							v1.Container{
-								Image: "gcr.io/v2-namespace/hello-world:10",
+								Image: "gcr.io/v2-namespace/hello-world:10.0.0",
 							},
 						},
 					},
@@ -845,7 +845,7 @@ func TestEventSentWithReleaseNotes(t *testing.T) {
 
 	repo := types.Repository{
 		Name: "gcr.io/v2-namespace/hello-world",
-		Tag:  "11",
+		Tag:  "11.0.0",
 	}
 
 	event := &types.Event{Repository: repo}
@@ -858,8 +858,8 @@ func TestEventSentWithReleaseNotes(t *testing.T) {
 		t.Errorf("expected to find a deployment with updated image but found: %s", fp.updated.Containers()[0].Image)
 	}
 
-	if fs.sentEvent.Message != "Successfully updated deployment xxxx/deployment-1 10->11 (gcr.io/v2-namespace/hello-world:11). Release notes: https://github.com/keel-hq/keel/releases" {
+	if fs.sentEvent.Message != "Successfully updated deployment xxxx/deployment-1 10.0.0->11.0.0 (gcr.io/v2-namespace/hello-world:11.0.0). Release notes: https://github.com/keel-hq/keel/releases" {
-		t.Errorf("expected 'Successfully updated deployment xxxx/deployment-1 10->11 (gcr.io/v2-namespace/hello-world:11). Release notes: https://github.com/keel-hq/keel/releases' sent message, got: %s", fs.sentEvent.Message)
+		t.Errorf("expected 'Successfully updated deployment xxxx/deployment-1 10.0.0->11.0.0 (gcr.io/v2-namespace/hello-world:11.0.0). Release notes: https://github.com/keel-hq/keel/releases' sent message, got: %s", fs.sentEvent.Message)
 	}
 }
 

tests/acceptance_test.go

@@ -143,6 +143,136 @@ func TestWebhooksSemverUpdate(t *testing.T) {
 	})
 }
 
+// Test to ensure Keel doesn't try to be tolerant and parse integers as semver versions, for example
+// 45000 shouldn't become 45000.0.0 version (https://github.com/keel-hq/keel/issues/296)
+func TestWebhookHighIntegerUpdate(t *testing.T) {
+
+	// stop := make(chan struct{})
+	ctx, cancel := context.WithCancel(context.Background())
+	// defer close(ctx)
+	defer cancel()
+
+	// go startKeel(ctx)
+	keel := &KeelCmd{}
+	go func() {
+		err := keel.Start(ctx)
+		if err != nil {
+			log.WithFields(log.Fields{
+				"error": err,
+			}).Error("failed to start Keel process")
+		}
+	}()
+
+	defer func() {
+		err := keel.Stop()
+		if err != nil {
+			log.WithFields(log.Fields{
+				"error": err,
+			}).Error("failed to stop Keel process")
+		}
+	}()
+
+	_, kcs := getKubernetesClient()
+
+	t.Run("UpdateThroughDockerHubWebhook", func(t *testing.T) {
+
+		testNamespace := createNamespaceForTest()
+		defer deleteTestNamespace(testNamespace)
+
+		dep := &apps_v1.Deployment{
+			meta_v1.TypeMeta{},
+			meta_v1.ObjectMeta{
+				Name:        "deployment-1",
+				Namespace:   testNamespace,
+				Labels:      map[string]string{types.KeelPolicyLabel: "all"},
+				Annotations: map[string]string{},
+			},
+			apps_v1.DeploymentSpec{
+				Selector: &meta_v1.LabelSelector{
+					MatchLabels: map[string]string{
+						"app": "wd-1",
+					},
+				},
+				Template: v1.PodTemplateSpec{
+					ObjectMeta: meta_v1.ObjectMeta{
+						Labels: map[string]string{
+							"app":     "wd-1",
+							"release": "1",
+						},
+					},
+					Spec: v1.PodSpec{
+						Containers: []v1.Container{
+							v1.Container{
+								Name:  "wd-1",
+								Image: "karolisr/webhook-demo:0.0.14",
+							},
+						},
+					},
+				},
+			},
+			apps_v1.DeploymentStatus{},
+		}
+
+		_, err := kcs.AppsV1().Deployments(testNamespace).Create(dep)
+		if err != nil {
+			t.Fatalf("failed to create deployment: %s", err)
+		}
+		// giving some time to get started
+		// TODO: replace with a readiness check function to wait for 1/1 READY
+		time.Sleep(2 * time.Second)
+
+		var payload = `{
+			"push_data": {
+				"pushed_at": 1497467660,
+				"images": [],
+				"tag": "45000",
+				"pusher": "karolisr"
+			},
+			"repository": {
+				"status": "Active",
+				"description": "",
+				"is_trusted": false,		
+				"repo_url": "https://hub.docker.com/r/webhook-demo",
+				"owner": "karolisr",
+				"is_official": false,
+				"is_private": false,
+				"name": "keel",
+				"namespace": "karolisr",
+				"star_count": 0,
+				"comment_count": 0,
+				"date_created": 1497032538,	
+				"repo_name": "karolisr/webhook-demo"
+			}
+		}`
+
+		// sending webhook
+		client := http.DefaultClient
+		buf := bytes.NewBufferString(payload)
+		req, err := http.NewRequest("POST", "http://localhost:9300/v1/webhooks/dockerhub", buf)
+		if err != nil {
+			t.Fatalf("failed to create req: %s", err)
+		}
+		resp, err := client.Do(req)
+		if err != nil {
+			t.Errorf("failed to make a webhook request to keel: %s", err)
+		}
+
+		if resp.StatusCode != 200 {
+			t.Errorf("unexpected webhook response from keel: %d", resp.StatusCode)
+		}
+
+		ctx, cancel := context.WithTimeout(context.Background(), time.Second*10)
+		defer cancel()
+
+		time.Sleep(3 * time.Second)
+
+		err = waitFor(ctx, kcs, testNamespace, dep.ObjectMeta.Name, "karolisr/webhook-demo:0.0.14")
+		if err != nil {
+			t.Errorf("update failed: %s", err)
+		}
+	})
+}
+
 func TestApprovals(t *testing.T) {
 
 	ctx, cancel := context.WithCancel(context.Background())

tests/helpers.go

@@ -84,6 +84,9 @@ func (kc *KeelCmd) Start(ctx context.Context) error {
 	cmd := "keel"
 	args := []string{"--no-incluster", "--kubeconfig", getKubeConfig()}
 	c := exec.CommandContext(ctx, cmd, args...)
+	c.Env = []string{
+		"DEBUG=true",
+	}
 	c.Stdout = os.Stdout
 	c.Stderr = os.Stderr