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: Remove incorrect KubeObject validations #7771

Merged
merged 5 commits into from May 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -86,7 +86,7 @@ class NonInjectedClusterRoleBindingDialog extends React.Component<ClusterRoleBin
}

@observable selectedRoleRef: ClusterRole | undefined = undefined;
selectedAccounts = new ObservableHashSet<ServiceAccount>([], sa => sa.metadata.uid);
selectedAccounts = new ObservableHashSet<ServiceAccount>([], sa => sa.getId());
selectedUsers = observable.set<string>([]);
selectedGroups = observable.set<string>([]);

Expand Down
Expand Up @@ -73,7 +73,7 @@ class NonInjectedRoleBindingDialog extends React.Component<RoleBindingDialogProp
@observable.ref selectedRoleRef: Role | ClusterRole | null | undefined = null;
@observable bindingName = "";
@observable bindingNamespace: string | null = null;
selectedAccounts = new ObservableHashSet<ServiceAccount>([], sa => sa.metadata.uid);
selectedAccounts = new ObservableHashSet<ServiceAccount>([], sa => sa.getId());
selectedUsers = observable.set<string>([]);
selectedGroups = observable.set<string>([]);

Expand Down
7 changes: 2 additions & 5 deletions packages/kube-object/src/api-types.ts
Expand Up @@ -25,8 +25,8 @@ export interface KubeJsonApiData<
Status = unknown,
Spec = unknown,
> {
kind: string;
apiVersion: string;
readonly kind: string;
readonly apiVersion: string;
metadata: Metadata;
status?: Status;
spec?: Spec;
Expand Down Expand Up @@ -227,9 +227,6 @@ export type KubeJsonApiObjectMetadata<Namespaced extends KubeObjectScope = KubeO
export type KubeObjectMetadata<Namespaced extends KubeObjectScope = KubeObjectScope> =
KubeJsonApiObjectMetadata<Namespaced> & {
readonly selfLink: string;
readonly uid: string;
readonly name: string;
readonly resourceVersion: string;
};

export type NamespaceScopedMetadata = KubeObjectMetadata<KubeObjectScope.Namespace>;
Expand Down
72 changes: 72 additions & 0 deletions packages/kube-object/src/kube-object.test.ts
@@ -0,0 +1,72 @@
import { KubeObject } from "./kube-object";

const getStubData = () => ({
apiVersion: "metrics.k8s.io/v1beta1",
kind: "PodMetrics",
metadata: {
creationTimestamp: "2023-05-24T14:17:01Z",
name: "cert-manager-54cbdfb45c-n4kp9",
namespace: "cert-manager",
selfLink: "/apis/metrics.k8s.io/v1beta1/namespaces/cert-manager/pods/cert-manager-54cbdfb45c-n4kp9",
uid: "123" as string | undefined,
resourceVersion: "foobar" as string | undefined,
},
});

describe("kube object tests", () => {
it("given '.metadata.uid' is missing, then KubeObject constructor does not throw", () => {
const data = getStubData();

delete data.metadata.uid;
expect(() => new KubeObject(data)).not.toThrow();
});

it("given '.metadata.resourceVersion' is missing, then KubeObject constructor does not throw", () => {
const data = getStubData();

delete data.metadata.resourceVersion;
expect(() => new KubeObject(data)).not.toThrow();
});

it("given both '.metadata.resourceVersion' and '.metadata.uid' are missing, then KubeObject constructor does not throw", () => {
const data = getStubData();

delete data.metadata.uid;
delete data.metadata.resourceVersion;
expect(() => new KubeObject(data)).not.toThrow();
});

it("given '.metadata.uid' exist, then KubeObject.getId() should return it", () => {
const data = getStubData();
const obj = new KubeObject(data);

expect(obj.getId()).toEqual("123");
});

it("given '.metadata.uid' is missing, then KubeObject.getId() should return '.metadata.selfLink'", () => {
const data = getStubData();

delete data.metadata.uid;
const obj = new KubeObject(data);

expect(obj.getId()).toEqual(
"/apis/metrics.k8s.io/v1beta1/namespaces/cert-manager/pods/cert-manager-54cbdfb45c-n4kp9",
);
});

it("given '.metadata.resourceVersion' exist, then KubeObject.getResourceVersion() should return it", () => {
const data = getStubData();
const obj = new KubeObject(data);

expect(obj.getResourceVersion()).toEqual("foobar");
});

it("given '.metadata.resourceVersion' is missing, then KubeObject.getResourceVersion() should return an empty string", () => {
const data = getStubData();

delete data.metadata.resourceVersion;
const obj = new KubeObject(data);

expect(obj.getResourceVersion()).toEqual("");
});
});
26 changes: 6 additions & 20 deletions packages/kube-object/src/kube-object.ts
Expand Up @@ -111,20 +111,6 @@ export class KubeObject<
);
}

if (!isString(data.metadata.uid)) {
throw new KubeCreationError(
`Cannot create a KubeObject from an object without metadata.uid being a string`,
data,
);
}

if (!isString(data.metadata.resourceVersion)) {
throw new KubeCreationError(
`Cannot create a KubeObject from an object without metadata.resourceVersion being a string`,
data,
);
}

if (!isString(data.metadata.selfLink)) {
Nokel81 marked this conversation as resolved.
Show resolved Hide resolved
throw new KubeCreationError(
`Cannot create a KubeObject from an object without metadata.selfLink being a string`,
Expand All @@ -136,23 +122,23 @@ export class KubeObject<
autoBind(this);
}

get selfLink() {
get selfLink(): string {
return this.metadata.selfLink;
}

getId() {
return this.metadata.uid;
getId(): string {
return this.metadata.uid ?? this.metadata.selfLink;
}

getResourceVersion() {
return this.metadata.resourceVersion;
getResourceVersion(): string {
return this.metadata.resourceVersion ?? "";
}

getScopedName() {
return [this.getNs(), this.getName()].filter(Boolean).join("/");
}

getName() {
getName(): string {
return this.metadata.name;
}

Expand Down