Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add CIDR support to Address structure #158

Open
wants to merge 1 commit into
base: v7
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 61 additions & 15 deletions address.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type Address interface {
Scope() string
Origin() string
SpaceID() string
CIDR() string
}

// AddressArgs is an argument struct used to create a new internal address
Expand All @@ -25,16 +26,18 @@ type AddressArgs struct {
Scope string
Origin string
SpaceID string
CIDR string
}

func newAddress(args AddressArgs) *address {
return &address{
Version: 2,
Version: 3,
Value_: args.Value,
Type_: args.Type,
Scope_: args.Scope,
Origin_: args.Origin,
SpaceID_: args.SpaceID,
CIDR_: args.CIDR,
}
}

Expand All @@ -47,6 +50,7 @@ type address struct {
Scope_ string `yaml:"scope,omitempty"`
Origin_ string `yaml:"origin,omitempty"`
SpaceID_ string `yaml:"spaceid,omitempty"`
CIDR_ string `yaml:"cidr,omitempty"`
}

// Value implements Address.
Expand Down Expand Up @@ -74,6 +78,11 @@ func (a *address) SpaceID() string {
return a.SpaceID_
}

// CIDR implements Address.
func (a *address) CIDR() string {
return a.CIDR_
}

func importAddresses(sourceList []interface{}) ([]*address, error) {
var result []*address
for i, value := range sourceList {
Expand Down Expand Up @@ -111,6 +120,7 @@ type addressDeserializationFunc func(map[string]interface{}) (*address, error)
var addressDeserializationFuncs = map[int]addressDeserializationFunc{
1: importAddressV1,
2: importAddressV2,
3: importAddressV3,
}

func importAddressV1(source map[string]interface{}) (*address, error) {
Expand All @@ -135,24 +145,12 @@ func importAddressV1(source map[string]interface{}) (*address, error) {
}

func importAddressV2(source map[string]interface{}) (*address, error) {
fields, defaults := addressV1Fields()
fields["spaceid"] = schema.String()

// We must allow for an empty space ID because:
// - newAddress always returns a V2 address.
// - newAddress is called by methods in Machine that do not negotiate a
// version.
// If an old version of Juju not supporting address spaces upgrades to this
// version of the library, we need to allow export and import of V2
// addresses that tolerate a missing space ID.
// Ensuring correct defaults for this field must be ensured in the Juju
// migration code itself.
defaults["spaceid"] = ""
fields, defaults := addressV2Fields()
checker := schema.FieldMap(fields, defaults)

coerced, err := checker.Coerce(source, nil)
if err != nil {
return nil, errors.Annotatef(err, "address v1 schema check failed")
return nil, errors.Annotatef(err, "address v2 schema check failed")
}
valid := coerced.(map[string]interface{})
// From here we know that the map returned from the schema coercion
Expand All @@ -168,6 +166,29 @@ func importAddressV2(source map[string]interface{}) (*address, error) {
}, nil
}

func importAddressV3(source map[string]interface{}) (*address, error) {
fields, defaults := addressV3Fields()
checker := schema.FieldMap(fields, defaults)

coerced, err := checker.Coerce(source, nil)
if err != nil {
return nil, errors.Annotatef(err, "address v3 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 &address{
Version: 3,
Value_: valid["value"].(string),
Type_: valid["type"].(string),
Scope_: valid["scope"].(string),
Origin_: valid["origin"].(string),
SpaceID_: valid["spaceid"].(string),
CIDR_: valid["cidr"].(string),
}, nil
}

func addressV1Fields() (schema.Fields, schema.Defaults) {
fields := schema.Fields{
"value": schema.String(),
Expand All @@ -182,3 +203,28 @@ func addressV1Fields() (schema.Fields, schema.Defaults) {
}
return fields, defaults
}

// We must allow for an empty value for fields introduced after v1 because:
// - newAddress always returns an address at the latest version
// - newAddress is called by methods in Machine that do not negotiate a
// version.
//
// If an old version of Juju not supporting new fields upgrades to this
// version of the library, we need to allow export and import of V2
// addresses that tolerate a missing space ID or CIDR.
// Ensuring correct defaults for this field must be ensured in the Juju
// migration code itself.

func addressV2Fields() (schema.Fields, schema.Defaults) {
fields, defaults := addressV1Fields()
fields["spaceid"] = schema.String()
defaults["spaceid"] = "" // must be allowed empty
return fields, defaults
}

func addressV3Fields() (schema.Fields, schema.Defaults) {
fields, defaults := addressV2Fields()
fields["cidr"] = schema.String()
defaults["cidr"] = "" // must be allowed empty
return fields, defaults
}
24 changes: 24 additions & 0 deletions address_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,27 @@ func (*AddressSerializationSuite) TestParsingSerializedDataV2(c *gc.C) {

c.Assert(addresss, jc.DeepEquals, initial)
}

func (*AddressSerializationSuite) TestParsingSerializedDataV3(c *gc.C) {
initial := &address{
Version: 3,
Value_: "no",
Type_: "content",
Scope_: "done",
Origin_: "here",
SpaceID_: "666",
CIDR_: "no/24",
}

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)

addresss, err := importAddress(source)
c.Assert(err, jc.ErrorIsNil)

c.Assert(addresss, jc.DeepEquals, initial)
}
4 changes: 2 additions & 2 deletions application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ func minimalApplicationMapCAAS() map[interface{}]interface{} {
"version": 1,
"provider-id": "some-provider",
"addresses": []interface{}{
map[interface{}]interface{}{"version": 2, "value": "10.0.0.1", "type": "special"},
map[interface{}]interface{}{"version": 2, "value": "10.0.0.2", "type": "other"},
map[interface{}]interface{}{"version": 3, "value": "10.0.0.1", "type": "special"},
map[interface{}]interface{}{"version": 3, "value": "10.0.0.2", "type": "other"},
},
}
result["units"] = map[interface{}]interface{}{
Expand Down
2 changes: 1 addition & 1 deletion cloudcontainer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (s *CloudContainerSerializationSuite) TestAllArgs(c *gc.C) {
container := newCloudContainer(&args)

c.Check(container.ProviderId(), gc.Equals, args.ProviderId)
c.Check(container.Address(), jc.DeepEquals, &address{Version: 2, Value_: "10.0.0.1", Type_: "special"})
c.Check(container.Address(), jc.DeepEquals, &address{Version: 3, Value_: "10.0.0.1", Type_: "special"})

c.Check(container.Ports(), jc.DeepEquals, args.Ports)
}
Expand Down
4 changes: 2 additions & 2 deletions cloudservice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ func (s *CloudServiceSerializationSuite) TestAllArgs(c *gc.C) {

c.Check(container.ProviderId(), gc.Equals, args.ProviderId)
c.Check(container.Addresses(), jc.DeepEquals, []Address{
&address{Version: 2, Value_: "10.0.0.1", Type_: "special"},
&address{Version: 2, Value_: "10.0.0.2", Type_: "other"},
&address{Version: 3, Value_: "10.0.0.1", Type_: "special"},
&address{Version: 3, Value_: "10.0.0.2", Type_: "other"},
})
}

Expand Down
2 changes: 1 addition & 1 deletion unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func minimalUnitMapCAAS() map[interface{}]interface{} {
result["cloud-container"] = map[interface{}]interface{}{
"version": 1,
"provider-id": "some-provider",
"address": map[interface{}]interface{}{"version": 2, "value": "10.0.0.1", "type": "special"},
"address": map[interface{}]interface{}{"version": 3, "value": "10.0.0.1", "type": "special"},
"ports": []interface{}{"80", "443"},
}
return result
Expand Down