Skip to content

Commit a556e55

Browse files
committed
VolumeAttachment: Handle unsupported attachment types.
If an unsupported attachment type is returned by the service, we should use the SDK's base interface to populate common fields. This is better than panicking if we don't know the type. This fixes github issue #477
1 parent 090f889 commit a556e55

File tree

2 files changed

+147
-46
lines changed

2 files changed

+147
-46
lines changed

provider/core_volume_attachments_data_source.go

Lines changed: 55 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package provider
44

55
import (
66
"context"
7+
"log"
78

89
"github.com/hashicorp/terraform/helper/schema"
910
oci_core "github.com/oracle/oci-go-sdk/core"
@@ -132,71 +133,79 @@ func (s *VolumeAttachmentsDataSourceCrud) SetData() {
132133
resources := []map[string]interface{}{}
133134

134135
for _, r := range s.Res.Items {
135-
iscsiVolumeAttachment, castOk := r.(oci_core.IScsiVolumeAttachment)
136-
if !castOk {
137-
panic("unexpected VolumeAttachment type on item.")
138-
}
136+
resources = append(resources, volumeAttachmentToMap(r))
137+
}
139138

140-
volumeAttachment := map[string]interface{}{
141-
"compartment_id": *r.GetCompartmentId(),
142-
"attachment_type": IScsiVolumeAttachmentDiscriminator,
143-
}
139+
if f, fOk := s.D.GetOkExists("filter"); fOk {
140+
resources = ApplyFilters(f.(*schema.Set), resources)
141+
}
144142

145-
if iscsiVolumeAttachment.AvailabilityDomain != nil {
146-
volumeAttachment["availability_domain"] = *iscsiVolumeAttachment.AvailabilityDomain
147-
}
143+
if err := s.D.Set("volume_attachments", resources); err != nil {
144+
panic(err)
145+
}
148146

149-
if iscsiVolumeAttachment.DisplayName != nil {
150-
volumeAttachment["display_name"] = *iscsiVolumeAttachment.DisplayName
151-
}
147+
return
148+
}
152149

153-
if iscsiVolumeAttachment.Id != nil {
154-
volumeAttachment["id"] = *iscsiVolumeAttachment.Id
155-
}
150+
func volumeAttachmentToMap(r oci_core.VolumeAttachment) map[string]interface{} {
151+
volumeAttachment := map[string]interface{}{
152+
"compartment_id": *r.GetCompartmentId(),
153+
}
156154

157-
if iscsiVolumeAttachment.InstanceId != nil {
158-
volumeAttachment["instance_id"] = *iscsiVolumeAttachment.InstanceId
159-
}
155+
if availabilityDomain := r.GetAvailabilityDomain(); availabilityDomain != nil {
156+
volumeAttachment["availability_domain"] = *availabilityDomain
157+
}
160158

161-
volumeAttachment["state"] = iscsiVolumeAttachment.LifecycleState
159+
if displayName := r.GetDisplayName(); displayName != nil {
160+
volumeAttachment["display_name"] = *displayName
161+
}
162162

163-
volumeAttachment["time_created"] = iscsiVolumeAttachment.TimeCreated.String()
163+
if id := r.GetId(); id != nil {
164+
volumeAttachment["id"] = *id
165+
}
164166

165-
if iscsiVolumeAttachment.VolumeId != nil {
166-
volumeAttachment["volume_id"] = *iscsiVolumeAttachment.VolumeId
167-
}
167+
if instanceId := r.GetInstanceId(); instanceId != nil {
168+
volumeAttachment["instance_id"] = *instanceId
169+
}
170+
171+
volumeAttachment["state"] = string(r.GetLifecycleState())
172+
173+
if timeCreated := r.GetTimeCreated(); timeCreated != nil {
174+
volumeAttachment["time_created"] = timeCreated.String()
175+
}
176+
177+
if volumeId := r.GetVolumeId(); volumeId != nil {
178+
volumeAttachment["volume_id"] = *volumeId
179+
}
180+
181+
switch typedValue := r.(type) {
182+
case oci_core.IScsiVolumeAttachment:
183+
volumeAttachment["attachment_type"] = IScsiVolumeAttachmentDiscriminator
168184

169185
// IScsiVolumeAttachment-specific fields:
170-
if iscsiVolumeAttachment.ChapSecret != nil {
171-
volumeAttachment["chap_secret"] = *iscsiVolumeAttachment.ChapSecret
186+
if typedValue.ChapSecret != nil {
187+
volumeAttachment["chap_secret"] = *typedValue.ChapSecret
172188
}
173189

174-
if iscsiVolumeAttachment.ChapUsername != nil {
175-
volumeAttachment["chap_username"] = *iscsiVolumeAttachment.ChapUsername
190+
if typedValue.ChapUsername != nil {
191+
volumeAttachment["chap_username"] = *typedValue.ChapUsername
176192
}
177193

178-
if iscsiVolumeAttachment.Ipv4 != nil {
179-
volumeAttachment["ipv4"] = *iscsiVolumeAttachment.Ipv4
194+
if typedValue.Ipv4 != nil {
195+
volumeAttachment["ipv4"] = *typedValue.Ipv4
180196
}
181197

182-
if iscsiVolumeAttachment.Iqn != nil {
183-
volumeAttachment["iqn"] = *iscsiVolumeAttachment.Iqn
198+
if typedValue.Iqn != nil {
199+
volumeAttachment["iqn"] = *typedValue.Iqn
184200
}
185201

186-
if iscsiVolumeAttachment.Port != nil {
187-
volumeAttachment["port"] = *iscsiVolumeAttachment.Port
202+
if typedValue.Port != nil {
203+
volumeAttachment["port"] = *typedValue.Port
188204
}
189-
190-
resources = append(resources, volumeAttachment)
205+
default:
206+
volumeAttachment["attachment_type"] = "Unknown"
207+
log.Printf("[WARNING] Retrieved a volume attachment of unknown type.")
191208
}
192209

193-
if f, fOk := s.D.GetOkExists("filter"); fOk {
194-
resources = ApplyFilters(f.(*schema.Set), resources)
195-
}
196-
197-
if err := s.D.Set("volume_attachments", resources); err != nil {
198-
panic(err)
199-
}
200-
201-
return
210+
return volumeAttachment
202211
}

provider/core_volume_attachments_data_source_test.go

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@ package provider
44

55
import (
66
"testing"
7+
"time"
8+
9+
common "github.com/oracle/oci-go-sdk/common"
10+
11+
oci_core "github.com/oracle/oci-go-sdk/core"
712

813
"github.com/hashicorp/terraform/helper/resource"
914
"github.com/hashicorp/terraform/terraform"
@@ -78,6 +83,93 @@ func (s *DatasourceCoreVolumeAttachmentTestSuite) TestAccDatasourceCoreVolumeAtt
7883
)
7984
}
8085

86+
type customVolumeAttachment struct {
87+
ad string
88+
compartmentId string
89+
id string
90+
instanceId string
91+
volumeId string
92+
displayName string
93+
timeCreated common.SDKTime
94+
state oci_core.VolumeAttachmentLifecycleStateEnum
95+
}
96+
97+
//GetAvailabilityDomain returns AvailabilityDomain
98+
func (m customVolumeAttachment) GetAvailabilityDomain() *string {
99+
return &m.ad
100+
}
101+
102+
//GetCompartmentId returns CompartmentId
103+
func (m customVolumeAttachment) GetCompartmentId() *string {
104+
return &m.compartmentId
105+
}
106+
107+
//GetId returns Id
108+
func (m customVolumeAttachment) GetId() *string {
109+
return &m.id
110+
}
111+
112+
//GetInstanceId returns InstanceId
113+
func (m customVolumeAttachment) GetInstanceId() *string {
114+
return &m.instanceId
115+
}
116+
117+
//GetLifecycleState returns LifecycleState
118+
func (m customVolumeAttachment) GetLifecycleState() oci_core.VolumeAttachmentLifecycleStateEnum {
119+
return m.state
120+
}
121+
122+
//GetTimeCreated returns TimeCreated
123+
func (m customVolumeAttachment) GetTimeCreated() *common.SDKTime {
124+
return &m.timeCreated
125+
}
126+
127+
//GetVolumeId returns VolumeId
128+
func (m customVolumeAttachment) GetVolumeId() *string {
129+
return &m.volumeId
130+
}
131+
132+
//GetDisplayName returns DisplayName
133+
func (m customVolumeAttachment) GetDisplayName() *string {
134+
return &m.displayName
135+
}
136+
137+
func checkExpectedValue(mapped map[string]interface{}, key string, expected string, t *testing.T) {
138+
if value := mapped[key].(string); value != expected {
139+
t.Errorf("Expected attachment to have type %s, but got %s", expected, value)
140+
}
141+
}
142+
143+
// This unit tests that any datasource result that implements the SDK's VolumeAttachment interface can
144+
// be converted to a map to be stored in Terraform.
145+
func TestUnitVolumeAttachmentToMap_unknownType(t *testing.T) {
146+
customAttachment := customVolumeAttachment{
147+
ad: "ad1",
148+
compartmentId: "compartment",
149+
id: "myId",
150+
instanceId: "myInstanceId",
151+
volumeId: "myVolumeId",
152+
displayName: "myDisplayName",
153+
timeCreated: common.SDKTime{time.Now()},
154+
state: oci_core.VolumeAttachmentLifecycleStateDetached,
155+
}
156+
157+
result := volumeAttachmentToMap(customAttachment)
158+
159+
// Check that type is set to Unknown for unsupported VolumeAttachment types
160+
checkExpectedValue(result, "attachment_type", "Unknown", t)
161+
162+
// Check that all VolumeAttachment base class attributes are set
163+
checkExpectedValue(result, "availability_domain", customAttachment.ad, t)
164+
checkExpectedValue(result, "compartment_id", customAttachment.compartmentId, t)
165+
checkExpectedValue(result, "id", customAttachment.id, t)
166+
checkExpectedValue(result, "instance_id", customAttachment.instanceId, t)
167+
checkExpectedValue(result, "volume_id", customAttachment.volumeId, t)
168+
checkExpectedValue(result, "display_name", customAttachment.displayName, t)
169+
checkExpectedValue(result, "time_created", customAttachment.timeCreated.String(), t)
170+
checkExpectedValue(result, "state", string(customAttachment.state), t)
171+
}
172+
81173
func TestDatasourceCoreVolumeAttachmentTestSuite(t *testing.T) {
82174
suite.Run(t, new(DatasourceCoreVolumeAttachmentTestSuite))
83175
}

0 commit comments

Comments
 (0)