Skip to content

Commit

Permalink
feat(beego): upgrade beego to v1.12 which support middleware (goharbo…
Browse files Browse the repository at this point in the history
…r#10524)

1. Upgrade beego to v1.12.0
2. Add RequestID middleware to all HTTP requests.
3. Add Orm middleware to v2 and v2.0 APIs.
4. Remove OrmFilter from all HTTP requests.
5. Fix some test cases which cause panic in API controllers.
6. Enable XSRF for test cases of CommonController.
7. Imporve ReadOnly middleware.

Signed-off-by: He Weiwei <[email protected]>
  • Loading branch information
heww authored and xhxh committed Feb 7, 2020
1 parent f0c0f51 commit e63fdd6
Show file tree
Hide file tree
Showing 115 changed files with 6,614 additions and 1,125 deletions.
2 changes: 1 addition & 1 deletion src/core/api/admin_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ func (aj *AJAPI) getLog(id int64) {
// submit submits a job to job service per request
func (aj *AJAPI) submit(ajr *models.AdminJobReq) {
// when the schedule is saved as None without any schedule, just return 200 and do nothing.
if ajr.Schedule.Type == models.ScheduleNone {
if ajr.Schedule == nil || ajr.Schedule.Type == models.ScheduleNone {
return
}

Expand Down
2 changes: 1 addition & 1 deletion src/core/api/reg_gc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func TestGCPost(t *testing.T) {
t.Error("Error occurred while add a admin job", err.Error())
t.Log(err)
} else {
assert.Equal(200, code, "Add adminjob status should be 200")
assert.Equal(201, code, "Add adminjob status should be 201")
}
}

Expand Down
54 changes: 49 additions & 5 deletions src/core/api/retention_test.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,33 @@
// Copyright Project Harbor 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 api

import (
"encoding/json"
"fmt"
"net/http"
"testing"
"time"

"github.com/goharbor/harbor/src/pkg/retention/dao"
"github.com/goharbor/harbor/src/pkg/retention/dao/models"
"github.com/goharbor/harbor/src/pkg/retention/mocks"
"github.com/goharbor/harbor/src/pkg/retention/policy"
"github.com/goharbor/harbor/src/pkg/retention/policy/rule"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"net/http"
"testing"
"time"
)

func TestGetMetadatas(t *testing.T) {
Expand All @@ -30,6 +47,16 @@ func TestGetMetadatas(t *testing.T) {
}

func TestCreatePolicy(t *testing.T) {
// mock retention api controller
mockController := &mocks.APIController{}
mockController.On("CreateRetention", mock.AnythingOfType("*policy.Metadata")).Return(int64(1), nil)

controller := retentionController
retentionController = mockController
defer func() {
retentionController = controller
}()

p1 := &policy.Metadata{
Algorithm: "or",
Rules: []rule.Metadata{
Expand Down Expand Up @@ -87,7 +114,7 @@ func TestCreatePolicy(t *testing.T) {
bodyJSON: p1,
credential: sysAdmin,
},
code: http.StatusOK,
code: http.StatusCreated,
},
{
request: &testingRequest{
Expand Down Expand Up @@ -267,6 +294,23 @@ func TestPolicy(t *testing.T) {
require.Nil(t, err)
require.True(t, id > 0)

// mock retention api controller
mockController := &mocks.APIController{}
mockController.On("GetRetention", mock.AnythingOfType("int64")).Return(p, nil)
mockController.On("UpdateRetention", mock.AnythingOfType("*policy.Metadata")).Return(nil)
mockController.On("TriggerRetentionExec",
mock.AnythingOfType("int64"),
mock.AnythingOfType("string"),
mock.AnythingOfType("bool")).Return(int64(1), nil)
mockController.On("ListRetentionExecs", mock.AnythingOfType("int64"), mock.AnythingOfType("*q.Query")).Return(nil, nil)
mockController.On("GetTotalOfRetentionExecs", mock.AnythingOfType("int64")).Return(int64(0), nil)

controller := retentionController
retentionController = mockController
defer func() {
retentionController = controller
}()

cases := []*codeCheckingCase{
{
request: &testingRequest{
Expand Down Expand Up @@ -408,7 +452,7 @@ func TestPolicy(t *testing.T) {
},
credential: sysAdmin,
},
code: http.StatusOK,
code: http.StatusCreated,
},
{
request: &testingRequest{
Expand Down
16 changes: 15 additions & 1 deletion src/core/api/scan_all_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
// Copyright Project Harbor 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 api

import (
Expand Down Expand Up @@ -56,7 +70,7 @@ func (suite *ScanAllAPITestSuite) TestScanAllPost() {
// case 1: add a new scan all job
code, err := apiTest.AddScanAll(*admin, adminJob002)
require.NoError(suite.T(), err, "Error occurred while add a scan all job")
suite.Equal(200, code, "Add scan all status should be 200")
suite.Equal(201, code, "Add scan all status should be 200")
}

func (suite *ScanAllAPITestSuite) TestScanAllGet() {
Expand Down
14 changes: 5 additions & 9 deletions src/core/controllers/controllers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,21 @@ package controllers

import (
"context"
"github.com/goharbor/harbor/src/core/filter"
"fmt"
"net/http"
"net/http/httptest"
// "net/url"
"os"
"path/filepath"
"runtime"
"testing"

"fmt"
"os"
"strings"
"testing"

"github.com/astaxie/beego"
"github.com/goharbor/harbor/src/common"
"github.com/goharbor/harbor/src/common/models"
utilstest "github.com/goharbor/harbor/src/common/utils/test"
"github.com/goharbor/harbor/src/core/config"
"github.com/goharbor/harbor/src/core/filter"
"github.com/goharbor/harbor/src/core/middlewares"
"github.com/stretchr/testify/assert"
)
Expand All @@ -41,6 +39,7 @@ func init() {
dir := filepath.Dir(file)
dir = filepath.Join(dir, "..")
apppath, _ := filepath.Abs(dir)
beego.BConfig.WebConfig.EnableXSRF = true
beego.BConfig.WebConfig.Session.SessionOn = true
beego.TestBeegoInit(apppath)
beego.AddTemplateExt("htm")
Expand Down Expand Up @@ -106,9 +105,6 @@ func TestAll(t *testing.T) {
err := middlewares.Init()
assert.Nil(err)

// Has to set to dev so that the xsrf panic can be rendered as 403
beego.BConfig.RunMode = beego.DEV

r, _ := http.NewRequest("POST", "/c/login", nil)
w := httptest.NewRecorder()
beego.BeeApp.Handlers.ServeHTTP(w, r)
Expand Down
32 changes: 0 additions & 32 deletions src/core/filter/orm.go

This file was deleted.

23 changes: 21 additions & 2 deletions src/core/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ package main
import (
"encoding/gob"
"fmt"
"net/http"
"os"
"os/signal"
"strconv"
"strings"
"syscall"
"time"

Expand Down Expand Up @@ -53,6 +55,8 @@ import (
"github.com/goharbor/harbor/src/pkg/types"
"github.com/goharbor/harbor/src/pkg/version"
"github.com/goharbor/harbor/src/replication"
"github.com/goharbor/harbor/src/server/middleware/orm"
"github.com/goharbor/harbor/src/server/middleware/requestid"
)

const (
Expand Down Expand Up @@ -247,7 +251,6 @@ func main() {

filter.Init()
beego.InsertFilter("/api/*", beego.BeforeStatic, filter.SessionCheck)
beego.InsertFilter("/*", beego.BeforeRouter, filter.OrmFilter)
beego.InsertFilter("/*", beego.BeforeRouter, filter.SecurityFilter)
beego.InsertFilter("/*", beego.BeforeRouter, filter.ReadonlyFilter)

Expand Down Expand Up @@ -288,6 +291,22 @@ func main() {
}

log.Infof("Version: %s, Git commit: %s", version.ReleaseVersion, version.GitCommit)
beego.Run()

middlewares := []beego.MiddleWare{
requestid.Middleware(),
orm.Middleware(legacyAPISkipper),
}
beego.RunWithMiddleWares("", middlewares...)

}

// legacyAPISkipper skip middleware for legacy APIs
func legacyAPISkipper(r *http.Request) bool {
for _, prefix := range []string{"/v2/", "/api/v2.0/"} {
if strings.HasPrefix(r.URL.Path, prefix) {
return false
}
}

return true
}
4 changes: 2 additions & 2 deletions src/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@ go 1.12
replace github.com/goharbor/harbor => ../

require (
github.com/Knetic/govaluate v3.0.0+incompatible // indirect
github.com/Masterminds/semver v1.4.2
github.com/Microsoft/go-winio v0.4.12 // indirect
github.com/Shopify/logrus-bugsnag v0.0.0-20171204204709-577dee27f20d // indirect
github.com/Unknwon/goconfig v0.0.0-20160216183935-5f601ca6ef4d // indirect
github.com/agl/ed25519 v0.0.0-20170116200512-5312a6153412 // indirect
github.com/aliyun/alibaba-cloud-sdk-go v0.0.0-20190726115642-cd293c93fd97
github.com/astaxie/beego v1.9.0
github.com/astaxie/beego v1.12.0
github.com/aws/aws-sdk-go v1.19.47
github.com/beego/i18n v0.0.0-20140604031826-e87155e8f0c0
github.com/bitly/go-hostpool v0.0.0-20171023180738-a3a6125de932 // indirect
Expand Down Expand Up @@ -71,6 +70,7 @@ require (
github.com/pquerna/cachecontrol v0.0.0-20180517163645-1555304b9b35 // indirect
github.com/prometheus/client_golang v0.9.4 // indirect
github.com/robfig/cron v1.0.0
github.com/shiena/ansicolor v0.0.0-20151119151921-a422bbe96644 // indirect
github.com/sirupsen/logrus v1.4.1 // indirect
github.com/spf13/viper v1.4.0 // indirect
github.com/stretchr/testify v1.4.0
Expand Down
Loading

0 comments on commit e63fdd6

Please sign in to comment.