Skip to content
This repository has been archived by the owner on Sep 4, 2021. It is now read-only.

Fix vpc validation for multiple subnets #507

Merged
merged 3 commits into from
May 26, 2016
Merged

Fix vpc validation for multiple subnets #507

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c705400
config: use Subnets for existing vpc validation
colhom May 24, 2016
d470f08
config: add omitempty yaml tags for Cluster
colhom May 25, 2016
43e7ac3
cluster: unit tests for multi-subnet / existing vpc
colhom May 25, 2016
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
61 changes: 53 additions & 8 deletions multi-node/aws/pkg/cluster/cluster_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,44 @@ import (
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/aws/aws-sdk-go/service/route53"
"github.com/coreos/coreos-kubernetes/multi-node/aws/pkg/config"
yaml "gopkg.in/yaml.v2"
)

const minimalConfigYaml = `
/*
TODO(colhom): when we fully deprecate instanceCIDR/availabilityZone, this block of
logic will go away and be replaced by a single constant string
*/
func defaultConfigValues(t *testing.T, configYaml string) string {
defaultYaml := `
externalDNSName: test.staging.core-os.net
keyName: test-key-name
region: us-west-1
availabilityZone: us-west-1c
clusterName: test-cluster-name
kmsKeyArn: "arn:aws:kms:us-west-1:xxxxxxxxx:key/xxxxxxxxxxxxxxxxxxx"
`
yamlStr := defaultYaml + configYaml

c := config.Cluster{}
if err := yaml.Unmarshal([]byte(yamlStr), &c); err != nil {
t.Errorf("failed umarshalling config yaml: %v :\n%s", err, yamlStr)
}

if len(c.Subnets) > 0 {
for i := range c.Subnets {
c.Subnets[i].AvailabilityZone = fmt.Sprintf("dummy-az-%d", i)
}
} else {
//Legacy behavior
c.AvailabilityZone = "dummy-az-0"
}

out, err := yaml.Marshal(&c)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is why 5341bf3 exists. We're using the config.Cluster to automate pre-processing of test yaml data.

if err != nil {
t.Errorf("error marshalling cluster: %v", err)
}

return string(out)
}

type VPC struct {
cidr string
Expand Down Expand Up @@ -103,6 +131,14 @@ vpcCIDR: 192.168.1.0/24
vpcId: vpc-xxx2
instanceCIDR: 192.168.1.50/28
controllerIP: 192.168.1.50
`, `
vpcCIDR: 192.168.1.0/24
vpcId: vpc-xxx2
controllerIP: 192.168.1.5
subnets:
- instanceCIDR: 192.168.1.0/28
- instanceCIDR: 192.168.1.32/28
- instanceCIDR: 192.168.1.64/28
`,
}

