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: x-kubernetes-preserve-unknown-fields incorrectly applied on List<T> #850

Merged
merged 3 commits into from
Mar 23, 2025

Conversation

rgrama
Copy link
Contributor

@rgrama rgrama commented Mar 12, 2025

Describe the bug

PreserveUnknownFieldsAttribute can currently only be applied to properties. If within my CRD I have a List and I set the attribute, it will render in the CRD at the items level, not within the object itself.

To reproduce

[KubernetesEntity(Group = "test.io", Kind = "Test", ApiVersion = "v1")]
public partial class TestCrd : CustomKubernetesEntity<TestSpec> { }

public class TestSpec
{
    [PreserveUnknownFields]
    public List<Item> Items { get; set; } = default!;
}

public class Item
{
    public string Name { get; set; } = default!;
}

Generates

  - name: v1
    schema:
      openAPIV3Schema:
        properties:
          spec:
            properties:
              items:
                items:
                  properties:
                    name:
                      type: string
                  type: object
                type: array
                x-kubernetes-preserve-unknown-fields: true
            type: object
        type: object

Expected behavior

x-kubernetes-preserve-unknown-fields: true should be applied to the object within items

  - name: v1
    schema:
      openAPIV3Schema:
        properties:
          spec:
            properties:
              items:
                items:
                  properties:
                    name:
                      type: string
                  type: object
                  x-kubernetes-preserve-unknown-fields: true
                type: array
            type: object
        type: object

To achieve this result, the attribute should be allowed on classes

[KubernetesEntity(Group = "test.io", Kind = "Test", ApiVersion = "v1")]
public partial class TestCrd : CustomKubernetesEntity<TestSpec> { }

public class TestSpec
{
    public List<Item> Items { get; set; } = default!;
}

[PreserveUnknownFields]
public class Item
{
    public string Name { get; set; } = default!;
}

Copy link
Owner

@buehler buehler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely, very nice addition!

Just one thing: would you mind to add some tests for the new behavior?

Much appreciated!

@rgrama
Copy link
Contributor Author

rgrama commented Mar 19, 2025

Added a couple of tests

@rgrama
Copy link
Contributor Author

rgrama commented Mar 19, 2025

btw I have one more change on my local branch which I require - handling of object. Currently object or List< object > is not supported. Should i do that as a separate PR or can I add to this one?

@buehler
Copy link
Owner

buehler commented Mar 23, 2025

it would be really cool to have it in a separate PR

@rgrama
Copy link
Contributor Author

rgrama commented Mar 23, 2025

alright, I'll create a separate one tomorrow

@buehler buehler merged commit 201e14e into buehler:main Mar 23, 2025
3 checks passed
@rgrama rgrama deleted the preserve-unknown-fields-objects branch March 24, 2025 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants