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

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

Closed
8 tasks done
Tracked by #5021
maisarissi opened this issue Aug 14, 2024 · 13 comments · May be fixed by #5691
Closed
8 tasks done
Tracked by #5021

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

maisarissi opened this issue Aug 14, 2024 · 13 comments · May be fixed by #5691

Comments

@maisarissi
Copy link
Contributor

maisarissi commented Aug 14, 2024

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

Preview Give feedback
  1. WIP
    calebkiage
  2. blocked
    calebkiage maisarissi
  3. vscode-extension
    calebkiage thewahome

Requirements

  • Requirement 1: Validate all the above rules before generating a plugin and only generate the plugin if there are no issues.
  • Requirement 2: Warn the user if their OpenAPI is not valid for plugin creation and don't generate the plugin
  • Requirement 3: Inform the user which selected endpoints are failing validation and the specific reason. For example, one has 10 endpoints selected to create a plugin and just 1 fails due to nested objects in the PUT method; notify of the specific endpoints and the reason for the failure.
@maisarissi
Copy link
Contributor Author

maisarissi commented Aug 14, 2024

Tasks:

  • 4 Only authorization code are supported as OAuth grant flows
  • 5 API Key in custom headers, query params or cookies are not supported
  • 6 Dual authentication flows (API Key AND OAuth token) for single API endpoint are not supported

Can be resolved with #5071

@baywet
Copy link
Member

baywet commented Aug 14, 2024

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"

@andrueastman andrueastman added needs more information and removed status:waiting-for-triage An issue that is yet to be reviewed or assigned labels Aug 15, 2024
@andrueastman andrueastman moved this from Needs Triage 🔍 to In Review 💭 in Kiota Aug 15, 2024
@andrueastman andrueastman moved this from In Review 💭 to Proposed 💡 in Kiota Aug 15, 2024
@petrhollayms
Copy link
Contributor

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

@maisarissi
Copy link
Contributor Author

maisarissi commented Aug 23, 2024

@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:

  • The OpenAPI description version needs to be < 3.1.0
  • There can't be circular reference in the selected paths by the user

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: #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 petrhollayms moved this from Proposed 💡 to Todo 📃 in Kiota Sep 2, 2024
@petrhollayms petrhollayms moved this from Todo 📃 to In Progress 🚧 in Kiota Sep 4, 2024
@petrhollayms petrhollayms changed the title Validation rules for Plugin Generation in Kiota [Epic] Validation rules for Plugin Generation in Kiota Sep 4, 2024
@petrhollayms petrhollayms added this to the Kiota v1.18 milestone Sep 5, 2024
@baywet baywet modified the milestones: Kiota v1.18, Kiota v1.19 Sep 5, 2024
@petrhollayms
Copy link
Contributor

@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 microsoft/OpenAPI.NET#795 will be released in the upcoming 2.0 version.

@sebastienlevert
Copy link
Contributor

Absolutely.

@calebkiage
Copy link
Contributor

The OpenAPI.NET throws an exception on this

@petrhollayms
Copy link
Contributor

Error shall be shown to the user when using 3.1

@calebkiage
Copy link
Contributor

Can we have more description for

Circular references in open API spec are not supported

@petrhollayms
Copy link
Contributor

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

@darrelmiller
Copy link
Member

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
Copy link
Contributor

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

@maisarissi
Copy link
Contributor Author

Closing this epic as the last item remaining is the circular reference and this might be fixed by SK when they support nested objects that contain parameters that have the same property names.

@github-project-automation github-project-automation bot moved this from In Progress 🚧 to Done ✔️ in Kiota Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants