Fix custom resources not having a fallback details component #3542
Conversation
Nokel81
requested review from
aleksfront,
jim-docker and
jakolehm
and removed request for
a team
src/renderer/api/kube-object.ts
Outdated
| export interface KubeObjectStatus { | ||
| conditions: { | ||
| lastTransitionTime: string; | ||
| message: string; | ||
| reason: string; | ||
| status: string; | ||
| type?: string; | ||
| }[]; | ||
| } |
There was a problem hiding this comment.
This is the starting of a similar type to KubeObjectMetadata. In the future we should be able to use it everywhere because kube uses this type for object (even custom objects) it seems.
There was a problem hiding this comment.
Are we sure all kubernetes objects have these?
There was a problem hiding this comment.
Also I think this interface does not match actual implementation, conditions seems to be an array of objects.
There was a problem hiding this comment.
So I am not sure if all kube controllers do use this (or something like this...)
After reading:
- Should we deprecate status.conditions? kubernetes/kubernetes#50798
- Eliminate Phase and simplify Conditions kubernetes/kubernetes#7856
- Update Condition guidance kubernetes/community#4521
and https://maelvls.dev/kubernetes-conditions/
It seems clear that not everything does use this type (or at least MUST use something like this). But that it is still the recommended way of displaying the history of data.
| interface Props extends KubeObjectDetailsProps<KubeObject> { | ||
| crd: CustomResourceDefinition; |
There was a problem hiding this comment.
I changed to passing in the crd (which should be the parent crd of object) because the single use of this component already needs to have it.
| } | ||
|
|
||
| renderAdditionalColumns(crd: CustomResourceDefinition, columns: AdditionalPrinterColumnsV1[]) { | ||
| renderAdditionalColumns(resource: KubeObject, columns: AdditionalPrinterColumnsV1[]) { |
There was a problem hiding this comment.
This component was incorrect because it said it was dealing with CustomResourceDefenition's as its main type. It is not, it is dealing with custom resources (which are from Lens' perspective, just KubeObject's).
| </DrawerItem> | ||
| )); | ||
| } | ||
|
|
||
| renderStatus(crd: CustomResourceDefinition, columns: AdditionalPrinterColumnsV1[]) { | ||
| renderStatus(crd: KubeObject<KubeObjectMetadata, KubeObjectStatus, any>, columns: AdditionalPrinterColumnsV1[]) { |
There was a problem hiding this comment.
Hopefully in the future, we won't need to specify KubeObjectStatus and we can just specify KubeObject but we are not there yet.
| if (crd) { | ||
| details.push(<CrdResourceDetails key={object.getId()} object={object} crd={crd} />); | ||
| } |
There was a problem hiding this comment.
By fixing the typing on CrdResourceDetails (to not saying we want a CustomResourceDefenition as object, because we don't) it is no longer necessary to use a type cast.
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Signed-off-by: Sebastian Malton <[email protected]>
Signed-off-by: Sebastian Malton <[email protected]>
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
Signed-off-by: Sebastian Malton <[email protected]>
Signed-off-by: Sebastian Malton [email protected]
fixes #3541