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

Open API 3 Schema #9

Closed
coryodaniel opened this issue Dec 20, 2018 · 12 comments · Fixed by #149
Closed

Open API 3 Schema #9

coryodaniel opened this issue Dec 20, 2018 · 12 comments · Fixed by #149
Milestone

Comments

@coryodaniel
Copy link
Owner

coryodaniel commented Dec 20, 2018

Not sure if validation itself should be a part of Bonny since kubernetes >1.13 will validate the HTTP operation.

We should support generating the Open API as a part of the CRD though. This will help with kubectl explain my-resource, enable validation at the kube API, and allow use to generate resource manifests #10

It could be very similar to how phoenix generates models:

mix bonny.gen.controller Foo foos name:string fav_number:integer fav_color:string

Open API 3 Schema Validation

Elixir Library

Include schema in #26

@mindreframer
Copy link
Contributor

mindreframer commented Jan 4, 2019

also possible alternative:
https://github.com/jonasschmidt/ex_json_schema

simpler API, less functionality (in a good way), less dependencies.

@coryodaniel
Copy link
Owner Author

Ive been bike shedding on this one a bit. Kube 1.13+ will do the validation for the custom resource on creation, so I'm not positive if we'd even need a library. Ive been wondering if its worth while to add validation into the lifecycle and for <1.13.

I was kinda rolling this up with the prereq for #10, but we could get away with just enough logic to reflect on the OAI spec enough that its possible to generate a YAML resource template 🤷‍♂️

@mindreframer
Copy link
Contributor

sounds great, all for offloading responsibility. 👍 we can always pick this up when this becomes a pain point

@mindreframer
Copy link
Contributor

mindreframer commented Jan 4, 2019

a bit offtopic: we should have labels with prios / or difficulty levels :)

@coryodaniel coryodaniel changed the title Open API 3 Schema Validation Open API 3 Schema Jan 5, 2019
@coryodaniel
Copy link
Owner Author

Started adding some labels!

@FreedomBen
Copy link
Contributor

FWIW, lacking CRD validation/definition is the only thing that has made Elixir/Bonny a much harder sell for operator development than the operator sdk. For me personally it's worth giving that up to get to use such a pleasant framework, but it would be really great to have.

@mruoss
Copy link
Collaborator

mruoss commented Feb 2, 2022

Do I get this wrong or what is really missing is bonny generating CRD of apiVersion apiextensions.k8s.io/v1 (instead of v1beta1)? If we follow that path - just brainstorming - we'd have to have a way to define types of the data fields (string, integer, object, ...). Elixir not being typed does not help here. But I guess typespecs could be the way to go, no? If the manifest generator is used, resources would have to be defined with typespecs so the generator can define the OpenApiV3Schema in the yaml...

CRD v1 documentation: https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/

in other words (i.e. brainstorming more): How do we define the schema of the CRD in the controller? If I'd have to write a ton of module constants, I'd rather just write the yaml directly... hence the idea of somehow leveraging typespecs...

@FreedomBen
Copy link
Contributor

Yeah great thoughts mruoss. I'm planning to fix #117 in a bit because v1beta1 is now removed from k8s so it's a matter of time until it starts breaking. (and for me I can't even get off the ground because we're on k8s 1.21). I'll definitely put some thought into this as well while I'm at it. I may even do an experiment and see how it feels.

@mruoss
Copy link
Collaborator

mruoss commented Sep 1, 2022

Alright, with #143 we now generate a valid apiextensions.k8s.io/v1 CRD. But still without the OpenAPI schema... I've wrapped my head around using typespecs - not a good idea. You can't define a schema with typespecs. I.e. you can't define map keys in typespecs - only their types.

I guess we need to be able to define the whole schema as a plain map in the controller somewhere. OR - I'm still a fan of alternatively letting the user define the CRD YAML first and let the controller use that to create the watcher/reconciler streams.

@sleipnir
Copy link
Contributor

sleipnir commented Sep 1, 2022

Alright, with #143 we now generate a valid apiextensions.k8s.io/v1 CRD. But still without the OpenAPI schema... I've wrapped my head around using typespecs - not a good idea. You can't define a schema with typespecs. I.e. you can't define map keys in typespecs - only their types.

I guess we need to be able to define the whole schema as a plain map in the controller somewhere. OR - I'm still a fan of alternatively letting the user define the CRD YAML first and let the controller use that to create the watcher/reconciler streams.

Contract first approach is good but it transfers the yaml creation to the user and not all users may have complete mastery of the correct format. We should add good documentation and examples for those who don't know how to set their types to the correct pattern

@mruoss
Copy link
Collaborator

mruoss commented Sep 1, 2022

Contract first approach is good but it transfers the yaml creation to the user and not all users may have complete mastery of the correct format. We should add good documentation and examples for those who don't know how to set their types to the correct pattern

Thanks for chipping in! Is it really the case that "users" who write operators don't have mastery of the CRD YAML? In any case they are gonna have to have complete mastery of the OpenAPI V3 schema if they want to declare it. Because there's no taking that away it seems.
That being said: I don't want to turn everything around. Bonny should still be able to generate a manifest. I just think bonny cannot support all the possibilities of the resulting YAML. Otherwise we end up with thousands of switches and @constants in the code that are only used by the manifest generator.

Another idea I had (very off topic): The manifest generator could send each resource (deployment, roles, service account, crds) to a user defined callback where the user could pattern match and modify the resulting structure (e.g. add labels, annotations, OR an OpenAPI schema,...)

@sleipnir
Copy link
Contributor

sleipnir commented Sep 1, 2022

Thanks for chipping in! Is it really the case that "users" who write operators don't have mastery of the CRD YAML? In any case they are gonna have to have complete mastery of the OpenAPI V3 schema if they want to declare it. Because there's no taking that away it seems. That being said: I don't want to turn everything around. Bonny should still be able to generate a manifest. I just think bonny cannot support all the possibilities of the resulting YAML. Otherwise we end up with thousands of switches and @constants in the code that are only used by the manifest generator.

I totally agree, I just think we should be careful to educate users through good examples.

Another idea I had (very off topic): The manifest generator could send each resource (deployment, roles, service account, crds) to a user defined callback where the user could pattern match and modify the resulting structure (e.g. add labels, annotations, OR an OpenAPI schema,...)

About the second case of the yaml generator I agree that the user needs to be able to add things like resources, health checks, annotations and any other things that can be customizable

@mruoss mruoss linked a pull request Sep 5, 2022 that will close this issue
9 tasks
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 a pull request may close this issue.

5 participants