Merge pull request #1627 from ywk253100/170315_ping_email

Ping email server API enhancement

docs/swagger.yaml

@@ -2098,7 +2098,7 @@ definitions:
         type: string
         description: The host of email server.
       email_port:
-        type: string
+        type: integer
         description: The port of email server.
       email_username:
         type: string
@@ -2107,7 +2107,7 @@ definitions:
         type: string
         description: The password of email server.
       email_ssl:
-        type: string
+        type: boolean
         description: Use ssl/tls or not.
       email_identity:
         type: string

src/ui/api/email.go

@@ -16,13 +16,11 @@
 package api
 
 import (
-	"fmt"
 	"net"
 	"net/http"
 	"strconv"
 
 	"github.com/vmware/harbor/src/common/api"
-	comcfg "github.com/vmware/harbor/src/common/config"
 	"github.com/vmware/harbor/src/common/dao"
 	"github.com/vmware/harbor/src/common/utils/email"
 	"github.com/vmware/harbor/src/common/utils/log"
@@ -54,63 +52,60 @@ func (e *EmailAPI) Prepare() {
 
 // Ping tests connection and authentication with email server
 func (e *EmailAPI) Ping() {
-	m := map[string]string{}
-	e.DecodeJSONReq(&m)
-
-	settings, err := config.Email()
-	if err != nil {
-		e.CustomAbort(http.StatusInternalServerError,
-			http.StatusText(http.StatusInternalServerError))
-	}
-
-	host, ok := m[comcfg.EmailHost]
-	if ok {
-		if len(host) == 0 {
-			e.CustomAbort(http.StatusBadRequest, "empty email server host")
+	var host, username, password, identity string
+	var port int
+	var ssl bool
+	body := e.Ctx.Input.CopyBody(1 << 32)
+	if body == nil || len(body) == 0 {
+		cfg, err := config.Email()
+		if err != nil {
+			log.Errorf("failed to get email configurations: %v", err)
+			e.CustomAbort(http.StatusInternalServerError,
+				http.StatusText(http.StatusInternalServerError))
 		}
-		settings.Host = host
-	}
+		host = cfg.Host
+		port = cfg.Port
+		username = cfg.Username
+		password = cfg.Password
+		identity = cfg.Identity
+		ssl = cfg.SSL
+	} else {
+		settings := &struct {
+			Host     string  `json:"email_host"`
+			Port     *int    `json:"email_port"`
+			Username string  `json:"email_username"`
+			Password *string `json:"email_password"`
+			SSL      bool    `json:"email_ssl"`
+			Identity string  `json:"email_identity"`
+		}{}
+		e.DecodeJSONReq(&settings)
 
-	port, ok := m[comcfg.EmailPort]
-	if ok {
-		if len(port) == 0 {
-			e.CustomAbort(http.StatusBadRequest, "empty email server port")
+		if len(settings.Host) == 0 || settings.Port == nil {
+			e.CustomAbort(http.StatusBadRequest, "empty host or port")
 		}
-		p, err := strconv.Atoi(port)
-		if err != nil || p <= 0 {
-			e.CustomAbort(http.StatusBadRequest, "invalid email server port")
+
+		if settings.Password == nil {
+			cfg, err := config.Email()
+			if err != nil {
+				log.Errorf("failed to get email configurations: %v", err)
+				e.CustomAbort(http.StatusInternalServerError,
+					http.StatusText(http.StatusInternalServerError))
+			}
+
+			settings.Password = &cfg.Password
 		}
-		settings.Port = p
+
+		host = settings.Host
+		port = *settings.Port
+		username = settings.Username
+		password = *settings.Password
+		identity = settings.Identity
+		ssl = settings.SSL
 	}
 
-	username, ok := m[comcfg.EmailUsername]
-	if ok {
-		settings.Username = username
-	}
-
-	password, ok := m[comcfg.EmailPassword]
-	if ok {
-		settings.Password = password
-	}
-
-	identity, ok := m[comcfg.EmailIdentity]
-	if ok {
-		settings.Identity = identity
-	}
-
-	ssl, ok := m[comcfg.EmailSSL]
-	if ok {
-		if ssl != "0" && ssl != "1" {
-			e.CustomAbort(http.StatusBadRequest,
-				fmt.Sprintf("%s should be 0 or 1", comcfg.EmailSSL))
-		}
-		settings.SSL = ssl == "1"
-	}
-
-	addr := net.JoinHostPort(settings.Host, strconv.Itoa(settings.Port))
-	if err := email.Ping(
-		addr, settings.Identity, settings.Username,
-		settings.Password, pingEmailTimeout, settings.SSL, false); err != nil {
+	addr := net.JoinHostPort(host, strconv.Itoa(port))
+	if err := email.Ping(addr, identity, username,
+		password, pingEmailTimeout, ssl, false); err != nil {
 		log.Debugf("ping %s failed: %v", addr, err)
 		e.CustomAbort(http.StatusBadRequest, err.Error())
 	}

src/ui/api/email_test.go

@@ -21,7 +21,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	comcfg "github.com/vmware/harbor/src/common/config"
 )
 
 func TestPingEmail(t *testing.T) {
@@ -38,17 +37,29 @@ func TestPingEmail(t *testing.T) {
 
 	assert.Equal(401, code, "the status code of ping email server with non-admin user should be 401")
 
-	settings := map[string]string{
-		comcfg.EmailHost:     "smtp.gmail.com",
-		comcfg.EmailPort:     "465",
-		comcfg.EmailIdentity: "",
-		comcfg.EmailUsername: "wrong_username",
-		comcfg.EmailPassword: "wrong_password",
-		comcfg.EmailSSL:      "1",
+	//case 2: bad request
+	settings := `{
+		"email_host":     ""
+	}`
+
+	code, _, err = apiTest.PingEmail(*admin, []byte(settings))
+	if err != nil {
+		t.Errorf("failed to test ping email server: %v", err)
+		return
 	}
 
-	//case 2: secure connection with admin role
-	code, body, err := apiTest.PingEmail(*admin, settings)
+	assert.Equal(400, code, "the status code of ping email server should be 400")
+
+	//case 3: secure connection with admin role
+	settings = `{
+		"email_host":     "smtp.gmail.com",
+		"email_port":     465,
+		"email_identity": "",
+		"email_username": "wrong_username",
+		"email_ssl":      true
+	}`
+
+	code, body, err := apiTest.PingEmail(*admin, []byte(settings))
 	if err != nil {
 		t.Errorf("failed to test ping email server: %v", err)
 		return
@@ -60,4 +71,13 @@ func TestPingEmail(t *testing.T) {
 		t.Errorf("unexpected error: %s does not contains 535", body)
 		return
 	}
+
+	//case 4: ping email server whose settings are read from config
+	code, _, err = apiTest.PingEmail(*admin, nil)
+	if err != nil {
+		t.Errorf("failed to test ping email server: %v", err)
+		return
+	}
+
+	assert.Equal(400, code, "the status code of ping email server should be 400")
 }

src/ui/api/harborapi_test.go

@@ -3,6 +3,7 @@
 package api
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
@@ -1038,8 +1039,8 @@ func (a testapi) ResetConfig(authInfo usrInfo) (int, error) {
 	return code, err
 }
 
-func (a testapi) PingEmail(authInfo usrInfo, settings map[string]string) (int, string, error) {
-	_sling := sling.New().Base(a.basePath).Post("/api/email/ping").BodyJSON(settings)
+func (a testapi) PingEmail(authInfo usrInfo, settings []byte) (int, string, error) {
+	_sling := sling.New().Base(a.basePath).Post("/api/email/ping").Body(bytes.NewReader(settings))
 
 	code, body, err := request(_sling, jsonAcceptHeader, authInfo)