app relation data cannot be read in non leader units

Bug #1911010 reported by David
30
This bug affects 5 people
Affects Status Importance Assigned to Milestone
Canonical Juju
Opinion
Medium
Unassigned

Bug Description

Hello,

I found a bug similar to this one: https://bugs.launchpad.net/juju/+bug/1865229

Non-leaders shouldn't have rights to write into the application data, but I'm getting permission denied error reading the application relation data from a non-leader unit.

Logs:
Traceback (most recent call last):
  File "/var/lib/juju/agents/unit-provides-8/charm/venv/ops/model.py", line 1049, in _run
    result = run(args, **kwargs)
  File "/usr/lib/python3.8/subprocess.py", line 512, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '('/var/lib/juju/tools/unit-provides-8/relation-get', '-r', '2', '-', 'provides', '--app', '--format=json')' returned non-zero exit status 1.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "./src/charm.py", line 81, in <module>
    main(ProvidesCharm)
  File "/var/lib/juju/agents/unit-provides-8/charm/venv/ops/main.py", line 402, in main
    _emit_charm_event(charm, dispatcher.event_name)
  File "/var/lib/juju/agents/unit-provides-8/charm/venv/ops/main.py", line 140, in _emit_charm_event
    event_to_emit.emit(*args, **kwargs)
  File "/var/lib/juju/agents/unit-provides-8/charm/venv/ops/framework.py", line 278, in emit
    framework._emit(event)
  File "/var/lib/juju/agents/unit-provides-8/charm/venv/ops/framework.py", line 722, in _emit
    self._reemit(event_path)
  File "/var/lib/juju/agents/unit-provides-8/charm/venv/ops/framework.py", line 767, in _reemit
    custom_handler(event)
  File "./src/charm.py", line 44, in on_relation_joined
    logger.error(f"joined: {rel_id}{data}")
  File "/var/lib/juju/agents/unit-provides-8/charm/venv/ops/model.py", line 702, in __repr__
    return repr(self._data)
  File "/var/lib/juju/agents/unit-provides-8/charm/venv/ops/model.py", line 386, in __repr__
    return repr(self._data)
  File "/var/lib/juju/agents/unit-provides-8/charm/venv/ops/model.py", line 367, in _data
    data = self._lazy_data = self._load()
  File "/var/lib/juju/agents/unit-provides-8/charm/venv/ops/model.py", line 719, in _load
    return self._backend.relation_get(self.relation.id, self._entity.name, self._is_app)
  File "/var/lib/juju/agents/unit-provides-8/charm/venv/ops/model.py", line 1090, in relation_get
    return self._run(*args, return_output=True, use_json=True)
  File "/var/lib/juju/agents/unit-provides-8/charm/venv/ops/model.py", line 1051, in _run
    raise ModelError(e.stderr)
ops.model.ModelError: b'ERROR permission denied\n'

The provides/8 unit, as you can see here, is not the leader:

Model Controller Cloud/Region Version SLA Timestamp
test microk8s-localhost microk8s/localhost 2.8.7 unsupported 15:57:46+01:00

App Version Status Scale Charm Store Rev OS Address N
otes
provides ubuntu:latest active 2 provides local 7 kubernetes 10.152.183.207

Unit Workload Agent Address Ports Message
provides/7* active idle 10.1.245.85 80/TCP
provides/8 error idle 10.1.245.87 80/TCP hook failed: "dummy-relation-joined"

Revision history for this message
Ian Booth (wallyworld) wrote :

IIRC, this is by design.

Only leaders can read/write application relation data.
To propagate that data to other application units, use a peer relation and have the leader update the data in that peer relation as needed.

Revision history for this message
David (davigar15) wrote :

I'm closing this bug. This message from John A Meinel explains it very clearly:

"""
you can read the data that is "targeted at you". iow, if mysql <-> wordpress, then wordpress/0 can read the application data from mysql, and mysql can read the application data from wordpress

however, mysql/1 can't read the application data from mysql for wordpress. If mysql wants to share data with mysql, then it should use a peer relation.

(we don't let you read the data if you wouldn't get an event if that data changed, which is what peer relations do)
"""

Thanks John and Ian!

Changed in juju:
status: New → Invalid
Revision history for this message
Cory Johns (johnsca) wrote :

This was also filed here https://bugs.launchpad.net/juju/+bug/1869915 and I really think this is a bad design. Particularly for request-response type interfaces, you often want to have access to some of the data that was sent as part of the request when handling the response. Some examples include:

  * The ID of the request, if multiple requests can be made, so you know which response goes with which request

  * The hash of the request sent, so that you can tell if a given response has been updated to handle an updated request

  * Some data from the request that the leader generated but is relevant to you

There are ways to work around this, such as using a peer relation as suggested, but then you're using a secondary relation to track data used by the primary and it just makes things messy and confusing.

I don't think restricting this protects the charmer from anything significant, breaks natural usage patterns, makes charm implementations messy, requires special-casing peer relations (which opens you up to bugs: https://bugs.launchpad.net/juju/+bug/1865229), and doesn't mirror the way that unit-level relation data is handled.

On that last point, we've never considered it a problem that units can see the data sent by their peers on the relation, so why is it somehow a problem if that data happens to be app-level? That also shows that the claim that "we don't let you read the data if you wouldn't get an event if that data changed" isn't true, because units don't get relation-changed hooks when their peers change their unit-level relation data yet they can still read that data. And that's never been an issue because they only care about that data (if they do at all) when the response comes, which would be the same for app-level data.

Changed in juju:
status: Invalid → New
Revision history for this message
Cory Johns (johnsca) wrote :
Revision history for this message
John A Meinel (jameinel) wrote :

I think it is worth bringing this up at a wider meeting to discuss the workflows that we would want to have.
The problem with a request / response pairing and introspection of an application data bag is that we don't have a mechanism to inform the unit that the data has changed. So you'll always be running the risk that what you see as the application request is no longer accurate.
So either we start providing events so that the units can be aware the data has changed, or you use a peer relation which communicates with the exact units you want to be sharing that information with, and has the correct update semantics.

I don't think we want to make charm writing harder than we need to. But we *do* want to provide accurate semantics and make sure we don't lead people down the "its easy to do it this way, but subtly wrong and hard to debug when it goes wrong".

Changed in juju:
importance: Undecided → Medium
status: New → Opinion
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.