Common base class for Kubernetes resource models

[tintoy]

Hi.

I was wondering if it might be useful to have a shared base class for models representing Kubernetes resources / resource-lists (the script I wrote to generate models from Swagger does this, and I imagine autorest can do something similar).

My existing client library does this and I've found that it makes it easier to deal with resources in a generic fashion:

https://github.com/tintoy/dotnet-kube-client/blob/develop/src/KubeClient/Models/PodV1.cs
https://github.com/tintoy/dotnet-kube-client/blob/develop/src/KubeClient/Models/PodListV1.cs
https://github.com/tintoy/dotnet-kube-client/blob/develop/src/KubeClient/ResourceClients/KubeResourceClient.cs#L90
https://github.com/tintoy/dotnet-kube-client/blob/develop/src/KubeClient/ResourceClients/KubeResourceClient.cs#L122

TBH given that the official client is mostly generated code, I don't know how useful it will be within the client itself, but it's almost certainly going to be useful to some of its consumers.

Again, I get that there are existing users and this might be a breaking change (at the binary-compatibility level at least) but I still think it's something worth considering.

[tintoy]

Here's an example of where it's useful:

https://github.com/DimensionDataResearch/glider-gun-v2/blob/master/src/GliderGun.Actors/Events/JobEvents.cs
https://github.com/DimensionDataResearch/glider-gun-v2/blob/master/src/GliderGun.Actors/Events/ResourceEventBus.cs

[brendandburns]

Yeah, I see the utility, but I don't want to manually generate the code.

Do you know if AutoRest supports an extension to add a parent class? If so we can look to add that extension...

[tintoy]

Agree 😊

The classes are marked as partial, so even if it doesn't I can easily use Roslyn for "post-processing". Will have a think about it this weekend

[tintoy]

Sorry, have been a bit flat-out at work, so I haven't had time to follow up on this; I'll try to have a proper look into it this weekend.

[tintoy]

@brendandburns - looking at the genrepository, is the directives / transforms the right place to make this change, or do you think I'm better off creating an Autorest extension to add the base class? I could easily write a Roslyn-based console app that can parse the generated code and add the base class as required. Which would be a lot simpler but probably not fit with the "official" generation process...

[tintoy]

@brendandburns - I think part of the problem is that the swagger definitions don't directly support specifying a base class (JSON schema doesn't support inheritance). This can be done at the code level, but not the swagger level.

FWIW, I did this in my own project when generating the code (after the Swagger JSON has been processed):

https://github.com/tintoy/dotnet-kube-client/blob/ff2d9bf577663e9c29994b383e67125ea60c13d0/src/Swagger/generate_models.py#L467-L475

And the detection of resources / resource-lists is done like so:

https://github.com/tintoy/dotnet-kube-client/blob/ff2d9bf577663e9c29994b383e67125ea60c13d0/src/Swagger/generate_models.py#L62-L102

Note that this also correctly handles StatusV1 which is not a resource list, but has a meta property of type ListMetaV1.

[tintoy]

@brendandburns - here's an example implementation. Only required a minor amount of yak-shaving (due to use of .NET Core 2.1-preview2 SDK):

https://gist.github.com/tintoy/93a8f50fb3f4dafcfa835cbeb804b441

[tintoy]

Otherwise, we might be able to do what we need using the allOf directive.

[tintoy]

I'll try modifying that Python script to add allOf and move the kind, apiVersion, and metadataproperties out to the relevant base classes.

[tintoy]

Would be nice to add a [KubeResource] attribute to each model as appropriate but given autorest's model this may not be worth the effort involved.

[tintoy]

@brendandburns - looks like I'd have to modify the gen repository to make this work, but there seems to be only a single script for preprocessing the Swagger document used for all language targets and many other languages (e.g. Go) may not want to use inheritance like this. While I have some ideas for tackling this issue, I'd appreciate it if you could provide some direction in terms of a "blessed" approach :)

(for what it's worth, I still think the Roslyn-based postprocessing might be a more workable idea in the long run even if it does make the model-generation process a two-step affair)

CC: @qmfrederik @tg123

[qmfrederik]

@tintoy An easier way may be to use interfaces instead of base classes, like in #152 .

Would interfaces work for your use cases or do you need base classes?

What would the KubeResource attribute do?

[tintoy]

Yeah possibly - but we'd still have the same problem of needing to somehow declare the interface implementation on each model class. So back to square one :)

[tintoy]

Will explain properly tomorrow morning :)

[tintoy]

Ok, now that I'm in front of a proper keyboard (rather than on my phone):

The issue is that, either way, we have to be able to declare a base class or interface for each model (but the current process for generating the models does not allow for this). If we forked the Python script that modifies the Swagger to create a C#-specific version then that could do the trick (but I'm not sure how willing others are to do it this way). Otherwise, we could perhaps add a separate Python script to modify the swagger (to be run after the main one) that's only run when generating for C#?

[qmfrederik]

Got it. The advantage of interfaces over base classes is that you don't have to remove any autogenerated code - you just add the interface implementation, which can be done in a partial class definition.

#152 tries to do that - would that approach work for you?

[tintoy]

#152 tries to do that - would that approach work for you?

Nice trick, yeah that'd work.

BTW, for places where I need a common base class I still have the implementation in KubeClient (but figured it'd be useful for more general use-cases / consumers to be able to implement generic resource handling). The only reason I was using a base class was because that way I don't need to fill out Kind or ApiVersion every time I create one of these objects (since that information is inherent in the model type via a custom attribute). Either way, this sounds like a good-enough solution, so I'm happy to close this once #152 is merged :)

[brendandburns]

#152 is merged, so closing.