Merge lp:~chipaca/snappy/mangle into lp:~snappy-dev/snappy/snappy-moved-to-github

[John Lenton]

Added integrate() to set Integration to default values needed for integration.

[Sergio Schvezov (sergiusens)]

Hey this looks good, lots of confusing red going down the drain, I did add a couple of comments and maybe it's the time, but I see no tests for services or Description of services, which I have a weird opinion of anyways ;-)

I'll give it another view in the morning.

Also, again, thanks for fixing!

[lp:~chipaca/snappy/mangle]

485. By John Lenton on 2015-06-08

addressed issues raised in review

486. By John Lenton on 2015-06-08

merged trunk

[Michael Vogt (mvo)]

Looks good, thanks a lot for working on this!

review: Approve

=== modified file 'snappy/build.go'
--- snappy/build.go	2015-05-28 11:58:30 +0000
+++ snappy/build.go	2015-06-08 16:38:59 +0000
@@ -128,13 +128,6 @@
 func handleBinaries(buildDir string, m *packageYaml) error {
 	for _, v := range m.Binaries {
 		hookName := filepath.Base(v.Name)
-
-		if _, ok := m.Integration[hookName]; !ok {
-			m.Integration[hookName] = make(map[string]string)
-		}
-		// legacy click hook
-		m.Integration[hookName]["bin-path"] = v.Exec
-
 		// handle the apparmor stuff
 		if err := handleApparmor(buildDir, m, hookName, &v.SecurityDefinitions); err != nil {
 			return err
@@ -145,36 +138,9 @@
 }
 
 func handleServices(buildDir string, m *packageYaml) error {
-	_, description, err := parseReadme(filepath.Join(buildDir, "meta", "readme.md"))
-	if err != nil {
-		return err
-	}
-
 	for _, v := range m.Services {
 		hookName := filepath.Base(v.Name)
 
-		if _, ok := m.Integration[hookName]; !ok {
-			m.Integration[hookName] = make(map[string]string)
-		}
-
-		// generate snappyd systemd unit json
-		if v.Description == "" {
-			v.Description = description
-		}
-
-		// omit the name from the json to make the
-		// click-reviewers-tool happy
-		v.Name = ""
-		snappySystemdContent, err := json.MarshalIndent(v, "", " ")
-		if err != nil {
-			return err
-		}
-		snappySystemdContentFile := filepath.Join("meta", hookName+".snappy-systemd")
-		if err := ioutil.WriteFile(filepath.Join(buildDir, snappySystemdContentFile), []byte(snappySystemdContent), 0644); err != nil {
-			return err
-		}
-		m.Integration[hookName]["snappy-systemd"] = snappySystemdContentFile
-
 		// handle the apparmor stuff
 		if err := handleApparmor(buildDir, m, hookName, &v.SecurityDefinitions); err != nil {
 			return err
@@ -499,11 +465,6 @@
 		return "", err
 	}
 
-	// defaults, mangling
-	if m.Integration == nil {
-		m.Integration = make(map[string]clickAppHook)
-	}
-
 	// generate compat hooks for binaries
 	if err := handleBinaries(buildDir, m); err != nil {
 		return "", err

=== modified file 'snappy/build_test.go'
--- snappy/build_test.go	2015-06-02 20:46:07 +0000
+++ snappy/build_test.go	2015-06-08 16:38:59 +0000
@@ -217,27 +217,13 @@
  "title": "some title",
  "hooks": {
   "foo": {
-   "apparmor": "meta/foo.apparmor",
-   "snappy-systemd": "meta/foo.snappy-systemd"
+   "apparmor": "meta/foo.apparmor"
   }
  }
 }`
 	readJSON, err := exec.Command("dpkg-deb", "-I", "hello_3.0.1_all.snap", "manifest").Output()
 	c.Assert(err, IsNil)
 	c.Assert(string(readJSON), Equals, expectedJSON)
-
-	// check the generated meta file
-	unpackDir := c.MkDir()
-	err = exec.Command("dpkg-deb", "-x", "hello_3.0.1_all.snap", unpackDir).Run()
-	c.Assert(err, IsNil)
-
-	snappySystemdContent, err := ioutil.ReadFile(filepath.Join(unpackDir, "meta/foo.snappy-systemd"))
-	c.Assert(err, IsNil)
-	c.Assert(string(snappySystemdContent), Equals, `{
- "description": "some description",
- "start": "bin/hello-world",
- "stop-timeout": "30s"
-}`)
 }
 
 func (s *SnapTestSuite) TestBuildAutoGenerateConfigAppArmor(c *C) {

=== modified file 'snappy/click.go'
--- snappy/click.go	2015-06-04 22:21:51 +0000
+++ snappy/click.go	2015-06-08 16:38:59 +0000
@@ -457,12 +457,17 @@
 
 	udevPartName := m.qualifiedName(originFromBasedir(baseDir))
 
+	desc := service.Description
+	if desc == "" {
+		desc = fmt.Sprintf("service %s for package %s", service.Name, m.Name)
+	}
+
 	return systemd.New(globalRootDir, nil).GenServiceFile(
 		&systemd.ServiceDescription{
 			AppName:     m.Name,
 			ServiceName: service.Name,
 			Version:     m.Version,
-			Description: service.Description,
+			Description: desc,
 			AppPath:     baseDir,
 			Start:       service.Start,
 			Stop:        service.Stop,

=== modified file 'snappy/security.go'
--- snappy/security.go	2015-05-28 11:58:30 +0000
+++ snappy/security.go	2015-06-08 16:38:59 +0000
@@ -89,34 +89,15 @@
 }
 
 func handleApparmor(buildDir string, m *packageYaml, hookName string, s *SecurityDefinitions) error {
-
-	// ensure we have a hook
-	if _, ok := m.Integration[hookName]; !ok {
-		m.Integration[hookName] = clickAppHook{}
-	}
-
-	// legacy use of "Integration" - the user should
-	// use the new format, nothing needs to be done
-	_, hasApparmor := m.Integration[hookName]["apparmor"]
-	_, hasApparmorProfile := m.Integration[hookName]["apparmor-profile"]
-	if hasApparmor || hasApparmorProfile {
-		return nil
-	}
-
-	// see if we have a custom security policy
-	if s.SecurityPolicy != nil && s.SecurityPolicy.Apparmor != "" {
-		m.Integration[hookName]["apparmor-profile"] = s.SecurityPolicy.Apparmor
-		return nil
-	}
-
-	// see if we have a security override
-	if s.SecurityOverride != nil && s.SecurityOverride.Apparmor != "" {
-		m.Integration[hookName]["apparmor"] = s.SecurityOverride.Apparmor
+	hasSecPol := s.SecurityPolicy != nil && s.SecurityPolicy.Apparmor != ""
+	hasSecOvr := s.SecurityOverride != nil && s.SecurityOverride.Apparmor != ""
+
+	if hasSecPol || hasSecOvr {
 		return nil
 	}
 
 	// generate apparmor template
-	apparmorJSONFile := filepath.Join("meta", hookName+".apparmor")
+	apparmorJSONFile := m.Integration[hookName]["apparmor"]
 	securityJSONContent, err := s.generateApparmorJSONContent()
 	if err != nil {
 		return err
@@ -125,8 +106,6 @@
 		return err
 	}
 
-	m.Integration[hookName]["apparmor"] = apparmorJSONFile
-
 	return nil
 }
 

=== modified file 'snappy/security_test.go'
--- snappy/security_test.go	2015-06-02 20:46:07 +0000
+++ snappy/security_test.go	2015-06-08 16:38:59 +0000
@@ -62,6 +62,9 @@
 func (a *SecurityTestSuite) TestSnappyHandleApparmorSecurityDefault(c *C) {
 	sec := &SecurityDefinitions{}
 
+	a.m.Binaries = append(a.m.Binaries, Binary{Name: "app", SecurityDefinitions: *sec})
+	a.m.legacyIntegration()
+
 	err := handleApparmor(a.buildDir, a.m, "app", sec)
 	c.Assert(err, IsNil)
 
@@ -81,6 +84,9 @@
 		SecurityCaps: []string{"cap1", "cap2"},
 	}
 
+	a.m.Binaries = append(a.m.Binaries, Binary{Name: "app", SecurityDefinitions: *sec})
+	a.m.legacyIntegration()
+
 	err := handleApparmor(a.buildDir, a.m, "app", sec)
 	c.Assert(err, IsNil)
 
@@ -101,6 +107,9 @@
 		SecurityTemplate: "docker-client",
 	}
 
+	a.m.Binaries = append(a.m.Binaries, Binary{Name: "app", SecurityDefinitions: *sec})
+	a.m.legacyIntegration()
+
 	err := handleApparmor(a.buildDir, a.m, "app", sec)
 	c.Assert(err, IsNil)
 
@@ -120,6 +129,9 @@
 		},
 	}
 
+	a.m.Binaries = append(a.m.Binaries, Binary{Name: "app", SecurityDefinitions: *sec})
+	a.m.legacyIntegration()
+
 	err := handleApparmor(a.buildDir, a.m, "app", sec)
 	c.Assert(err, IsNil)
 
@@ -133,6 +145,9 @@
 		},
 	}
 
+	a.m.Binaries = append(a.m.Binaries, Binary{Name: "app", SecurityDefinitions: *sec})
+	a.m.legacyIntegration()
+
 	err := handleApparmor(a.buildDir, a.m, "app", sec)
 	c.Assert(err, IsNil)
 

=== modified file 'snappy/snapp.go'
--- snappy/snapp.go	2015-06-04 12:55:54 +0000
+++ snappy/snapp.go	2015-06-08 16:38:59 +0000
@@ -323,6 +323,8 @@
 		}
 	}
 
+	m.legacyIntegration()
+
 	return &m, nil
 }
 
@@ -445,6 +447,59 @@
 	return nil
 }
 
+func (m *packageYaml) legacyIntegrateSecDef(hookName string, s *SecurityDefinitions) {
+	// see if we have a custom security policy
+	if s.SecurityPolicy != nil && s.SecurityPolicy.Apparmor != "" {
+		m.Integration[hookName]["apparmor-profile"] = s.SecurityPolicy.Apparmor
+		return
+	}
+
+	// see if we have a security override
+	if s.SecurityOverride != nil && s.SecurityOverride.Apparmor != "" {
+		m.Integration[hookName]["apparmor"] = s.SecurityOverride.Apparmor
+		return
+	}
+
+	// apparmor template
+	m.Integration[hookName]["apparmor"] = filepath.Join("meta", hookName+".apparmor")
+
+	return
+}
+
+// legacyIntegration sets up the Integration property of packageYaml from its other attributes
+func (m *packageYaml) legacyIntegration() {
+	if m.Integration != nil {
+		// TODO: append "Overriding user-provided values." to the end of the blurb.
+		logger.Noticef(`The "integration" key is deprecated, and all uses of "integration" should be rewritten; see https://developer.ubuntu.com/en/snappy/guides/package-metadata/ (the "binaries" and "services" sections are probably especially relevant)."`)
+	} else {
+		// TODO: do this always, not just when Integration is not set
+		m.Integration = make(map[string]clickAppHook)
+	}
+
+	for _, v := range m.Binaries {
+		hookName := filepath.Base(v.Name)
+
+		if _, ok := m.Integration[hookName]; !ok {
+			m.Integration[hookName] = clickAppHook{}
+		}
+		// legacy click hook
+		m.Integration[hookName]["bin-path"] = v.Exec
+
+		m.legacyIntegrateSecDef(hookName, &v.SecurityDefinitions)
+	}
+
+	for _, v := range m.Services {
+		hookName := filepath.Base(v.Name)
+
+		if _, ok := m.Integration[hookName]; !ok {
+			m.Integration[hookName] = clickAppHook{}
+		}
+
+		// handle the apparmor stuff
+		m.legacyIntegrateSecDef(hookName, &v.SecurityDefinitions)
+	}
+}
+
 // NewInstalledSnapPart returns a new SnapPart from the given yamlPath
 func NewInstalledSnapPart(yamlPath, origin string) (*SnapPart, error) {
 	m, err := parsePackageYamlFile(yamlPath)

=== modified file 'snappy/snapp_test.go'
--- snappy/snapp_test.go	2015-06-04 13:34:57 +0000
+++ snappy/snapp_test.go	2015-06-08 16:38:59 +0000
@@ -1433,3 +1433,70 @@
 	udevName := packageYaml.qualifiedName("")
 	c.Assert(udevName, Equals, "foo")
 }
+
+func (s *SnapTestSuite) TestIntegrateBoring(c *C) {
+	m := &packageYaml{}
+	m.legacyIntegration()
+
+	// no binaries, no service, no legacyIntegration
+	c.Check(m.Integration, HasLen, 0)
+}
+
+func (s *SnapTestSuite) TestIntegrateBinary(c *C) {
+	m := &packageYaml{
+		Binaries: []Binary{
+			{
+				Name: "testme",
+				Exec: "bin/testme",
+			},
+			{
+				Name: "testme-override",
+				Exec: "bin/testme-override",
+				SecurityDefinitions: SecurityDefinitions{
+					SecurityOverride: &SecurityOverrideDefinition{Apparmor: "meta/testme-override.apparmor"},
+				},
+			},
+			{
+				Name: "testme-policy",
+				Exec: "bin/testme-policy",
+				SecurityDefinitions: SecurityDefinitions{
+					SecurityPolicy: &SecurityPolicyDefinition{Apparmor: "meta/testme-policy.profile"},
+				},
+			},
+		},
+	}
+	m.legacyIntegration()
+
+	c.Check(m.Integration, DeepEquals, map[string]clickAppHook{
+		"testme": {
+			"apparmor": "meta/testme.apparmor",
+			"bin-path": "bin/testme",
+		},
+		"testme-override": {
+			"apparmor": "meta/testme-override.apparmor",
+			"bin-path": "bin/testme-override",
+		},
+		"testme-policy": {
+			"apparmor-profile": "meta/testme-policy.profile",
+			"bin-path":         "bin/testme-policy",
+		},
+	})
+}
+
+func (s *SnapTestSuite) TestIntegrateService(c *C) {
+	m := &packageYaml{
+		Services: []Service{
+			{
+				Name: "svc",
+			},
+		},
+	}
+
+	m.legacyIntegration()
+
+	// no binaries, no service, no integrate
+	c.Check(m.Integration, DeepEquals, map[string]clickAppHook{
+		"svc": clickAppHook{
+			"apparmor": "meta/svc.apparmor",
+		}})
+}