Expand Down Expand Up @@ -131,6 +167,14 @@ instanceCIDR: 192.168.1.100/26 #instance cidr conflicts with existing subnet
controllerIP: 192.168.1.80
vpcId: vpc-xxx2
routeTableId: rtb-xxxxxx
`, `
vpcCIDR: 192.168.1.0/24
controllerIP: 192.168.1.80
vpcId: vpc-xxx2
routeTableId: rtb-xxxxxx
subnets:
- instanceCIDR: 192.168.1.100/26 #instance cidr conflicts with existing subnet
- instanceCIDR: 192.168.1.0/26
`,
}

Expand All @@ -156,7 +200,7 @@ routeTableId: rtb-xxxxxx
}

validateCluster := func(networkConfig string) error {
configBody := minimalConfigYaml + networkConfig
configBody := defaultConfigValues(t, networkConfig)
clusterConfig, err := config.ClusterFromBytes([]byte(configBody))
if err != nil {
t.Errorf("could not get valid cluster config: %v", err)
Expand Down Expand Up @@ -185,7 +229,7 @@ routeTableId: rtb-xxxxxx

func TestValidateKeyPair(t *testing.T) {

clusterConfig, err := config.ClusterFromBytes([]byte(minimalConfigYaml))
clusterConfig, err := config.ClusterFromBytes([]byte(defaultConfigValues(t, "")))
if err != nil {
t.Errorf("could not get valid cluster config: %v", err)
}
Expand Down Expand Up @@ -241,6 +285,7 @@ func (r53 dummyR53Service) ListResourceRecordSets(input *route53.ListResourceRec
}
return output, nil
}

func (r53 dummyR53Service) GetHostedZone(input *route53.GetHostedZoneInput) (*route53.GetHostedZoneOutput, error) {
for _, zone := range r53.HostedZones {
if zone.Id == aws.StringValue(input.Id) {
Expand All @@ -252,9 +297,9 @@ func (r53 dummyR53Service) GetHostedZone(input *route53.GetHostedZoneInput) (*ro
}, nil
}
}

return nil, fmt.Errorf("dummy route53 service: no hosted zone with id '%s'", aws.StringValue(input.Id))
}

func TestValidateDNSConfig(t *testing.T) {
r53 := dummyR53Service{
HostedZones: []Zone{
Expand Down Expand Up @@ -321,7 +366,7 @@ hostedZone: unicorns.core-os.net #non-existant hostedZone DNS name
}

for _, validConfig := range validDNSConfigs {
configBody := minimalConfigYaml + validConfig
configBody := defaultConfigValues(t, validConfig)
clusterConfig, err := config.ClusterFromBytes([]byte(configBody))
if err != nil {
t.Errorf("could not get valid cluster config: %v", err)
Expand All @@ -335,7 +380,7 @@ hostedZone: unicorns.core-os.net #non-existant hostedZone DNS name
}

for _, invalidConfig := range invalidDNSConfigs {
configBody := minimalConfigYaml + invalidConfig
configBody := defaultConfigValues(t, invalidConfig)
clusterConfig, err := config.ClusterFromBytes([]byte(configBody))
if err != nil {
t.Errorf("could not get valid cluster config: %v", err)
Expand Down Expand Up @@ -427,7 +472,7 @@ stackTags:
}

for _, testCase := range testCases {
configBody := minimalConfigYaml + testCase.clusterYaml
configBody := defaultConfigValues(t, testCase.clusterYaml)
clusterConfig, err := config.ClusterFromBytes([]byte(configBody))
if err != nil {
t.Errorf("could not get valid cluster config: %v", err)
Expand Down
94 changes: 50 additions & 44 deletions multi-node/aws/pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,41 +98,41 @@ func ClusterFromBytes(data []byte) (*Cluster, error) {
}

type Cluster struct {
ClusterName string `yaml:"clusterName"`
ExternalDNSName string `yaml:"externalDNSName"`
KeyName string `yaml:"keyName"`
Region string `yaml:"region"`
AvailabilityZone string `yaml:"availabilityZone"`
ReleaseChannel string `yaml:"releaseChannel"`
ControllerInstanceType string `yaml:"controllerInstanceType"`
ControllerRootVolumeSize int `yaml:"controllerRootVolumeSize"`
WorkerCount int `yaml:"workerCount"`
WorkerInstanceType string `yaml:"workerInstanceType"`
WorkerRootVolumeSize int `yaml:"workerRootVolumeSize"`
WorkerSpotPrice string `yaml:"workerSpotPrice"`
VPCID string `yaml:"vpcId"`
RouteTableID string `yaml:"routeTableId"`
VPCCIDR string `yaml:"vpcCIDR"`
InstanceCIDR string `yaml:"instanceCIDR"`
ControllerIP string `yaml:"controllerIP"`
PodCIDR string `yaml:"podCIDR"`
ServiceCIDR string `yaml:"serviceCIDR"`
DNSServiceIP string `yaml:"dnsServiceIP"`
K8sVer string `yaml:"kubernetesVersion"`
HyperkubeImageRepo string `yaml:"hyperkubeImageRepo"`
KMSKeyARN string `yaml:"kmsKeyArn"`
CreateRecordSet bool `yaml:"createRecordSet"`
RecordSetTTL int `yaml:"recordSetTTL"`
HostedZone string `yaml:"hostedZone"`
HostedZoneID string `yaml:"hostedZoneId"`
StackTags map[string]string `yaml:"stackTags"`
UseCalico bool `yaml:"useCalico"`
Subnets []Subnet `yaml:"subnets"`
ClusterName string `yaml:"clusterName,omitempty"`
ExternalDNSName string `yaml:"externalDNSName,omitempty"`
KeyName string `yaml:"keyName,omitempty"`
Region string `yaml:"region,omitempty"`
AvailabilityZone string `yaml:"availabilityZone,omitempty"`
ReleaseChannel string `yaml:"releaseChannel,omitempty"`
ControllerInstanceType string `yaml:"controllerInstanceType,omitempty"`
ControllerRootVolumeSize int `yaml:"controllerRootVolumeSize,omitempty"`
WorkerCount int `yaml:"workerCount,omitempty"`
WorkerInstanceType string `yaml:"workerInstanceType,omitempty"`
WorkerRootVolumeSize int `yaml:"workerRootVolumeSize,omitempty"`
WorkerSpotPrice string `yaml:"workerSpotPrice,omitempty"`
VPCID string `yaml:"vpcId,omitempty"`
RouteTableID string `yaml:"routeTableId,omitempty"`
VPCCIDR string `yaml:"vpcCIDR,omitempty"`
InstanceCIDR string `yaml:"instanceCIDR,omitempty"`
ControllerIP string `yaml:"controllerIP,omitempty"`
PodCIDR string `yaml:"podCIDR,omitempty"`
ServiceCIDR string `yaml:"serviceCIDR,omitempty"`
DNSServiceIP string `yaml:"dnsServiceIP,omitempty"`
K8sVer string `yaml:"kubernetesVersion,omitempty"`
HyperkubeImageRepo string `yaml:"hyperkubeImageRepo,omitempty"`
KMSKeyARN string `yaml:"kmsKeyArn,omitempty"`
CreateRecordSet bool `yaml:"createRecordSet,omitempty"`
RecordSetTTL int `yaml:"recordSetTTL,omitempty"`
HostedZone string `yaml:"hostedZone,omitempty"`
HostedZoneID string `yaml:"hostedZoneId,omitempty"`
StackTags map[string]string `yaml:"stackTags,omitempty"`
UseCalico bool `yaml:"useCalico,omitempty"`
Subnets []Subnet `yaml:"subnets,omitempty"`
}

type Subnet struct {
AvailabilityZone string `yaml:"availabilityZone"`
InstanceCIDR string `yaml:"instanceCIDR"`
AvailabilityZone string `yaml:"availabilityZone,omitempty"`
InstanceCIDR string `yaml:"instanceCIDR,omitempty"`
}

const (
Expand Down Expand Up @@ -570,10 +570,7 @@ func (c *Cluster) ValidateExistingVPC(existingVPCCIDR string, existingSubnetCIDR
)
}
}
_, instanceNet, err := net.ParseCIDR(c.InstanceCIDR)
if err != nil {
return fmt.Errorf("error parsing instances cidr %s : %v", c.InstanceCIDR, err)
}

_, vpcNet, err := net.ParseCIDR(c.VPCCIDR)
if err != nil {
return fmt.Errorf("error parsing vpc cidr %s: %v", c.VPCCIDR, err)
Expand All @@ -588,14 +585,23 @@ func (c *Cluster) ValidateExistingVPC(existingVPCCIDR string, existingSubnetCIDR
)
}

//Loop through all existing subnets in the VPC and look for conflicting CIDRS
for _, existingSubnet := range existingSubnets {
if cidrOverlap(instanceNet, existingSubnet) {
return fmt.Errorf(
"instance cidr (%s) conflicts with existing subnet cidr=%s",
instanceNet,
existingSubnet,
)
// Loop through all subnets
// Note: legacy instanceCIDR/availabilityZone stuff has already been marshalled into this format
for _, subnet := range c.Subnets {
_, instanceNet, err := net.ParseCIDR(subnet.InstanceCIDR)
if err != nil {
return fmt.Errorf("error parsing instances cidr %s : %v", c.InstanceCIDR, err)
}

//Loop through all existing subnets in the VPC and look for conflicting CIDRS
for _, existingSubnet := range existingSubnets {
if cidrOverlap(instanceNet, existingSubnet) {
return fmt.Errorf(
"instance cidr (%s) conflicts with existing subnet cidr=%s",
instanceNet,
existingSubnet,
)
}
}
}

Expand Down