Support the new security-override semantics #222

Merged
merged 1 commit into from Jan 12, 2016

Conversation

Projects
None yet
3 participants
Collaborator

sergiusens commented Jan 12, 2016

LP: #1533293

schema/snapcraft.yaml
+ type: string
+ abstractions:
+ type: array
+ description: a list of additional Apparmor abstractions for the app.
@kyrofa

kyrofa Jan 12, 2016

Member

AppArmor. Only slightly nit-picky.

@sergiusens

sergiusens Jan 12, 2016

Collaborator

fair enough :-)

+ 'apparmor': 'file.apparmor',
+ },
+ },
+ }
@kyrofa

kyrofa Jan 12, 2016

Member

You're missing three combos here, right? Worth adding them?

@sergiusens

sergiusens Jan 12, 2016

Collaborator

what combos? with subTest I cover

  • all
  • caps & security-policy
  • security-override & security-policy
  • security-override & caps
@kyrofa

kyrofa Jan 12, 2016

Member

Ah, I read too quickly. However, I can break your code above by removing all but one invalid combo and this test would still pass, correct? Should you be explicitly testing each invalid combination rather than throwing them all together?

@kyrofa

kyrofa Jan 12, 2016

Member

Ah, I missed the deletion below. You do exactly as I ask, just by altering the app on the fly a few times. It could be a bit clearer, but I'm fine with this.

Member

kyrofa commented Jan 12, 2016

Couple nits, but overall looks good.

schema/snapcraft.yaml
+ minItems: 1
+ uniqueItems: true
+ items:
+ type: string
@jdstrand

jdstrand Jan 12, 2016

You added caps, security-override and security-policy but left out security-template. Since you seem to be going for completeness of the current implementation, you should include security-template. It is of type string. It may be used with caps and security-override, but not security-policy.

@sergiusens

sergiusens Jan 12, 2016

Collaborator

Added. Thanks!

sergiusens added a commit that referenced this pull request Jan 12, 2016

Merge pull request #222 from sergiusens/security-override
Support the new security-override semantics

@sergiusens sergiusens merged commit b36c094 into snapcore:master Jan 12, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.08%) to 90.174%
Details

@sergiusens sergiusens deleted the sergiusens:security-override branch Jan 12, 2016

kalikiana pushed a commit to kalikiana/snapcraft that referenced this pull request Apr 6, 2017

Merge pull request #222 from sergiusens/security-override
Support the new security-override semantics
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment