Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
snappy: rename "migration-skill" to "old-security" and use new interface names instead of skills #518
Conversation
mvo5
added some commits
Feb 24, 2016
|
@sergiusens breakage is coming. |
elopio
reviewed
Feb 24, 2016
| -Most of the security aspects of the system will be done via skills and | ||
| -skill slots. However for compatibility with the 15.04 snappy | ||
| -architecture there is a special skill type called `migration-skill` | ||
| +Most of the security aspects of the system will be done via interfaces and |
elopio
reviewed
Feb 24, 2016
| -architecture there is a special skill type called `migration-skill` | ||
| +Most of the security aspects of the system will be done via interfaces and | ||
| +slots and plugs. However for compatibility with the 15.04 snappy | ||
| +architecture there is a special interface type called `old-security` |
elopio
Feb 24, 2016
Member
From Gustavo's mail, I got that instead of saying "interface type" we should say only "interface".
elopio
reviewed
Feb 24, 2016
| if err := verifyStructStringsAgainstWhitelist(*uses, servicesBinariesStringsWhitelist); err != nil { | ||
| return err | ||
| } | ||
| - if uses.Type != "migration-skill" { | ||
| - return fmt.Errorf("can not use skill %q, only migration-skill supported", uses.Type) | ||
| + if uses.Type != "old-security" { |
mvo5
closed this
Feb 24, 2016
mvo5
added some commits
Feb 24, 2016
mvo5
reopened this
Feb 24, 2016
|
Does this really have more prio than getting the kernel snaps in place? |
|
+1 |
niemeyer
reviewed
Feb 25, 2016
| - if len(app.UsesRef) != 1 { | ||
| - return nil, fmt.Errorf("only a single skill is supported, %d found", len(app.UsesRef)) | ||
| + if len(app.SlotsRef) != 1 { | ||
| + return nil, fmt.Errorf("only a single skill is supported, %d found", len(app.SlotsRef)) |
niemeyer
reviewed
Feb 25, 2016
| if !ok { | ||
| - return nil, fmt.Errorf("can not find skill %q", app.UsesRef[0]) | ||
| + return nil, fmt.Errorf("can not find skill %q", app.SlotsRef[0]) |
niemeyer
reviewed
Feb 25, 2016
| that can be used to migrate using the 15.04 syntax. See the example | ||
| -below for the various ways the migration-skill can be used. | ||
| +below for the various ways the `old-security` interface can be used. | ||
| ## Security with the migration skill |
niemeyer
reviewed
Feb 25, 2016
| - Type: "some-skill", | ||
| - }), ErrorMatches, ".*can not use skill.* only migration-skill supported") | ||
| + c.Check(verifySlotYaml(&slotYaml{ | ||
| + Interface: "some-skill", |
niemeyer
reviewed
Feb 25, 2016
| @@ -742,17 +742,17 @@ func hasConfig(baseDir string) bool { | ||
| return helpers.FileExists(filepath.Join(baseDir, "meta", "hooks", "config")) | ||
| } | ||
| -func findSkillForApp(m *snapYaml, app *AppYaml) (*usesYaml, error) { | ||
| - if len(app.UsesRef) == 0 { | ||
| +func findSkillForApp(m *snapYaml, app *AppYaml) (*slotYaml, error) { |
niemeyer
reviewed
Feb 25, 2016
| } | ||
| - skill, ok := m.Uses[app.UsesRef[0]] | ||
| + skill, ok := m.Slots[app.SlotsRef[0]] |
niemeyer
reviewed
Feb 25, 2016
| @@ -1044,7 +1044,7 @@ func (a *SecurityTestSuite) TestFindSkillForAppEmpty(c *C) { | ||
| func (a *SecurityTestSuite) TestFindSkillForAppTooMany(c *C) { |
niemeyer
reviewed
Feb 25, 2016
| @@ -77,9 +77,9 @@ The following keys are optional: | ||
| * `uses`: a map of names and skills |
niemeyer
Feb 25, 2016
Contributor
* `slots`: a map of interfaces
We need more details here at some point, that'll do for now.
niemeyer
Feb 25, 2016
Contributor
There's also another entry for uses above which needs conversion. Can't comment there as it's out of the diff.
|
LGTM, just a few more occurrences to replace. @sergiusens This is really not the place (or the tone) for that kind of conversation. |
niemeyer
added
the
Reviewed
label
Feb 26, 2016
mvo5
added some commits
Feb 29, 2016
|
@niemeyer thanks for this excellent review and sorry for overlooking these. I did a |
zyga
commented on 8067567
Feb 29, 2016
|
+1 |
|
retest this please |
mvo5 commentedFeb 24, 2016
This branch follows thenew
interfacesapproach and renames the following items:It is pretty mechanical. Note that the integration tests will start failing. I will prepare new versions of all our examples so ideally we only merge once the new examples are uploaded.