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

Resolves #1074 - Update openapi.json bindings #1095

Merged
merged 5 commits into from
Aug 10, 2023
Merged

Resolves #1074 - Update openapi.json bindings #1095

merged 5 commits into from
Aug 10, 2023

Conversation

david-rocca
Copy link
Collaborator

Closes Issue #1074

Summary

Updated all put / post endpoints for /cve to update the openapi.json bindings / swagger docs.

Testing

Steps to manually test updated functionality, if possible

  • 1) Use the tool of your choice to parse the openapi.json to create bindings.

@david-rocca
Copy link
Collaborator Author

Rebased to dev.

@gwd
Copy link

gwd commented Jul 21, 2023

Thanks for this. When I use openapi to generate the bindings, I get the following errors (repeated several times):

[main] WARN  o.o.codegen.utils.ModelUtils - #/definitions/cnaPublishedContainer is not defined
[main] WARN  o.o.codegen.utils.ModelUtils - #/definitions/adpContainer is not defined

And when I then go into the directory and run go build, I get the following errors, which match the errors above:

./model_published_1.go:22:16: undefined: AdpContainer
./model_published.go:22:16: undefined: CnaPublishedContainer
./model_published.go:43:39: undefined: CnaPublishedContainer
./model_published.go:53:43: undefined: CnaPublishedContainer

Note that it's just those two which are missing. Unfortunately I don't understand how the system works enough to be able to figure out why it can find (for instance) [Pp]roviderMetadata, but not [Cc]naPublishedContainer.

To run openapi-generator, I had to first run a tiny web server from the root directory of the cve-services repo:

$ cd cve-services
$ python3 -m http.server
Serving HTTP on :: port 8000 (http://[::]:8000/) ...

Then I executed the following command:

$  openapi-generator generate -i http://localhost:8000/api-docs/openapi.json -g go -o go-cveapi --package-name cveapi --skip-validate-spec

@david-rocca
Copy link
Collaborator Author

Thanks for trying this. One point of clarification, if you start the simple http server, do the bindings generate or is it the same missing imports?

@gwd
Copy link

gwd commented Jul 21, 2023

Thanks for the quick response; unfortunately I don't really understand the question.

If I don't use the webserver, and instead give it the file directly, then it doesn't work: it can't resolve the relative references.

If I do use the webserver, then it no longer complains that it can't find the references; it manages to find the files, but there are exactly two definitions it claims not to be able to find. Lots of code is generated, including lots of structures, functions, tests, and documentation; but the tests don't run, because it seems to be missing those two structures.

Let me know if you want me to record all the output in a snippet somewhere.

@david-rocca
Copy link
Collaborator Author

Your answer helps me quite a bit. I'll do a bit of digging to see if I can fix the includes. I have an idea of what I can do. Thanks!

@david-rocca
Copy link
Collaborator Author

david-rocca commented Jul 21, 2023

@gwd I was able to reproduce the exact errors you mentioned. I pushed a fix that should fix the import warnings that were being created.

I installed golang on my machine and tried to do a quick go test, and I am not quite sure what to make of the output as I am not very familiar with go. At your convince, please feel free to give it another try and see if you have better luck. Either way I think this is a step in the right direction.

@gwd
Copy link

gwd commented Jul 28, 2023

Sorry for the delay; I twice tried to respond to this issue and
somehow managed to lose the content of the comment each time.

Yes, that gets rid of the "is not defined" warnings.

I now get these warnings:

[main] WARN  o.o.codegen.DefaultCodegen - allOf with multiple schemas defined. Using only the first one: AnyType
...
[main] WARN  o.o.codegen.DefaultCodegen - allOf with multiple schemas defined. Using only the first one: AnyType
...
[main] WARN  o.o.codegen.DefaultCodegen - reference_tags_inner (oneOf schema) already has `string` defined and therefore it's skipped.
[main] WARN  o.o.codegen.DefaultCodegen - metrics_inner (anyOf schema) already has `interface{}` defined and therefore it's skipped.
[main] WARN  o.o.codegen.DefaultCodegen - metrics_inner (anyOf schema) already has `interface{}` defined and therefore it's skipped.
[main] WARN  o.o.codegen.DefaultCodegen - metrics_inner (anyOf schema) already has `interface{}` defined and therefore it's skipped.
[main] WARN  o.o.codegen.DefaultCodegen - cnaTags_inner (oneOf schema) already has `string` defined and therefore it's skipped.

When I build, I get the errors below. (FYI the Go compiler automatically
stops after 10 errors, since you normally only look at the first
handful of compilation errors anyway.)

./model_metrics_inner.go:20:2: syntax error: unexpected interface, expected field name or embedded type
./model_metrics_inner.go:27:34: syntax error: unexpected interface, expected name or (
./model_metrics_inner.go:29:42: syntax error: unexpected interface, expected name or (
./model_metrics_inner.go:31:8: syntax error: unexpected interface, expected name or (
./model_metrics_inner.go:36:7: syntax error: unexpected interface, expected name or (
./model_metrics_inner.go:44:9: syntax error: unexpected interface, expected name or (
./model_metrics_inner.go:45:3: syntax error: unexpected return, expected {
./model_metrics_inner.go:48:2: syntax error: non-declaration statement outside function body

Looking at the context of the first one:

// MetricsInner struct for MetricsInner
type MetricsInner struct {
        interface{} *interface{}
}

This is gibberish. interface{} is a type, and it's being used as
a field name for the struct. The other errors are basically
variations on a theme; for example:

        // try to unmarshal JSON data into interface{}
        err = json.Unmarshal(data, &dst.interface{});

It's trying to use interface{} as a field name of dst, which is of
type MetricsInner; but of course it doesn't work because it's a
type.

FWIW it looks like that code is present before #ef9c8e3; the change
didn't introduce the error, but by fixing the other error exposed this
one.

I thought it might have something to do with the fact that Golang
doesn't have sum types; but you get similar problems if you try to
generate Rust (which does have sum types):

error[E0412]: cannot find type `OneOfLessThanAnyTypeCommaAnyTypeGreaterThan` in module `crate::models`
  --> src/models/product.rs:39:45
   |
39 |     pub versions: Option<Vec<crate::models::OneOfLessThanAnyTypeCommaAnyTypeGreaterThan>>,
   |                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in `crate::models`

error[E0412]: cannot find type `OneOfLessThanAnyTypeCommaAnyTypeGreaterThan` in module `crate::models`
  --> src/models/product_1.rs:39:45
   |
39 |     pub versions: Option<Vec<crate::models::OneOfLessThanAnyTypeCommaAnyTypeGreaterThan>>,
   |                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in `crate::models`

I think I tried to generate Python bindings on Monday too, and also had errors (IIRC).

FWIW:

  • On the client side, anyOf, allOf, and oneOf only seem to be used in the new code you introduced
  • The examples I found in the OpenAPI docs for how to use the above don't look much like the instances in the code you introduced

But I definitely don't understand much about the schemas, and haven't had time to dig into it.

Did you mention that some other client bindings generator worked for
you? Which one was that?

I'm happy to try to continue testing it for you, but in the mean time I
think I'll just write what I need to by hand; I don't expect the
surface area of the interface I need to be that large.

@jdaigneau5
Copy link
Collaborator

This at least makes the bindings more complete than before. Merging this and will capture remaining concerns in a separate ticket.

@jdaigneau5 jdaigneau5 merged commit 0d4a7c4 into dev Aug 10, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants