fix bug in CompareGeneratePolicyFromFile for sideloaded pkgs

snappy/security.go

@@ -827,10 +827,11 @@ func compareSinglePolicyToCurrent(oldPolicyFn, newPolicy string) error {
 // CompareGeneratePolicyFromFile is used to simulate security policy
 // generation and returns if the policy would have changed
 func CompareGeneratePolicyFromFile(fn string) error {
-	m, err := parsePackageYamlFile(fn)
+	m, err := parsePackageYamlFileWithVersion(fn)
 	if err != nil {
 		return err
 	}
+
 	baseDir := filepath.Dir(filepath.Dir(fn))
 
 	for _, service := range m.ServiceYamls {
@@ -872,18 +873,25 @@ func CompareGeneratePolicyFromFile(fn string) error {
 	return nil
 }
 
+// FIXME: refactor so that we don't need this
+func parsePackageYamlFileWithVersion(fn string) (*packageYaml, error) {
+	m, err := parsePackageYamlFile(fn)
+
+	// FIXME: duplicated code from snapp.go:NewSnapPartFromYaml,
+	//        version is overriden by sideloaded versions
+	m.Version = filepath.Base(filepath.Dir(filepath.Dir(fn)))
+
+	return m, err
+}
+
 // GeneratePolicyFromFile is used to generate security policy on the system
 // from the specified manifest file name
 func GeneratePolicyFromFile(fn string, force bool) error {
 	// FIXME: force not used yet
-
-	m, err := parsePackageYamlFile(fn)
+	m, err := parsePackageYamlFileWithVersion(fn)
 	if err != nil {
 		return err
 	}
-	// FIXME: duplicated code from snapp.go:NewSnapPartFromYaml,
-	//        version is overriden by sideloaded versions
-	m.Version = filepath.Base(filepath.Dir(filepath.Dir(fn)))
 
 	if m.Type == "" || m.Type == pkg.TypeApp {
 		_, err = originFromYamlPath(fn)

snappy/security_test.go

@@ -931,11 +931,7 @@ func (a *SecurityTestSuite) TestSecurityWarnsOnDeprecatedSeccomp(c *C) {
 	}
 }
 
-func (a *SecurityTestSuite) TestSecurityGeneratePolicyFromFileSideload(c *C) {
-	// we need to create some fake data
-	makeMockApparmorTemplate(c, "default", []byte(``))
-	makeMockSeccompTemplate(c, "default", []byte(``))
-
+func makeInstalledMockSnapSideloaded(c *C) string {
 	mockPackageYamlFn, err := makeInstalledMockSnap(dirs.GlobalRootDir, mockSecurityPackageYaml)
 	c.Assert(err, IsNil)
 	// pretend its sideloaded
@@ -945,8 +941,18 @@ func (a *SecurityTestSuite) TestSecurityGeneratePolicyFromFileSideload(c *C) {
 	err = os.Rename(oldPath, newPath)
 	mockPackageYamlFn = filepath.Join(basePath, "IsSideloadVer", "meta", "package.yaml")
 
+	return mockPackageYamlFn
+}
+
+func (a *SecurityTestSuite) TestSecurityGeneratePolicyFromFileSideload(c *C) {
+	// we need to create some fake data
+	makeMockApparmorTemplate(c, "default", []byte(``))
+	makeMockSeccompTemplate(c, "default", []byte(``))
+
+	mockPackageYamlFn := makeInstalledMockSnapSideloaded(c)
+
 	// the acutal thing that gets tested
-	err = GeneratePolicyFromFile(mockPackageYamlFn, false)
+	err := GeneratePolicyFromFile(mockPackageYamlFn, false)
 	c.Assert(err, IsNil)
 
 	// ensure the apparmor policy got loaded
@@ -961,6 +967,21 @@ func (a *SecurityTestSuite) TestSecurityGeneratePolicyFromFileSideload(c *C) {
 	c.Assert(helpers.FileExists(generatedProfileFn), Equals, true)
 }
 
+func (a *SecurityTestSuite) TestSecurityCompareGeneratePolicyFromFileSideload(c *C) {
+	// we need to create some fake data
+	makeMockApparmorTemplate(c, "default", []byte(``))
+	makeMockSeccompTemplate(c, "default", []byte(``))
+
+	mockPackageYamlFn := makeInstalledMockSnapSideloaded(c)
+	// generate policy
+	err := GeneratePolicyFromFile(mockPackageYamlFn, false)
+	c.Assert(err, IsNil)
+
+	// nothing changed, ensure compare is happy even for sideloaded pkgs
+	err = CompareGeneratePolicyFromFile(mockPackageYamlFn)
+	c.Assert(err, IsNil)
+}
+
 func (a *SecurityTestSuite) TestSecurityGeneratePolicyForServiceBinaryFramework(c *C) {
 	makeMockSecurityEnv(c)