Skip to content
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

Support +mapType, +structType markers #178

Merged
merged 7 commits into from
Jan 30, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions pkg/generators/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,16 @@ var tagToExtension = map[string]extensionAttributes{
kind: types.Slice,
allowedValues: sets.NewString("atomic", "set", "map"),
},
"mapType": {
xName: "x-kubernetes-map-type",
kind: types.Map,
allowedValues: sets.NewString("atomic", "granular"),
},
"structType": {
xName: "x-kubernetes-map-type",
kind: types.Struct,
allowedValues: sets.NewString("atomic", "granular"),
},
}

// Extension encapsulates information necessary to generate an OpenAPI extension.
Expand Down
58 changes: 58 additions & 0 deletions pkg/generators/extension_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ func TestSingleTagExtension(t *testing.T) {
extensionName: "x-kubernetes-list-map-keys",
extensionValues: []string{"port"},
},
{
comments: []string{"+mapType=granular"},
extensionTag: "mapType",
extensionName: "x-kubernetes-map-type",
extensionValues: []string{"granular"},
},
{
comments: []string{"+k8s:openapi-gen=x-kubernetes-member-tag:member_test"},
extensionTag: "k8s:openapi-gen",
Expand Down Expand Up @@ -205,6 +211,18 @@ func TestExtensionAllowedValues(t *testing.T) {
},
allowedValues: nil,
},
{
e: extension{
idlTag: "mapType",
},
allowedValues: sets.NewString("atomic", "granular"),
},
{
e: extension{
idlTag: "structType",
},
allowedValues: sets.NewString("atomic", "granular"),
},
{
e: extension{
idlTag: "k8s:openapi-gen",
Expand Down Expand Up @@ -259,6 +277,20 @@ func TestExtensionAllowedValues(t *testing.T) {
values: []string{"atomic"},
},
},
{
e: extension{
idlTag: "mapType",
xName: "x-kubernetes-map-type",
values: []string{"atomic"},
},
},
{
e: extension{
idlTag: "structType",
xName: "x-kubernetes-map-type",
values: []string{"granular"},
},
},
}
for _, test := range successTests {
actualErr := test.e.validateAllowedValues()
Expand Down Expand Up @@ -292,6 +324,20 @@ func TestExtensionAllowedValues(t *testing.T) {
values: []string{"not-allowed"},
},
},
{
e: extension{
idlTag: "mapType",
xName: "x-kubernetes-map-type",
values: []string{"something-pretty-wrong"},
},
},
{
e: extension{
idlTag: "structType",
xName: "x-kubernetes-map-type",
values: []string{"not-quite-right"},
},
},
}
for _, test := range failureTests {
actualErr := test.e.validateAllowedValues()
Expand Down Expand Up @@ -326,6 +372,18 @@ func TestExtensionKind(t *testing.T) {
},
kind: types.Slice,
},
{
e: extension{
idlTag: "mapType",
},
kind: types.Map,
},
{
e: extension{
idlTag: "structType",
},
kind: types.Struct,
},
{
e: extension{
idlTag: "listMapKey",
Expand Down
22 changes: 13 additions & 9 deletions pkg/idl/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,15 @@ type ListType string
type ListMapKey string

// MapType annotates a map to further describe its topology. It may
// have only one value: "atomic". Atomic means that the entire map is
// considered as a whole, rather than as distinct values.
// have one of two values: `atomic` or `granular`. `atomic` means that the entire map is
// considered as a whole; actors that wish to update the map can only
// entirely replace it. `granular` means that specific values in the map can be
// updated separately from other fields.
//
// By default, a map will be considered as a set of distinct values that
// can be updated individually. This default WILL NOT generate any
// openapi extension, as this will also be interpreted as the default
// behavior in the openapi definition.
// can be updated individually (i.e. the equivalent of `granular`).
// This default WILL NOT generate any openapi extension, as this will also
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which doc are you referring to?

This doc :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We talked about it during the meeting, but making my comment "official". I think it's strictly better to have the "granular" explicitly in the openapi than rely on the missing value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 👌 (for structType too).

// be interpreted as the default behavior in the openapi definition.
//
// This tag MUST only be used on maps, or the generation step will fail.
//
Expand Down Expand Up @@ -114,11 +116,13 @@ type PatchMergeKey string
type PatchStrategy string

// StructType annotates a struct to further describe its topology. It may
// have only one value: "atomic". Atomic means that the entire struct is
// considered as a whole, rather than as distinct values.
// have one of two values: `atomic` or `granular`. `atomic` means that the entire struct is
// considered as a whole; actors that wish to update the struct can only
// entirely replace it. `granular` means that specific fields in the struct can be
// updated separately from other fields.
//
// By default, a struct will be considered as a set of distinct values that
// can be updated individually. This default WILL NOT generate any
// can be updated individually (`granular`). This default WILL NOT generate any
// openapi extension, as this will also be interpreted as the default
// behavior in the openapi definition.
//
Expand All @@ -133,7 +137,7 @@ type PatchStrategy string
// used on any struct.
//
// Using this tag will generate the following OpenAPI extension:
// "x-kubernetes-struct-type": "atomic"
// "x-kubernetes-map-type": "atomic"
mariantalla marked this conversation as resolved.
Show resolved Hide resolved
type StructType string

// Union is TBD.
Expand Down
10 changes: 5 additions & 5 deletions test/integration/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,26 @@ $ go test -v .
First, run the generator to create `openapi_generated.go` file which specifies
the `OpenAPIDefinition` for each type, and generate the golden API rule
violation report file . Note that if you do not pass a report
filename (`./testdata/golden.report` in the command below) to let the generator
filename (`./testdata/golden.v2.report` in the command below) to let the generator
mariantalla marked this conversation as resolved.
Show resolved Hide resolved
to print API rule violations to the file, the generator will return error to stderr
on API rule violations.

```bash
$ go run ../../cmd/openapi-gen/openapi-gen.go \
-i "k8s.io/kube-openapi/test/integration/testdata/listtype,k8s.io/kube-openapi/test/integration/testdata/dummytype,k8s.io/kube-openapi/test/integration/testdata/uniontype" \
-i "k8s.io/kube-openapi/test/integration/testdata/listtype,k8s.io/kube-openapi/test/integration/testdata/maptype,k8s.io/kube-openapi/test/integration/testdata/structtype,k8s.io/kube-openapi/test/integration/testdata/dummytype,k8s.io/kube-openapi/test/integration/testdata/uniontype" \
-o pkg \
-p generated \
-O openapi_generated \
-r ./testdata/golden.report
-r ./testdata/golden.v2.report
```
The generated file `pkg/generated/openapi_generated.go` should have been created.

Next, run the OpenAPI builder to create the Swagger file which includes
the definitions. The output file named `golden.json` will be output in
the definitions. The output file named `golden.v2.json` will be output in
the current directory.

```bash
$ go run builder/main.go testdata/golden.json
$ go run builder/main.go testdata/golden.v2.json
```

After the golden spec is generated, please clean up the generated file
Expand Down
4 changes: 4 additions & 0 deletions test/integration/builder/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ func createWebServices() []*restful.WebService {
w.Route(buildRouteForType(w, "custom", "Bak"))
w.Route(buildRouteForType(w, "custom", "Bac"))
w.Route(buildRouteForType(w, "custom", "Bah"))
w.Route(buildRouteForType(w, "maptype", "GranularMap"))
w.Route(buildRouteForType(w, "maptype", "AtomicMap"))
w.Route(buildRouteForType(w, "structtype", "GranularStruct"))
w.Route(buildRouteForType(w, "structtype", "AtomicStruct"))
return []*restful.WebService{w}
}

Expand Down
2 changes: 2 additions & 0 deletions test/integration/integration_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ const (
testdataDir = "./testdata"
testPkgDir = "k8s.io/kube-openapi/test/integration/testdata"
inputDir = testPkgDir + "/listtype" +
"," + testPkgDir + "/maptype" +
"," + testPkgDir + "/structtype" +
"," + testPkgDir + "/dummytype" +
"," + testPkgDir + "/uniontype" +
"," + testPkgDir + "/custom"
Expand Down
170 changes: 141 additions & 29 deletions test/integration/pkg/generated/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading