fix: support YAML 1.1 parsing in Kubernetes manifests by replacing js-yaml with yaml #2548

Tanmayshi · 2025-07-08T15:16:54Z

🛠️ What’s Changed

Removed: js-yaml dependency

Added: yaml library

Updated manifest parsing logic to:

parseDocument(manifest, { version: '1.1' });
Ensures values are interpreted using YAML 1.1 rules.

🐛 Problems Solved

✅ Incorrect parsing of octal values
defaultMode: 0644 was being parsed as 644 in YAML 1.2 (incorrect)

In YAML 1.1, it's correctly interpreted as octal 0644 → decimal 420

This fixes configMap and secret mount permission issues in Kubernetes

✅ Boolean string interpretation issues
YAML 1.2 doesn't treat yes, no, on, off as booleans

In YAML 1.1, these are properly recognized as true/false

Prevents unexpected behavior in manifest logic that depends on boolean flags

✅ Tested

Applied a manifest with defaultMode: 0644 — parsed correctly as 420
Applied a manifest with on, yes, no — parsed correctly as booleans
Resource created successfully using KubernetesObjectApi

✅ Result
Manifests parsed through this library now match Kubernetes expectations

No more silent failures or misconfigurations due to YAML parsing issues

Fixes #2539

linux-foundation-easycla · 2025-07-08T15:16:59Z

The committers listed above are authorized under a signed CLA.

✅ login: Tanmayshi / name: Tanmay Shirbhayye (edd426d, dddce79, 06d5731, 80cbb7a)

k8s-ci-robot · 2025-07-08T15:17:00Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Tanmayshi
Once this PR has been reviewed and has the lgtm label, please assign cjihrig for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot · 2025-07-08T15:17:04Z

Welcome @Tanmayshi!

It looks like this is your first PR to kubernetes-client/javascript 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-client/javascript has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

cjihrig

Can you please sign the CLA.

cjihrig · 2025-07-08T15:29:11Z

package.json

@@ -62,15 +62,15 @@
        "form-data": "^4.0.0",
        "hpagent": "^1.2.0",
        "isomorphic-ws": "^5.0.0",
-        "js-yaml": "^4.1.0",


@types/js-yaml should also be removed. Is there a @types/yaml that should be added in its place?

Yes, I will remove @types/js-yaml since it's no longer used.

Regarding @types/yaml:
I won't be adding it because the yaml package already includes its own TypeScript definitions, so there's no need for a separate type package.

If you still think adding it is necessary for a specific reason, please let me know — happy to update accordingly.

cjihrig · 2025-07-08T15:29:28Z

src/yaml.ts

@@ -1,4 +1,4 @@
-import yaml from 'js-yaml';
+import YAML from 'yaml';


Can you leave this name as it was please.

cjihrig · 2025-07-08T15:30:14Z

src/yaml.ts

