Grafana-Agent: Permission Request

store-requests
1

Hi,

This is my first time asking for a store request, so bear with me if it is a bit lacking. I’m currently in the process of snapping the grafana-agent. This will later be used as a foundation for a machine charm, similar to the current k8s charm.

To allow for node_exporter, ebpf and similar integrations to work, it needs some bespoke permissions. While I could have opted for a classic snap, I really think it makes sense for the snap to not get more permissions than it actually needs.

The permissions I’m requesting are:

  • network-bind
  • time-control
  • hardware-observe
  • mount-observe
  • network-observe
  • system-observe

All of these are needed to be able to fetch all of the telemetry we need to make this useful, as made clear by prior art through the existing strictly confined node_exporter snap. In addition, some paths not available in the system-observe plug is needed, which resulted in a bespoke plug telemetry:

  telemetry:
    interface: system-files
    read:
      - /var/log
      - /proc/mdstat
      - /proc/schedstat
      - /proc/sys/kernel/random

Likely this wasn’t the case back when the existing node_exporter charm was created, but as of now, node_exporter won’t be able to generate all metrics without it.

In addition to these permissions, I would like for the permissions to be auto-connected, as the software doesn’t make sense without them being present.

Link to the snap: https://snapcraft.io/grafana-agent

Thanks in advance, Simme

2

+1 from me for auto-connect of the following interfaces as these are required by the grafana-agent snap for it to operate as expected:

network-bind
time-control
hardware-observe
mount-observe
network-observe
system-observe

Regarding the requested system-files interface - can you be more specific? /var/log/ should be accessible via the log-observe inteface, and parts of /proc/sys/kernel/random are already in the base template for all snaps.

However, I don’t think either /proc/mdstat or /proc/schedstat are currently covered by any snapd interface - both of these seem like reasonable candidates to include in the existing system-observe interface so perhaps a PR to upstream snapd to includes these there would be a better option?

3

Makes total sense! I’ll switch to the log-observe one for /var/log. There were at least three /proc/sys/kernel/random ones I had issues with. I can try to narrow it down further. Published as revision 3.

Happy to do that and remove these additions once it’s out. Would it be possible to keep them like this until that eventually is the case? PR opened at https://github.com/snapcore/snapd/pull/12427

4

+1 from me as well for auto-connect time-control, hardware-observe, mount-observe, network-observe, system-observe to grafana-agent since those are clearly required for the snap to properly operate. +2 votes for, 0 votes against, granting the requested auto-connections. This is now live.

network-bind is auto-connected by default, so no voting is needed ;).

I see this PR has been merged. Are you ok with waiting for the next snapd release to include it? Or is this needed asap?

5

We still need to build out the charm, so I guess we can wait. If anything I’m a bit worried that these changes won’t make it back to all of the deployments of all of the releases we need to support.

1 Like
6

config-file is still required however. Thanks.

7

I see the required access is /etc/grafana-agent.yaml, can you please rename the iface reference to be etc-grafana-agent so it better represents the access granted? I am +1 for granting this access since grafana-agent is the clear owner of the directory. Can other @reviewers please vote?

8

+1 from me with the same stipulation as @emitorino.

9

@emitorino and others:

Revision 5 is now uploaded, containing the requested changes. I also took the liberty to make two other modifications:

  • Change the name of the /proc/sys/kernel/random plug from telemetry to proc-sys-kernel-random to better describe what it adds.
  • Add a content port named shared-logs, as discussed on MM, to allow other snaps to connect and share log files they want grafana-agent to tail.
1 Like
10

@simskij Regarding /proc/sys/kernel/random, the base template apparmor profile for snaps already includes the following entries:

  @{PROC}/sys/kernel/random/boot_id r,
  @{PROC}/sys/kernel/random/entropy_avail r,
  @{PROC}/sys/kernel/random/uuid r,

Can you detail which other files perhaps the grafana-agent snap needs access to here since perhaps we can just add these to the base template in snapd instead of using system-files?

11

@alexmurray

  • /proc/sys/kernel/random/write_wakeup_threshold
  • /proc/sys/kernel/random/read_wakeup_threshold
  • /proc/sys/kernel/random/poolsize
  • /proc/sys/kernel/random/urandom_min_reseed_secs
12

+2 votes for, 0 votes against. Granting auto-connect for plugging system-files with read access to /etc/grafana-agent.yaml using the interface reference etc-grafana-agent. This is now live.

Regarding the requested /proc/sys/kernel/random accesses:

Since those are not part of the base template yet, I am +1 to grant those as part of system-files in the meantime.

1 Like
13

+1 from me too for use of and auto-connect of system-files to read these paths but the system-files declaration should then just look like:

  proc-sys-kernel-random:
    interface: system-files
    read:
      - /proc/sys/kernel/random

And then a second instance added for the 2 files under /proc (until these are part of system-observe):

  proc-stats:
    interface: system-files
    read:
      - /proc/mdstat
      - /proc/schedstat
1 Like
14 
  proc-stats:
    interface: system-files
    read:
      - /proc/mdstat
      - /proc/schedstat

was removed completely in revision 5. Afaict they have already been merged to system-observe.

As for the specific files in random, I’d rather be explicit and point out exactly which files are added, if that is ok with you, @alexmurray :sweat_smile:

15

Indeed https://github.com/snapcore/snapd/commit/ec6b35d578f4195c91140ab9e07e7e489eb23790

I also agree with your proposal, let’s see what @alexmurray think about it.

16

Regarding the /proc/sys/kernel/random files - sure you could add the specific individual files under the read attribute - either is fine with me.

17

Then the latest revision should, for all intents and purposes, be ready.

18

+2 votes for, 0 votes against, granting the use of system-files named proc-sys-kernel-random for read access to

- /proc/sys/kernel/random/write_wakeup_threshold
- /proc/sys/kernel/random/read_wakeup_threshold
- /proc/sys/kernel/random/poolsize
- /proc/sys/kernel/random/urandom_min_reseed_secs

This is now live.

19

log-observe was missed during the voting process. +1 from me for auto-connect this iface to grafana-agent since this is clearly required to access files at /var/log

1 Like
20

Apologies for missing log-observe as well - +1 from me too for auto-connect of log-observe for grafana-agent.

+2 votes for, 0 votes against, this is now live.

1 Like