Skip to content

Commit 560898f

Browse files
committed
subnet: fix security_list_ids ordering
`baremetal_core_subnet.security_list_ids` with multiple items causes destroy/add on every `terraform apply`, even with no changes. API state does not respect item ordering, so config and API state do not match (except when items happen to have exactly the same sorting as used by API state). Because local and remote state do not match, terraform will attempt to change remote state to match on every `apply`. Change `security_list_ids` from `TypeList` to `TypeSet`, so terraform only considers membership, not ordering. Fixes #44
1 parent ee2d09c commit 560898f

File tree

3 files changed

+49
-35
lines changed

3 files changed

+49
-35
lines changed

core/subnet_resource.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
33
package core
44

55
import (
6+
"github.com/hashicorp/terraform/helper/schema"
67
"github.com/oracle/terraform-provider-baremetal/client"
78
"github.com/oracle/terraform-provider-baremetal/crud"
8-
"github.com/hashicorp/terraform/helper/schema"
99
)
1010

1111
func SubnetResource() *schema.Resource {
@@ -41,9 +41,10 @@ func SubnetResource() *schema.Resource {
4141
ForceNew: true,
4242
},
4343
"security_list_ids": {
44-
Type: schema.TypeList,
44+
Type: schema.TypeSet,
4545
Required: true,
4646
ForceNew: true,
47+
Set: schema.HashString,
4748
Elem: &schema.Schema{
4849
Type: schema.TypeString,
4950
},
@@ -54,11 +55,11 @@ func SubnetResource() *schema.Resource {
5455
Computed: true,
5556
},
5657
"dhcp_options_id": {
57-
Type: schema.TypeString,
58+
Type: schema.TypeString,
5859
Optional: true,
5960
},
6061
"dns_label": {
61-
Type: schema.TypeString,
62+
Type: schema.TypeString,
6263
Optional: true,
6364
},
6465
"id": {

core/subnet_resource_crud.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"time"
77

88
"github.com/MustWin/baremetal-sdk-go"
9+
"github.com/hashicorp/terraform/helper/schema"
910
"github.com/oracle/terraform-provider-baremetal/crud"
1011
)
1112

@@ -57,7 +58,7 @@ func (s *SubnetResourceCrud) Create() (e error) {
5758

5859
if rawSecurityListIDs, ok := s.D.GetOk("security_list_ids"); ok {
5960
securityListIDs := []string{}
60-
for _, val := range rawSecurityListIDs.([]interface{}) {
61+
for _, val := range rawSecurityListIDs.(*schema.Set).List() {
6162
securityListIDs = append(securityListIDs, val.(string))
6263
}
6364
opts.SecurityListIDs = securityListIDs
@@ -104,7 +105,7 @@ func (s *SubnetResourceCrud) SetData() {
104105
s.D.Set("dns_label", s.Resource.DNSLabel)
105106
s.D.Set("route_table_id", s.Resource.RouteTableID)
106107
s.D.Set("vcn_id", s.Resource.VcnID)
107-
s.D.Set("security_list_ids", s.Resource.SecurityListIDs)
108+
s.D.Set("security_list_ids", makeSetFromStrings(s.Resource.SecurityListIDs))
108109
s.D.Set("state", s.Resource.State)
109110
s.D.Set("time_created", s.Resource.TimeCreated.String())
110111
s.D.Set("virtual_router_ip", s.Resource.VirtualRouterIP)
@@ -114,3 +115,13 @@ func (s *SubnetResourceCrud) SetData() {
114115
func (s *SubnetResourceCrud) Delete() (e error) {
115116
return s.Client.DeleteSubnet(s.D.Id(), nil)
116117
}
118+
119+
// makeSetFromStrings encodes an []string into a
120+
// *schema.Set in the appropriate structure for the schema
121+
func makeSetFromStrings(ss []string) *schema.Set {
122+
st := &schema.Set{F: schema.HashString}
123+
for _, s := range ss {
124+
st.Add(s)
125+
}
126+
return st
127+
}

core_subnet_test.go

Lines changed: 31 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ import (
77
"time"
88

99
"github.com/MustWin/baremetal-sdk-go"
10-
"github.com/oracle/terraform-provider-baremetal/client/mocks"
1110
"github.com/hashicorp/terraform/helper/resource"
1211
"github.com/hashicorp/terraform/helper/schema"
1312
"github.com/hashicorp/terraform/terraform"
13+
"github.com/oracle/terraform-provider-baremetal/client/mocks"
1414

1515
"github.com/stretchr/testify/suite"
1616
)
@@ -43,15 +43,15 @@ func (s *ResourceCoreSubnetTestSuite) SetupTest() {
4343
s.TimeCreated = baremetal.Time{Time: time.Now()}
4444

4545
s.Config = `
46-
resource "baremetal_core_subnet" "t" {
47-
availability_domain = "availabilitydomainid"
48-
compartment_id = "compartmentid"
49-
display_name = "display_name"
50-
cidr_block = "10.10.10.0/24"
51-
route_table_id = "routetableid"
52-
vcn_id = "vcnid"
53-
security_list_ids = ["slid1", "slid2"]
54-
}
46+
resource "baremetal_core_subnet" "t" {
47+
availability_domain = "availabilitydomainid"
48+
compartment_id = "compartmentid"
49+
display_name = "display_name"
50+
cidr_block = "10.10.10.0/24"
51+
route_table_id = "routetableid"
52+
vcn_id = "vcnid"
53+
security_list_ids = ["slid1", "slid2"]
54+
}
5555
`
5656

5757
s.Config += testProviderConfig
@@ -65,8 +65,9 @@ func (s *ResourceCoreSubnetTestSuite) SetupTest() {
6565
ID: "id",
6666
RouteTableID: "routetableid",
6767
SecurityListIDs: []string{
68-
"slid1",
68+
// Note: sorted by schema.HashString
6969
"slid2",
70+
"slid1",
7071
},
7172
State: baremetal.ResourceAvailable,
7273
TimeCreated: baremetal.Time{
@@ -122,14 +123,14 @@ func (s *ResourceCoreSubnetTestSuite) TestCreateResourceCoreSubnetWithoutDisplay
122123
s.Client.On("GetSubnet", "id").Return(s.DeletedRes, nil).Times(2)
123124

124125
s.Config = `
125-
resource "baremetal_core_subnet" "t" {
126-
availability_domain = "availabilitydomainid"
127-
compartment_id = "compartmentid"
128-
cidr_block = "10.10.10.0/24"
129-
route_table_id = "routetableid"
130-
vcn_id = "vcnid"
131-
security_list_ids = ["slid1", "slid2"]
132-
}
126+
resource "baremetal_core_subnet" "t" {
127+
availability_domain = "availabilitydomainid"
128+
compartment_id = "compartmentid"
129+
cidr_block = "10.10.10.0/24"
130+
route_table_id = "routetableid"
131+
vcn_id = "vcnid"
132+
security_list_ids = ["slid1", "slid2"]
133+
}
133134
`
134135

135136
s.Config += testProviderConfig
@@ -162,15 +163,15 @@ func (s *ResourceCoreSubnetTestSuite) TestCreateResourceCoreSubnetWithoutDisplay
162163

163164
func (s ResourceCoreSubnetTestSuite) TestUpdateCompartmentIDForcesNewSubnet() {
164165
config := `
165-
resource "baremetal_core_subnet" "t" {
166-
availability_domain = "availabilitydomainid"
167-
compartment_id = "new_compartmentid"
168-
display_name = "display_name"
169-
cidr_block = "10.10.10.0/24"
170-
route_table_id = "routetableid"
171-
vcn_id = "vcnid"
172-
security_list_ids = ["slid1", "slid2"]
173-
}
166+
resource "baremetal_core_subnet" "t" {
167+
availability_domain = "availabilitydomainid"
168+
compartment_id = "new_compartmentid"
169+
display_name = "display_name"
170+
cidr_block = "10.10.10.0/24"
171+
route_table_id = "routetableid"
172+
vcn_id = "vcnid"
173+
security_list_ids = ["slid1", "slid2"]
174+
}
174175
`
175176

176177
config += testProviderConfig
@@ -183,8 +184,9 @@ func (s ResourceCoreSubnetTestSuite) TestUpdateCompartmentIDForcesNewSubnet() {
183184
ID: "new_id",
184185
RouteTableID: "routetableid",
185186
SecurityListIDs: []string{
186-
"slid1",
187+
// Note: sorted by schema.HashString
187188
"slid2",
189+
"slid1",
188190
},
189191
State: baremetal.ResourceAvailable,
190192
TimeCreated: baremetal.Time{

0 commit comments

Comments
 (0)