Merge pull request #403 from barrettj12/reserved-name

#403

Backport of #327

actions.go

@@ -95,7 +95,7 @@ func (spec *ActionSpec) InsertDefaults(target map[string]interface{}) (map[strin
 }
 
 // ReadActionsYaml builds an Actions spec from a charm's actions.yaml.
-func ReadActionsYaml(r io.Reader) (*Actions, error) {
+func ReadActionsYaml(charmName string, r io.Reader) (*Actions, error) {
 	data, err := ioutil.ReadAll(r)
 	if err != nil {
 		return nil, err
@@ -114,7 +114,7 @@ func ReadActionsYaml(r io.Reader) (*Actions, error) {
 		if valid := actionNameRule.MatchString(name); !valid {
 			return nil, fmt.Errorf("bad action name %s", name)
 		}
-		if reserved, reason := reservedName(name); reserved {
+		if reserved, reason := reservedName(charmName, name); reserved {
 			return nil, fmt.Errorf(
 				"cannot use action name %s: %s",
 				name, reason,

actions_test.go

@@ -577,12 +577,41 @@ snapshot-0-foo:
 	for i, test := range goodActionsYamlTests {
 		c.Logf("test %d: %s", i, test.description)
 		reader := bytes.NewReader([]byte(test.yaml))
-		loadedAction, err := ReadActionsYaml(reader)
+		loadedAction, err := ReadActionsYaml("somecharm", reader)
 		c.Assert(err, gc.IsNil)
 		c.Check(loadedAction, jc.DeepEquals, test.expectedActions)
 	}
 }
 
+func (s *ActionsSuite) TestJujuCharmActionsYaml(c *gc.C) {
+	actionsYaml := `
+juju-snapshot:
+   description: Take a snapshot of the database.
+   params:
+      outfile:
+         description: "The file to write out to."
+         type: string
+   required: ["outfile"]
+`
+	expectedActions := &Actions{map[string]ActionSpec{
+		"juju-snapshot": {
+			Description: "Take a snapshot of the database.",
+			Params: map[string]interface{}{
+				"title":       "juju-snapshot",
+				"description": "Take a snapshot of the database.",
+				"type":        "object",
+				"properties": map[string]interface{}{
+					"outfile": map[string]interface{}{
+						"description": "The file to write out to.",
+						"type":        "string"}},
+				"required": []interface{}{"outfile"}}}}}
+
+	reader := bytes.NewReader([]byte(actionsYaml))
+	loadedAction, err := ReadActionsYaml("juju-charm", reader)
+	c.Assert(err, gc.IsNil)
+	c.Check(loadedAction, jc.DeepEquals, expectedActions)
+}
+
 func (s *ActionsSuite) TestReadBadActionsYaml(c *gc.C) {
 
 	var badActionsYamlTests = []struct {
@@ -747,7 +776,7 @@ snapshot:
 	for i, test := range badActionsYamlTests {
 		c.Logf("test %d: %s", i, test.description)
 		reader := bytes.NewReader([]byte(test.yaml))
-		_, err := ReadActionsYaml(reader)
+		_, err := ReadActionsYaml("somecharm", reader)
 		c.Check(err, gc.ErrorMatches, test.expectedError)
 	}
 }
@@ -956,7 +985,7 @@ act:
 func getSchemaForAction(c *gc.C, wholeSchema string) ActionSpec {
 	// Load up the YAML schema definition.
 	reader := bytes.NewReader([]byte(wholeSchema))
-	loadedActions, err := ReadActionsYaml(reader)
+	loadedActions, err := ReadActionsYaml("somecharm", reader)
 	c.Assert(err, gc.IsNil)
 	// Same action name for all tests, "act".
 	return loadedActions.ActionSpecs["act"]

charmarchive.go

@@ -117,6 +117,7 @@ func readCharmArchive(zopen zipOpener) (archive *CharmArchive, err error) {
 	}
 
 	if b.actions, err = getActions(
+		b.meta.Name,
 		func(file string) (io.ReadCloser, error) {
 			return zipOpenFile(zipr, file)
 		},
@@ -171,11 +172,11 @@ func readCharmArchive(zopen zipOpener) (archive *CharmArchive, err error) {
 
 type fileOpener func(string) (io.ReadCloser, error)
 
-func getActions(open fileOpener, isNotFound func(error) bool) (actions *Actions, err error) {
+func getActions(charmName string, open fileOpener, isNotFound func(error) bool) (actions *Actions, err error) {
 	reader, err := open("actions.yaml")
 	if err == nil {
 		defer reader.Close()
-		return ReadActionsYaml(reader)
+		return ReadActionsYaml(charmName, reader)
 	} else if !isNotFound(err) {
 		return nil, err
 	}

charmarchive_test.go

@@ -148,6 +148,13 @@ func (s *CharmArchiveSuite) TestReadCharmArchiveWithActions(c *gc.C) {
 	c.Assert(archive.Actions().ActionSpecs, gc.HasLen, 1)
 }
 
+func (s *CharmDirSuite) TestReadCharmArchiveWithJujuActions(c *gc.C) {
+	path := archivePath(c, readCharmDir(c, "juju-charm"))
+	archive, err := charm.ReadCharmArchive(path)
+	c.Assert(err, gc.IsNil)
+	c.Assert(archive.Actions().ActionSpecs, gc.HasLen, 1)
+}
+
 func (s *CharmArchiveSuite) TestReadCharmArchiveBytes(c *gc.C) {
 	data, err := ioutil.ReadFile(s.archivePath)
 	c.Assert(err, gc.IsNil)

charmdir.go

@@ -115,6 +115,7 @@ func ReadCharmDir(path string) (*CharmDir, error) {
 	}
 
 	if b.actions, err = getActions(
+		b.meta.Name,
 		func(file string) (io.ReadCloser, error) {
 			return os.Open(b.join(file))
 		},

charmdir_test.go

@@ -106,6 +106,13 @@ func (s *CharmDirSuite) TestReadCharmDirWithActions(c *gc.C) {
 	c.Assert(dir.Actions().ActionSpecs, gc.HasLen, 1)
 }
 
+func (s *CharmDirSuite) TestReadCharmDirWithJujuActions(c *gc.C) {
+	path := charmDirPath(c, "juju-charm")
+	dir, err := charm.ReadCharmDir(path)
+	c.Assert(err, gc.IsNil)
+	c.Assert(dir.Actions().ActionSpecs, gc.HasLen, 1)
+}
+
 func (s *CharmDirSuite) TestReadCharmDirManifest(c *gc.C) {
 	path := charmDirPath(c, "dummy")
 	dir, err := charm.ReadCharmDir(path)

internal/test-charm-repo/quantal/juju-charm/.notignored

@@ -0,0 +1 @@
+#

internal/test-charm-repo/quantal/juju-charm/actions.yaml

@@ -0,0 +1,7 @@
+juju-snapshot:
+   description: Take a snapshot of the database.
+   params:
+      outfile:
+         description: The file to write out to.
+         type: string
+         default: foo.bz2

internal/test-charm-repo/quantal/juju-charm/actions/juju-snapshot

@@ -0,0 +1,2 @@
+#!/bin/bash
+echo "function $0"

internal/test-charm-repo/quantal/juju-charm/config.yaml

@@ -0,0 +1,5 @@
+options:
+  title: {default: My Title, description: A descriptive title used for the application., type: string}
+  outlook: {description: No default outlook., type: string}
+  username: {default: admin001, description: The name of the initial account (given admin permissions)., type: string}
+  skill-level: {description: A number indicating skill., type: int}

internal/test-charm-repo/quantal/juju-charm/hooks/install

@@ -0,0 +1,2 @@
+#!/bin/bash
+echo "Done!"

internal/test-charm-repo/quantal/juju-charm/lxd-profile.yaml

@@ -0,0 +1,9 @@
+name: "test"
+description: "sample lxdprofile for testing"
+config:
+  security.nesting: "true"
+  security.privileged: "true"
+devices:
+  tun:
+    path: /dev/net/tun
+    type: unix-char

internal/test-charm-repo/quantal/juju-charm/metadata.yaml

@@ -0,0 +1,8 @@
+name: juju-charm
+summary: "That's a dummy charm."
+description: |
+    This is a longer description which
+    potentially contains multiple lines.
+provides:
+    dashboard:
+        interface: juju-dashboard

internal/test-charm-repo/quantal/juju-charm/revision

@@ -0,0 +1 @@
+1

meta.go

@@ -756,12 +756,12 @@ func (m Meta) Check(format Format, reasons ...FormatSelectionReason) error {
 			// Container-scoped require relations on subordinates are allowed
 			// to use the otherwise-reserved juju-* namespace.
 			if !m.Subordinate || role != RoleRequirer || rel.Scope != ScopeContainer {
-				if reserved, _ := reservedName(name); reserved {
+				if reserved, _ := reservedName(m.Name, name); reserved {
 					return errors.Errorf("charm %q using a reserved relation name: %q", m.Name, name)
 				}
 			}
 			if role != RoleRequirer {
-				if reserved, _ := reservedName(rel.Interface); reserved {
+				if reserved, _ := reservedName(m.Name, rel.Interface); reserved {
 					return errors.Errorf("charm %q relation %q using a reserved interface: %q", m.Name, name, rel.Interface)
 				}
 			}
@@ -904,11 +904,14 @@ func hasReason(reasons []FormatSelectionReason, reason FormatSelectionReason) bo
 	return set.NewStrings(reasons...).Contains(reason)
 }
 
-func reservedName(name string) (reserved bool, reason string) {
-	if name == "juju" {
+func reservedName(charmName, endpointName string) (reserved bool, reason string) {
+	if strings.HasPrefix(charmName, "juju-") {
+		return false, ""
+	}
+	if endpointName == "juju" {
 		return true, `"juju" is a reserved name`
 	}
-	if strings.HasPrefix(name, "juju-") {
+	if strings.HasPrefix(endpointName, "juju-") {
 		return true, `the "juju-" prefix is reserved`
 	}
 	return false, ""

meta_test.go

@@ -459,6 +459,17 @@ func (s *MetaSuite) TestCombinedRelations(c *gc.C) {
 	})
 }
 
+func (s *MetaSuite) TestParseJujuRelations(c *gc.C) {
+	meta, err := charm.ReadMeta(repoMeta(c, "juju-charm"))
+	c.Assert(err, gc.IsNil)
+	c.Assert(meta.Provides["dashboard"], gc.Equals, charm.Relation{
+		Name:      "dashboard",
+		Role:      charm.RoleProvider,
+		Interface: "juju-dashboard",
+		Scope:     charm.ScopeGlobal,
+	})
+}
+
 var relationsConstraintsTests = []struct {
 	rels string
 	err  string