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

Recommendations for interacting with 3rd party (non k8s) CRDs? #313

Closed
rrichardson opened this issue Aug 20, 2020 · 8 comments · Fixed by #321
Closed

Recommendations for interacting with 3rd party (non k8s) CRDs? #313

rrichardson opened this issue Aug 20, 2020 · 8 comments · Fixed by #321
Labels
derive kube-derive proc_macro related

Comments

@rrichardson
Copy link
Contributor

My controller has a need to create DNS records for the endpoints that it creates.
I would like to rely on the DNSEndpoint type that is managed by external-dns as the default.
https://github.com/kubernetes-sigs/external-dns/blob/master/docs/contributing/crd-source/crd-manifest.yaml

At the moment I've hacked together a minimal struct that serializes into the desired JSON object that I can POST to the API.
I'm hoping for a more correct and type-safe approach.

Is there an suggestions for how I might incorporate the "official" schema of 3rd party types into my controller?

@nightkr
Copy link
Member

nightkr commented Aug 20, 2020

Yeah, that's pretty much the current state of the art, I'm afraid.

@clux
Copy link
Member

clux commented Aug 20, 2020

Hm, yeah, this would be like the reverse of #264. Generate code code from existing kubernetes yaml. Would be a difficult one to approach. Even if we had this, you'd still have to update your code when the schema changes (if you touched that part of the schema).

For now I would stick with minimal struct with a derive around it, and use Api::patch when modifying the parts you know about.

@clux clux added the derive kube-derive proc_macro related label Aug 20, 2020
@rrichardson
Copy link
Contributor Author

One other snafu. I can't derive a CustomResource for a DNSEndpoint because it is not PascalCase. Is there a workaround for this? I have created DnsEndpointSpec and set the kind to DNSEndpoint but this fails with this message:

error: #[derive(CustomResource)] requires a non-plural PascalCase `kind = "..."` or non-plural PascalCase struct name

Is there a way I can override this restriction?
Note that the compiler doesn't mind if I create a vanilla struct called DNSEndpoint. Either it fits Rust's qualifications for PascalCase, or it doesn't care.

@rrichardson
Copy link
Contributor Author

Ah. I think I have a very simple solution. I can create the DnsEndpoint struct as such, but then explicitly set the kind and api_version when constructing the object. I haven't tested this yet, but I expect it to work.

@clux
Copy link
Member

clux commented Aug 29, 2020

Ah, if that works, then that's great. But I expect we have to loosen the derive restriction just like we did for DynamicResource about two weeks ago. If you can't get around it I'm happy to just remove that from kube-derive.

@rrichardson
Copy link
Contributor Author

@clux - I do consider this a workaround. I think the right course of action would be to remove the PascalCase restriction.. or if it's possible, make it a bit less restrictive.

If it matters to have some level of validation, a Regex check might be good enough: \b[A-Z][a-z]*([A-Z][a-z]*)*\b (I tested this with a few items using https://rustexp.lpil.uk/) It doesn't do plural checking, though. inflector does provide a plural check with

For future reference: inflector::cases::classcase::is_class_case will check both the "PascalCase" and Plurality in one function call, so it would simplify the existing implementation.

@nightkr
Copy link
Member

nightkr commented Aug 30, 2020

IMO we should drop this validation completely, that's the apiserver's job to decide.

clux added a commit that referenced this issue Sep 11, 2020
also refactors kube-derive a bit for reabability.
also fixed a bug that caused it not to pick up on the shortname attr
@clux clux linked a pull request Sep 11, 2020 that will close this issue
@clux clux closed this as completed in #321 Sep 17, 2020
@clux
Copy link
Member

clux commented Oct 8, 2020

Validation in kube-derive removed in 0.43.0 (same commit as a month ago, just hadn't done a release)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
derive kube-derive proc_macro related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants