maxmustermann/pulumi
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Respond to CR feedback on PR #186

Browse source 
This change responds to some great feedback from @lukehoban on
https://github.com/pulumi/lumi/pull/186.  Namely:

* Relax many assertions in the providers.  A failure here is pretty
  bad because it takes down the entire provider due to fail-fast.
  That said, I'd rather fail in response to a bug than let it go
  silently.  Nevertheless, a few of them were candidates for dynamic
  failures.

* Use `aws.StringValue` and friends in more places.

* Remove a superfluous waitForTableState in DynamoDB.

* Fix some erroneous references to `Read` in some `Get` comments.
This commit is contained in:
joeduffy 2017-06-01 13:25:49 -07:00
parent b07056ab10
commit fff4a9e8af
8 changed files with 72 additions and 64 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

45
lib/aws/provider/dynamodb/table.go
View file

@ -23,6 +23,7 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
awsdynamodb "github.com/aws/aws-sdk-go/service/dynamodb"
"github.com/pkg/errors"
"github.com/pulumi/lumi/pkg/resource"
"github.com/pulumi/lumi/pkg/util/contract"
"github.com/pulumi/lumi/pkg/util/mapper"
@ -228,10 +229,9 @@ func (p *tableProvider) Create(ctx context.Context, obj *dynamodb.Table) (resour
var arn string
if resp, err := p.ctx.DynamoDB().CreateTable(create); err != nil {
return "", err
} else if resp == nil || resp.TableDescription == nil || resp.TableDescription.TableArn == nil {
return "", errors.New("DynamoDB table created, but AWS did not return an ARN for it")
} else {
contract.Assert(resp != nil)
contract.Assert(resp.TableDescription != nil)
contract.Assert(resp.TableDescription.TableArn != nil)
arn = *resp.TableDescription.TableArn
}
@ -255,33 +255,39 @@ func (p *tableProvider) Get(ctx context.Context, id resource.ID) (*dynamodb.Tabl
resp, err := p.ctx.DynamoDB().DescribeTable(&awsdynamodb.DescribeTableInput{TableName: aws.String(name)})
if err != nil {
return nil, err
} else if resp == nil || resp.Table == nil {
return nil, errors.New("DynamoDB query returned an empty response or table")
}
// The object was found, we need to reverse map a bunch of properties into the structure form.
contract.Assert(resp != nil)
contract.Assert(resp.Table != nil)
tab := resp.Table
var attributes []dynamodb.Attribute
for _, attr := range tab.AttributeDefinitions {
attributes = append(attributes, dynamodb.Attribute{
Name: *attr.AttributeName,
Type: dynamodb.AttributeType(*attr.AttributeType),
Name: aws.StringValue(attr.AttributeName),
Type: dynamodb.AttributeType(aws.StringValue(attr.AttributeType)),
})
}
hashKey, rangeKey := getHashRangeKeys(tab.KeySchema)
if hashKey == nil {
return nil, errors.New("Missing hash key in table schema")
}
var gsis *[]dynamodb.GlobalSecondaryIndex
if len(tab.GlobalSecondaryIndexes) > 0 {
var gis []dynamodb.GlobalSecondaryIndex
for _, gsid := range tab.GlobalSecondaryIndexes {
hk, rk := getHashRangeKeys(gsid.KeySchema)
if hk == nil {
return nil, errors.New("Missing hash key in table global secondary index")
}
gis = append(gis, dynamodb.GlobalSecondaryIndex{
IndexName: *gsid.IndexName,
HashKey: hk,
ReadCapacity: float64(*gsid.ProvisionedThroughput.ReadCapacityUnits),
WriteCapacity: float64(*gsid.ProvisionedThroughput.WriteCapacityUnits),
IndexName: aws.StringValue(gsid.IndexName),
HashKey: *hk,
ReadCapacity: float64(aws.Int64Value(gsid.ProvisionedThroughput.ReadCapacityUnits)),
WriteCapacity: float64(aws.Int64Value(gsid.ProvisionedThroughput.WriteCapacityUnits)),
RangeKey: rk,
NonKeyAttributes: aws.StringValueSlice(gsid.Projection.NonKeyAttributes),
ProjectionType: dynamodb.ProjectionType(*gsid.Projection.ProjectionType),
@ -291,21 +297,21 @@ func (p *tableProvider) Get(ctx context.Context, id resource.ID) (*dynamodb.Tabl
}
return &dynamodb.Table{
HashKey: hashKey,
HashKey: *hashKey,
Attributes: attributes,
ReadCapacity: float64(*tab.ProvisionedThroughput.ReadCapacityUnits),
WriteCapacity: float64(*tab.ProvisionedThroughput.WriteCapacityUnits),
ReadCapacity: float64(aws.Int64Value(tab.ProvisionedThroughput.ReadCapacityUnits)),
WriteCapacity: float64(aws.Int64Value(tab.ProvisionedThroughput.WriteCapacityUnits)),
RangeKey: rangeKey,
TableName: tab.TableName,
GlobalSecondaryIndexes: gsis,
}, nil
}
func getHashRangeKeys(schema []*awsdynamodb.KeySchemaElement) (string, *string) {
func getHashRangeKeys(schema []*awsdynamodb.KeySchemaElement) (*string, *string) {
var hashKey *string
var rangeKey *string
for _, elem := range schema {
switch *elem.KeyType {
switch aws.StringValue(elem.KeyType) {
case hashKeyAttribute:
hashKey = elem.AttributeName
case rangeKeyAttribute:
@ -314,8 +320,7 @@ func getHashRangeKeys(schema []*awsdynamodb.KeySchemaElement) (string, *string)
contract.Failf("Unexpected key schema attribute type: %v", *elem.KeyType)
}
}
contract.Assertf(hashKey != nil, "Expected to discover a hash partition key")
return *hashKey, rangeKey
return hashKey, rangeKey
}
// InspectChange checks what impacts a hypothetical update will have on the resource's properties.
@ -455,10 +460,6 @@ func (p *tableProvider) Update(ctx context.Context, id resource.ID,
return err
}
}
if err := p.waitForTableState(name, true); err != nil {
return err
}
}
return nil
}

24
lib/aws/provider/ec2/instance.go
View file

@ -103,15 +103,15 @@ func (p *instanceProvider) Create(ctx context.Context, obj *ec2.Instance) (resou
result, err := p.ctx.EC2().RunInstances(create)
if err != nil {
return "", err
} else if result == nil || len(result.Instances) == 0 {
return "", errors.New("EC2 instance created, but AWS did not return an instance ID for it")
}
// Get the unique ID from the created instance.
contract.Assert(result != nil)
contract.Assert(len(result.Instances) == 1)
inst := result.Instances[0]
contract.Assert(inst != nil)
contract.Assert(inst.InstanceId != nil)
id := *inst.InstanceId
contract.Assert(result.Instances[0] != nil)
contract.Assert(result.Instances[0].InstanceId != nil)
id := aws.StringValue(result.Instances[0].InstanceId)
// Before returning that all is okay, wait for the instance to reach the running state.
fmt.Fprintf(os.Stdout, "EC2 instance '%v' created; now waiting for it to become 'running'\n", id)
@ -144,26 +144,28 @@ func (p *instanceProvider) Get(ctx context.Context, id resource.ID) (*ec2.Instan
// If we are here, we know that there is a reservation that matched; read its fields and populate the object.
contract.Assert(len(resp.Reservations) == 1)
resv := resp.Reservations[0]
contract.Assert(resp.Reservations[0] != nil)
contract.Assert(resp.Reservations[0].Instances != nil)
contract.Assert(len(resp.Reservations[0].Instances) == 1)
inst := resv.Instances[0]
inst := resp.Reservations[0].Instances[0]
var secgrpIDs *[]resource.ID
if len(inst.SecurityGroups) > 0 {
var ids []resource.ID
for _, group := range inst.SecurityGroups {
ids = append(ids, arn.NewEC2SecurityGroupID(idarn.Region, idarn.AccountID, *group.GroupId))
ids = append(ids,
arn.NewEC2SecurityGroupID(idarn.Region, idarn.AccountID, aws.StringValue(group.GroupId)))
}
secgrpIDs = &ids
}
instanceType := ec2.InstanceType(*inst.InstanceType)
instanceType := ec2.InstanceType(aws.StringValue(inst.InstanceType))
return &ec2.Instance{
ImageID: *inst.ImageId,
ImageID: aws.StringValue(inst.ImageId),
InstanceType: &instanceType,
SecurityGroups: secgrpIDs,
KeyName: inst.KeyName,
AvailabilityZone: *inst.Placement.AvailabilityZone,
AvailabilityZone: aws.StringValue(inst.Placement.AvailabilityZone),
PrivateDNSName: inst.PrivateDnsName,
PublicDNSName: inst.PublicDnsName,
PrivateIP: inst.PrivateIpAddress,

15
lib/aws/provider/ec2/security_group.go
View file

@ -22,6 +22,7 @@ import (
"github.com/aws/aws-sdk-go/aws"
awsec2 "github.com/aws/aws-sdk-go/service/ec2"
"github.com/pkg/errors"
"github.com/pulumi/lumi/pkg/resource"
"github.com/pulumi/lumi/pkg/util/contract"
"github.com/pulumi/lumi/pkg/util/convutil"
@ -94,10 +95,11 @@ func (p *sgProvider) Create(ctx context.Context, obj *ec2.SecurityGroup) (resour
result, err := p.ctx.EC2().CreateSecurityGroup(create)
if err != nil {
return "", err
} else if result == nil || result.GroupId == nil {
return "", errors.New("EC2 security group created, but AWS did not return an ID for it")
}
contract.Assert(result != nil)
contract.Assert(result.GroupId != nil)
id := *result.GroupId
id := aws.StringValue(result.GroupId)
// Don't proceed until the security group exists.
fmt.Printf("EC2 security group created: %v; waiting for it to become active\n", id)
@ -175,19 +177,18 @@ func (p *sgProvider) Get(ctx context.Context, id resource.ID) (*ec2.SecurityGrou
}
// If we found one, fetch all the requisite properties and store them on the output.
contract.Assert(len(resp.SecurityGroups) == 1)
grp := resp.SecurityGroups[0]
var vpcID *resource.ID
if grp.VpcId != nil {
vpc := arn.NewEC2VPCID(p.ctx.Region(), p.ctx.AccountID(), *grp.VpcId)
vpc := arn.NewEC2VPCID(p.ctx.Region(), p.ctx.AccountID(), aws.StringValue(grp.VpcId))
vpcID = &vpc
}
return &ec2.SecurityGroup{
GroupID: *grp.GroupId,
GroupID: aws.StringValue(grp.GroupId),
GroupName: grp.GroupName,
GroupDescription: *grp.Description,
GroupDescription: aws.StringValue(grp.Description),
VPC: vpcID,
SecurityGroupEgress: createSecurityGroupRulesFromIPPermissions(grp.IpPermissionsEgress),
SecurityGroupIngress: createSecurityGroupRulesFromIPPermissions(grp.IpPermissions),

7
lib/aws/provider/elasticbeanstalk/application.go
View file

@ -53,7 +53,8 @@ type applicationProvider struct {
}
// Check validates that the given property bag is valid for a resource of the given type.
func (p *applicationProvider) Check(ctx context.Context, obj *elasticbeanstalk.Application) ([]mapper.FieldError, error) {
func (p *applicationProvider) Check(ctx context.Context,
obj *elasticbeanstalk.Application) ([]mapper.FieldError, error) {
var failures []mapper.FieldError
if name := obj.ApplicationName; name != nil {
if len(*name) < minApplicationName {
@ -99,7 +100,7 @@ func (p *applicationProvider) Create(ctx context.Context, obj *elasticbeanstalk.
return arn.NewElasticBeanstalkApplicationID(p.ctx.Region(), p.ctx.AccountID(), name), nil
}
// Read reads the instance state identified by ID, returning a populated resource object, or an error if not found.
// Get reads the instance state identified by ID, returning a populated resource object, or an error if not found.
func (p *applicationProvider) Get(ctx context.Context, id resource.ID) (*elasticbeanstalk.Application, error) {
name, err := arn.ParseResourceName(id)
if err != nil {
@ -115,7 +116,7 @@ func (p *applicationProvider) Get(ctx context.Context, id resource.ID) (*elastic
}
contract.Assert(len(resp.Applications) == 1)
app := resp.Applications[0]
contract.Assert(*app.ApplicationName == name)
contract.Assert(aws.StringValue(app.ApplicationName) == name)
return &elasticbeanstalk.Application{
ApplicationName: app.ApplicationName,
Description: app.Description,

10
lib/aws/provider/elasticbeanstalk/applicationVersion.go
View file

@ -101,7 +101,7 @@ func (p *applicationVersionProvider) Create(ctx context.Context,
return arn.NewElasticBeanstalkApplicationVersionID(p.ctx.Region(), p.ctx.AccountID(), appname, versionLabel), nil
}
// Read reads the instance state identified by ID, returning a populated resource object, or an error if not found.
// Get reads the instance state identified by ID, returning a populated resource object, or an error if not found.
func (p *applicationVersionProvider) Get(ctx context.Context,
id resource.ID) (*elasticbeanstalk.ApplicationVersion, error) {
idarn, err := arn.ARN(id).Parse()
@ -122,15 +122,17 @@ func (p *applicationVersionProvider) Get(ctx context.Context,
}
contract.Assert(len(resp.ApplicationVersions) == 1)
vers := resp.ApplicationVersions[0]
contract.Assert(*vers.ApplicationName == appname)
contract.Assert(aws.StringValue(vers.ApplicationName) == appname)
appid := arn.NewElasticBeanstalkApplication(idarn.Region, idarn.AccountID, appname)
contract.Assert(*vers.VersionLabel == version)
contract.Assert(aws.StringValue(vers.VersionLabel) == version)
s3buck := aws.StringValue(vers.SourceBundle.S3Bucket)
s3key := aws.StringValue(vers.SourceBundle.S3Key)
return &elasticbeanstalk.ApplicationVersion{
VersionLabel: vers.VersionLabel,
Application: resource.ID(appid),
Description: vers.Description,
SourceBundle: arn.NewS3ObjectID(*vers.SourceBundle.S3Bucket, *vers.SourceBundle.S3Key),
SourceBundle: arn.NewS3ObjectID(s3buck, s3key),
}, nil
}

14
lib/aws/provider/elasticbeanstalk/environment.go
View file

@ -136,7 +136,7 @@ func (p *environmentProvider) Create(ctx context.Context, obj *elasticbeanstalk.
return arn.NewElasticBeanstalkEnvironmentID(p.ctx.Region(), p.ctx.AccountID(), appname, name), nil
}
// Read reads the instance state identified by ID, returning a populated resource object, or an error if not found.
// Get reads the instance state identified by ID, returning a populated resource object, or an error if not found.
func (p *environmentProvider) Get(ctx context.Context, id resource.ID) (*elasticbeanstalk.Environment, error) {
appname, envname, err := arn.ParseResourceNamePair(id)
if err != nil {
@ -153,9 +153,9 @@ func (p *environmentProvider) Get(ctx context.Context, id resource.ID) (*elastic
} else if envresp.Environments == nil || len(envresp.Environments) == 0 {
return nil, nil
}
contract.Assert(len(envresp.Environments) == 1)
// Successfully found the environment, now map all of its properties onto the struct.
contract.Assert(len(envresp.Environments) == 1)
env := envresp.Environments[0]
if env.CNAME != nil || env.TemplateName != nil || env.Tier != nil {
return nil, fmt.Errorf("Properties not yet supported: CNAMEPrefix, TemplateName, Tier")
@ -163,7 +163,7 @@ func (p *environmentProvider) Get(ctx context.Context, id resource.ID) (*elastic
var versionLabel *resource.ID
if env.VersionLabel != nil {
version := arn.NewElasticBeanstalkApplicationVersionID(
p.ctx.Region(), p.ctx.AccountID(), appname, *env.VersionLabel)
p.ctx.Region(), p.ctx.AccountID(), appname, aws.StringValue(env.VersionLabel))
versionLabel = &version
}
envobj := &elasticbeanstalk.Environment{
@ -172,7 +172,7 @@ func (p *environmentProvider) Get(ctx context.Context, id resource.ID) (*elastic
EnvironmentName: env.EnvironmentName,
SolutionStackName: env.SolutionStackName,
Version: versionLabel,
EndpointURL: *env.EndpointURL,
EndpointURL: aws.StringValue(env.EndpointURL),
}
// Next see if there are any configuration option settings and, if so, set them on the return.
@ -186,9 +186,9 @@ func (p *environmentProvider) Get(ctx context.Context, id resource.ID) (*elastic
for _, setting := range confresp.ConfigurationSettings {
for _, option := range setting.OptionSettings {
options = append(options, elasticbeanstalk.OptionSetting{
Namespace: *option.Namespace,
OptionName: *option.OptionName,
Value: *option.Value,
Namespace: aws.StringValue(option.Namespace),
OptionName: aws.StringValue(option.OptionName),
Value: aws.StringValue(option.Value),
})
}
}

4
lib/aws/provider/iam/role.go
View file

@ -146,7 +146,7 @@ func (p *roleProvider) Get(ctx context.Context, id resource.ID) (*iam.Role, erro
if len(getpols.AttachedPolicies) > 0 {
var policies []awscommon.ARN
for _, policy := range getpols.AttachedPolicies {
policies = append(policies, awscommon.ARN(*policy.PolicyArn))
policies = append(policies, awscommon.ARN(aws.StringValue(policy.PolicyArn)))
}
managedPolicies = &policies
}
@ -156,7 +156,7 @@ func (p *roleProvider) Get(ctx context.Context, id resource.ID) (*iam.Role, erro
Path: role.Path,
RoleName: role.RoleName,
ManagedPolicyARNs: managedPolicies,
ARN: awscommon.ARN(*role.Arn),
ARN: awscommon.ARN(aws.StringValue(role.Arn)),
}, nil
}

17
lib/aws/provider/lambda/function.go
View file

@ -23,6 +23,7 @@ import (
"github.com/aws/aws-sdk-go/aws"
awslambda "github.com/aws/aws-sdk-go/service/lambda"
"github.com/pkg/errors"
"github.com/pulumi/lumi/pkg/resource"
"github.com/pulumi/lumi/pkg/util/contract"
"github.com/pulumi/lumi/pkg/util/convutil"
@ -153,9 +154,9 @@ func (p *funcProvider) Create(ctx context.Context, obj *lambda.Function) (resour
return false, nil // retry the condition.
}
return false, err
} else if resp == nil || resp.FunctionArn == nil {
return false, errors.New("Lambda Function created, but AWS did not respond with an ARN")
}
contract.Assert(resp != nil)
contract.Assert(resp.FunctionArn != nil)
arn = resource.ID(*resp.FunctionArn)
return true, nil
},
@ -177,7 +178,7 @@ func (p *funcProvider) Create(ctx context.Context, obj *lambda.Function) (resour
return arn, nil
}
// Read reads the instance state identified by ID, returning a populated resource object, or an error if not found.
// Get reads the instance state identified by ID, returning a populated resource object, or an error if not found.
func (p *funcProvider) Get(ctx context.Context, id resource.ID) (*lambda.Function, error) {
name, err := arn.ParseResourceName(id)
if err != nil {
@ -195,7 +196,7 @@ func (p *funcProvider) Get(ctx context.Context, id resource.ID) (*lambda.Functio
contract.Assert(funcresp != nil)
contract.Assert(funcresp.Code != nil)
// TODO: unclear if we need to consult RepositoryType.
code := resource.NewURIArchive(*funcresp.Code.Location)
code := resource.NewURIArchive(aws.StringValue(funcresp.Code.Location))
// Deserialize all configuration properties into prompt objects.
config := funcresp.Configuration
@ -207,11 +208,11 @@ func (p *funcProvider) Get(ctx context.Context, id resource.ID) (*lambda.Functio
}
return &lambda.Function{
ARN: awscommon.ARN(*config.FunctionArn),
ARN: awscommon.ARN(aws.StringValue(config.FunctionArn)),
Code: code,
Handler: *config.Handler,
Role: resource.ID(*config.Role),
Runtime: lambda.Runtime(*config.Runtime),
Handler: aws.StringValue(config.Handler),
Role: resource.ID(aws.StringValue(config.Role)),
Runtime: lambda.Runtime(aws.StringValue(config.Runtime)),
FunctionName: config.FunctionName,
Description: config.Description,
Environment: env,