Don't require names for built-in Kubernetes list types #294
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`.metadata.name` is not required for built-in list types. Our provider expects this property, which means it will crash if it's not given. This commit changes ouur provider to not expect such. The specifics are slightly more complex. Here is the background: Kubernetes does not instantiate or manage the lifecycle of list types like `v1.List`. If it receives a list type, it will recursively traverse it for resource definitions that it is supposed to manage, and then perform the relevant operation on each. (e.g., `apply`'ing each `v1.ConfigMap` in a `v1.ConfigMapList`.) Because of this property, lists are not required to have names. Unfortunately, while our code handles `v1.List` correctly, it does not handle all these other built-in lists. A resource is not specified as a list in the OpenAPI spec, either, so we will simply generate a set of checks for things that "look like" lists, and trust that when we update the OpenAPI spec, we will catch any weirdness in the diffs.
hausdorff
force-pushed
the
hausdorff/lists
branch
from
5ed7938
to
e2e65b5
Compare
hausdorff
changed the title
lblackstone
reviewed
Nov 26, 2018
| // Since Kubernetes does not instantiate list types directly, Pulumi also traverses lists | ||
| // for resource definitions that can be managed by Kubernetes, and registers those with the | ||
| // engine instead. | ||
| if ( |
There was a problem hiding this comment.
How was this list generated?
What's the process for keeping it updated with newer k8s versions?
There was a problem hiding this comment.
It's a mustache template, right? It's generated here: https://github.com/pulumi/pulumi-kubernetes/pull/294/files#diff-551fbe30151ad564ca8ed6b6c8f46e40R98
That's picked up from the OpenAPI spec we provide. So it's always automatically up to date.
lblackstone
approved these changes
Nov 26, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
.metadata.nameis not required for built-in list types. Our providerexpects this property, which means it will crash if it's not given. This
commit changes ouur provider to not expect such.
The specifics are slightly more complex. Here is the background:
Kubernetes does not instantiate or manage the lifecycle of list types
like
v1.List. If it receives a list type, it will recursively traverseit for resource definitions that it is supposed to manage, and then
perform the relevant operation on each. (e.g.,
apply'ing eachv1.ConfigMapin av1.ConfigMapList.)Because of this property, lists are not required to have names.
Unfortunately, while our code handles
v1.Listcorrectly, it does nothandle all these other built-in lists. A resource is not specified as a
list in the OpenAPI spec, either, so we will simply generate a set of
checks for things that "look like" lists, and trust that when we update
the OpenAPI spec, we will catch any weirdness in the diffs.