Merge pull request #9189 from justinsb/fix_9123

Avoid nil-pointer dereference panics in AWS
This commit is contained in:
Brian Grant 2015-06-03 12:34:34 -07:00
commit d6d52b41c6

View File

@ -152,7 +152,7 @@ type ec2InstanceFilter struct {
// True if the passed instance matches the filter // True if the passed instance matches the filter
func (f *ec2InstanceFilter) Matches(instance *ec2.Instance) bool { func (f *ec2InstanceFilter) Matches(instance *ec2.Instance) bool {
if f.PrivateDNSName != "" && *instance.PrivateDNSName != f.PrivateDNSName { if f.PrivateDNSName != "" && orEmpty(instance.PrivateDNSName) != f.PrivateDNSName {
return false return false
} }
return true return true
@ -549,11 +549,11 @@ func (aws *AWSCloud) NodeAddresses(name string) ([]api.NodeAddress, error) {
addresses := []api.NodeAddress{} addresses := []api.NodeAddress{}
if *instance.PrivateIPAddress != "" { if !isNilOrEmpty(instance.PrivateIPAddress) {
ipAddress := *instance.PrivateIPAddress ipAddress := *instance.PrivateIPAddress
ip := net.ParseIP(ipAddress) ip := net.ParseIP(ipAddress)
if ip == nil { if ip == nil {
return nil, fmt.Errorf("EC2 instance had invalid private address: %s (%s)", *instance.InstanceID, ipAddress) return nil, fmt.Errorf("EC2 instance had invalid private address: %s (%s)", orEmpty(instance.InstanceID), ipAddress)
} }
addresses = append(addresses, api.NodeAddress{Type: api.NodeInternalIP, Address: ip.String()}) addresses = append(addresses, api.NodeAddress{Type: api.NodeInternalIP, Address: ip.String()})
@ -562,11 +562,11 @@ func (aws *AWSCloud) NodeAddresses(name string) ([]api.NodeAddress, error) {
} }
// TODO: Other IP addresses (multiple ips)? // TODO: Other IP addresses (multiple ips)?
if *instance.PublicIPAddress != "" { if !isNilOrEmpty(instance.PublicIPAddress) {
ipAddress := *instance.PublicIPAddress ipAddress := *instance.PublicIPAddress
ip := net.ParseIP(ipAddress) ip := net.ParseIP(ipAddress)
if ip == nil { if ip == nil {
return nil, fmt.Errorf("EC2 instance had invalid public address: %s (%s)", *instance.InstanceID, ipAddress) return nil, fmt.Errorf("EC2 instance had invalid public address: %s (%s)", orEmpty(instance.InstanceID), ipAddress)
} }
addresses = append(addresses, api.NodeAddress{Type: api.NodeExternalIP, Address: ip.String()}) addresses = append(addresses, api.NodeAddress{Type: api.NodeExternalIP, Address: ip.String()})
} }
@ -580,7 +580,7 @@ func (aws *AWSCloud) ExternalID(name string) (string, error) {
if err != nil { if err != nil {
return "", err return "", err
} }
return *inst.InstanceID, nil return orEmpty(inst.InstanceID), nil
} }
// InstanceID returns the cloud provider ID of the specified instance. // InstanceID returns the cloud provider ID of the specified instance.
@ -591,7 +591,7 @@ func (aws *AWSCloud) InstanceID(name string) (string, error) {
} }
// In the future it is possible to also return an endpoint as: // In the future it is possible to also return an endpoint as:
// <endpoint>/<zone>/<instanceid> // <endpoint>/<zone>/<instanceid>
return "/" + *inst.Placement.AvailabilityZone + "/" + *inst.InstanceID, nil return "/" + orEmpty(inst.Placement.AvailabilityZone) + "/" + orEmpty(inst.InstanceID), nil
} }
// Return the instances matching the relevant private dns name. // Return the instances matching the relevant private dns name.
@ -611,7 +611,7 @@ func (s *AWSCloud) getInstanceByDnsName(name string) (*ec2.Instance, error) {
continue continue
} }
if *instance.PrivateDNSName != name { if orEmpty(instance.PrivateDNSName) != name {
// TODO: Should we warn here? - the filter should have caught this // TODO: Should we warn here? - the filter should have caught this
// (this will happen in the tests if they don't fully mock the EC2 API) // (this will happen in the tests if they don't fully mock the EC2 API)
continue continue
@ -632,14 +632,18 @@ func (s *AWSCloud) getInstanceByDnsName(name string) (*ec2.Instance, error) {
// Check if the instance is alive (running or pending) // Check if the instance is alive (running or pending)
// We typically ignore instances that are not alive // We typically ignore instances that are not alive
func isAlive(instance *ec2.Instance) bool { func isAlive(instance *ec2.Instance) bool {
state := *instance.State if instance.State == nil {
switch *state.Name { glog.Warning("Instance state was unexpectedly nil: ", instance)
return false
}
stateName := orEmpty(instance.State.Name)
switch stateName {
case "shutting-down", "terminated", "stopping", "stopped": case "shutting-down", "terminated", "stopping", "stopped":
return false return false
case "pending", "running": case "pending", "running":
return true return true
default: default:
glog.Errorf("unknown EC2 instance state: %s", *instance.State) glog.Errorf("unknown EC2 instance state: %s", stateName)
return false return false
} }
} }
@ -668,26 +672,26 @@ func (aws *AWSCloud) getInstancesByRegex(regex string) ([]string, error) {
for _, instance := range instances { for _, instance := range instances {
// TODO: Push filtering down into EC2 API filter? // TODO: Push filtering down into EC2 API filter?
if !isAlive(instance) { if !isAlive(instance) {
glog.V(2).Infof("skipping EC2 instance (%s): %s",
*instance.State.Name, *instance.InstanceID)
continue continue
} }
// Only return fully-ready instances when listing instances // Only return fully-ready instances when listing instances
// (vs a query by name, where we will return it if we find it) // (vs a query by name, where we will return it if we find it)
if *instance.State.Name == "pending" { if orEmpty(instance.State.Name) == "pending" {
glog.V(2).Infof("skipping EC2 instance (pending): %s", *instance.InstanceID) glog.V(2).Infof("skipping EC2 instance (pending): %s", *instance.InstanceID)
continue continue
} }
if *instance.PrivateDNSName == "" {
privateDNSName := orEmpty(instance.PrivateDNSName)
if privateDNSName == "" {
glog.V(2).Infof("skipping EC2 instance (no PrivateDNSName): %s", glog.V(2).Infof("skipping EC2 instance (no PrivateDNSName): %s",
*instance.InstanceID) orEmpty(instance.InstanceID))
continue continue
} }
for _, tag := range instance.Tags { for _, tag := range instance.Tags {
if *tag.Key == "Name" && re.MatchString(*tag.Value) { if orEmpty(tag.Key) == "Name" && re.MatchString(orEmpty(tag.Value)) {
matchingInstances = append(matchingInstances, *instance.PrivateDNSName) matchingInstances = append(matchingInstances, privateDNSName)
break break
} }
} }
@ -709,7 +713,7 @@ func (aws *AWSCloud) GetNodeResources(name string) (*api.NodeResources, error) {
return nil, err return nil, err
} }
resources, err := getResourcesByInstanceType(*instance.InstanceType) resources, err := getResourcesByInstanceType(orEmpty(instance.InstanceType))
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -950,7 +954,7 @@ func (self *awsInstance) assignMountDevice(volumeID string) (mountDevice string,
} }
deviceMappings := map[string]string{} deviceMappings := map[string]string{}
for _, blockDevice := range info.BlockDeviceMappings { for _, blockDevice := range info.BlockDeviceMappings {
deviceMappings[*blockDevice.DeviceName] = *blockDevice.EBS.VolumeID deviceMappings[orEmpty(blockDevice.DeviceName)] = orEmpty(blockDevice.EBS.VolumeID)
} }
self.deviceMappings = deviceMappings self.deviceMappings = deviceMappings
} }
@ -1077,7 +1081,12 @@ func (self *awsDisk) waitForAttachmentStatus(status string) error {
if attachmentStatus != "" { if attachmentStatus != "" {
glog.Warning("Found multiple attachments: ", info) glog.Warning("Found multiple attachments: ", info)
} }
attachmentStatus = *attachment.State if attachment.State != nil {
attachmentStatus = *attachment.State
} else {
// Shouldn't happen, but don't panic...
glog.Warning("Ignoring nil attachment state: ", attachment)
}
} }
if attachmentStatus == "" { if attachmentStatus == "" {
attachmentStatus = "detached" attachmentStatus = "detached"
@ -1144,7 +1153,7 @@ func (aws *AWSCloud) getAwsInstance(instanceName string) (*awsInstance, error) {
return nil, fmt.Errorf("error finding instance: %v", err) return nil, fmt.Errorf("error finding instance: %v", err)
} }
awsInstance = newAWSInstance(aws.ec2, *instance.InstanceID) awsInstance = newAWSInstance(aws.ec2, orEmpty(instance.InstanceID))
} }
return awsInstance, nil return awsInstance, nil
@ -1248,10 +1257,10 @@ func (aws *AWSCloud) CreateVolume(volumeOptions *VolumeOptions) (string, error)
return "", err return "", err
} }
az := response.AvailabilityZone az := orEmpty(response.AvailabilityZone)
awsID := response.VolumeID awsID := orEmpty(response.VolumeID)
volumeName := "aws://" + *az + "/" + *awsID volumeName := "aws://" + az + "/" + awsID
return volumeName, nil return volumeName, nil
} }