Merge pull request #139 from juju/add-uuid-spaces

Add UUID fields into spaces and subnets

Makefile

@@ -1,4 +1,4 @@
-PROJECT := github.com/juju/description/v5
+PROJECT := github.com/juju/description/v6
 
 .PHONY: check-licence check-go check
 

go.mod

@@ -1,4 +1,4 @@
-module github.com/juju/description/v5
+module github.com/juju/description/v6
 
 go 1.21
 

interfaces.go

@@ -22,6 +22,7 @@ type AgentTools interface {
 // Space represents a network space, which is a named collection of subnets.
 type Space interface {
 	Id() string
+	UUID() string
 	Name() string
 	Public() bool
 	ProviderID() string
@@ -204,6 +205,7 @@ type StorageInstanceConstraints struct {
 // Subnet represents a network subnet.
 type Subnet interface {
 	ID() string
+	UUID() string
 	ProviderId() string
 	ProviderNetworkId() string
 	ProviderSpaceId() string
@@ -212,6 +214,7 @@ type Subnet interface {
 	AvailabilityZones() []string
 	IsPublic() bool
 	SpaceID() string
+	SpaceUUID() string
 	SpaceName() string
 	FanLocalUnderlay() string
 	FanOverlay() string

model.go

@@ -565,7 +565,7 @@ func (m *model) AddSpace(args SpaceArgs) Space {
 
 func (m *model) setSpaces(spaceList []*space) {
 	m.Spaces_ = spaces{
-		Version: 2,
+		Version: 3,
 		Spaces_: spaceList,
 	}
 }
@@ -1234,18 +1234,18 @@ func (m *model) validateStorage(validationCtx *validationContext) error {
 
 // validateSubnets makes sure that any spaces referenced by subnets exist.
 func (m *model) validateSubnets() error {
-	spaceIDs := set.NewStrings()
+	spaceUUIDs := set.NewStrings()
 	for _, space := range m.Spaces_.Spaces_ {
-		spaceIDs.Add(space.Id())
+		spaceUUIDs.Add(space.UUID())
 	}
 	for _, subnet := range m.Subnets_.Subnets_ {
 		// space "0" is the new, in juju 2.7, default space,
 		// created with each new model.
-		if subnet.SpaceID() == "" || subnet.SpaceID() == "0" {
+		if subnet.SpaceUUID() == "" || subnet.SpaceUUID() == "0" {
 			continue
 		}
-		if !spaceIDs.Contains(subnet.SpaceID()) {
-			return errors.Errorf("subnet %q references non-existent space %q", subnet.CIDR(), subnet.SpaceID())
+		if !spaceUUIDs.Contains(subnet.SpaceUUID()) {
+			return errors.Errorf("subnet %q references non-existent space %q", subnet.CIDR(), subnet.SpaceUUID())
 		}
 	}
 

model_test.go

@@ -150,7 +150,7 @@ func (s *ModelSerializationSuite) TestVersions(c *gc.C) {
 	c.Assert(initial.Relations_.Version, gc.Equals, len(relationFieldsFuncs))
 	c.Assert(initial.RemoteEntities_.Version, gc.Equals, len(remoteEntityFieldsFuncs))
 	c.Assert(initial.RemoteApplications_.Version, gc.Equals, len(remoteApplicationFieldsFuncs))
-	c.Assert(initial.Spaces_.Version, gc.Equals, len(spaceDeserializationFuncs))
+	c.Assert(initial.Spaces_.Version, gc.Equals, len(spaceFieldsFuncs))
 	c.Assert(initial.Volumes_.Version, gc.Equals, len(volumeDeserializationFuncs))
 	c.Assert(initial.FirewallRules_.Version, gc.Equals, len(firewallRuleFieldsFuncs))
 	c.Assert(initial.OfferConnections_.Version, gc.Equals, len(offerConnectionDeserializationFuncs))
@@ -594,11 +594,11 @@ func (s *ModelSerializationSuite) TestModelSerializationWithRelationNetworks(c *
 
 func (s *ModelSerializationSuite) TestModelValidationChecksSubnets(c *gc.C) {
 	model := s.newModel(ModelArgs{Owner: names.NewUserTag("owner")})
-	model.AddSubnet(SubnetArgs{CIDR: "10.0.0.0/24", SpaceID: "3"})
+	model.AddSubnet(SubnetArgs{CIDR: "10.0.0.0/24", SpaceUUID: "deadbeef-1bad-500d-9000-4b1d0d06f00d"})
 	model.AddSubnet(SubnetArgs{CIDR: "10.0.1.0/24"})
 	err := model.Validate()
-	c.Assert(err, gc.ErrorMatches, `subnet "10.0.0.0/24" references non-existent space "3"`)
-	model.AddSpace(SpaceArgs{Id: "3"})
+	c.Assert(err, gc.ErrorMatches, `subnet "10.0.0.0/24" references non-existent space "deadbeef-1bad-500d-9000-4b1d0d06f00d"`)
+	model.AddSpace(SpaceArgs{UUID: "deadbeef-1bad-500d-9000-4b1d0d06f00d"})
 	err = model.Validate()
 	c.Assert(err, jc.ErrorIsNil)
 }
@@ -1206,9 +1206,9 @@ func (s *ModelSerializationSuite) TestVersion1IgnoresRemoteApplications(c *gc.C)
 
 func (s *ModelSerializationSuite) TestSpaces(c *gc.C) {
 	initial := s.newModel(ModelArgs{Owner: names.NewUserTag("owner")})
-	space := initial.AddSpace(SpaceArgs{Id: "1", Name: "special"})
+	space := initial.AddSpace(SpaceArgs{UUID: "deadbeef-1bad-500d-9000-4b1d0d06f00d", Name: "special"})
 	c.Assert(space.Name(), gc.Equals, "special")
-	c.Assert(space.Id(), gc.Equals, "1")
+	c.Assert(space.UUID(), gc.Equals, "deadbeef-1bad-500d-9000-4b1d0d06f00d")
 
 	spaces := initial.Spaces()
 	c.Assert(spaces, gc.HasLen, 1)

package_test.go

@@ -25,7 +25,7 @@ var _ = gc.Suite(&ImportTest{})
 
 func (*ImportTest) TestImports(c *gc.C) {
 	imps, err := jtesting.FindImports(
-		"github.com/juju/description/v5",
+		"github.com/juju/description/v6",
 		"github.com/juju/juju/")
 	c.Assert(err, jc.ErrorIsNil)
 	// This package brings in nothing else from juju/juju

remoteapplication_test.go

@@ -77,8 +77,10 @@ func minimalRemoteApplicationMapWithoutStatus() map[interface{}]interface{} {
 					"subnets": []interface{}{map[interface{}]interface{}{
 						"cidr":                "2.3.4.0/24",
 						"subnet-id":           "",
+						"uuid":                "",
 						"is-public":           false,
 						"space-id":            "",
+						"space-uuid":          "",
 						"space-name":          "",
 						"vlan-tag":            0,
 						"provider-id":         "juju-subnet-1",

remotespace_test.go

@@ -40,8 +40,10 @@ func minimalRemoteSpaceMap() map[interface{}]interface{} {
 			"subnets": []interface{}{map[interface{}]interface{}{
 				"cidr":                "2.3.4.0/24",
 				"subnet-id":           "",
+				"uuid":                "",
 				"is-public":           false,
 				"space-id":            "",
+				"space-uuid":          "",
 				"space-name":          "a-space",
 				"vlan-tag":            64,
 				"provider-id":         "juju-subnet-1",

space.go

@@ -15,6 +15,7 @@ type spaces struct {
 
 type space struct {
 	Id_         string `yaml:"id"`
+	UUID_       string `yaml:"uuid"`
 	Name_       string `yaml:"name"`
 	Public_     bool   `yaml:"public"`
 	ProviderID_ string `yaml:"provider-id,omitempty"`
@@ -24,6 +25,7 @@ type space struct {
 // type that supports the Space interface.
 type SpaceArgs struct {
 	Id         string
+	UUID       string
 	Name       string
 	Public     bool
 	ProviderID string
@@ -32,6 +34,7 @@ type SpaceArgs struct {
 func newSpace(args SpaceArgs) *space {
 	return &space{
 		Id_:         args.Id,
+		UUID_:       args.UUID,
 		Name_:       args.Name,
 		Public_:     args.Public,
 		ProviderID_: args.ProviderID,
@@ -43,6 +46,11 @@ func (s *space) Id() string {
 	return s.Id_
 }
 
+// UUID implements Space.
+func (s *space) UUID() string {
+	return s.UUID_
+}
+
 // Name implements Space.
 func (s *space) Name() string {
 	return s.Name_
@@ -67,22 +75,28 @@ func importSpaces(source map[string]interface{}) ([]*space, error) {
 	valid := coerced.(map[string]interface{})
 
 	version := int(valid["version"].(int64))
-	importFunc, ok := spaceDeserializationFuncs[version]
+	getFields, ok := spaceFieldsFuncs[version]
 	if !ok {
 		return nil, errors.NotValidf("version %d", version)
 	}
 	sourceList := valid["spaces"].([]interface{})
-	return importSpaceList(sourceList, importFunc)
+	return importSpaceList(sourceList, schema.FieldMap(getFields()), version)
 }
 
-func importSpaceList(sourceList []interface{}, importFunc spaceDeserializationFunc) ([]*space, error) {
+func importSpaceList(sourceList []interface{}, checker schema.Checker, version int) ([]*space, error) {
 	result := make([]*space, 0, len(sourceList))
 	for i, value := range sourceList {
 		source, ok := value.(map[string]interface{})
 		if !ok {
 			return nil, errors.Errorf("unexpected value for space %d, %T", i, value)
 		}
-		space, err := importFunc(source)
+		coerced, err := checker.Coerce(source, nil)
+
+		if err != nil {
+			return nil, errors.Annotatef(err, "space %d v%d schema check failed", i, version)
+		}
+		valid := coerced.(map[string]interface{})
+		space, err := newSpaceFromValid(valid, version)
 		if err != nil {
 			return nil, errors.Annotatef(err, "space %d", i)
 		}
@@ -91,51 +105,26 @@ func importSpaceList(sourceList []interface{}, importFunc spaceDeserializationFu
 	return result, nil
 }
 
-type spaceDeserializationFunc func(map[string]interface{}) (*space, error)
-
-var spaceDeserializationFuncs = map[int]spaceDeserializationFunc{
-	1: importSpaceV1,
-	2: importSpaceV2,
-}
-
-func importSpaceV1(source map[string]interface{}) (*space, error) {
-	fields, defaults := spaceV1Fields()
-	checker := schema.FieldMap(fields, defaults)
-
-	coerced, err := checker.Coerce(source, nil)
-	if err != nil {
-		return nil, errors.Annotatef(err, "space v1 schema check failed")
-	}
-	valid := coerced.(map[string]interface{})
-	// From here we know that the map returned from the schema coercion
-	// contains fields of the right type.
-
-	return &space{
+func newSpaceFromValid(valid map[string]interface{}, version int) (*space, error) {
+	result := space{
 		Name_:       valid["name"].(string),
 		Public_:     valid["public"].(bool),
 		ProviderID_: valid["provider-id"].(string),
-	}, nil
-}
-
-func importSpaceV2(source map[string]interface{}) (*space, error) {
-	fields, defaults := spaceV1Fields()
-	fields["id"] = schema.String()
-	checker := schema.FieldMap(fields, defaults)
-
-	coerced, err := checker.Coerce(source, nil)
-	if err != nil {
-		return nil, errors.Annotatef(err, "space v2 schema check failed")
 	}
-	valid := coerced.(map[string]interface{})
-	// From here we know that the map returned from the schema coercion
-	// contains fields of the right type.
+	// id was added in V2 and removed in V3.
+	if version == 2 {
+		result.Id_ = valid["id"].(string)
+	}
+	if version >= 3 {
+		result.UUID_ = valid["uuid"].(string)
+	}
+	return &result, nil
+}
 
-	return &space{
-		Id_:         valid["id"].(string),
-		Name_:       valid["name"].(string),
-		Public_:     valid["public"].(bool),
-		ProviderID_: valid["provider-id"].(string),
-	}, nil
+var spaceFieldsFuncs = map[int]fieldsFunc{
+	1: spaceV1Fields,
+	2: spaceV2Fields,
+	3: spaceV3Fields,
 }
 
 func spaceV1Fields() (schema.Fields, schema.Defaults) {
@@ -150,3 +139,18 @@ func spaceV1Fields() (schema.Fields, schema.Defaults) {
 	}
 	return fields, defaults
 }
+
+func spaceV2Fields() (schema.Fields, schema.Defaults) {
+	fields, defaults := spaceV1Fields()
+	fields["id"] = schema.String()
+
+	return fields, defaults
+}
+
+func spaceV3Fields() (schema.Fields, schema.Defaults) {
+	fields, defaults := spaceV2Fields()
+	fields["uuid"] = schema.String()
+	delete(fields, "id")
+
+	return fields, defaults
+}

space_test.go

@@ -35,6 +35,7 @@ func (s *SpaceSerializationSuite) TestNewSpace(c *gc.C) {
 	}
 	space := newSpace(args)
 	c.Assert(space.Id(), gc.Equals, "")
+	c.Assert(space.UUID(), gc.Equals, "")
 	c.Assert(space.Name(), gc.Equals, args.Name)
 	c.Assert(space.Public(), gc.Equals, args.Public)
 	c.Assert(space.ProviderID(), gc.Equals, args.ProviderID)
@@ -92,3 +93,30 @@ func (s *SpaceSerializationSuite) TestParsingSerializedDataV2(c *gc.C) {
 
 	c.Assert(spaces, jc.DeepEquals, initial.Spaces_)
 }
+
+func (s *SpaceSerializationSuite) TestParsingSerializedDataV3(c *gc.C) {
+	initial := spaces{
+		Version: 3,
+		Spaces_: []*space{
+			newSpace(SpaceArgs{
+				UUID:       "018ea48e-c6a6-7d51-ae76-9bfee4a6b6dd",
+				Name:       "special",
+				Public:     true,
+				ProviderID: "magic",
+			}),
+			newSpace(SpaceArgs{Name: "foo"}),
+		},
+	}
+
+	bytes, err := yaml.Marshal(initial)
+	c.Assert(err, jc.ErrorIsNil)
+
+	var source map[string]interface{}
+	err = yaml.Unmarshal(bytes, &source)
+	c.Assert(err, jc.ErrorIsNil)
+
+	spaces, err := importSpaces(source)
+	c.Assert(err, jc.ErrorIsNil)
+
+	c.Assert(spaces, jc.DeepEquals, initial.Spaces_)
+}