microsoft / kiota

OpenAPI based HTTP Client code generator
https://aka.ms/kiota/docs
MIT License
2.86k stars 194 forks source link

[Epic] Validation rules for Plugin Generation in Kiota #5162

Open maisarissi opened 1 month ago

maisarissi commented 1 month ago

We need to enhance the plugin generation process in Kiota by implementing validation rules. These validations will ensure that the generated plugins function correctly. Some of these validations address current limitations that may be resolved in the future. However, it is crucial to ensure that the generated plugins work with the current constraints.

### Tasks
- [ ] https://github.com/microsoft/Microsoft.Plugins.Manifest/issues/255
- [ ] https://github.com/microsoft/kiota/issues/5164
- [ ] https://github.com/microsoft/kiota/issues/5381
- [ ] https://github.com/microsoft/Microsoft.Plugins.Manifest/issues/259
- [ ] https://github.com/microsoft/Microsoft.Plugins.Manifest/issues/256
- [ ] https://github.com/microsoft/Microsoft.Plugins.Manifest/issues/257
- [ ] https://github.com/microsoft/kiota/issues/5380
- [ ] https://github.com/microsoft/Microsoft.Plugins.Manifest/issues/258

Requirements

maisarissi commented 1 month ago

Tasks:

Can be resolved with https://github.com/microsoft/kiota/issues/5071

baywet commented 1 month ago

I do have a bunch of questions about those rules...

Nested objects in response or parameters in API methods are not supported

What qualifies as nested object? an inline object type definition? or are you referring to properties that are of object types in general? (e.g. assigned licenses for user in Graph)

Polymorphic references in open API spec (oneOf, allOf, anyOf) are not supported

This is how we define inheritance (entity<-directory object<- user), which would in effect rule out most API definitions out there. We need to be much more specific here.

Circular references in open API spec are not supported

Likewise we have plenty of those in Microsoft Graph. E.g. a User has a manager property which is a user... Also should we consider multiple degrees of separation? (e.g. user.memberof ->group, group.members -> user)

Only authorization code and PKCE are supported as OAuth grant flows

Providing examples would help here.

API Key in custom headers, query params or cookies are not supported

Examples please.

Dual authentication flows (OAuth/Entra SSO + http Bearer token) for single API endpoint

Examples please.

OpenAPI descriptions needs to be 3.1.0 or previous versions

Considering 3.1.0 is not supported yet by OAI.net we should revisit this one for our current timelines.

Server url should be an absolute url with https protocol

Or in simple terms "should start with https://, case insensitive"

petrhollayms commented 1 month ago

@maisarissi Shall we move it from Proposed to Todo? (meaning, is it a must-have for GA?)

maisarissi commented 1 month ago

@baywet I've added the examples and details around the validations in each of the new issues created.

After talking to @darrelmiller, I've created validation issues in the validation library. On top of the validations in the validation lib, linked to this epic, In Kiota we should still validate:

If any above validation fails, we need throw an error.

For inheritance (allOf, oneOf, anyOf) in request bodies, I've created a new issue for Kiota to handle the scenario: https://github.com/microsoft/kiota/issues/5164

For the circular reference we should list all selected paths that are causing the error. I'm good with moving to Todo @petrhollayms

petrhollayms commented 2 weeks ago

@sebastienlevert To confirm- OpenAPI version 3.1.0 shall throw an error, given that it is not yet supported in OpenAPI.NET library, right?

Support for 3.1 https://github.com/microsoft/OpenAPI.NET/issues/795 will be released in the upcoming 2.0 version.

sebastienlevert commented 2 weeks ago

Absolutely.

calebkiage commented 2 weeks ago

The OpenAPI.NET throws an exception on this

petrhollayms commented 2 weeks ago

Error shall be shown to the user when using 3.1

calebkiage commented 2 weeks ago

Can we have more description for

Circular references in open API spec are not supported

petrhollayms commented 2 weeks ago

@darrelmiller Do we have any better description or examples? It shall not be too restrictive now, I see some risk in there.

darrelmiller commented 2 weeks ago

I have no idea what is meant by Circular references are not supported.

I can imagine that if you try to create a plugin with a request payload that has a schema that is self referencing then Semantic Kernel will not be able to call that API. That is probably because SK cannot support nested objects that contain parameters that have the same property names. This is a bug they are planning to fix in the next sprint.

petrhollayms commented 2 weeks ago

Thanks @darrelmiller , so suggesting we keep it for post-GA and re-evaluate later on based on the feedback from users.