redhat-cop / resource-locker-operator

Apache License 2.0
30 stars 14 forks source link

Resource locking does not enforce additions #49

Open erzhan46 opened 3 years ago

erzhan46 commented 3 years ago

I tried to deploy example resource locking - Config Map with 'foo:bar' data. While operator 'enforce' any changes in the existing entry - it ignores any entries added manually. My expectation was that it should delete any lines added manually - but it just ignores them.

Original ConfigMap:

apiVersion: v1
data:
  foo: bar
kind: ConfigMap
metadata:
  name: foo-bar-configmap
  namespace: resource-locker-test-ns

Manually edited ConfigMap:

apiVersion: v1
data:
  foo: bar
  foo2: bar2                        <---- This is the line added via 'oc edit'
kind: ConfigMap
metadata:
  name: foo-bar-configmap
  namespace: resource-locker-test-ns
raffaelespazzoli commented 3 years ago

can you share your resource locking CR?

erzhan46 commented 3 years ago

I used example here: https://github.com/redhat-cop/resource-locker-operator/blob/master/EXAMPLES.md

apiVersion: redhatcop.redhat.io/v1alpha1 kind: ResourceLocker metadata: name: locked-configmap-foo-bar-configmap namespace: resource-locker-test-ns spec: resources:

erzhan46 commented 3 years ago

Another issue I noticed is that if I edit ConfigMap and change name 'foo' to something else , e.g. 'foo2' - then operator will leave it as is and just add new 'foo: bar' line

raffaelespazzoli commented 3 years ago

yeah, I was able to reproduce the behavior. I am not sure it's an error, but I am pretty sure that I cannot do much about it. Here is why: to be able to manage the excludedPaths, when the object changes, I need to update it as with a patch, in which the non excludedPaths are merged over the existing object. The behavior of a merge on a list/collection, as is the .data field, is controlled by the api definition. In this case it is granular:

mapType
string
specifies the level of atomicity of the map; i.e. whether each item in the map is independent of the others, or all fields are treated as a single unit.
Possible values:

“granular“: items in the map are independent of each other, and can be manipulated by different actors. This is the default behavior.
“atomic“: all fields are treated as one unit. Any changes have to replace the entire map.

I tried different types of merges:

const (
    JSONPatchType           PatchType = "application/json-patch+json"
    MergePatchType          PatchType = "application/merge-patch+json"
    StrategicMergePatchType PatchType = "application/strategic-merge-patch+json"
    ApplyPatchType          PatchType = "application/apply-patch+yaml"
)

but those that work all honor the granular semantic.

raffaelespazzoli commented 3 years ago

I agree that this is not the expected behavior.