Malformed payloads cause excessive warning headers, potentially leading to 502 responses

[mantis-toboggan-md]

This is a follow up from #9012 and there's more detail in the comments there.

Steve adds a few fields to all resources:
metadata.fields
metadata.state
metadata.relationships
status.conditions[i].error
status.conditions[i].transitioning

When the user edits a resource, these are included in the payload, and each results in a warning header in the response.

The ui has been sending these extra fields for a while and it's only recently become a problem due to the combo of k8s 1.25 validating by default, and steve including warning headers as of 2.7.2. There will need to be more discussion around how to handle this, potentially in coordination with backend work.

[gaktive]

@KevinJoiner to flesh out some of the impact here and then file a related backend ticket to see what can be done in the API since though UI can truncate, new fields could be added.

@richard-cox to explore around this a bit.

[richard-cox]

Useful References

original PR that brought this in - kubernetes/enhancements#2885
limited docs - https://kubernetes.io/docs/reference/using-api/api-concepts/#validation-for-unrecognized-or-duplicate-fields-setting-the-field-validation-level

Current state

I've created #9030 to help identify where we get warnings. Some output below (create followed by edit for pod, deployment and secret resources)

Validation Warnings for https://localhost:8005/v1/pods

299 - unknown field "spec.containers[0].__active"
299 - unknown field "spec.containers[0]._init"
299 - unknown field "spec.selector"
299 - unknown field "type"


Validation Warnings for https://localhost:8005/v1/pods/default/aaaaa

299 - unknown field "metadata.fields"
299 - unknown field "metadata.relationships"
299 - unknown field "metadata.state"
299 - unknown field "spec.containers[0].__active"
299 - unknown field "spec.containers[0]._init"
299 - unknown field "status.conditions[0].error"
299 - unknown field "status.conditions[0].lastUpdateTime"
299 - unknown field "status.conditions[0].transitioning"
299 - unknown field "status.conditions[1].error"
299 - unknown field "status.conditions[1].lastUpdateTime"
299 - unknown field "status.conditions[1].transitioning"
299 - unknown field "status.conditions[2].error"
299 - unknown field "status.conditions[2].lastUpdateTime"
299 - unknown field "status.conditions[2].transitioning"
299 - unknown field "status.conditions[3].error"
299 - unknown field "status.conditions[3].lastUpdateTime"
299 - unknown field "status.conditions[3].transitioning"



Validation Warnings for https://localhost:8005/v1/apps.deployments

299 - unknown field "spec.template.spec.containers[0].__active"
299 - unknown field "spec.template.spec.containers[0]._init"
299 - unknown field "type"

console.js:31 Validation Warnings for https://localhost:8005/v1/apps.deployments/default/aaaaa

299 - unknown field "metadata.fields"
299 - unknown field "metadata.relationships"
299 - unknown field "metadata.state"
299 - unknown field "spec.template.spec.containers[0].__active"
299 - unknown field "spec.template.spec.containers[0]._init"
299 - unknown field "status.conditions[0].error"
299 - unknown field "status.conditions[0].transitioning"
299 - unknown field "status.conditions[1].error"
299 - unknown field "status.conditions[1].transitioning"



Validation Warnings for https://localhost:8005/v1/secrets

299 - unknown field "_type"

Validation Warnings for https://localhost:8005/v1/secrets/default/asdsadsad

299 - unknown field "metadata.fields"
299 - unknown field "metadata.relationships"
299 - unknown field "metadata.state"

Create vs Edit

We do strip out some properties already, but only when cloning or saving as yaml. Importantly NOT when editing

• see

  dashboard/shell/plugins/steve/actions.js

  Line 274 in c1e3897

  cleanForNew(ctx, obj) {

• root properties - 'actions', 'links', 'status', 'id'
• metadata properties - 'uid', 'ownerReferences', 'selfLink', 'creationTimestamp', 'deletionTimestamp', 'state', 'fields', 'relationships', 'generation', 'managedFields', 'resourceVersion',

Edit - Actually it looks like cleanForDownload does pretty much what we want to do, it even contains references to the error and transitioning status condition keys

Options

• Head in sand / Ignore warnings
  • Add fieldValidation=Ignore to all post/put requests
• Remove fields client side (basic edition)
  • We can add a toSave fn to the steve model (this pattern already exists) that calls a beefed up version of toJSON. The toJson will remove a similar set of properties as per cleanForNew
    • Should status be also removed?
  • On a more case by case basis remove properties outside of those removed above (in particular properties with a preceding _)
• Remove fields client side (advanced edition)
  • Iterate through all fields removing properties not in schema
  • This does raise the question, shouldn't calculated fields added by steve be in the schema? Should they be marked as something that shouldn't be persisted?
• Remove fields server side
  • Kind of like two client side options above, it either removes properties it added... or removes any not in a schema
  • This has the added benefit of supporting users who edit steve resources outside of the UI

[aalves08]

FYI @aalves08

[gaktive]

We need to dig into this some more since we could tackle the generic ones, but that may be 80% of what's there.

This feels like an epic that needs to be split up.

[richard-cox]

The two issues required to resolve this are...

• Remove common invalid properties when updating kube resources #9371
• Remove resource specific invalid properties when updating kube resources #9372

[richard-cox]

Linking backend changes (which covers some of the common cases) - rancher/rancher#41772