From cfc15e030bc976f99e15a215dd624fed5739cf52 Mon Sep 17 00:00:00 2001 From: He Guimin Date: Tue, 6 Jun 2017 19:34:07 +0800 Subject: [PATCH] fix security group rules bug (#15114) --- .../alicloud/resource_alicloud_instance.go | 2 +- .../resource_alicloud_security_group_rule.go | 72 ++++++++++--------- .../alicloud/r/instance.html.markdown | 2 +- 3 files changed, 40 insertions(+), 36 deletions(-) diff --git a/builtin/providers/alicloud/resource_alicloud_instance.go b/builtin/providers/alicloud/resource_alicloud_instance.go index 31e03bac5..f6884ea83 100644 --- a/builtin/providers/alicloud/resource_alicloud_instance.go +++ b/builtin/providers/alicloud/resource_alicloud_instance.go @@ -42,7 +42,7 @@ func resourceAliyunInstance() *schema.Resource { "security_groups": &schema.Schema{ Type: schema.TypeSet, Elem: &schema.Schema{Type: schema.TypeString}, - Optional: true, + Required: true, }, "allocate_public_ip": &schema.Schema{ diff --git a/builtin/providers/alicloud/resource_alicloud_security_group_rule.go b/builtin/providers/alicloud/resource_alicloud_security_group_rule.go index 56e4de670..dd671879c 100644 --- a/builtin/providers/alicloud/resource_alicloud_security_group_rule.go +++ b/builtin/providers/alicloud/resource_alicloud_security_group_rule.go @@ -66,15 +66,17 @@ func resourceAliyunSecurityGroupRule() *schema.Resource { }, "cidr_ip": &schema.Schema{ - Type: schema.TypeString, - Optional: true, - ForceNew: true, + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ConflictsWith: []string{"source_security_group_id"}, }, "source_security_group_id": &schema.Schema{ - Type: schema.TypeString, - Optional: true, - ForceNew: true, + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ConflictsWith: []string{"cidr_ip"}, }, "source_group_owner_account": &schema.Schema{ @@ -248,21 +250,12 @@ func buildAliyunSecurityIngressArgs(d *schema.ResourceData, meta interface{}) (* args.Priority = v } - if v := d.Get("nic_type").(string); v != "" { - args.NicType = ecs.NicType(v) + if v := d.Get("cidr_ip").(string); v != "" { + args.SourceCidrIp = v } - cidrIp := d.Get("cidr_ip").(string) - sourceGroupId := d.Get("source_security_group_id").(string) - if err := checkCidrAndSourceGroupId(cidrIp, sourceGroupId); err != nil { - return nil, err - } - if cidrIp != "" { - args.SourceCidrIp = cidrIp - } - - if sourceGroupId != "" { - args.SourceGroupId = sourceGroupId + if v := d.Get("source_security_group_id").(string); v != "" { + args.SourceGroupId = v } if v := d.Get("source_group_owner_account").(string); v != "" { @@ -276,11 +269,21 @@ func buildAliyunSecurityIngressArgs(d *schema.ResourceData, meta interface{}) (* RegionId: getRegion(d, meta), } - _, err := conn.DescribeSecurityGroupAttribute(sgArgs) + group, err := conn.DescribeSecurityGroupAttribute(sgArgs) if err != nil { return nil, fmt.Errorf("Error get security group %s error: %#v", sgId, err) } + if v := d.Get("nic_type").(string); v != "" { + if (group != nil && group.VpcId != "") || args.SourceGroupId != "" { + if GroupRuleNicType(v) != GroupRuleIntranet { + return nil, fmt.Errorf("When security group in the vpc or authorizing permission for source security group, " + + "the nic_type must be 'intranet'.") + } + } + args.NicType = ecs.NicType(v) + } + args.SecurityGroupId = sgId return args, nil @@ -309,21 +312,12 @@ func buildAliyunSecurityEgressArgs(d *schema.ResourceData, meta interface{}) (*e args.Priority = v } - if v := d.Get("nic_type").(string); v != "" { - args.NicType = ecs.NicType(v) + if v := d.Get("cidr_ip").(string); v != "" { + args.DestCidrIp = v } - cidrIp := d.Get("cidr_ip").(string) - sourceGroupId := d.Get("source_security_group_id").(string) - if err := checkCidrAndSourceGroupId(cidrIp, sourceGroupId); err != nil { - return nil, err - } - if cidrIp != "" { - args.DestCidrIp = cidrIp - } - - if sourceGroupId != "" { - args.DestGroupId = sourceGroupId + if v := d.Get("source_security_group_id").(string); v != "" { + args.DestGroupId = v } if v := d.Get("source_group_owner_account").(string); v != "" { @@ -337,11 +331,21 @@ func buildAliyunSecurityEgressArgs(d *schema.ResourceData, meta interface{}) (*e RegionId: getRegion(d, meta), } - _, err := conn.DescribeSecurityGroupAttribute(sgArgs) + group, err := conn.DescribeSecurityGroupAttribute(sgArgs) if err != nil { return nil, fmt.Errorf("Error get security group %s error: %#v", sgId, err) } + if v := d.Get("nic_type").(string); v != "" { + if (group != nil && group.VpcId != "") || args.DestGroupId != "" { + if GroupRuleNicType(v) != GroupRuleIntranet { + return nil, fmt.Errorf("When security group in the vpc or authorizing permission for destination security group, " + + "the nic_type must be 'intranet'.") + } + } + args.NicType = ecs.NicType(v) + } + args.SecurityGroupId = sgId return args, nil diff --git a/website/source/docs/providers/alicloud/r/instance.html.markdown b/website/source/docs/providers/alicloud/r/instance.html.markdown index a1c68d467..a6fe7587c 100644 --- a/website/source/docs/providers/alicloud/r/instance.html.markdown +++ b/website/source/docs/providers/alicloud/r/instance.html.markdown @@ -57,7 +57,7 @@ The following arguments are supported: * `image_id` - (Required) The Image to use for the instance. ECS instance's image can be replaced via changing 'image_id'. * `instance_type` - (Required) The type of instance to start. * `io_optimized` - (Required) Valid values are `none`, `optimized`, If `optimized`, the launched ECS instance will be I/O optimized. -* `security_groups` - (Optional) A list of security group ids to associate with. +* `security_groups` - (Required) A list of security group ids to associate with. * `availability_zone` - (Optional) The Zone to start the instance in. * `instance_name` - (Optional) The name of the ECS. This instance_name can have a string of 2 to 128 characters, must contain only alphanumeric characters or hyphens, such as "-",".","_", and must not begin or end with a hyphen, and must not begin with http:// or https://. If not specified, Terraform will autogenerate a default name is `ECS-Instance`.