Merge pull request #3662 from ywk253100/171122_ping_endpoint

Refactor ping target API

docs/swagger.yaml

@@ -1670,31 +1670,6 @@ paths:
           description: Target not found.
         '500':
           description: Unexpected internal errors.
-  '/targets/{id}/ping':
-    post:
-      summary: Ping target.
-      description: |
-        This endpoint is for ping target.
-      parameters:
-        - name: id
-          in: path
-          type: integer
-          format: int64
-          required: true
-          description: The replication's target ID.
-      tags:
-        - Products
-      responses:
-        '200':
-          description: Ping replication's target successfully.
-        '400':
-          description: Can not ping target.
-        '401':
-          description: User need to log in first.
-        '404':
-          description: Target ID does not exist.
-        '500':
-          description: Unexpected internal errors.
   '/targets/{id}':
     put:
       summary: Update replication's target.
@@ -2512,6 +2487,10 @@ definitions:
   PingTarget:
     type: object
     properties:
+      id:
+        type: integer
+        format: int
+        description: Target ID.
       endpoint:
         type: string
         description: The target address URL string.

src/ui/api/harborapi_test.go

@@ -118,7 +118,6 @@ func init() {
 	beego.Router("/api/targets/:id([0-9]+)", &TargetAPI{})
 	beego.Router("/api/targets/:id([0-9]+)/policies/", &TargetAPI{}, "get:ListPolicies")
 	beego.Router("/api/targets/ping", &TargetAPI{}, "post:Ping")
-	beego.Router("/api/targets/:id([0-9]+)/ping", &TargetAPI{}, "post:PingByID")
 	beego.Router("/api/policies/replication/:id([0-9]+)", &RepPolicyAPI{})
 	beego.Router("/api/policies/replication", &RepPolicyAPI{}, "get:List")
 	beego.Router("/api/policies/replication", &RepPolicyAPI{}, "post:Post;delete:Delete")
@@ -636,18 +635,6 @@ func (a testapi) PingTarget(authInfo usrInfo, body interface{}) (int, error) {
 	return httpStatusCode, err
 }
 
-//PingTargetByID ...
-func (a testapi) PingTargetByID(authInfo usrInfo, id int) (int, error) {
-	_sling := sling.New().Post(a.basePath)
-
-	path := fmt.Sprintf("/api/targets/%d/ping", id)
-
-	_sling = _sling.Path(path)
-
-	httpStatusCode, _, err := request(_sling, jsonAcceptHeader, authInfo)
-	return httpStatusCode, err
-}
-
 //Get target by targetID
 func (a testapi) GetTargetByID(authInfo usrInfo, targetID string) (int, error) {
 	_sling := sling.New().Get(a.basePath)

src/ui/api/target.go

@@ -84,48 +84,57 @@ func (t *TargetAPI) ping(endpoint, username, password string, insecure bool) {
 	}
 }
 
-// PingByID ping target by ID
-func (t *TargetAPI) PingByID() {
-	id := t.GetIDFromURL()
-
-	target, err := dao.GetRepTarget(id)
-	if err != nil {
-		log.Errorf("failed to get target %d: %v", id, err)
-		t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError))
-	}
-	if target == nil {
-		t.CustomAbort(http.StatusNotFound, fmt.Sprintf("target %d not found", id))
-	}
-
-	endpoint := target.URL
-	username := target.Username
-	password := target.Password
-	insecure := target.Insecure
-	if len(password) != 0 {
-		password, err = utils.ReversibleDecrypt(password, t.secretKey)
-		if err != nil {
-			log.Errorf("failed to decrypt password: %v", err)
-			t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError))
-		}
-	}
-	t.ping(endpoint, username, password, insecure)
-}
-
 // Ping validates whether the target is reachable and whether the credential is valid
 func (t *TargetAPI) Ping() {
 	req := struct {
-		Endpoint string `json:"endpoint"`
-		Username string `json:"username"`
-		Password string `json:"password"`
-		Insecure bool   `json:"insecure"`
+		ID       *int64  `json:"id"`
+		Endpoint *string `json:"endpoint"`
+		Username *string `json:"username"`
+		Password *string `json:"password"`
+		Insecure *bool   `json:"insecure"`
 	}{}
 	t.DecodeJSONReq(&req)
 
-	if len(req.Endpoint) == 0 {
-		t.CustomAbort(http.StatusBadRequest, "endpoint is required")
+	target := &models.RepTarget{}
+	if req.ID != nil {
+		var err error
+		target, err = dao.GetRepTarget(*req.ID)
+		if err != nil {
+			t.HandleInternalServerError(fmt.Sprintf("failed to get target %d: %v", *req.ID, err))
+			return
+		}
+		if target == nil {
+			t.HandleNotFound(fmt.Sprintf("target %d not found", *req.ID))
+			return
+		}
+		if len(target.Password) != 0 {
+			target.Password, err = utils.ReversibleDecrypt(target.Password, t.secretKey)
+			if err != nil {
+				t.HandleInternalServerError(fmt.Sprintf("failed to decrypt password: %v", err))
+				return
+			}
+		}
+	}
+
+	if req.Endpoint != nil {
+		target.URL = *req.Endpoint
+	}
+	if req.Username != nil {
+		target.Username = *req.Username
+	}
+	if req.Password != nil {
+		target.Password = *req.Password
+	}
+	if req.Insecure != nil {
+		target.Insecure = *req.Insecure
+	}
+
+	if len(target.URL) == 0 {
+		t.HandleBadRequest("empty endpoint")
+		return
 	}
 
-	t.ping(req.Endpoint, req.Username, req.Password, req.Insecure)
+	t.ping(target.URL, target.Username, target.Password, target.Insecure)
 }
 
 // Get ...

src/ui/api/target_test.go

@@ -15,11 +15,13 @@ package api
 
 import (
 	"fmt"
+	"net/http"
 	"os"
 	"strconv"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/vmware/harbor/tests/apitests/apilib"
 )
 
@@ -110,72 +112,46 @@ func TestTargetsGet(t *testing.T) {
 }
 
 func TestTargetPing(t *testing.T) {
-	var httpStatusCode int
-	var err error
-
-	assert := assert.New(t)
 	apiTest := newHarborAPI()
 
-	fmt.Println("Testing Targets Ping Post API")
-	//case 1
-	body := struct {
+	// 404: not exist target
+	target01 := struct {
+		ID int64 `json:"id"`
+	}{
+		ID: 10000,
+	}
+
+	code, err := apiTest.PingTarget(*admin, target01)
+	require.Nil(t, err)
+	assert.Equal(t, http.StatusNotFound, code)
+
+	// 400: empty endpoint
+	target02 := struct {
+		Endpoint string `json:"endpoint"`
+	}{
+		Endpoint: "",
+	}
+	code, err = apiTest.PingTarget(*admin, target02)
+	require.Nil(t, err)
+	assert.Equal(t, http.StatusBadRequest, code)
+
+	// 200
+	target03 := struct {
+		ID       int64  `json:"id"`
 		Endpoint string `json:"endpoint"`
 		Username string `json:"username"`
 		Password string `json:"password"`
+		Insecure bool   `json:"insecure"`
 	}{
+		ID:       int64(addTargetID),
 		Endpoint: os.Getenv("REGISTRY_URL"),
 		Username: adminName,
 		Password: adminPwd,
+		Insecure: true,
 	}
-	httpStatusCode, err = apiTest.PingTarget(*admin, body)
-	if err != nil {
-		t.Error("Error while ping target", err.Error())
-		t.Log(err)
-	} else {
-		assert.Equal(int(200), httpStatusCode, "")
-	}
-
-	//case 2
-	body.Endpoint = ""
-	httpStatusCode, err = apiTest.PingTarget(*admin, body)
-	if err != nil {
-		t.Error("Error while ping target", err.Error())
-	} else {
-		assert.Equal(int(400), httpStatusCode, "")
-	}
-}
-
-func TestTargetPingByID(t *testing.T) {
-	var httpStatusCode int
-	var err error
-
-	assert := assert.New(t)
-	apiTest := newHarborAPI()
-
-	fmt.Println("Testing Targets Ping Post API")
-
-	//-------------------case 1 : response code = 200------------------------//
-	fmt.Println("case 1 : response code = 200")
-	id := addTargetID
-	httpStatusCode, err = apiTest.PingTargetByID(*admin, id)
-	if err != nil {
-		t.Error("Error whihle ping target", err.Error())
-		t.Log(err)
-	} else {
-		assert.Equal(int(200), httpStatusCode, "httpStatusCode should be 200")
-	}
-
-	//--------------case 2 : response code = 404,target not found------------//
-	fmt.Println("case 2 : response code = 404,target not found")
-
-	id = 1111
-	httpStatusCode, err = apiTest.PingTargetByID(*admin, id)
-	if err != nil {
-		t.Error("Error whihle ping target", err.Error())
-		t.Log(err)
-	} else {
-		assert.Equal(int(404), httpStatusCode, "httpStatusCode should be 404")
-	}
+	code, err = apiTest.PingTarget(*admin, target03)
+	require.Nil(t, err)
+	assert.Equal(t, http.StatusOK, code)
 }
 
 func TestTargetGetByID(t *testing.T) {

src/ui/router.go

@@ -114,7 +114,6 @@ func initRouters() {
 	beego.Router("/api/targets/:id([0-9]+)", &api.TargetAPI{})
 	beego.Router("/api/targets/:id([0-9]+)/policies/", &api.TargetAPI{}, "get:ListPolicies")
 	beego.Router("/api/targets/ping", &api.TargetAPI{}, "post:Ping")
-	beego.Router("/api/targets/:id([0-9]+)/ping", &api.TargetAPI{}, "post:PingByID")
 	beego.Router("/api/logs", &api.LogAPI{})
 	beego.Router("/api/configurations", &api.ConfigAPI{})
 	beego.Router("/api/configurations/reset", &api.ConfigAPI{}, "post:Reset")