Skip to content

Commit 396dd25

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#91993 from nodo/89274-change-of-managefields-via-subresources
Do not allow manual changes to manageFields via subresources
2 parents 983265a + c522ee0 commit 396dd25

File tree

10 files changed

+310
-33
lines changed

10 files changed

+310
-33
lines changed

‎staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,7 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd
849849
reqScope.Kind,
850850
reqScope.HubGroupVersion,
851851
crd.Spec.PreserveUnknownFields,
852+
false,
852853
)
853854
if err != nil {
854855
return nil, err
@@ -876,6 +877,21 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd
876877
// override status subresource values
877878
// shallow copy
878879
statusScope := *requestScopes[v.Name]
880+
if utilfeature.DefaultFeatureGate.Enabled(features.ServerSideApply) {
881+
statusScope.FieldManager, err = fieldmanager.NewDefaultCRDFieldManager(
882+
openAPIModels,
883+
statusScope.Convertor,
884+
statusScope.Defaulter,
885+
statusScope.Creater,
886+
statusScope.Kind,
887+
statusScope.HubGroupVersion,
888+
crd.Spec.PreserveUnknownFields,
889+
true,
890+
)
891+
if err != nil {
892+
return nil, err
893+
}
894+
}
879895
statusScope.Subresource = "status"
880896
statusScope.Namer = handlers.ContextBasedNaming{
881897
SelfLinker: meta.NewAccessor(),

‎staging/src/k8s.io/apiextensions-apiserver/test/integration/fixtures/resources.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,9 @@ func NewMultipleVersionNoxuCRD(scope apiextensionsv1beta1.ResourceScope) *apiext
205205
Storage: false,
206206
},
207207
},
208+
Subresources: &apiextensionsv1beta1.CustomResourceSubresources{
209+
Status: &apiextensionsv1beta1.CustomResourceSubresourceStatus{},
210+
},
208211
},
209212
}
210213
}

