'jsonpatch remove operation does not apply' when updating rolebinding

[henrjk]

The symptom I am seeing is

Patching rolebinding/admin-0 ... failed
2019/10/11 07:17:26 Update aborted: Error from server: jsonpatch remove operation does not apply: doc is missing path: "/groupNames/0"

Excerpt from the tailor update log:

...
~ rolebinding/admin-0 to update
--- Current State (OpenShift cluster)
+++ Desired State (Processed template)
@@ -1,16 +1,14 @@
 apiVersion: authorization.openshift.io/v1
-groupNames:
-- dedicated-admins
-- system:serviceaccounts:dedicated-admin
+groupNames: null
 kind: RoleBinding
 metadata:
   name: admin-0
 roleRef:
   name: admin
 subjects:
-- kind: Group
-  name: dedicated-admins
-- kind: SystemGroup
-  name: system:serviceaccounts:dedicated-admin
-userNames: null
+- kind: ServiceAccount
+  name: jenkins
+  namespace: asap-cd
+userNames:
+- system:serviceaccount:asap-cd:jenkins

...
Apply changes? [y/n]: y

===== Applying changes related to context directory . =====
Creating rolebinding/admin-2 ... done
Creating rolebinding/view-0 ... done
Deleting rolebinding/dedicated-project-admin ... done
Patching rolebinding/admin-0 ... failed
2019/10/11 07:17:26 Update aborted: Error from server: jsonpatch remove operation does not apply: doc is missing path: "/groupNames/0"

After this excerpt from issuing tailor with --diff=json

...
~ rolebinding/admin-0 to update
[
  {
    "op": "replace",
    "path": "/groupNames"
  },
  {
    "op": "remove",
    "path": "/groupNames/0"
  },
  {
    "op": "remove",
    "path": "/groupNames/1"
  },
  {
    "op": "replace",
    "path": "/subjects/0/kind",
    "value": "ServiceAccount"
  },
  {
    "op": "replace",
    "path": "/subjects/0/name",
    "value": "jenkins"
  },
  {
    "op": "add",
    "path": "/subjects/0/namespace",
    "value": "asap-cd"
  },
  {
    "op": "remove",
    "path": "/subjects/1"
  },
  {
    "op": "add",
    "path": "/userNames/0",
    "value": "system:serviceaccount:asap-cd:jenkins"
  }
]
...

Definition of admin-0 in rb.yml

- apiVersion: authorization.openshift.io/v1
  groupNames: null
  kind: RoleBinding
  metadata:
    name: admin-0
  roleRef:
    name: admin
  subjects:
  - kind: ServiceAccount
    name: jenkins
    namespace: asap-cd
  userNames:
  - system:serviceaccount:asap-cd:jenkins

and exported from asap-demo1:

- apiVersion: authorization.openshift.io/v1
  groupNames:
  - dedicated-admins
  - system:serviceaccounts:dedicated-admin
  kind: RoleBinding
  metadata:
    creationTimestamp: null
    name: admin-0
  roleRef:
    name: admin
  subjects:
  - kind: Group
    name: dedicated-admins
  - kind: SystemGroup
    name: system:serviceaccounts:dedicated-admin
  userNames: null

I would guess replace operations need to always have a value.

[michaelsauter]

Hmm. The issue might be that it is trying to remove and replace at the same time:

  {
    "op": "replace",
    "path": "/groupNames"
  },
  {
    "op": "remove",
    "path": "/groupNames/0"
  },
  {
    "op": "remove",
    "path": "/groupNames/1"
  }

And yes, the replace without a value seems fishy ... I will investigate.

[michaelsauter]

I figured out that the JSON patches are correct if groupNames: null is replaced with groupNames: []. groupNames should be an array but since YAML does not enforce it Tailor should probably handle the case when it is null.

[michaelsauter]

Today I realised that when you look at the YAML of the rolebinding in the console UI, the field groupNames or userNames does not actually exist. I read up on it in the API:

(array) UserNames holds all the usernames directly bound to the role. This field should only be specified when supporting legacy clients and servers. See Subjects for further details.

My conclusion is that I will not care about those fields and simply strip them from the config ... which should also fix this issue here.

@henrjk Does that make sense to you?

[henrjk]

what happens for rolebindings that have groupnames or usernames. If they are added to the subjects after application, then I would prefer if tailor would warn users that these are being ignored.

[michaelsauter]

OK, I'll think about it that ... but might be just a "verbose message" when it is being removed ... generally I don't think we run into that problem. Remember that on the cluster, the YAML does not even contain those fields - they are ADDED during export ...

[gerardcl]

hi! I was also doind some research about the rolebinding export/import in the past, for the autoclone feature, I implemented this python script for testing, which uses a JSON formatted export from a oc get --export rolibidings -o json -n project > jsonfile.json:

import sys
import json

# print('Number of arguments:', len(sys.argv), 'arguments.')
# print('Argument List:', str(sys.argv))

if len(sys.argv) < 2 or len(sys.argv) > 2:
    print('only one input JSON file is accepted')
    exit(1)

users = []
groups = []

with open(sys.argv[1], 'r') as infile:
    d = json.load(infile)

    for rb in d['items']:
        # print(rb['roleRef']['name'])
        if rb['kind'] == 'RoleBinding':
            if rb['userNames']:
                for username in rb['userNames']:
                    # print("\tuser:\t"+username)
                    users.append(rb['roleRef']['name'] +" "+ username)
            if rb['groupNames']:
                for groupname in rb['groupNames']:
                    # print("\tgroup:\t"+groupname)
                    groups.append(rb['roleRef']['name'] +" "+ groupname)

# print(users)
# print(groups)

with open('users-rolebindings.tsv', 'w') as outfile:
    for user in users:
        outfile.write(str(user)+'\n')

with open('groups-rolebindings.tsv', 'w') as outfile:
    for group in groups:
        outfile.write(str(group)+'\n')

which is basically creating two different files, one for group role bindings and the other for user role bindings, which then can be just used for iterating and applying each respective role and user/group policy to the new enviroment to creat.

My question is, couldn't we integrate it into tailor directly at some point?

If now, shall we use it directly so to integrate it from export to import scripts?

[michaelsauter]

@gerardcl Thanks for chiming in.

However, I am not sure how that would help the current situation?

I think moving rolebindings from one cluster to another is a tricky beast and can't easily be done. Tailor assumes "same config across envs, just different params". Anything that falls outside this perspective is not suited to how Tailor "thinks".

It could be that dealing with "rolebinding migration" is done better outside of Tailor (e.g. with a script like yours) but even that assumes certain roles and users to match between clusters, right?

For this issue, I want to just tackle the bug that @henrjk encountered - and that can be done by removing the paths altogether, which I intend to implement on Friday if I get to it.

I am more than open though to explore how this rolebinding migration can be handled in general though ... but a bit out of scope here.

[gerardcl]

totally agree! I jumped into this thread and I got caught by the rolebinding user and group topic aforementioned...this topic is indeed related to export and import steps for autocloning feature, for sure :) will come back to it at some point!