-
Notifications
You must be signed in to change notification settings - Fork 123
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
pkg/k8s/backend.go: use apiextensions/v1 #183
Conversation
This works, but I am not sure about a lot of things :( |
|
||
_, err := pb.extensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions().Create(context.TODO(), crd, metav1.CreateOptions{}) | ||
_, err := pb.extensionsClient.ApiextensionsV1().CustomResourceDefinitions().Create(context.TODO(), crd, metav1.CreateOptions{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we try printing this to the console and sharing here so we can see what the generated CRD looks like? Alternatively, we can get a dump of kubectl get crd peer -o yaml
. We should ensure that the CRD spec has all of the necessary OpenAPI v3 validation stuff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v1beta1:
&CustomResourceDefinition{ObjectMeta:{peers.kilo.squat.ai 0 0001-01-01 00:00:00 +0000 UTC <nil> <nil> map[] map[] [] [] []},Spec:CustomResourceDefinitionSpec{Group:kilo.squat.ai,Version:v1alpha1,Names:CustomResourceDefinitionNames{Plural:peers,Singular:,ShortNames:[peer],Kind:Peer,ListKind:,Categories:[],},Scope:Cluster,Validation:&CustomResourceValidation{OpenAPIV3Schema:&JSONSchemaProps{ID:,Schema:,Ref:nil,Description:,Type:,Format:,Title:,Default:nil,Maximum:nil,ExclusiveMaximum:false,Minimum:nil,ExclusiveMinimum:false,MaxLength:nil,MinLength:nil,Pattern:,MaxItems:nil,MinItems:nil,UniqueItems:false,MultipleOf:nil,Enum:[]JSON{},MaxProperties:nil,MinProperties:nil,Required:[],Items:nil,AllOf:[]JSONSchemaProps{},OneOf:[]JSONSchemaProps{},AnyOf:[]JSONSchemaProps{},Not:nil,Properties:map[string]JSONSchemaProps{},AdditionalProperties:nil,PatternProperties:map[string]JSONSchemaProps{},Dependencies:JSONSchemaDependencies{},AdditionalItems:nil,Definitions:JSONSchemaDefinitions{},ExternalDocs:nil,Example:nil,Nullable:false,XPreserveUnknownFields:nil,XEmbeddedResource:false,XIntOrString:false,XListMapKeys:[],XListType:nil,XMapType:nil,},},Subresources:&CustomResourceSubresources{Status:nil,Scale:nil,},Versions:[]CustomResourceDefinitionVersion{},AdditionalPrinterColumns:[]CustomResourceColumnDefinition{},Conversion:nil,PreserveUnknownFields:nil,},Status:CustomResourceDefinitionStatus{Conditions:[]CustomResourceDefinitionCondition{},AcceptedNames:CustomResourceDefinitionNames{Plural:,Singular:,ShortNames:[],Kind:,ListKind:,Categories:[],},StoredVersions:[],},}
v1:
&CustomResourceDefinition{ObjectMeta:{peers.kilo.squat.ai 0 0001-01-01 00:00:00 +0000 UTC <nil> <nil> map[] map[] [] [] []},Spec:CustomResourceDefinitionSpec{Group:kilo.squat.ai,Names:CustomResourceDefinitionNames{Plural:peers,Singular:,ShortNames:[peer],Kind:Peer,ListKind:,Categories:[],},Scope:Cluster,Versions:[]CustomResourceDefinitionVersion{CustomResourceDefinitionVersion{Name:v1alpha1,Served:true,Storage:true,Schema:&CustomResourceValidation{OpenAPIV3Schema:&JSONSchemaProps{ID:,Schema:,Ref:nil,Description:,Type:object,Format:,Title:,Default:nil,Maximum:nil,ExclusiveMaximum:false,Minimum:nil,ExclusiveMinimum:false,MaxLength:nil,MinLength:nil,Pattern:,MaxItems:nil,MinItems:nil,UniqueItems:false,MultipleOf:nil,Enum:[]JSON{},MaxProperties:nil,MinProperties:nil,Required:[],Items:nil,AllOf:[]JSONSchemaProps{},OneOf:[]JSONSchemaProps{},AnyOf:[]JSONSchemaProps{},Not:nil,Properties:map[string]JSONSchemaProps{},AdditionalProperties:nil,PatternProperties:map[string]JSONSchemaProps{},Dependencies:JSONSchemaDependencies{},AdditionalItems:nil,Definitions:JSONSchemaDefinitions{},ExternalDocs:nil,Example:nil,Nullable:false,XPreserveUnknownFields:nil,XEmbeddedResource:false,XIntOrString:false,XListMapKeys:[],XListType:nil,XMapType:nil,},},Subresources:&CustomResourceSubresources{Status:nil,Scale:nil,},AdditionalPrinterColumns:[]CustomResourceColumnDefinition{},Deprecated:false,DeprecationWarning:nil,},},Conversion:nil,PreserveUnknownFields:false,},Status:CustomResourceDefinitionStatus{Conditions:[]CustomResourceDefinitionCondition{},AcceptedNames:CustomResourceDefinitionNames{Plural:,Singular:,ShortNames:[],Kind:,ListKind:,Categories:[],},StoredVersions:[],},}
The Version is missing in v1, but that was expected.
The openAPIv3 stuff used to be nil and is now almost nil. I only set the type to "object", but I just found out that you can set the hole thing to nil. However controller-gen generates the openAPIv3 stuff. I will look into that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The openAPIv3 stuff used to be nil and is now almost nil. I only set the type to "object", but I just found out that you can set the hole thing to nil. However controller-gen generates the openAPIv3 stuff. I will look into that.
nvm, the schema may not be empty anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, so the schema used to be nil, which is why we never had and validation by the API server. We should really improve that this time around. Not only because it improves life when using Kilo without the admission webhook but also because it enables discovery of the Kilo API via OpenAPI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this change is definitely no worse than before, so we could merge this and at least ensure Kilo works with K8s v1.22. Or we can use it as an opportunity to get better OpenAPI generation. I'm happy with either way (:
close this for #186 |
No description provided.