+export function loadAllYaml(data: string): any[] {
+    const ymls = YAML.parseAllDocuments(data, { version: '1.1' });
+    return ymls.map((doc) => {
+        const obj = doc.toJS() as KubernetesObject;


Can you keep the name as obj to keep the diff smaller.

feloy · 2025-07-08T15:44:14Z

src/yaml.ts

@@ -25,12 +24,12 @@ export function loadYaml<T>(data: string, opts?: yaml.LoadOptions): T {
 * @param opts - Optional YAML load options.
 * @returns An array of deserialized Kubernetes objects.
 */
-export function loadAllYaml(data: string, opts?: yaml.LoadOptions): any[] {


I think users may still want to pass options to the parser

I think we may need to define our own types for the options passed to loadYaml(), loadAllYaml(), and dumpYaml(). These are part of the public API, and changing the underlying YAML library like this changes the actual options that are supported.

feloy · 2025-07-08T15:44:52Z

src/yaml.ts

-    return ymls.map((yml) => {
-        const obj = yml as KubernetesObject;
+export function loadAllYaml(data: string): any[] {
+    const ymls = YAML.parseAllDocuments(data, { version: '1.1' });


I think this value 1.1 for version option should be default, but the user may be able to overwrite it

Yes sir @feloy, you're absolutely right

I've updated the function so that version: '1.1' is now used as the default, but users can still override it by passing their own options.

yaml.parseAllDocuments(data, { version: '1.1', ...opts }); ,

cjihrig · 2025-07-08T17:11:03Z

package-lock.json

@@ -16,15 +16,15 @@
                "form-data": "^4.0.0",
                "hpagent": "^1.2.0",
                "isomorphic-ws": "^5.0.0",
-                "js-yaml": "^4.1.0",


I don't think @types/js-yaml has been removed in this file.

cjihrig · 2025-07-08T17:12:16Z

src/yaml.ts

    if (!yml) {
-        throw new Error('Failed to load YAML');
+        throw new Error('Failed to load yaml');


Please undo this change.

cjihrig · 2025-07-08T17:13:54Z

src/yaml.ts

        const type = getSerializationType(obj.apiVersion, obj.kind);
-        return ObjectSerializer.deserialize(yml, type);
+        return ObjectSerializer.deserialize(obj, type);


If you don't rename things and make unrelated changes, the diff stays smaller and easier to review. It also makes git blame easier to use too.

cjihrig · 2025-07-08T17:14:33Z

I started the CI. It looks like there are relevant failures.

cjihrig · 2025-07-08T20:19:07Z

src/yaml_test.ts

-        strictEqual(objects[2].apiVersion, 'apiextensions.k8s.io/v1');
-        strictEqual(objects[2].kind, 'CustomResourceDefinition');
-        strictEqual(objects[2].metadata!.name, 'my-crd.example.com');
+        // Assert specific types for each object


The fact that an existing test broke implies to me that this may be a subtly breaking change.

The tests themselves were not failing — the issue was purely related to TypeScript type-checking. TypeScript was warning that some properties might be undefined or missing. I've addressed those issues, and CI is now passing

If you'd prefer a more strictly type-safe solution, please let me knowd.

feloy · 2025-07-09T06:15:35Z

src/yaml.ts

 * @returns The deserialized Kubernetes object.
 */
-export function loadYaml<T>(data: string, opts?: yaml.LoadOptions): T {
-    const yml = yaml.load(data, opts) as any as KubernetesObject;
+export function loadYaml<T>(data: string, opts?: yaml.ParseOptions): T {


yaml.parse accepts options of type ParseOptions & DocumentOptions & SchemaOptions & ToJSOptions. Let's the user have access to all the options, not only ParseOptions ones (especially the version one, which is in the DocumentOptions)

Thanks! Updated the type to ParseOptions & DocumentOptions so version: '1.1' and similar options work correctly now. Let me know if more should be added.

I cannot be sure some users won't have an interest on using options from the other ones, I would set all of them: ParseOptions & DocumentOptions & SchemaOptions & ToJSOptions

feloy · 2025-07-09T06:18:10Z

src/yaml.ts

-    const ymls = yaml.loadAll(data, undefined, opts);
-    return ymls.map((yml) => {
-        const obj = yml as KubernetesObject;
+export function loadAllYaml(data: string, opts?: yaml.ParseOptions): KubernetesObject[] {


type of opts would be ParseOptions & DocumentOptions & SchemaOptions here

feloy · 2025-07-09T06:33:06Z

src/yaml.ts

-    const ymls = yaml.loadAll(data, undefined, opts);
-    return ymls.map((yml) => {
-        const obj = yml as KubernetesObject;
+export function loadAllYaml(data: string, opts?: yaml.ParseOptions): KubernetesObject[] {


I'm not sure we want to change the type of the return value, as it will be a breaking change for user (and is what makes the tests fail)

We should not. We should try to keep backwards compatibility as much as possible.

We initially used KubernetesObject[] for better type safety and IntelliSense — especially when accessing standard fields like apiVersion, kind, or metadata.

But you're right — changing it from any[] would be a breaking change for users. So we’ve reverted back to any[] to preserve backward compatibility.

chore: replace js-yaml with yaml for YAML 1.1 compliance

80cbb7a

k8s-ci-robot requested review from brendandburns and mstruebing July 8, 2025 15:16

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jul 8, 2025

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 8, 2025

cjihrig reviewed Jul 8, 2025

View reviewed changes

feloy reviewed Jul 8, 2025

View reviewed changes

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jul 8, 2025

chore: address reviewer feedback

edd426d

cjihrig reviewed Jul 8, 2025

View reviewed changes

chore: address reviewer feedback 2

06d5731

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 8, 2025

cjihrig reviewed Jul 8, 2025

View reviewed changes

feloy reviewed Jul 9, 2025

View reviewed changes

chore: address reviewer feedback 3

dddce79

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 9, 2025

		@@ -1,4 +1,4 @@
		import yaml from 'js-yaml';
		import YAML from 'yaml';

fix: support YAML 1.1 parsing in Kubernetes manifests by replacing js-yaml with yaml #2548

Are you sure you want to change the base?

fix: support YAML 1.1 parsing in Kubernetes manifests by replacing js-yaml with yaml #2548

Conversation

Tanmayshi commented Jul 8, 2025

✅ Tested

Uh oh!

linux-foundation-easycla bot commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Jul 8, 2025

Uh oh!

k8s-ci-robot commented Jul 8, 2025

Uh oh!

cjihrig left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjihrig commented Jul 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tanmayshi Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

linux-foundation-easycla bot commented Jul 8, 2025 •

edited

Loading

Tanmayshi Jul 9, 2025 •

edited

Loading