🌱 Add field.Path aggregation for Machine webhook #6368
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
Jont828
changed the title
k8s-ci-robot
added
the
size/S
|
/assign @killianmuldoon |
|
@killianmuldoon I followed the basic idea of #6322. Are these the changes we want to see? |
There was a problem hiding this comment.
Looking good - one tiny change to make the ClusterName field change forbidden rather than invalid.
Thanks @Jont828
api/v1beta1/machine_webhook.go
Outdated
| @@ -126,13 +127,14 @@ func (m *Machine) validate(old *Machine) error { | |||
| if old != nil && old.Spec.ClusterName != m.Spec.ClusterName { | |||
| allErrs = append( | |||
| allErrs, | |||
| field.Invalid(field.NewPath("spec", "clusterName"), m.Spec.ClusterName, "field is immutable"), | |||
| // Should this be field.Forbidden? | |||
| field.Invalid(specPath.Child("clusterName"), m.Spec.ClusterName, "field is immutable"), | |||
There was a problem hiding this comment.
Good idea to change this to field.Forbidden
|
@killianmuldoon Just added the |
api/v1beta1/machine_webhook.go
Outdated
| @@ -126,13 +127,13 @@ func (m *Machine) validate(old *Machine) error { | |||
| if old != nil && old.Spec.ClusterName != m.Spec.ClusterName { | |||
| allErrs = append( | |||
| allErrs, | |||
| field.Invalid(field.NewPath("spec", "clusterName"), m.Spec.ClusterName, "field is immutable"), | |||
| field.Forbidden(specPath.Child("clusterName"), "spec.clusterName is immutable"), | |||
There was a problem hiding this comment.
I think we can leave the message the same here as the path will be part of the message elsewhere.
| field.Forbidden(specPath.Child("clusterName"), "spec.clusterName is immutable"), | |
| field.Forbidden(specPath.Child("clusterName"), "field is immutable"), |
There was a problem hiding this comment.
@killianmuldoon Sounds good, just squashed and pushed.
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fabriziopandini The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
What this PR does / why we need it: Update usage of
field.Pathin Machine webhook as part of an effort to make core webhook validation more consistent.Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #6249