From 69d38b924b1d8301f59f9db4fdec6e57fb240fea Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Tue, 22 Aug 2023 11:07:45 +0800 Subject: [PATCH] feat: add diff of route resource (#3) --- .github/workflows/unit_test.yaml | 61 +++++++++++ Makefile | 4 + adc.yaml | 2 + internal/pkg/db/memdb.go | 35 ++++++- internal/pkg/db/memdb_test.go | 45 ++++++++- internal/pkg/differ/differ.go | 65 +++++++++++- internal/pkg/differ/differ_test.go | 156 ++++++++++++++++++++++++++++- pkg/common/file_test.go | 4 +- pkg/data/types.go | 29 +++++- 9 files changed, 382 insertions(+), 19 deletions(-) create mode 100644 .github/workflows/unit_test.yaml diff --git a/.github/workflows/unit_test.yaml b/.github/workflows/unit_test.yaml new file mode 100644 index 0000000..5c1ebba --- /dev/null +++ b/.github/workflows/unit_test.yaml @@ -0,0 +1,61 @@ +name: Run Unit Test Suites + +on: + push: + branches: + - main + - release/** + pull_request: + branches: + - main + - release/** +permissions: read-all +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true +jobs: + changes: + runs-on: ubuntu-latest + timeout-minutes: 5 + outputs: + src: ${{ steps.filter.outputs.src }} + steps: + - name: Checkout + uses: actions/checkout@v3 + - uses: dorny/paths-filter@v2 + id: filter + with: + token: ${{ secrets.GITHUB_TOKEN }} + filters: | + src: + - '*.go' + - '**/*.go' + - 'go.mod' + - 'go.sum' + - 'Makefile' + - '.github/**' + - 'internal/**' + run-test: + needs: changes + if: | + (needs.changes.outputs.src == 'true') + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@v3 + - uses: actions/cache@v3 + with: + path: | + ~/.cache/go-build + ~/go/pkg/mod + key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ runner.os }}-go- + - name: Setup Go Environment + uses: actions/setup-go@v3 + with: + go-version: '1.20.3' + - name: Run Unit Test Suites + working-directory: ./ + run: | + make test diff --git a/Makefile b/Makefile index 476ca75..eb4f0e5 100644 --- a/Makefile +++ b/Makefile @@ -8,3 +8,7 @@ help: ## Display this help build: ## Build adc @go build -o bin/adc main.go .PHONY: build + +test: + @go test -v ./... +.PHONY: test diff --git a/adc.yaml b/adc.yaml index 5637393..d60ff57 100644 --- a/adc.yaml +++ b/adc.yaml @@ -1,3 +1,5 @@ +name: "test" +version: "1.0.0" services: - name: svc1 hosts: diff --git a/internal/pkg/db/memdb.go b/internal/pkg/db/memdb.go index 551469d..d7188c0 100644 --- a/internal/pkg/db/memdb.go +++ b/internal/pkg/db/memdb.go @@ -17,10 +17,15 @@ var schema = &memdb.DBSchema{ Unique: true, Indexer: &memdb.StringFieldIndex{Field: "ID"}, }, - "name": { - Name: "name", + }, + }, + "routes": { + Name: "routes", + Indexes: map[string]*memdb.IndexSchema{ + "id": { + Name: "id", Unique: true, - Indexer: &memdb.StringFieldIndex{Field: "Name"}, + Indexer: &memdb.StringFieldIndex{Field: "ID"}, }, }, }, @@ -47,8 +52,17 @@ func NewMemDB(configure *data.Configuration) (*DB, error) { if service.ID == "" { service.ID = service.Name } + err = txn.Insert("services", service) + if err != nil { + return nil, err + } + } - err := txn.Insert("services", service) + for _, routes := range configure.Routes { + if routes.ID == "" { + routes.ID = routes.Name + } + err = txn.Insert("routes", routes) if err != nil { return nil, err } @@ -70,3 +84,16 @@ func (db *DB) GetServiceByID(id string) (*data.Service, error) { return obj.(*data.Service), err } + +func (db *DB) GetRouteByID(id string) (*data.Route, error) { + obj, err := db.memDB.Txn(false).First("routes", "id", id) + if err != nil { + return nil, err + } + + if obj == nil { + return nil, NotFound + } + + return obj.(*data.Route), err +} diff --git a/internal/pkg/db/memdb_test.go b/internal/pkg/db/memdb_test.go index 7d3c497..3e3e294 100644 --- a/internal/pkg/db/memdb_test.go +++ b/internal/pkg/db/memdb_test.go @@ -1,6 +1,7 @@ package db import ( + "net/http" "testing" "github.com/stretchr/testify/assert" @@ -36,6 +37,15 @@ var ( }, UpstreamInUse: "upstream1", } + + route = &data.Route{ + ID: "route", + Name: "route", + Labels: []string{"lable1", "label2"}, + Methods: []string{http.MethodGet}, + Paths: []string{"/get"}, + ServiceID: "svc", + } ) func TestGetServiceByID(t *testing.T) { @@ -54,14 +64,41 @@ func TestGetServiceByID(t *testing.T) { assert.Equal(t, NotFound, err, "check the error") // Test Case 3: Service don't have id - svc.ID = "" - svc.Name = "test" + svc1 := *svc + svc1.ID = "" config = data.Configuration{ - Services: []*data.Service{svc}, + Services: []*data.Service{&svc1}, } db, _ = NewMemDB(&config) - service, err = db.GetServiceByID("test") + service, err = db.GetServiceByID("svc") assert.Nil(t, err, "check the error") assert.Equal(t, svc, service, "check the service") } + +func TestGetRouteByID(t *testing.T) { + // Test Case 1: get route by id + config := data.Configuration{ + Routes: []*data.Route{route}, + } + + db, _ := NewMemDB(&config) + route1, err := db.GetRouteByID("route") + assert.Nil(t, err, "check the error") + assert.Equal(t, route, route1, "check the route") + + // Test Case 2: get route by id (not found) + route1, err = db.GetRouteByID("not-found") + assert.Equal(t, NotFound, err, "check the error") + + // Test Case 3: Route don't have id + route.ID = "" + config = data.Configuration{ + Routes: []*data.Route{route}, + } + + db, _ = NewMemDB(&config) + route1, err = db.GetRouteByID("route") + assert.Nil(t, err, "check the error") + assert.Equal(t, route, route1, "check the route") +} diff --git a/internal/pkg/differ/differ.go b/internal/pkg/differ/differ.go index 2e74a62..6e0d045 100644 --- a/internal/pkg/differ/differ.go +++ b/internal/pkg/differ/differ.go @@ -35,19 +35,23 @@ func (d *Differ) Diff() ([]*data.Event, error) { var events []*data.Event var err error - serviceEvents, err := d.diffService() + serviceEvents, err := d.diffServices() if err != nil { return nil, err } - events = append(events, serviceEvents...) - // TODO: diff routes + routeEvents, err := d.diffRoutes() + if err != nil { + return nil, err + } + events = append(events, serviceEvents...) + events = append(events, routeEvents...) return events, nil } // diffService compares the services between local and remote. -func (d *Differ) diffService() ([]*data.Event, error) { +func (d *Differ) diffServices() ([]*data.Event, error) { var events []*data.Event var mark = make(map[string]bool) @@ -98,3 +102,56 @@ func (d *Differ) diffService() ([]*data.Event, error) { return events, nil } + +// diffRoutes compares the routes between local and remote. +func (d *Differ) diffRoutes() ([]*data.Event, error) { + var events []*data.Event + var mark = make(map[string]bool) + + for _, remoteRoute := range d.remoteConfig.Routes { + localRoute, err := d.localDB.GetRouteByID(remoteRoute.ID) + if err != nil { + // If we can't find the route in local, it means the route should be deleted. + // So we add a delete event and the value is the route from remote. + if err == db.NotFound { + e := data.Event{ + ResourceType: data.RouteResourceType, + Option: data.DeleteOption, + Value: remoteRoute, + } + events = append(events, &e) + continue + } + + return nil, err + } + + mark[localRoute.ID] = true + // If the route is equal, we don't need to add an event. + // Else, we use the local routes to update the remote routes. + if equal := reflect.DeepEqual(localRoute, remoteRoute); equal { + continue + } + + events = append(events, &data.Event{ + ResourceType: data.RouteResourceType, + Option: data.UpdateOption, + Value: localRoute, + }) + } + + // If the route is not in the remote configuration, it means the route should be created. + for _, route := range d.localConfig.Routes { + if mark[route.ID] { + continue + } + + events = append(events, &data.Event{ + ResourceType: data.RouteResourceType, + Option: data.CreateOption, + Value: route, + }) + } + + return events, nil +} diff --git a/internal/pkg/differ/differ_test.go b/internal/pkg/differ/differ_test.go index 776321f..3842560 100644 --- a/internal/pkg/differ/differ_test.go +++ b/internal/pkg/differ/differ_test.go @@ -1,6 +1,7 @@ package differ import ( + "net/http" "testing" "github.com/stretchr/testify/assert" @@ -36,19 +37,65 @@ var ( }, UpstreamInUse: "upstream1", } + + route = &data.Route{ + ID: "route", + Name: "route", + Labels: []string{"lable1", "label2"}, + Methods: []string{http.MethodGet}, + Paths: []string{"/get"}, + ServiceID: "svc", + } ) func TestDiff(t *testing.T) { // Test case 1: delete events localConfig := &data.Configuration{ Services: []*data.Service{}, + Routes: []*data.Route{route}, } + + route1 := *route + route1.ID = "route1" + route1.Name = "route1" remoteConfig := &data.Configuration{ Services: []*data.Service{svc}, + Routes: []*data.Route{&route1}, } differ, _ := NewDiffer(localConfig, remoteConfig) events, _ := differ.Diff() + assert.Equal(t, 3, len(events), "check the number of delete events") + assert.Equal(t, []*data.Event{ + { + ResourceType: data.ServiceResourceType, + Option: data.DeleteOption, + Value: svc, + }, + { + ResourceType: data.RouteResourceType, + Option: data.DeleteOption, + Value: &route1, + }, + { + ResourceType: data.RouteResourceType, + Option: data.CreateOption, + Value: route, + }, + }, events, "check the content of delete events") +} + +func TestDiffServices(t *testing.T) { + // Test case 1: delete events + localConfig := &data.Configuration{ + Services: []*data.Service{}, + } + remoteConfig := &data.Configuration{ + Services: []*data.Service{svc}, + } + + differ, _ := NewDiffer(localConfig, remoteConfig) + events, _ := differ.diffServices() assert.Equal(t, 1, len(events), "check the number of delete events") assert.Equal(t, []*data.Event{ { @@ -70,7 +117,7 @@ func TestDiff(t *testing.T) { } differ, _ = NewDiffer(localConfig, remoteConfig) - events, _ = differ.Diff() + events, _ = differ.diffServices() assert.Equal(t, 1, len(events), "check the number of update events") assert.Equal(t, []*data.Event{ { @@ -89,7 +136,7 @@ func TestDiff(t *testing.T) { } differ, _ = NewDiffer(localConfig, remoteConfig) - events, _ = differ.Diff() + events, _ = differ.diffServices() assert.Equal(t, 1, len(events), "check the number of create events") assert.Equal(t, []*data.Event{ { @@ -109,7 +156,7 @@ func TestDiff(t *testing.T) { } differ, _ = NewDiffer(localConfig, remoteConfig) - events, _ = differ.Diff() + events, _ = differ.diffServices() assert.Equal(t, 0, len(events), "check the number of no events") // Test case 5: delete and create events @@ -125,7 +172,7 @@ func TestDiff(t *testing.T) { } differ, _ = NewDiffer(localConfig, remoteConfig) - events, _ = differ.Diff() + events, _ = differ.diffServices() assert.Equal(t, 2, len(events), "check the number of delete and create events") assert.Equal(t, []*data.Event{ { @@ -140,3 +187,104 @@ func TestDiff(t *testing.T) { }, }, events, "check the content of delete and create events") } + +func TestDifferRoutes(t *testing.T) { + // Test case 1: delete events + localConfig := &data.Configuration{ + Routes: []*data.Route{}, + } + remoteConfig := &data.Configuration{ + Routes: []*data.Route{route}, + } + + differ, _ := NewDiffer(localConfig, remoteConfig) + events, _ := differ.diffRoutes() + assert.Equal(t, 1, len(events), "check the number of delete events") + assert.Equal(t, []*data.Event{ + { + ResourceType: data.RouteResourceType, + Option: data.DeleteOption, + Value: route, + }, + }, events, "check the content of delete events") + + // Test case 2: update events + localConfig = &data.Configuration{ + Routes: []*data.Route{route}, + } + route1 := *route + route1.Name = "route1" + remoteConfig = &data.Configuration{ + Routes: []*data.Route{&route1}, + } + + differ, _ = NewDiffer(localConfig, remoteConfig) + events, _ = differ.diffRoutes() + assert.Equal(t, 1, len(events), "check the number of update events") + assert.Equal(t, []*data.Event{ + { + ResourceType: data.RouteResourceType, + Option: data.UpdateOption, + Value: route, + }, + }, events, "check the content of update events") + + // Test case 3: create events + localConfig = &data.Configuration{ + Routes: []*data.Route{route}, + } + remoteConfig = &data.Configuration{ + Routes: []*data.Route{}, + } + + differ, _ = NewDiffer(localConfig, remoteConfig) + events, _ = differ.diffRoutes() + assert.Equal(t, 1, len(events), "check the number of create events") + assert.Equal(t, []*data.Event{ + { + ResourceType: data.RouteResourceType, + Option: data.CreateOption, + Value: route, + }, + }, events, "check the content of create events") + + // Test case 4: no events + localConfig = &data.Configuration{ + Routes: []*data.Route{}, + } + + remoteConfig = &data.Configuration{ + Routes: []*data.Route{}, + } + + differ, _ = NewDiffer(localConfig, remoteConfig) + events, _ = differ.diffRoutes() + assert.Equal(t, 0, len(events), "check the number of no events") + + // Test case 5: delete and create events + localConfig = &data.Configuration{ + Routes: []*data.Route{route}, + } + route1 = *route + route1.ID = "route1" + route1.Name = "route1" + remoteConfig = &data.Configuration{ + Routes: []*data.Route{&route1}, + } + + differ, _ = NewDiffer(localConfig, remoteConfig) + events, _ = differ.diffRoutes() + assert.Equal(t, 2, len(events), "check the number of delete and create events") + assert.Equal(t, []*data.Event{ + { + ResourceType: data.RouteResourceType, + Option: data.DeleteOption, + Value: &route1, + }, + { + ResourceType: data.RouteResourceType, + Option: data.CreateOption, + Value: route, + }, + }, events, "check the content of delete and create events") +} diff --git a/pkg/common/file_test.go b/pkg/common/file_test.go index 2b810b7..63bea8c 100644 --- a/pkg/common/file_test.go +++ b/pkg/common/file_test.go @@ -17,7 +17,9 @@ func TestGetContentFromFile(t *testing.T) { defer os.Remove(tmpfile.Name()) // clean up // write some content to the file - expectedContent := `services: + expectedContent := `name: test +version: "1.0.0" +services: - name: svc1 hosts: - svc1.example.com diff --git a/pkg/data/types.go b/pkg/data/types.go index 693ce7d..d53f0c0 100644 --- a/pkg/data/types.go +++ b/pkg/data/types.go @@ -56,6 +56,26 @@ type Upstream struct { Checks *Checks `json:"checks,omitempty"` } +// Route is the API endpoint which is exposed to the outside world by its service. +type Route struct { + ID string `json:"id"` + Name string `json:"name"` + Description string `json:"description"` + // Labels are used for resource classification and indexing + Labels StringArray `json:"labels,omitempty"` + Methods []string `json:"methods"` + // Paths is the route paths to match this route. + Paths []string `json:"paths"` + // StripePathPrefix indicates whether to strip the path prefix defined on service. + StripPathPrefix bool `json:"strip_path_prefix"` + // Plugin settings on Service level + Plugins Plugins `json:"plugins,omitempty"` + // ServiceID is id of service that current route belong with. + ServiceID string `json:"service_id"` + // EnableWebSocket indicates whether this route should support websocket upgrade. + EnableWebSocket bool `json:"enable_websocket"` +} + // UpstreamTarget is the definition for an upstream endpoint. type UpstreamTarget struct { Host string `json:"host"` @@ -216,16 +236,21 @@ type TCPUnhealthyPredicatesForPassive struct { Timeouts int64 `json:"timeouts,omitempty"` } -// ServiceConfig is the configuration of services +// Configuration is the configuration of services type Configuration struct { + Name string `yaml:"name" json:"name"` + Version string `yaml:"version" json:"version"` Services []*Service `yaml:"services" json:"services"` + Routes []*Route `yaml:"routes" json:"routes"` } type ResourceType string var ( - // ResourceTypeService is the resource type of service + // ServiceResourceType is the resource type of service ServiceResourceType ResourceType = "service" + // RouteResourceType is the resource type of route + RouteResourceType ResourceType = "route" ) const (