-
Notifications
You must be signed in to change notification settings - Fork 294
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
Escape hatches seem to not be working as expected #2163
Comments
When doing this kind of thing I found that For what it's worth... |
Thanks @cm3brian but when I try running this: var test = new KubeNamespace(this, "test", {
metadata: {
name: "test"
}
})
test.addJsonPatch(JsonPatch.add("/spec/test", 1)) I get an exception: Synthesizing application
.../llm-playground/k8s-setup-crd/node_modules/cdk8s/src/api-object.ts:224
throw new Error(`Failed serializing construct at path '${this.node.path}' with name '${this.name}': ${e}`);
^
Error: Failed serializing construct at path 'setup-crd/test' with name 'test': TypeError: Cannot set properties of undefined (setting 'test')
at KubeNamespace.toJson (.../llm-playground/k8s-setup-crd/node_modules/cdk8s/src/api-object.ts:224:13)
at KubeNamespace.toJson (.../llm-playground/k8s-setup-crd/imports/k8s.ts:2664:28)
at .../llm-playground/k8s-setup-crd/node_modules/cdk8s/src/app.ts:116:46
at Array.map (<anonymous>)
at Function._synthChart (.../llm-playground/k8s-setup-crd/node_modules/cdk8s/src/app.ts:116:31)
at MyChart.toJson (.../llm-playground/k8s-setup-crd/node_modules/cdk8s/src/chart.ts:148:16)
at App.synth (.../llm-playground/k8s-setup-crd/node_modules/cdk8s/src/app.ts:216:47)
at Object.<anonymous> (.../llm-playground/k8s-setup-crd/main.ts:25:5)
at Module._compile (node:internal/modules/cjs/loader:1364:14)
at Module.m._compile (.../llm-playground/k8s-setup-crd/node_modules/ts-node/src/index.ts:1618:23)
Error: command "npx ts-node main.ts " at .../llm-playground/k8s-setup-crd returned a non-zero exit code 1
at ChildProcess.<anonymous> (.../llm-playground/k8s-setup-crd/node_modules/cdk8s-cli/lib/util.js:54:27)
at Object.onceWrapper (node:events:632:26)
at ChildProcess.emit (node:events:517:28)
at ChildProcess._handle.onexit (node:internal/child_process:292:12)
.../llm-playground Is there any working example you can send? EDIT: I tried to extend your example like so: var test = new KubeNamespace(this, "test", {
metadata: {
name: "test"
}
})
test.addJsonPatch(JsonPatch.add("/spec", {}))
test.addJsonPatch(JsonPatch.add("/spec/test", 1)) This time I got no exception but output is the same: apiVersion: v1
kind: Namespace
metadata:
name: test
spec: {} |
Ah, I did not expect you to be directly running the contrived code I provided, it was an example only, pseudo-code, if you will. As your example was quite contrived I didn't expect it to be what you were actually using, but a public-friendly sharable snippet. I question why you are trying to use patching at all where you are using First things first, in order to patch something; its root must exist, so you should define it first (see below, where I have defined There are also other considerations, like the fact you need to work within the spec of the resource you are trying to create, for Namespace, it's here: https://kubernetes.io/docs/reference/kubernetes-api/cluster-resources/namespace-v1/ const test = new KubeNamespace(this, 'test', {
metadata: {
name: 'test'
},
spec: {}
})
//won't work, this isn't in the resource spec
test.addJsonPatch(JsonPatch.add('/spec/test', 1))
// works as its in the spec
test.addJsonPatch(JsonPatch.add('/spec/finalizers', ['test'])) outputs: apiVersion: v1
kind: Namespace
metadata:
name: test
spec:
finalizers:
- test |
Thanks @cm3brian that explains some stuff. So the real issue is that I'm failing to add a "status" definition which I can see in the link you sent. (https://kubernetes.io/docs/reference/kubernetes-api/cluster-resources/namespace-v1/#NamespaceStatus) var test = new KubeNamespace(this, "test", {
metadata: {
name: "test"
}
})
test.addJsonPatch(JsonPatch.add("/status", {
conditions: {
"status": "ready"
}
})) |
Trying to set the I suggest brushing up on your Kubernetes conventions, the specific part relating to this is here: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#spec-and-status This is a user error, not an issue with cdk8s. |
To be honest I was not saying this is "an issue with cdk8s" - but rather that it seems to not be doing what the documentation is saying. To begin with I was trying to wrap a valid manifest file for MetalLB (see here). Also I'm seeing this pattern repeat itself many times (see here) so I'm not so convinced this really goes against the convention. Regardless of the specific issue - maybe the documentation would benefit from some of the information you've provided in this thread?
It doesn't mention anything about the other considerations you have mentioned. In short I thought it might be good to bring that to your attention, for my own use case I have already found a workaround that doesn't involve cdk8s. If you think I'm wrong - happy to drop this thread. |
I appear to be having a similar issue. I've imported a CRD (the AWS CNI Plugin CRD, here: https://raw.githubusercontent.com/aws/amazon-vpc-cni-k8s/master/charts/aws-vpc-cni/crds/customresourcedefinition.yaml), but the CRD appears to be defective (it doesn't specify a spec for a resource). The yaml definition looks like this:
So I'm attempting to create the resource using ApiObject, but ApiObject doesn't allow passing a spec:
|
@srfrnk Thanks for brining this to our attention. Your expectation of how it should work makes sense. @cm3brian the considerations you laid out here for escape hatches also make sense, but they are actually inconsistent with our documentation, and with how we envisioned escape patching should be used. The fact escape hatches cannot be used on properties that don't appear in the schema is a bug. @johncuyle the problem (and a workaround for it) you are describing is explained here. |
Duplicate of #2172 |
Wow this is rather frustrating. The purpose of this escape hatch as stated in the docs is to break through the abstraction and override the output, but its actually validated by the abstraction and allows no actual override of the output. I just spent a ton of time trying to use this with absolutely zero errors or hint that it is just going to eat the calls. Maybe the docs should be changed to reflect the lack of this functionality. |
Description of the bug:
The following code:
Generates this:
While AFAIK it should generate:
Reproduction Steps:
npm run synth
Environment:
This is 🐛 Bug Report
The text was updated successfully, but these errors were encountered: