Ros2-snapd: ros-snapd-support and snap-refresh-observe

store-requests privileged-interfaces
1

This request is following the discussion: Request snapd-control interface for ros2-snapd And the development of the ros-snapd-support interface.

2

This request has been added to the queue for review by the @reviewers team.

3

Hi @RoboticsCommunity!

Based on the previous discussion here, both requested interfaces make sense to me.

+1 vote for granting ros-snapd-support and snap-refresh-observe to snap ros2-snapd (#voteFor).

What do the other @reviewers think?

4

Hey @RoboticsCommunity

Regarding the reasoning for snap-refresh-observe, access to /apps endpoint should be granted by ros-snapd-support, should it? Moreover, snap-refresh-observe is intended to be used only to mark the existence of a refresh awareness client, according to the documentation (#askForInfo)

5

@jslarraz we used the refresh-observe to get access to /apps indeed. We could move the access to our ros-snapd-support interface, it’s a good suggestion. But right now we would like to drop the snapd-control as soon as possible since we have a better and more secure solution even if not perfect. After this change we will apply a new PR in our support interface and eventually when it reaches snapd stable we can also drop the refresh-observe. What do you think?

6

Hey @mirkoferrati

I don’t know if I’m missing something but according to the documentation snap-refresh-observe does not grant access to /apps

Endpoint access permissions

    /v2/changes
    /v2/changes/{id}
    /v2/icons/{name}/icon
    /v2/notices
    /v2/notices/{id}
    /v2/snaps
    /v2/snaps/{name}

On the other hand, ros-snapd-support seems to actually grant access to /apps already:

var (
	appsCmd = &Command{
		Path:        "/v2/apps",
		GET:         getAppsInfo,
		POST:        postApps,
		ReadAccess:  interfaceOpenAccess{Interfaces: []string{"ros-snapd-support"}},
		WriteAccess: interfaceAuthenticatedAccess{Interfaces: []string{"ros-snapd-support"}, Polkit: polkitActionManage},
	}

	logsCmd = &Command{
		Path:       "/v2/logs",
		GET:        getLogs,
		ReadAccess: authenticatedAccess{Polkit: polkitActionManage},
	}
)
7

So, it seems that there was a mistake in the initial post that carried throughout the discussion and made things a little obfuscated. The request for the snap-refresh-observe is actually aiming to access both the /snaps and /changes endpoints, not /apps which is indeed provided by the specific ros-snapd-support interface. Sorry for the confusion.

This being said, @mirkoferrati reasoning still applies. We could be granted both the ros-snapd-support and the snap-refresh-observe in order to drop the much wider scope snapd-control which we currently have granted. Albeit the access to snap-refresh-observe could be only temporary until we go through another cycle of updating the ros-snapd-support to fully support our needs and seeing it released in snapd. The alternative to this is status quo and this snap keeps the current snapd-control access.

8

Hey @artivis,

Thanks for the clarification!

I’m certainly happy to grant both, ros-snapd-support and the snap-refresh-observe, to remove the need for snapd-control.

My only concern at this point is that snap-refresh-observe documentation says that “it is intended to be used only to mark the existence of a refresh awareness client”. Thus, I would like to know if @pedronis is fine with it.

1 Like
9

the correct thing is to expand ros-snapd-support to include the other permissions it needs, as ros-snapd it is not observing refreshes it’s confusing to give it snap-refresh-observe. The point of “support” interfaces is to cover the special requirements of the snap they are for, so from that point of view this is fine

10

given the reasoning and discussion above, +1 (#voteFor) from me as well for auto-connecting ros-snapd-support and the snap-refresh-observe

11

Voting period has ended. This request is approved with 2 votes for and 0 votes against.