‎staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/capmanagers_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ func (*fakeManager) Apply(_, _ runtime.Object, _ fieldmanager.Managed, _ string,
4848

4949
func TestCapManagersManagerMergesEntries(t *testing.T) {
5050
f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"),
51+
false,
5152
func(m fieldmanager.Manager) fieldmanager.Manager {
5253
return fieldmanager.NewCapManagersManager(m, 3)
5354
})
@@ -113,6 +114,7 @@ func TestCapManagersManagerMergesEntries(t *testing.T) {
113114

114115
func TestCapUpdateManagers(t *testing.T) {
115116
f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"),
117+
false,
116118
func(m fieldmanager.Manager) fieldmanager.Manager {
117119
return fieldmanager.NewCapManagersManager(m, 3)
118120
})

‎staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go‎

Lines changed: 52 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -67,18 +67,19 @@ type Manager interface {
6767
// FieldManager updates the managed fields and merge applied
6868
// configurations.
6969
type FieldManager struct {
70-
fieldManager Manager
70+
fieldManager Manager
71+
ignoreManagedFieldsFromRequestObject bool
7172
}
7273

7374
// NewFieldManager creates a new FieldManager that decodes, manages, then re-encodes managedFields
7475
// on update and apply requests.
75-
func NewFieldManager(f Manager) *FieldManager {
76-
return &FieldManager{f}
76+
func NewFieldManager(f Manager, ignoreManagedFieldsFromRequestObject bool) *FieldManager {
77+
return &FieldManager{fieldManager: f, ignoreManagedFieldsFromRequestObject: ignoreManagedFieldsFromRequestObject}
7778
}
7879

7980
// NewDefaultFieldManager creates a new FieldManager that merges apply requests
8081
// and update managed fields for other types of requests.
81-
func NewDefaultFieldManager(models openapiproto.Models, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, hub schema.GroupVersion) (*FieldManager, error) {
82+
func NewDefaultFieldManager(models openapiproto.Models, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, hub schema.GroupVersion, ignoreManagedFieldsFromRequestObject bool) (*FieldManager, error) {
8283
typeConverter, err := internal.NewTypeConverter(models, false)
8384
if err != nil {
8485
return nil, err
@@ -88,13 +89,13 @@ func NewDefaultFieldManager(models openapiproto.Models, objectConverter runtime.
8889
if err != nil {
8990
return nil, fmt.Errorf("failed to create field manager: %v", err)
9091
}
91-
return newDefaultFieldManager(f, typeConverter, objectConverter, objectCreater, kind), nil
92+
return newDefaultFieldManager(f, typeConverter, objectConverter, objectCreater, kind, ignoreManagedFieldsFromRequestObject), nil
9293
}
9394

9495
// NewDefaultCRDFieldManager creates a new FieldManager specifically for
9596
// CRDs. This allows for the possibility of fields which are not defined
9697
// in models, as well as having no models defined at all.
97-
func NewDefaultCRDFieldManager(models openapiproto.Models, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, hub schema.GroupVersion, preserveUnknownFields bool) (_ *FieldManager, err error) {
98+
func NewDefaultCRDFieldManager(models openapiproto.Models, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, hub schema.GroupVersion, preserveUnknownFields, ignoreManagedFieldsFromRequestObject bool) (_ *FieldManager, err error) {
9899
var typeConverter internal.TypeConverter = internal.DeducedTypeConverter{}
99100
if models != nil {
100101
typeConverter, err = internal.NewTypeConverter(models, preserveUnknownFields)
@@ -106,11 +107,11 @@ func NewDefaultCRDFieldManager(models openapiproto.Models, objectConverter runti
106107
if err != nil {
107108
return nil, fmt.Errorf("failed to create field manager: %v", err)
108109
}
109-
return newDefaultFieldManager(f, typeConverter, objectConverter, objectCreater, kind), nil
110+
return newDefaultFieldManager(f, typeConverter, objectConverter, objectCreater, kind, ignoreManagedFieldsFromRequestObject), nil
110111
}
111112

112113
// newDefaultFieldManager is a helper function which wraps a Manager with certain default logic.
113-
func newDefaultFieldManager(f Manager, typeConverter internal.TypeConverter, objectConverter runtime.ObjectConvertor, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind) *FieldManager {
114+
func newDefaultFieldManager(f Manager, typeConverter internal.TypeConverter, objectConverter runtime.ObjectConvertor, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, ignoreManagedFieldsFromRequestObject bool) *FieldManager {
114115
f = NewStripMetaManager(f)
115116
f = NewManagedFieldsUpdater(f)
116117
f = NewBuildManagerInfoManager(f, kind.GroupVersion())
@@ -119,36 +120,59 @@ func newDefaultFieldManager(f Manager, typeConverter internal.TypeConverter, obj
119120
f = NewLastAppliedManager(f, typeConverter, objectConverter, kind.GroupVersion())
120121
f = NewLastAppliedUpdater(f)
121122

122-
return NewFieldManager(f)
123+
return NewFieldManager(f, ignoreManagedFieldsFromRequestObject)
123124
}
124125

125-
// Update is used when the object has already been merged (non-apply
126-
// use-case), and simply updates the managed fields in the output
127-
// object.
128-
func (f *FieldManager) Update(liveObj, newObj runtime.Object, manager string) (object runtime.Object, err error) {
126+
func decodeLiveManagedFields(liveObj runtime.Object) (Managed, error) {
127+
liveAccessor, err := meta.Accessor(liveObj)
128+
if err != nil {
129+
return nil, err
130+
}
131+
managed, err := internal.DecodeObjectManagedFields(liveAccessor.GetManagedFields())
132+
if err != nil {
133+
return internal.NewEmptyManaged(), nil
134+
}
135+
return managed, nil
136+
}
137+
138+
func decodeManagedFields(liveObj, newObj runtime.Object, ignoreManagedFieldsFromRequestObject bool) (Managed, error) {
139+
// We take the managedFields of the live object in case the request tries to
140+
// manually set managedFields via a subresource.
141+
if ignoreManagedFieldsFromRequestObject {
142+
return decodeLiveManagedFields(liveObj)
143+
}
144+
129145
// If the object doesn't have metadata, we should just return without trying to
130146
// set the managedFields at all, so creates/updates/patches will work normally.
131147
newAccessor, err := meta.Accessor(newObj)
132148
if err != nil {
133-
return newObj, nil
149+
return nil, err
150+
}
151+
152+
if isResetManagedFields(newAccessor.GetManagedFields()) {
153+
return internal.NewEmptyManaged(), nil
154+
}
155+
156+
managed, err := internal.DecodeObjectManagedFields(newAccessor.GetManagedFields())
157+
// If the managed field is empty or we failed to decode it,
158+
// let's try the live object. This is to prevent clients who
159+
// don't understand managedFields from deleting it accidentally.
160+
if err != nil || len(managed.Fields()) == 0 {
161+
return decodeLiveManagedFields(liveObj)
134162
}
135163

164+
return managed, nil
165+
}
166+
167+
// Update is used when the object has already been merged (non-apply
168+
// use-case), and simply updates the managed fields in the output
169+
// object.
170+
func (f *FieldManager) Update(liveObj, newObj runtime.Object, manager string) (object runtime.Object, err error) {
136171
// First try to decode the managed fields provided in the update,
137172
// This is necessary to allow directly updating managed fields.
138-
var managed Managed
139-
if isResetManagedFields(newAccessor.GetManagedFields()) {
140-
managed = internal.NewEmptyManaged()
141-
} else if managed, err = internal.DecodeObjectManagedFields(newAccessor.GetManagedFields()); err != nil || len(managed.Fields()) == 0 {
142-
liveAccessor, err := meta.Accessor(liveObj)
143-
if err != nil {
144-
return newObj, nil
145-
}
146-
// If the managed field is empty or we failed to decode it,
147-
// let's try the live object. This is to prevent clients who
148-
// don't understand managedFields from deleting it accidentally.
149-
if managed, err = internal.DecodeObjectManagedFields(liveAccessor.GetManagedFields()); err != nil {
150-
managed = internal.NewEmptyManaged()
151-
}
173+
managed, err := decodeManagedFields(liveObj, newObj, f.ignoreManagedFieldsFromRequestObject)
174+
if err != nil {
175+
return newObj, nil
152176
}
153177

154178
internal.RemoveObjectManagedFields(liveObj)

‎staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go‎

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,14 @@ type TestFieldManager struct {
8686
}
8787

8888
func NewDefaultTestFieldManager(gvk schema.GroupVersionKind) TestFieldManager {
89-
return NewTestFieldManager(gvk, nil)
89+
return NewTestFieldManager(gvk, false, nil)
9090
}
9191

92-
func NewTestFieldManager(gvk schema.GroupVersionKind, chainFieldManager func(fieldmanager.Manager) fieldmanager.Manager) TestFieldManager {
92+
func NewSubresourceTestFieldManager(gvk schema.GroupVersionKind) TestFieldManager {
93+
return NewTestFieldManager(gvk, true, nil)
94+
}
95+
96+
func NewTestFieldManager(gvk schema.GroupVersionKind, ignoreManagedFieldsFromRequestObject bool, chainFieldManager func(fieldmanager.Manager) fieldmanager.Manager) TestFieldManager {
9397
m := NewFakeOpenAPIModels()
9498
typeConverter := NewFakeTypeConverter(m)
9599
converter := internal.NewVersionConverter(typeConverter, &fakeObjectConvertor{}, gvk.GroupVersion())
@@ -118,7 +122,7 @@ func NewTestFieldManager(gvk schema.GroupVersionKind, chainFieldManager func(fie
118122
f = chainFieldManager(f)
119123
}
120124
return TestFieldManager{
121-
fieldManager: fieldmanager.NewFieldManager(f),
125+
fieldManager: fieldmanager.NewFieldManager(f, ignoreManagedFieldsFromRequestObject),
122126
emptyObj: live,
123127
liveObj: live.DeepCopyObject(),
124128
}
@@ -1233,3 +1237,56 @@ func getLastApplied(obj runtime.Object) (string, error) {
12331237
}
12341238
return lastApplied, nil
12351239
}
1240+
1241+
func TestUpdateViaSubresources(t *testing.T) {
1242+
f := NewSubresourceTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"))
1243+
1244+
obj := &unstructured.Unstructured{Object: map[string]interface{}{}}
1245+
if err := yaml.Unmarshal([]byte(`{
1246+
"apiVersion": "v1",
1247+
"kind": "Pod",
1248+
"metadata": {
1249+
"labels": {
1250+
"a":"b"
1251+
},
1252+
}
1253+
}`), &obj.Object); err != nil {
1254+
t.Fatalf("error decoding YAML: %v", err)
1255+
}
1256+
obj.SetManagedFields([]metav1.ManagedFieldsEntry{
1257+
{
1258+
Manager: "test",
1259+
Operation: metav1.ManagedFieldsOperationApply,
1260+
APIVersion: "apps/v1",
1261+
FieldsType: "FieldsV1",
1262+
FieldsV1: &metav1.FieldsV1{
1263+
[]byte(`{"f:metadata":{"f:labels":{"f:another_field":{}}}}`),
1264+
},
1265+
},
1266+
})
1267+
1268+
// Check that managed fields cannot be changed via subresources
1269+
expectedManager := "fieldmanager_test_subresource"
1270+
if err := f.Update(obj, expectedManager); err != nil {
1271+
t.Fatalf("failed to apply object: %v", err)
1272+
}
1273+
1274+
managedFields := f.ManagedFields()
1275+
if len(managedFields) != 1 {
1276+
t.Fatalf("Expected new managed fields to have one entry. Got:\n%#v", managedFields)
1277+
}
1278+
if managedFields[0].Manager != expectedManager {
1279+
t.Fatalf("Expected first item to have manager set to: %s. Got: %s", expectedManager, managedFields[0].Manager)
1280+
}
1281+
1282+
// Check that managed fields cannot be reset via subresources
1283+
newObj := obj.DeepCopy()
1284+
newObj.SetManagedFields([]metav1.ManagedFieldsEntry{})
1285+
if err := f.Update(newObj, expectedManager); err != nil {
1286+
t.Fatalf("failed to apply object: %v", err)
1287+
}
1288+
newManagedFields := f.ManagedFields()
1289+
if len(newManagedFields) != 1 {
1290+
t.Fatalf("Expected new managed fields to have one entry. Got:\n%#v", newManagedFields)
1291+
}
1292+
}

‎staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/lastappliedupdater_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828

2929
func TestLastAppliedUpdater(t *testing.T) {
3030
f := NewTestFieldManager(schema.FromAPIVersionAndKind("apps/v1", "Deployment"),
31+
false,
3132
func(m fieldmanager.Manager) fieldmanager.Manager {
3233
return fieldmanager.NewLastAppliedUpdater(m)
3334
})

‎staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/skipnonapplied_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func (f *fakeObjectCreater) New(_ schema.GroupVersionKind) (runtime.Object, erro
4343
}
4444

4545
func TestNoUpdateBeforeFirstApply(t *testing.T) {
46-
f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"), func(m fieldmanager.Manager) fieldmanager.Manager {
46+
f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"), false, func(m fieldmanager.Manager) fieldmanager.Manager {
4747
return fieldmanager.NewSkipNonAppliedManager(
4848
m,
4949
&fakeObjectCreater{gvk: schema.GroupVersionKind{Version: "v1", Kind: "Pod"}},
@@ -83,7 +83,7 @@ func TestNoUpdateBeforeFirstApply(t *testing.T) {
8383
}
8484

8585
func TestUpdateBeforeFirstApply(t *testing.T) {
86-
f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"), func(m fieldmanager.Manager) fieldmanager.Manager {
86+
f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"), false, func(m fieldmanager.Manager) fieldmanager.Manager {
8787
return fieldmanager.NewSkipNonAppliedManager(
8888
m,
8989
&fakeObjectCreater{gvk: schema.GroupVersionKind{Version: "v1", Kind: "Pod"}},

‎staging/src/k8s.io/apiserver/pkg/endpoints/installer.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
570570
a.group.Creater,
571571
fqKindToRegister,
572572
reqScope.HubGroupVersion,
573+
isSubresource,
573574
)
574575
if err != nil {
575576
return nil, fmt.Errorf("failed to create field manager: %v", err)

‎test/integration/apiserver/apply/apply_crd_test.go‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
2828
"k8s.io/apiextensions-apiserver/test/integration/fixtures"
2929
apierrors "k8s.io/apimachinery/pkg/api/errors"
30+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3031
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
3132
"k8s.io/apimachinery/pkg/types"
3233
genericfeatures "k8s.io/apiserver/pkg/features"
@@ -129,6 +130,69 @@ spec:
129130
t.Fatalf("failed to apply object with force after updating replicas: %v:\n%v", err, string(result))
130131
}
131132
verifyReplicas(t, result, 1)
133+
134+
// Try to set managed fields using a subresource and verify that it has no effect
135+
existingManagedFields, err := getManagedFields(result)
136+
if err != nil {
137+
t.Fatalf("failed to get managedFields from response: %v", err)
138+
}
139+
updateBytes := []byte(`{
140+
"metadata": {
141+
"managedFields": [{
142+
"manager":"testing",
143+
"operation":"Update",
144+
"apiVersion":"v1",
145+
"fieldsType":"FieldsV1",
146+
"fieldsV1":{
147+
"f:spec":{
148+
"f:containers":{
149+
"k:{\"name\":\"testing\"}":{
150+
".":{},
151+
"f:image":{},
152+
"f:name":{}
153+
}
154+
}
155+
}
156+
}
157+
}]
158+
}
159+
}`)
160+
result, err = rest.Patch(types.MergePatchType).
161+
AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural).
162+
SubResource("status").
163+
Name(name).
164+
Param("fieldManager", "subresource_test").
165+
Body(updateBytes).
166+
DoRaw(context.TODO())
167+
if err != nil {
168+
t.Fatalf("Error updating subresource: %v ", err)
169+
}
170+
newManagedFields, err := getManagedFields(result)
171+
if err != nil {
172+
t.Fatalf("failed to get managedFields from response: %v", err)
173+
}
174+
if !reflect.DeepEqual(existingManagedFields, newManagedFields) {
175+
t.Fatalf("Expected managed fields to not have changed when trying manually settting them via subresoures.\n\nExpected: %#v\n\nGot: %#v", existingManagedFields, newManagedFields)
176+
}
177+
178+
// However, it is possible to modify managed fields using the main resource
179+
result, err = rest.Patch(types.MergePatchType).
180+
AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural).
181+
Name(name).
182+
Param("fieldManager", "subresource_test").
183+
Body([]byte(`{"metadata":{"managedFields":[{}]}}`)).
184+
DoRaw(context.TODO())
185+
if err != nil {
186+
t.Fatalf("Error updating managed fields of the main resource: %v ", err)
187+
}
188+
newManagedFields, err = getManagedFields(result)
189+
if err != nil {
190+
t.Fatalf("failed to get managedFields from response: %v", err)
191+
}
192+
193+
if len(newManagedFields) != 0 {
194+
t.Fatalf("Expected managed fields to have been reset, but got: %v", newManagedFields)
195+
}
132196
}
133197

134198
// TestApplyCRDStructuralSchema tests that when a CRD has a structural schema in its validation field,
@@ -753,3 +817,11 @@ spec:
753817
}
754818
verifyReplicas(t, result, 1)
755819
}
820+
821+
func getManagedFields(rawResponse []byte) ([]metav1.ManagedFieldsEntry, error) {
822+
obj := unstructured.Unstructured{}
823+
if err := obj.UnmarshalJSON(rawResponse); err != nil {
824+
return nil, err
825+
}
826+
return obj.GetManagedFields(), nil
827+
}

0 commit comments

Comments
 (0)