cannot compile personal-files/system-files snap declaration constraints

Bug #1814592 reported by Jamie Strandboge on 2019-02-04
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Snap Store
Undecided
Unassigned

Bug Description

I was trying to add the following to icdiff's (https://dashboard.snapcraft.io/snaps/icdiff) snap declaration:

{
  "personal-files": {
    "allow-auto-connection": "true",
    "allow-installation": {
      "plug-attributes": {
        "read": [
          "$HOME/.gitconfig"
        ]
      }
    }
  }
}

but it fails with:

could not sign assertion (assertion snap-declaration: cannot compile plug-attributes in allow-installation in plug rule for interface "personal-files": cannot compile "read/alt#1/" constraint "$HOME/.gitconfig": not a valid $SLOT()/$PLUG() constraint)

IIUC, it is having trouble because it doesn't expect 'read' to be of type 'list'. This is new with personal-files and system-files since they are the first interfaces to have an attribute that is a list. Please update the compiler to support lists of strings for plug-attributes and slot-attributes.

Thanks!

PS - the review-tools currently handle lists correctly

Jamie Strandboge (jdstrand) wrote :

FYI, the personal-files and system-files interfaces are highly anticipated in snapd 2.37.1 and we already have several requests coming in to use them and this snap declaration is required to scope the use of the interface. Please consider this when determining priority.

Samuele Pedroni (pedronis) wrote :

First of all that code is in snapd actually and gets incorporated in the behavior of the assertion service.

But is working as intend ed, *-attributes constraints as anything else consider a list as set of alternatives (OR),

single values are regexps so you will need quoting here, in which case if the actual value is a list all values in it must match the regexp.

Samuele Pedroni (pedronis) wrote :

Also we wrap ^$ automatically around the regexp,

so I think here you want something like (not tested):

"read": "\\$HOME/\\.gitconfig"

Jamie Strandboge (jdstrand) wrote :

Samuele, the OR is definitely the case for all previous interfaces, but these new personal-files and system-files interfaces are different in that the attribute itself is meant to be a list of paths for read and write such that the snap can do:

plugs:
  personal-files:
    read:
    - $HOME/.foo
    - $HOME/.bar
    write:
    - /etc/baz
    - /etc/norf

In this case, the snap is declaring it wants all of them. I would therefore expect the snap declaration to be of the form:

{
  "personal-files": {
    "allow-auto-connection": "true",
    "allow-installation": {
      "plug-attributes": {
        "read": [
          "\\$HOME/\\.foo",
          "\\$HOME/\\.bar"
        ],
        "write": [
          "/etc/baz",
          "/etc/norf"
      }
    }
  }
}

Rather than:

{
  "personal-files": {
    "allow-auto-connection": "true",
    "allow-installation": [
      {
        "plug-attributes": {
          "read": "\\$HOME/\\.foo"
        }
      },
      {
        "plug-attributes": {
          "read": "\\$HOME/\\.bar"
        }
      },
      {
        "plug-attributes": {
          "write": "/etc/baz"
        }
      },
      {
        "plug-attributes": {
          "write": "/etc/norf"
        }
      }
    ]
  }
}

This latter syntax seems to make more sense when the interface attribute is a list and the snap is plugging the interface multiple times, like with the dbus and the name attribute.

Granted, the syntax I was expecting does mean that the snap must match the complete list, which is potentially awkward if the snap decides to drop one of the paths. I don't think we ever explicitly discussed what happens when an interface attribute is itself a list, so it is good to have this conversation.

Jamie Strandboge (jdstrand) wrote :

Sigh, comment #4 had too many typos that I'm going to just restate it in corrected form here (sorry; editing comments would be a nice feature in LP...)

Samuele, the OR is definitely the case for all previous interfaces, but these new personal-files and system-files interfaces are different in that the attribute itself is meant to be a list of paths for read and write such that the snap can do:

plugs:
  personal-files:
    read:
    - $HOME/.foo
    - $HOME/.bar
    write:
    - /etc/baz
    - /etc/norf

In this case, the snap is declaring it wants all of them. I would therefore expect the snap declaration to be of the form:

{
  "personal-files": {
    "allow-auto-connection": "true",
    "allow-installation": {
      "plug-attributes": {
        "read": [
          "\\$HOME/\\.foo",
          "\\$HOME/\\.bar"
        ],
        "write": [
          "/etc/baz",
          "/etc/norf"
        ]
      }
    }
  }
}

Rather than:

{
  "personal-files": {
    "allow-auto-connection": "true",
    "allow-installation": [
      {
        "plug-attributes": {
          "read": "\\$HOME/\\.foo"
        }
      },
      {
        "plug-attributes": {
          "read": "\\$HOME/\\.bar"
        }
      },
      {
        "plug-attributes": {
          "write": "/etc/baz"
        }
      },
      {
        "plug-attributes": {
          "write": "/etc/norf"
        }
      }
    ]
  }
}

This latter syntax seems to make more sense when the interface attribute is a string and the snap is plugging the interface multiple times, like with the dbus and the name attribute for providing multiple dbus services in the snap. The latter syntax for when the interface attribute is a list of strings is a bit awkward.

Granted, the syntax I was expecting does mean that the snap must match the complete list, which is potentially awkward if the snap decides to drop one of the paths. I don't think we ever explicitly discussed what happens when an interface attribute is itself a list, so it is good to have this conversation.

Samuele Pedroni (pedronis) wrote :

@jdstrand when the attribute is a list all entries need to match the regexp, that is how this was designed.

Anyway at the level of these checks we don't know anything about specific interfaces, so we cannot make lists make different things.

For the multiple entries in read/write so you need to use a regexp with | and quoting/() as needed.

Samuele Pedroni (pedronis) wrote :

> Granted, the syntax I was expecting does mean that the snap must match the complete list, which is potentially awkward if the snap decides to drop one of the paths.

yes, that's why we went for the regexp matching all entries approach, it does mean tough that we should not design interfaces where a list is not actually a set of orthogonal things

Samuele Pedroni (pedronis) wrote :

> Anyway at the level of these checks we don't know anything about specific interfaces, so we cannot make lists make different things.

s/make lists make/make lists mean/

Changed in snapstore:
status: New → Invalid
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers