-
Notifications
You must be signed in to change notification settings - Fork 550
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: OpenAPI 3 parse and convert #2460
Changes from all commits
5b1c65b
7b6985d
8ac880d
2c3d930
ed2aed1
8f488df
2658bf2
2e8a99c
6747450
a4adf65
4834643
88f5d21
2d04feb
4202db8
6f4abe9
574bf6c
beaceb5
3e3cada
bd71046
06144b5
9a10f64
c879a13
9d3ffd1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one or more | ||
* contributor license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright ownership. | ||
* The ASF licenses this file to You 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 openapi3 | ||
|
||
import "github.com/apisix/manager-api/internal/handler/data_loader/loader" | ||
|
||
func (Loader) Export(data loader.DataSets) (interface{}, error) { | ||
//TODO implement me | ||
panic("implement me") | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,128 @@ | ||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one or more | ||
* contributor license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright ownership. | ||
* The ASF licenses this file to You 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 openapi3 | ||
|
||
import ( | ||
"fmt" | ||
"reflect" | ||
"strings" | ||
"time" | ||
|
||
"github.com/getkin/kin-openapi/openapi3" | ||
"github.com/pkg/errors" | ||
|
||
"github.com/apisix/manager-api/internal/core/entity" | ||
"github.com/apisix/manager-api/internal/handler/data_loader/loader" | ||
"github.com/apisix/manager-api/internal/utils/consts" | ||
) | ||
|
||
func (o Loader) Import(input interface{}) (*loader.DataSets, error) { | ||
if input == nil { | ||
panic("input is nil") | ||
} | ||
|
||
d, ok := input.([]byte) | ||
if !ok { | ||
panic(fmt.Sprintf("input format error: expected []byte but it is %s", reflect.TypeOf(input).Kind().String())) | ||
} | ||
|
||
// load OAS3 document | ||
swagger, err := openapi3.NewSwaggerLoader().LoadSwaggerFromData(d) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// no paths in OAS3 document | ||
if len(swagger.Paths) <= 0 { | ||
return nil, errors.Wrap(errors.New("OpenAPI documentation does not contain any paths"), consts.ErrImportFile.Error()) | ||
} | ||
|
||
if o.TaskName == "" { | ||
o.TaskName = "openapi_" + time.Now().Format("20060102150405") | ||
} | ||
|
||
data, err := o.convertToEntities(swagger) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
return data, nil | ||
} | ||
|
||
func (o Loader) convertToEntities(s *openapi3.Swagger) (*loader.DataSets, error) { | ||
var ( | ||
// temporarily save the parsed data | ||
data = &loader.DataSets{} | ||
// global upstream ID | ||
globalUpstreamID = o.TaskName | ||
// global uri prefix | ||
globalPath = "" | ||
) | ||
|
||
// create upstream when servers field not empty | ||
if len(s.Servers) > 0 { | ||
var upstream entity.Upstream | ||
upstream = entity.Upstream{ | ||
BaseInfo: entity.BaseInfo{ID: globalUpstreamID}, | ||
UpstreamDef: entity.UpstreamDef{ | ||
Name: globalUpstreamID, | ||
Type: "roundrobin", | ||
}, | ||
} | ||
data.Upstreams = append(data.Upstreams, upstream) | ||
} | ||
|
||
// each one will correspond to a route | ||
for uri, v := range s.Paths { | ||
// replace parameter in uri to wildcard | ||
realUri := regURIVar.ReplaceAllString(uri, "*") | ||
// generate route Name | ||
routeName := o.TaskName + "_" + strings.TrimPrefix(uri, "/") | ||
nic-6443 marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can follow APISIX's schema to check resources. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tao12345666333 Yes, the ability to check the schema is already built into the storage layer, so no matter what APISIX resource we try to create, it will perform the schema check. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BTW, I'm still not sure on the route name generation and can see from #2460 (comment), do you have any suggestions? |
||
|
||
// decide whether to merge multi-method routes based on configuration | ||
if o.MergeMethod { | ||
bzp2010 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// create a single route for each path, merge all methods | ||
route := generateBaseRoute(routeName, v.Summary) | ||
route.Uris = []string{globalPath + realUri} | ||
route.UpstreamID = globalUpstreamID | ||
for method := range v.Operations() { | ||
route.Methods = append(route.Methods, strings.ToUpper(method)) | ||
} | ||
data.Routes = append(data.Routes, route) | ||
} else { | ||
// create routes for each method of each path | ||
for method, operation := range v.Operations() { | ||
subRouteID := routeName + "_" + method | ||
route := generateBaseRoute(subRouteID, operation.Summary) | ||
route.Uris = []string{globalPath + realUri} | ||
route.Methods = []string{strings.ToUpper(method)} | ||
route.UpstreamID = globalUpstreamID | ||
data.Routes = append(data.Routes, route) | ||
} | ||
} | ||
} | ||
return data, nil | ||
} | ||
|
||
// Generate a base route for customize | ||
func generateBaseRoute(name string, desc string) entity.Route { | ||
return entity.Route{ | ||
Name: name, | ||
Desc: desc, | ||
Plugins: make(map[string]interface{}), | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one or more | ||
* contributor license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright ownership. | ||
* The ASF licenses this file to You 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 openapi3 | ||
|
||
import ( | ||
"io/ioutil" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/assert" | ||
|
||
"github.com/apisix/manager-api/internal/core/entity" | ||
) | ||
|
||
var ( | ||
TestAPI101 = "../../../../../test/testdata/import/Postman-API101.yaml" | ||
nic-6443 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
|
||
// Test API 101 on no MergeMethod mode | ||
func TestParseAPI101NoMerge(t *testing.T) { | ||
fileContent, err := ioutil.ReadFile(TestAPI101) | ||
assert.NoError(t, err) | ||
|
||
l := &Loader{MergeMethod: false, TaskName: "test"} | ||
data, err := l.Import(fileContent) | ||
assert.NoError(t, err) | ||
|
||
assert.Len(t, data.Routes, 5) | ||
assert.Len(t, data.Upstreams, 1) | ||
|
||
// Upstream | ||
assert.Equal(t, "test", data.Upstreams[0].Name) | ||
assert.Equal(t, "roundrobin", data.Upstreams[0].Type) | ||
|
||
// Route | ||
assert.Equal(t, data.Upstreams[0].ID, data.Routes[0].UpstreamID) | ||
for _, route := range data.Routes { | ||
switch route.Name { | ||
case "test_customers_GET": | ||
assert.Contains(t, route.Uris, "/customers") | ||
assert.Contains(t, route.Methods, "GET") | ||
assert.Equal(t, "Get all customers", route.Desc) | ||
assert.Equal(t, entity.Status(0), route.Status) | ||
case "test_customer_GET": | ||
assert.Contains(t, route.Uris, "/customer") | ||
assert.Contains(t, route.Methods, "GET") | ||
assert.Equal(t, "Get one customer", route.Desc) | ||
assert.Equal(t, entity.Status(0), route.Status) | ||
case "test_customer_POST": | ||
assert.Contains(t, route.Uris, "/customer") | ||
assert.Contains(t, route.Methods, "POST") | ||
assert.Equal(t, "Add new customer", route.Desc) | ||
assert.Equal(t, entity.Status(0), route.Status) | ||
case "test_customer/{customer_id}_PUT": | ||
assert.Contains(t, route.Uris, "/customer/*") | ||
assert.Contains(t, route.Methods, "PUT") | ||
assert.Equal(t, "Update customer", route.Desc) | ||
assert.Equal(t, entity.Status(0), route.Status) | ||
case "test_customer/{customer_id}_DELETE": | ||
assert.Contains(t, route.Uris, "/customer/*") | ||
assert.Contains(t, route.Methods, "DELETE") | ||
assert.Equal(t, "Remove customer", route.Desc) | ||
assert.Equal(t, entity.Status(0), route.Status) | ||
default: | ||
t.Fatal("bad route name exist") | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should add the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. default branch added |
||
} | ||
} | ||
|
||
// Test API 101 on MergeMethod mode | ||
func TestParseAPI101Merge(t *testing.T) { | ||
fileContent, err := ioutil.ReadFile(TestAPI101) | ||
assert.NoError(t, err) | ||
|
||
l := &Loader{MergeMethod: true, TaskName: "test"} | ||
data, err := l.Import(fileContent) | ||
assert.NoError(t, err) | ||
|
||
assert.Len(t, data.Routes, 3) | ||
assert.Len(t, data.Upstreams, 1) | ||
|
||
// Upstream | ||
assert.Equal(t, "test", data.Upstreams[0].Name) | ||
assert.Equal(t, "roundrobin", data.Upstreams[0].Type) | ||
|
||
// Route | ||
assert.Equal(t, data.Upstreams[0].ID, data.Routes[0].UpstreamID) | ||
for _, route := range data.Routes { | ||
switch route.Name { | ||
case "test_customer": | ||
assert.Contains(t, route.Uris, "/customer") | ||
assert.Contains(t, route.Methods, "GET", "GET") | ||
assert.Equal(t, entity.Status(0), route.Status) | ||
case "test_customers": | ||
assert.Contains(t, route.Uris, "/customers") | ||
assert.Contains(t, route.Methods, "GET") | ||
assert.Equal(t, entity.Status(0), route.Status) | ||
case "test_customer/{customer_id}": | ||
assert.Contains(t, route.Uris, "/customer/*") | ||
assert.Contains(t, route.Methods, "PUT", "DELETE") | ||
assert.Equal(t, entity.Status(0), route.Status) | ||
default: | ||
t.Fatal("bad route name exist") | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one or more | ||
* contributor license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright ownership. | ||
* The ASF licenses this file to You 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 openapi3 | ||
|
||
import ( | ||
"regexp" | ||
|
||
"github.com/getkin/kin-openapi/openapi3" | ||
) | ||
|
||
type OpenAPISpecFileType string | ||
|
||
type Loader struct { | ||
// MergeMethod indicates whether to merge routes when multiple HTTP methods are on the same path | ||
MergeMethod bool | ||
// TaskName indicates the name of current import/export task | ||
TaskName string | ||
tokers marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
type PathValue struct { | ||
Method string | ||
Value *openapi3.Operation | ||
} | ||
|
||
var ( | ||
regURIVar = regexp.MustCompile(`{.*?}`) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the taskName as upstream ID ?
It looks strange.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The taskname can be set to a service name, such as
user
ororder
, so that it will generate an upstream namedorder
and generate a route name similar toorder_user/list
. I think this is clearer.