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

Invalid value: "null" when field is removed from configuration and SSA is used to apply #103011

Closed
karunasagark opened this issue Jun 19, 2021 · 7 comments
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. triage/accepted Indicates an issue or PR is ready to be actively worked on. wg/api-expression Categorizes an issue or PR as relevant to WG API Expression.

Comments

@karunasagark
Copy link

karunasagark commented Jun 19, 2021

What happened:

We are developing a new k8s controller and decided to use SSA completely. However, ran into an issue when I removed a field from the configuration. I have a custom resource with field called 'prop1' and it is not a required field (see snippet below). Using SSA, I created an object of this CR kind with 'prop1' specified and this was successful. I changed the configuration by removing 'prop1' and used the same code aka SSA to apply this configuration, but this time I received an error - Invalid value: "null": spec.prop1 in body must be of type object: "null".

...
prop1:
  type: object
  properties:
    enabled:
      type: boolean
  required:
    - enabled
...

What you expected to happen:

However, I was not expecting this error. I understand that 'prop1' is not nullable as per JSON schema, but then I didn't set it to null, I removed it from the configuration and ensured that field is not present in the request body. So, it seems that something in the API server sets it to null. And as per the documentation here, if I remove the field it is supposed to be deleted from the live object.

Can you please help me understand this behavior?

How to reproduce it (as minimally and precisely as possible):

Attached zip file containing testcrd.yaml, testcr1.yaml and testcr2.yaml
reproYaml.zip

$kubectl apply --server-side -f testcrd.yaml
customresourcedefinition.apiextensions.k8s.io/testones.example.com serverside-applied

$kubectl apply --server-side -f testcr1.yaml
testone.example.com/objectone serverside-applied

$kubectl apply --server-side -f testcr2.yaml
The TestOne "objectone" is invalid: spec.prop1: Invalid value: "null": spec.prop1 in body must be of type object: "null"

$kubectl describe testone objectone
Name:         objectone
Namespace:    default
Labels:       <none>
Annotations:  <none>
API Version:  example.com/v1beta1
Kind:         TestOne
Metadata:
  Creation Timestamp:  2021-06-19T00:09:13Z
  Generation:          1
  Managed Fields:
    API Version:  example.com/v1beta1
    Fields Type:  FieldsV1
    fieldsV1:
      f:spec:
        f:prop1:
          f:enabled:
        f:prop2:
          f:enabled:
    Manager:         kubectl
    Operation:       Apply
    Time:            2021-06-19T00:09:13Z
  Resource Version:  27364718
  Self Link:         /apis/example.com/v1beta1/namespaces/default/testones/objectone
  UID:               036de8a1-00fb-45e2-a675-ff1124470e04
Spec:
  prop1:
    Enabled:  true
  prop2:
    Enabled:  false
Events:       <none>

Anything else we need to know?:

  • Controller code is written in Java, though the above repro steps with kubectl seems to be equivalent
  • The issue persists the same, though I remove the required from CRD
  • CCA was NOT used
  • Possibly related to Update sigs.k8s.io/structured-merge-diff to v4.0.3 #99014. If yes, which version of Kubernetes has the fix?
  • Ofcourse, we can add 'nullable: true' to the CRD and it does help. However, I'm trying to understand where 'null' is coming from and also the documentation says that SSA is supposed to remove this field without erroring.

Environment:

  • Kubernetes version (use kubectl version):
    Client Version: version.Info{Major:"1", Minor:"20", GitVersion:"v1.20.4", GitCommit:"e87da0bd6e03ec3fea7933c4b5263d151aafd07c", GitTreeState:"clean", BuildDate:"2021-02-18T16:12:00Z", GoVersion:"go1.15.8", Compiler:"gc", Platform:"linux/amd64"}
    Server Version: version.Info{Major:"1", Minor:"19", GitVersion:"v1.19.7", GitCommit:"8ab00ff68a1763b5a506a7073cb8e67b12dfbcd7", GitTreeState:"clean", BuildDate:"2021-03-10T23:40:01Z", GoVersion:"go1.15.5", Compiler:"gc", Platform:"linux/amd64"}

  • Cloud provider or hardware configuration:
    Azure Kubernetes Service

  • OS (e.g: cat /etc/os-release):
    Ubuntu 18.04

  • Kernel (e.g. uname -a):

  • Install tools:

  • Network plugin and version (if this is a network-related bug):

  • Others:

@karunasagark karunasagark added the kind/bug Categorizes issue or PR as related to a bug. label Jun 19, 2021
@k8s-ci-robot k8s-ci-robot added needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jun 19, 2021
@karunasagark
Copy link
Author

karunasagark commented Jun 19, 2021

/wg api-expression
/sig api-machinery

@k8s-ci-robot k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jun 19, 2021
@k8s-ci-robot
Copy link
Contributor

@karunasagark: The label(s) wg/wg-api-expression cannot be applied, because the repository doesn't have them.

In response to this:

/wg wg-api-expression
/sig api-machinery

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot removed the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Jun 19, 2021
@karunasagark
Copy link
Author

/wg api-expression

@k8s-ci-robot k8s-ci-robot added the wg/api-expression Categorizes an issue or PR as relevant to WG API Expression. label Jun 19, 2021
@fedebongio
Copy link
Contributor

/assign @leilajal @Jefftree
/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jun 22, 2021
@Jefftree
Copy link
Member

Jefftree commented Jun 22, 2021

This is a known issue and has been fixed and backported by this commit: kubernetes-sigs/structured-merge-diff#184 (SMD), #99038 (k/k)

The fix applies for server version >=1.19.10 and >=1.20.6. Looking at your server version, 1.19.7 doesn't yet include the fix, and upgrading should solve the problem.

Going to close this for now, feel free to reopen if the problem persists in the later versions.
/close

@k8s-ci-robot
Copy link
Contributor

@Jefftree: Closing this issue.

In response to this:

This is a known issue and has been fixed and backported by this commit: #99038

The fix applies for server version >=1.19.10 and >=1.20.6. Looking at your server version, 1.19.7 doesn't yet include the fix, and upgrading should solve the problem.

Going to close this for now, feel free to reopen if the problem persists in the later versions.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@karunasagark
Copy link
Author

To close out, I couldn't repro the bug using the repro steps mentioned above on 1.20.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. triage/accepted Indicates an issue or PR is ready to be actively worked on. wg/api-expression Categorizes an issue or PR as relevant to WG API Expression.
Projects
None yet
Development

No branches or pull requests

5 participants