Skip to content

Commit

Permalink
Fix path for the error when mutating managedBy
Browse files Browse the repository at this point in the history
  • Loading branch information
mimowo committed Apr 18, 2024
1 parent 51db356 commit 5f843af
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 6 deletions.
2 changes: 1 addition & 1 deletion pkg/webhooks/jobset_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ func (j *jobSetWebhook) ValidateUpdate(ctx context.Context, old, newObj runtime.
}
// Note that SucccessPolicy and failurePolicy are made immutable via CEL.
errs := apivalidation.ValidateImmutableField(mungedSpec.ReplicatedJobs, oldJS.Spec.ReplicatedJobs, field.NewPath("spec").Child("replicatedJobs"))
errs = append(errs, apivalidation.ValidateImmutableField(mungedSpec.ManagedBy, oldJS.Spec.ManagedBy, field.NewPath("spec").Child("labels").Key("managedBy"))...)
errs = append(errs, apivalidation.ValidateImmutableField(mungedSpec.ManagedBy, oldJS.Spec.ManagedBy, field.NewPath("spec").Child("managedBy"))...)
return nil, errs.ToAggregate()
}

Expand Down
31 changes: 26 additions & 5 deletions pkg/webhooks/jobset_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/stretchr/testify/assert"
batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -1051,6 +1052,26 @@ func TestValidateUpdate(t *testing.T) {
},
},
},
{
name: "managedBy is immutable",
js: &jobset.JobSet{
ObjectMeta: validObjectMeta,
Spec: jobset.JobSetSpec{
ManagedBy: ptr.To("example.com/one"),
ReplicatedJobs: validReplicatedJobs,
},
},
oldJs: &jobset.JobSet{
ObjectMeta: validObjectMeta,
Spec: jobset.JobSetSpec{
ManagedBy: ptr.To("example.com/two"),
ReplicatedJobs: validReplicatedJobs,
},
},
want: field.ErrorList{
field.Invalid(field.NewPath("spec").Child("managedBy"), "", "field is immutable"),
}.ToAggregate(),
},
{
name: "replicated jobs are immutable",
js: &jobset.JobSet{
Expand Down Expand Up @@ -1085,7 +1106,9 @@ func TestValidateUpdate(t *testing.T) {
ReplicatedJobs: validReplicatedJobs,
},
},
want: fmt.Errorf("field is immutable"),
want: field.ErrorList{
field.Invalid(field.NewPath("spec").Child("replicatedJobs"), "", "field is immutable"),
}.ToAggregate(),
},
}

Expand All @@ -1097,10 +1120,8 @@ func TestValidateUpdate(t *testing.T) {
newObj := tc.js.DeepCopyObject()
oldObj := tc.oldJs.DeepCopyObject()
_, err = webhook.ValidateUpdate(context.TODO(), oldObj, newObj)
if tc.want != nil {
assert.True(t, strings.Contains(err.Error(), tc.want.Error()))
} else {
assert.Nil(t, err)
if diff := cmp.Diff(tc.want, err, cmpopts.IgnoreFields(field.Error{}, "BadValue")); diff != "" {
t.Errorf("ValidateResources() mismatch (-want +got):\n%s", diff)
}
})
}
Expand Down

0 comments on commit 5f843af

Please sign in to comment.