Operator attempts to list all namespaces, fails, after 1Pass item update

[parente]

Your environment

Chart Version: 2.1.0

Helm Version:

Kubernetes Version: v1.20.4

What happened?

Operator logs endless permission error listing namespaces after a change to a 1Password vault item previously created as a secret on the cluster.

E0808 23:25:10.319571       1 reflector.go:178] sigs.k8s.io/controller-runtime/pkg/cache/internal/informers_map.go:224: Failed to list *v1.Namespace: namespaces is forbidden: User "system:serviceaccount:1password-connect:onepassword-connect-operator" cannot list resource "namespaces" in API group "" at the cluster scope

What did you expect to happen?

Operator should have updated the secret successfully and restarted the deployment using it.

Steps to reproduce

1. Deploy the v2.1.0 helm chart with both connect server and operator enabled, and the operator configured to watch the 1password-connect and default namespaces.
2. Create the deployment resource below in the default namespace.
3. Verify the k8s Secret associated with the example-secret 1Password item is created and available both in env vars and on disk in the deployment pod.
4. Make a change to the value associated with some-key in the 1Password UI (e.g., some-value -> some-value-changed).
5. Note the continual error messages logged by the operator (see above), that the k8s secret is not updated/recreated wit the new value, and the deployment is not rolled.

apiVersion: apps/v1
kind: Deployment
metadata:
  name: example-deployment
  labels:
    app.kubernetes.io/name: example-deployment
  annotations:
    operator.1password.io/item-path: 'vaults/sandbox-dev-k8s/items/example-secret'
    operator.1password.io/item-name: 'example-secret'
    operator.1password.io/auto-restart: 'true'
spec:
  replicas: 1
  selector:
    matchLabels:
      app.kubernetes.io/name: example-deployment
  template:
    metadata:
      labels:
        app.kubernetes.io/name: example-deployment
    spec:
      securityContext:
        runAsNonRoot: true
        runAsUser: 1000
      containers:
        - name: example-deployment
          image: alpine:3
          command: ['/bin/sh', '-c', '--']
          args: ['while true; do sleep 30; done;']
          resources:
            limits:
              memory: 64Mi
            requests:
              memory: 64Mi
          # Example mounting contents of key/value pairs from the 1Password entry as env vars
          env:
            - name: SECRET_SOME_KEY
              valueFrom:
                secretKeyRef:
                  name: example-secret
                  key: some-key
            - name: SECRET_ANOTHER_KEY
              valueFrom:
                secretKeyRef:
                  name: example-secret
                  key: another-key
          # Example mounting contents of key/value pairs from the 1Password entry as files on disk
          volumeMounts:
            - name: secret
              mountPath: '/mnt/secrets'
              readOnly: true
      volumes:
        - name: secret
          secret:
            secretName: example-secret

Notes & Logs

I verified the helm release created a ClusterRole with list permission for the namespaces resource. I also verified the helm release created a RoleBinding in the watched namespaces.

Should the operator be attempting to list all namespaces when the helm release is configured with an explicit set of namespaces? Is listing all namespaces an allowed operation for RoleBindings (perhaps only for ClusterRoleBindings)?

[parente]

The deployment example included in the 1Password/onepassword-operator repository uses ClusterRoleBindings instead which may work around the issue when the operator attempts to list all namespaces:

https://github.com/1Password/onepassword-operator/blob/b50d864b50760e3980d314b11db736dee8bd147b/deploy/permissions_multi_namespace_example.yaml#L6

[villesau]

I'm seeing this even with namespaced secrets and it prevents from updating secrets at all. Not sure why the namespaces needs listing when updating a secret in 1pw? In our setups we've given permission to two namespaces, in which the other is the operator namespace and set autoUpdate to true. Would be good if the operator could live without ClusterRoleBinding when the namespaces are defined.

I suspect that this is actually an issue with the operator and not with the helm chart since the operator should not need to list the namespaces if explicit list of them is given.

[jillianwilson]

Hi there, thank you for bringing this issue to our attention, we are currently investigating.

[villesau]

@jillianwilson I'm not 100% sure but I think this is the culprit: https://github.com/1Password/onepassword-operator/blob/b574e394ad649d2cbf6f61192f00a1918cef8df3/pkg/onepassword/secret_update_handler.go#L164

If the namespaces are given explicitly, there shouldn't be a need to list all of them, right?

[jillianwilson]

@villesau I originally suspected that as well, but after some investigation I was also running into issues with listing deployments and secrets which can not be avoided. I'm now thinking this might be more of a configuration issue with permissions in the helm chart itself.

[SimonBarendse]

Could we change those list operations to list the deployments and secrets in the watched namespaces specifically? We could then add a list permission for deployments so the operator is allowed to list deployments within the watched namespaces via the role binding.

[fractos]

Hit this one a couple of EKS clusters. OnePassword is configured with watchNamespace parameters which stops it from applying the ClusterRoleBinding, therefore it doesn't have the permissions it needs to retrieve the list of namespaces when it is looking for an updated list. To fix this, we call the chart from Helm and have a local ClusterRoleBinding file that applies it arbitrarily. I might be missing some nuance, here, but now it works as expected.

[villesau]

I, too, have watchNamespace set to watch some namespaces so that description would match to my case as well.

[alexbescond]

Any updates on this issue?

[stayanti]

The problem is still here and there is not yet a sufficient answer. Creating a clusterrolebinding manually to get the operator working is an ugly hack. Why would the operator need cluster wide roles when the namespaces are scoped by the watchNamespace variable? The least privilege approach is not followed here which worries me for such a sensitive operator.