Add OpenAPI V2 to V3 Converter #292

Jefftree · 2022-03-22T03:43:59Z

Use the CommonResponses object in OpenAPI V3 builder. (This adds the 404 Response structs)
Create a OpenAPIV3Config struct that we can transition to in the future. Currently it is just an upsampling from the v2 Config.
Add converter from OpenAPI V2 to V3
Verify converter by generating the OpenAPI v2/v3 from go structs, and attempt to convert v2 to v3 and check for equality.

Jefftree · 2022-03-22T23:34:09Z

/cc @alexzielenski @jiahuif @apelisse

k8s-ci-robot · 2022-03-22T23:34:11Z

@Jefftree: GitHub didn't allow me to request PR reviews from the following users: alexzielenski.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @alexzielenski @jiahuif @apelisse

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

test/integration/testutil/testutil.go

pkg/common/common.go

pkg/builder3/openapi.go

alexzielenski · 2022-03-22T23:53:46Z

pkg/builder3/openapi.go

@@ -154,18 +165,23 @@ func (o *openAPI) buildRequestBody(parameters []common.Parameter, bodySample int

 func newOpenAPI(config *common.Config) openAPI {
 	o := openAPI{
-		config: config,
+		config: common.ConvertConfigToV3(config),


Is common.Config meant to be used for both OpenAPIV2 and OpenAPIV3?

I am concerned that over time the OpenAPI v3 config might diverge and no longer be automatically convertible from the common.Config. It would be painful to switch the API to take a new config type at that point. We are already starting to see differences in how we want to treat GetDefinitionName of common.Config in OpenAPI v2 vs v3 (recall kubernetes/kubernetes#106778).

Are we able guarantee to our users the the OpenAPI config will always be automatically convertible from the OpenAPI v2 config?

No, we should have separate configs for OpenAPI V2 and V3. This should apply for structs as well (spec.Info, Schema, etc).

I think making that change would be too big given our current timeline, so as a compromise I'm starting to create the V3 config but generating that from the common Config.

pkg/builder3/openapi.go

pkg/openapiconv/convert.go

test/integration/openapiconv/convert_test.go

Jefftree · 2022-03-23T02:57:23Z

/cc @sttts

pkg/builder3/openapi.go

pkg/common/common.go

test/integration/openapiconv/convert_test.go

pkg/openapiconv/convert.go

test/integration/openapiconv/convert_test.go

Jefftree · 2022-03-26T05:00:45Z

/retest
hm, the tests all pass locally

apelisse · 2022-03-26T21:02:54Z

pkg/openapiconv/convert.go

+		v3Schema.Items = new(spec.SchemaOrArray)
+		v3Schema.Items.Schema = ConvertSchema(v2Schema.Items.Schema)
+		if v2Schema.Items.Schemas != nil {
+			v3Schema.Items.Schemas = []spec.Schema{}
+			for _, s := range v2Schema.Items.Schemas {
+				v3Schema.Items.Schemas = append(v3Schema.Items.Schemas, *ConvertSchema(&s))
+			}
+		}


Not something I specifically want you to change, but you could have written this like:

v3Schema.Items := spec.SchemaOrArray { Schema: ConvertSchema(v2schema.Items.Schema), Schemas: ConvertSchemaList(v2.schema.Items.Schemas), // Move the for loop into its own function that does its own nil checks appropriately. }

You could have just said you want me to change it :)

Updated.

apelisse

OK Thanks, I'm happy with it. @sttts Do you want to take a look?

apelisse · 2022-03-27T00:19:20Z

Feel free to squash I guess?

apelisse · 2022-03-28T20:08:35Z

/lgtm
/approve

k8s-ci-robot · 2022-03-28T20:08:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: apelisse, Jefftree

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [apelisse]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 22, 2022

k8s-ci-robot requested review from roycaihw and sttts March 22, 2022 03:44

Jefftree force-pushed the v2v3conv branch from 168aa7f to dcd6870 Compare March 22, 2022 05:02

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 22, 2022

Jefftree force-pushed the v2v3conv branch 2 times, most recently from 4515942 to 6ebf5ca Compare March 22, 2022 23:30

k8s-ci-robot requested a review from jiahuif March 22, 2022 23:34

k8s-ci-robot requested a review from apelisse March 22, 2022 23:34

Jefftree commented Mar 22, 2022

View reviewed changes

test/integration/testutil/testutil.go Show resolved Hide resolved

Jefftree commented Mar 23, 2022

View reviewed changes

pkg/common/common.go Show resolved Hide resolved

Jefftree changed the title ~~Add OpenAPI V2 to V3 Converter~~ [WIP] Add OpenAPI V2 to V3 Converter Mar 23, 2022

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 23, 2022

alexzielenski reviewed Mar 23, 2022

View reviewed changes

Jefftree mentioned this pull request Mar 23, 2022

Change aggregator to proxier for OpenAPI v3 kubernetes/kubernetes#108637

Merged

apelisse reviewed Mar 23, 2022

View reviewed changes

Jefftree force-pushed the v2v3conv branch from eee48e4 to e70839d Compare March 24, 2022 02:16

Jefftree changed the title ~~[WIP] Add OpenAPI V2 to V3 Converter~~ Add OpenAPI V2 to V3 Converter Mar 24, 2022

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 24, 2022

Jefftree force-pushed the v2v3conv branch 3 times, most recently from a5c6ebb to c680601 Compare March 24, 2022 15:55

Jefftree mentioned this pull request Mar 25, 2022

Add OpenAPI v2 conversion if APIService does not publish OpenAPI V3 kubernetes/kubernetes#109010

Closed

Jefftree force-pushed the v2v3conv branch 2 times, most recently from e0d9f07 to 7d73b45 Compare March 26, 2022 03:51

apelisse reviewed Mar 26, 2022

View reviewed changes

pkg/openapiconv/convert.go Outdated Show resolved Hide resolved

Jefftree force-pushed the v2v3conv branch from 7d73b45 to e80060d Compare March 26, 2022 04:30

apelisse reviewed Mar 26, 2022

View reviewed changes

test/integration/openapiconv/convert_test.go Show resolved Hide resolved

Jefftree force-pushed the v2v3conv branch 3 times, most recently from 28f39c4 to 0c9bc77 Compare March 26, 2022 20:16

apelisse reviewed Mar 26, 2022

View reviewed changes

apelisse reviewed Mar 27, 2022

View reviewed changes

Add OpenAPIConv

2cbad14

Jefftree force-pushed the v2v3conv branch from 232f4c0 to 2cbad14 Compare March 27, 2022 00:45

Jefftree mentioned this pull request Mar 28, 2022

Optimize GetOpenAPIDefinitions to avoid generating the map for all definitions on every BuildOpenAPISpec invocation #299

Merged

k8s-ci-robot assigned apelisse Mar 28, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 28, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 28, 2022

k8s-ci-robot merged commit 3a31a64 into kubernetes:master Mar 28, 2022

alexzielenski mentioned this pull request Mar 29, 2022

REQUEST: New membership for alexzielenski kubernetes/org#3343

Closed

9 tasks

Jefftree deleted the v2v3conv branch March 21, 2023 15:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add OpenAPI V2 to V3 Converter #292

Add OpenAPI V2 to V3 Converter #292

Jefftree commented Mar 22, 2022 •

edited

Loading

Jefftree commented Mar 22, 2022

k8s-ci-robot commented Mar 22, 2022

alexzielenski Mar 22, 2022

Jefftree Mar 23, 2022

Jefftree commented Mar 23, 2022

Jefftree commented Mar 26, 2022

apelisse Mar 26, 2022 •

edited

Loading

Jefftree Mar 26, 2022

apelisse left a comment

apelisse commented Mar 27, 2022 •

edited

Loading

apelisse commented Mar 28, 2022

k8s-ci-robot commented Mar 28, 2022

Add OpenAPI V2 to V3 Converter #292

Add OpenAPI V2 to V3 Converter #292

Conversation

Jefftree commented Mar 22, 2022 • edited Loading

Jefftree commented Mar 22, 2022

k8s-ci-robot commented Mar 22, 2022

alexzielenski Mar 22, 2022

Choose a reason for hiding this comment

Jefftree Mar 23, 2022

Choose a reason for hiding this comment

Jefftree commented Mar 23, 2022

Jefftree commented Mar 26, 2022

apelisse Mar 26, 2022 • edited Loading

Choose a reason for hiding this comment

Jefftree Mar 26, 2022

Choose a reason for hiding this comment

apelisse left a comment

Choose a reason for hiding this comment

apelisse commented Mar 27, 2022 • edited Loading

apelisse commented Mar 28, 2022

k8s-ci-robot commented Mar 28, 2022

Jefftree commented Mar 22, 2022 •

edited

Loading

apelisse Mar 26, 2022 •

edited

Loading

apelisse commented Mar 27, 2022 •

edited

Loading