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

Fix aliases for several resource kinds #990

Merged
merged 2 commits into from
Feb 13, 2020
Merged

Conversation

lblackstone
Copy link
Member

Proposed changes

The following resource Kinds had the wrong Group
specified in the aliases:

  • ClusterRole
  • ClusterRoleBinding
  • Ingress
  • NetworkPolicy
  • PriorityClass
  • Role
  • RoleBinding

Related issues (optional)

Fixes #989

The following resource Kinds had the wrong Group
specified in the aliases:

- ClusterRole
- ClusterRoleBinding
- Ingress
- NetworkPolicy
- PriorityClass
- Role
- RoleBinding
@lukehoban
Copy link
Member

Is there a source of truth for this data we can use to make sure we are right here (and link to from source code)? Is it in Kubernetes API docs anywhere? Is it in Kubernetes source code anywhere?

@lblackstone
Copy link
Member Author

Yeah, it's in the API docs

@lukehoban
Copy link
Member

Yeah, it's in the API docs

Specifically - you mean the "Other API versions of this object exist:" annotations? Do we now map precisely the same kinds as specified in these docs? Can we pull this from API specs somehow? (that is - where is this data for the API docs sourced from, and why can't we source from the same place)?

@lblackstone
Copy link
Member Author

Specifically - you mean the "Other API versions of this object exist:" annotations?

Yes

Do we now map precisely the same kinds as specified in these docs?

Yes

Can we pull this from API specs somehow? (that is - where is this data for the API docs sourced from, and why can't we source from the same place)?

It's generated from the OpenAPI spec, so it's doable with some caveats:

  1. We don't currently alias every resource Kind. We were concerned that some apiVersions might not be compatible, so we started with the subset that we were sure about. I'm still unsure about this.
  2. These don't change often, so that might be more work than it's worth.
  3. Old, deprecated versions are sometimes removed from the OpenAPI specs, which means the aliases would also be removed (barring additional logic to work around that). That's probably not what we want.

@lukehoban
Copy link
Member

We should take this PR as-is to fix the current issues.

That said:

We don't currently alias every resource Kind. We were concerned that some apiVersions might not be compatible, so we started with the subset that we were sure about. I'm still unsure about this.

It sounds rather suspicious to me that we are trying to outsmart Kubernetes' own notion of what resouce kinds are "the same". Any place we don't match we will run into very weird issues with Helm/YAML, and just with overall model compatibility. Barring any explicit Pulumi-specific issues, I'm inclined to align with the platform.

These don't change often, so that might be more work than it's worth.

I may be biased, but I think it is basically always going to be worth it to derive things from official specs than to rely on ourselves to do ongoing manual work. It removes so much risk around long term maintenance and just human error to derive these sorts of things programmatically.

Old, deprecated versions are sometimes removed from the OpenAPI spec

Seems we have to fundamentally solve for this anyway - we very much don't want to drop old resource types in general, even if they are deprecated.

@lblackstone
Copy link
Member Author

Tracking further work in #828

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.

Incorrect aliases for some k8s resources
2 participants