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

Generate provider SDK from Kubernetes v1.12 #293

Merged
merged 3 commits into from
Nov 26, 2018
Merged

Conversation

hausdorff
Copy link
Contributor

No description provided.

@hausdorff
Copy link
Contributor Author

hausdorff commented Nov 21, 2018

Hmm, it looks like Kubernetes changed apiregistration -> apiregistration.k8s.io. This will break URNs that used these APIs. We should definitely not merge this until we decide what to do. At the outset it seems like we have a couple of options:

  • Maintain a table of mappings from group name -> "URN group name", and keep emitting the latter. I really do not like making the URNs having a different form from the k8s API, though.
  • Find a way to do aliasing in the engine itself, maybe? So the engine would understand both apiregistration and apiregistration.k8s.io, but we could mark the former as deprecated, or something.

cc @lukehoban @lblackstone

pkg/gen/typegen.go Outdated Show resolved Hide resolved
pkg/gen/typegen.go Show resolved Hide resolved
@lblackstone
Copy link
Member

  • Find a way to do aliasing in the engine itself, maybe? So the engine would understand both apiregistration and apiregistration.k8s.io, but we could mark the former as deprecated, or something.

I like that option better.

@hausdorff
Copy link
Contributor Author

I think in the short term, we should do the first, and then file an issue to follow up and remove this lookup table thing and replace it with some kind of aliasing.

My reasoning is basically that we are absolutely, 100% going to encounter this again. This is the sort of thing Kubernetes does all the time.

@lukehoban
Copy link
Member

I love that we can so easily pick up all the new features (and docs!) - really nice to see this!

Maintain a table of mappings from group name -> "URN group name", and keep emitting the latter. I really do not like making the URNs having a different form from the k8s API, though.

To be clear, is your short term proposal to map the "new" names back to the "old" names for compatibility? I notice that in places like getResources there is also a breaking change associated with this.

@hausdorff
Copy link
Contributor Author

@lukehoban that's correct, yes. Temporarily mapping it back to the old name would solve this problem everywhere, except one place, which is our CRD class -- that we have to fix manually.

I do think we need to solve the problem though -- this situation is guaranteed to come up again.

@swgillespie
Copy link
Contributor

Mental note for me to do early in M20 that we need to manually regenerate the Python casing table again (#258).

@lukehoban
Copy link
Member

I do think we need to solve the problem though -- this situation is guaranteed to come up again.

Absolutely. Can you open an issue to track this so we make sure to tackle it?

@hausdorff
Copy link
Contributor Author

Ok, I think this is nearly ready to merge.

  • Use apiregistration instead of apiregistration.k8s.io in our URNs as a temporary solution for the OpenAPI spec changing the group name.
  • Filed Migration strategy for URNs whose group name changes #298 as a follow-up to fix this sort of issue permanently.
  • Updated Python's tables.py. If we have extra time this week, I'll resolve Python: Auto-generate camel case mapping table from OpenAPI spec #258 so that this is automated.
  • Updated Makefile to delete old Python SDK files before generating the new ones, so that we don't keep around auto-generated files from the last time. (Not applicable to TS, because it's one file which we re-generate every time.)
  • Looked at the diff a bunch over a period of several days to sanity-check.

@hausdorff hausdorff merged commit 87ec44e into master Nov 26, 2018
@pulumi-bot pulumi-bot deleted the hausdorff/openapi branch November 26, 2018 22:01
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 this pull request may close these issues.

None yet

4 participants