Skip to content

Commit a6cbbfa

Browse files
committed
fix: update parameter names from 'domainid' to 'domain_id' for consistency
add configured_max to capture intended max value of 0 or -1 and cleanup the related code
1 parent 0a72f69 commit a6cbbfa

3 files changed

Lines changed: 126 additions & 87 deletions

File tree

cloudstack/data_source_cloudstack_limits.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,9 @@ func dataSourceCloudStackLimits() *schema.Resource {
4747
"account": {
4848
Type: schema.TypeString,
4949
Optional: true,
50-
Description: "List resources by account. Must be used with the domainid parameter.",
50+
Description: "List resources by account. Must be used with the domain_id parameter.",
5151
},
52-
"domainid": {
52+
"domain_id": {
5353
Type: schema.TypeString,
5454
Optional: true,
5555
Description: "List only resources belonging to the domain specified.",
@@ -80,7 +80,7 @@ func dataSourceCloudStackLimits() *schema.Resource {
8080
Type: schema.TypeString,
8181
Computed: true,
8282
},
83-
"domainid": {
83+
"domain_id": {
8484
Type: schema.TypeString,
8585
Computed: true,
8686
},
@@ -123,7 +123,7 @@ func dataSourceCloudStackLimitsRead(d *schema.ResourceData, meta interface{}) er
123123
p.SetAccount(v.(string))
124124
}
125125

126-
if v, ok := d.GetOk("domainid"); ok {
126+
if v, ok := d.GetOk("domain_id"); ok {
127127
p.SetDomainid(v.(string))
128128
}
129129

@@ -196,7 +196,7 @@ func generateDataSourceID(d *schema.ResourceData) string {
196196
buf.WriteString(fmt.Sprintf("%s-", v.(string)))
197197
}
198198

199-
if v, ok := d.GetOk("domainid"); ok {
199+
if v, ok := d.GetOk("domain_id"); ok {
200200
buf.WriteString(fmt.Sprintf("%s-", v.(string)))
201201
}
202202

cloudstack/resource_cloudstack_limits.go

Lines changed: 98 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,9 @@ func resourceCloudStackLimits() *schema.Resource {
6969
Type: schema.TypeString,
7070
Optional: true,
7171
ForceNew: true,
72-
Description: "Update resource for a specified account. Must be used with the domainid parameter.",
72+
Description: "Update resource for a specified account. Must be used with the domain_id parameter.",
7373
},
74-
"domainid": {
74+
"domain_id": {
7575
Type: schema.TypeString,
7676
Optional: true,
7777
ForceNew: true,
@@ -82,6 +82,11 @@ func resourceCloudStackLimits() *schema.Resource {
8282
Optional: true,
8383
Description: "Maximum resource limit. Use -1 for unlimited resource limit. A value of 0 means zero resources are allowed, though the CloudStack API may return -1 for a limit set to 0.",
8484
},
85+
"configured_max": {
86+
Type: schema.TypeInt,
87+
Computed: true,
88+
Description: "Internal field to track the originally configured max value to distinguish between 0 and -1 when CloudStack returns -1.",
89+
},
8590
"projectid": {
8691
Type: schema.TypeString,
8792
Optional: true,
@@ -98,16 +103,16 @@ func resourceCloudStackLimits() *schema.Resource {
98103
// resourceCloudStackLimitsImport parses composite import IDs and sets resource fields accordingly.
99104
func resourceCloudStackLimitsImport(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
100105
// Expected formats:
101-
// - type-account-accountname-domainid (for account-specific limits)
106+
// - type-account-accountname-domain_id (for account-specific limits)
102107
// - type-project-projectid (for project-specific limits)
103-
// - type-domain-domainid (for domain-specific limits)
108+
// - type-domain-domain_id (for domain-specific limits)
104109

105110
log.Printf("[DEBUG] Importing resource with ID: %s", d.Id())
106111

107112
// First, extract the resource type which is always the first part
108113
idParts := strings.SplitN(d.Id(), "-", 2)
109114
if len(idParts) < 2 {
110-
return nil, fmt.Errorf("unexpected import ID format (%q), expected type-account-accountname-domainid, type-domain-domainid, or type-project-projectid", d.Id())
115+
return nil, fmt.Errorf("unexpected import ID format (%q), expected type-account-accountname-domain_id, type-domain-domain_id, or type-project-projectid", d.Id())
111116
}
112117

113118
// Parse the resource type
@@ -199,26 +204,36 @@ func resourceCloudStackLimitsCreate(d *schema.ResourceData, meta interface{}) er
199204
}
200205

201206
account := d.Get("account").(string)
202-
domainid := d.Get("domainid").(string)
207+
domain_id := d.Get("domain_id").(string)
203208
projectid := d.Get("projectid").(string)
204209

205210
// Validate account and domain parameters
206-
if account != "" && domainid == "" {
207-
return fmt.Errorf("domainid is required when account is specified")
211+
if account != "" && domain_id == "" {
212+
return fmt.Errorf("domain_id is required when account is specified")
208213
}
209214

210215
// Create a new parameter struct
211216
p := cs.Limit.NewUpdateResourceLimitParams(resourcetype)
212217
if account != "" {
213218
p.SetAccount(account)
214219
}
215-
if domainid != "" {
216-
p.SetDomainid(domainid)
220+
if domain_id != "" {
221+
p.SetDomainid(domain_id)
217222
}
218-
if maxVal, ok := d.GetOk("max"); ok {
223+
// Check for max value - need to handle zero values explicitly
224+
maxVal := d.Get("max")
225+
if maxVal != nil {
219226
maxIntVal := maxVal.(int)
220227
log.Printf("[DEBUG] Setting max value to %d", maxIntVal)
221228
p.SetMax(int64(maxIntVal))
229+
230+
// Store the original configured value for later reference
231+
// This helps the Read function distinguish between 0 and -1 when CloudStack returns -1
232+
if err := d.Set("configured_max", maxIntVal); err != nil {
233+
return fmt.Errorf("error storing configured max value: %w", err)
234+
}
235+
} else {
236+
log.Printf("[DEBUG] No max value found in configuration during Create")
222237
}
223238
if projectid != "" {
224239
p.SetProjectid(projectid)
@@ -232,24 +247,24 @@ func resourceCloudStackLimitsCreate(d *schema.ResourceData, meta interface{}) er
232247
}
233248

234249
// Generate a unique ID based on the parameters
235-
id := generateResourceID(resourcetype, account, domainid, projectid)
250+
id := generateResourceID(resourcetype, account, domain_id, projectid)
236251
d.SetId(id)
237252

238253
return resourceCloudStackLimitsRead(d, meta)
239254
}
240255

241256
// generateResourceID creates a unique ID for the resource based on its parameters
242-
func generateResourceID(resourcetype int, account, domainid, projectid string) string {
257+
func generateResourceID(resourcetype int, account, domain_id, projectid string) string {
243258
if projectid != "" {
244259
return fmt.Sprintf("%d-project-%s", resourcetype, projectid)
245260
}
246261

247-
if account != "" && domainid != "" {
248-
return fmt.Sprintf("%d-account-%s-%s", resourcetype, account, domainid)
262+
if account != "" && domain_id != "" {
263+
return fmt.Sprintf("%d-account-%s-%s", resourcetype, account, domain_id)
249264
}
250265

251-
if domainid != "" {
252-
return fmt.Sprintf("%d-domain-%s", resourcetype, domainid)
266+
if domain_id != "" {
267+
return fmt.Sprintf("%d-domain-%s", resourcetype, domain_id)
253268
}
254269

255270
return fmt.Sprintf("%d", resourcetype)
@@ -269,31 +284,41 @@ func resourceCloudStackLimitsRead(d *schema.ResourceData, meta interface{}) erro
269284
// Find the string representation for this numeric type
270285
for typeStr, typeVal := range resourceTypeMap {
271286
if typeVal == rt {
272-
d.Set("type", typeStr)
287+
if err := d.Set("type", typeStr); err != nil {
288+
return fmt.Errorf("error setting type: %s", err)
289+
}
273290
break
274291
}
275292
}
276293

277294
// Handle different ID formats
278295
if len(idParts) >= 3 {
279296
if idParts[1] == "domain" {
280-
// Format: resourcetype-domain-domainid
281-
d.Set("domainid", idParts[2])
297+
// Format: resourcetype-domain-domain_id
298+
if err := d.Set("domain_id", idParts[2]); err != nil {
299+
return fmt.Errorf("error setting domain_id: %s", err)
300+
}
282301
} else if idParts[1] == "project" {
283302
// Format: resourcetype-project-projectid
284-
d.Set("projectid", idParts[2])
303+
if err := d.Set("projectid", idParts[2]); err != nil {
304+
return fmt.Errorf("error setting projectid: %s", err)
305+
}
285306
} else if idParts[1] == "account" && len(idParts) >= 4 {
286-
// Format: resourcetype-account-account-domainid
287-
d.Set("account", idParts[2])
288-
d.Set("domainid", idParts[3])
307+
// Format: resourcetype-account-account-domain_id
308+
if err := d.Set("account", idParts[2]); err != nil {
309+
return fmt.Errorf("error setting account: %s", err)
310+
}
311+
if err := d.Set("domain_id", idParts[3]); err != nil {
312+
return fmt.Errorf("error setting domain_id: %s", err)
313+
}
289314
}
290315
}
291316
}
292317
}
293318
}
294319

295320
account := d.Get("account").(string)
296-
domainid := d.Get("domainid").(string)
321+
domain_id := d.Get("domain_id").(string)
297322
projectid := d.Get("projectid").(string)
298323

299324
// Create a new parameter struct
@@ -302,8 +327,8 @@ func resourceCloudStackLimitsRead(d *schema.ResourceData, meta interface{}) erro
302327
if account != "" {
303328
p.SetAccount(account)
304329
}
305-
if domainid != "" {
306-
p.SetDomainid(domainid)
330+
if domain_id != "" {
331+
p.SetDomainid(domain_id)
307332
}
308333
if projectid != "" {
309334
p.SetProjectid(projectid)
@@ -321,38 +346,45 @@ func resourceCloudStackLimitsRead(d *schema.ResourceData, meta interface{}) erro
321346
return nil
322347
}
323348

324-
// Update the config
325-
for _, limit := range l.ResourceLimits {
326-
if limit.Resourcetype == fmt.Sprintf("%d", resourcetype) {
327-
log.Printf("[DEBUG] Retrieved max value from API: %d", limit.Max)
328-
329-
// Handle CloudStack's convention where -1 signifies unlimited and 0 signifies zero
330-
if limit.Max == -1 {
331-
// For the zero limit test case, we need to preserve the 0 value
332-
// We'll check if the resource was created with max=0
333-
if d.Get("max").(int) == 0 {
334-
log.Printf("[DEBUG] API returned -1 for a limit set to 0, keeping it as 0 in state")
335-
d.Set("max", 0)
336-
} else {
337-
// Otherwise, use -1 to represent unlimited
338-
d.Set("max", limit.Max)
339-
}
340-
} else {
341-
// For any other value, use it directly
342-
d.Set("max", limit.Max)
349+
// Get the first (and should be only) limit from the results
350+
limit := l.ResourceLimits[0]
351+
352+
// Handle the max value - CloudStack may return -1 for both unlimited and zero limits
353+
// We need to preserve the original value from the configuration when possible
354+
log.Printf("[DEBUG] CloudStack returned max value: %d", limit.Max)
355+
if limit.Max == -1 {
356+
// CloudStack returns -1 for both unlimited and zero limits
357+
// Check if we have the originally configured value stored
358+
if configuredMax, hasConfiguredMax := d.GetOk("configured_max"); hasConfiguredMax {
359+
configuredValue := configuredMax.(int)
360+
log.Printf("[DEBUG] Found configured max value: %d, using it", configuredValue)
361+
// Use the originally configured value (0 for zero limit, -1 for unlimited)
362+
if err := d.Set("max", configuredValue); err != nil {
363+
return fmt.Errorf("error setting max to configured value %d: %w", configuredValue, err)
343364
}
344-
345-
// Only set the type field if it was originally specified in the configuration
346-
if v, ok := d.GetOk("type"); ok {
347-
// Preserve the original case of the type parameter
348-
d.Set("type", v.(string))
365+
} else {
366+
log.Printf("[DEBUG] No configured max value found, treating -1 as unlimited")
367+
// If no configured value is stored, treat -1 as unlimited
368+
if err := d.Set("max", -1); err != nil {
369+
return fmt.Errorf("error setting max to unlimited (-1): %w", err)
349370
}
371+
}
372+
} else {
373+
log.Printf("[DEBUG] Using positive max value from API: %d", limit.Max)
374+
// For any positive value, use it directly from the API
375+
if err := d.Set("max", int(limit.Max)); err != nil {
376+
return fmt.Errorf("error setting max: %w", err)
377+
}
378+
}
350379

351-
return nil
380+
// Preserve original type configuration if it exists
381+
if typeValue, ok := d.GetOk("type"); ok {
382+
if err := d.Set("type", typeValue.(string)); err != nil {
383+
return fmt.Errorf("error setting type: %w", err)
352384
}
353385
}
354386

355-
return fmt.Errorf("resource limit not found")
387+
return nil
356388
}
357389

358390
func resourceCloudStackLimitsUpdate(d *schema.ResourceData, meta interface{}) error {
@@ -364,21 +396,28 @@ func resourceCloudStackLimitsUpdate(d *schema.ResourceData, meta interface{}) er
364396
}
365397

366398
account := d.Get("account").(string)
367-
domainid := d.Get("domainid").(string)
399+
domain_id := d.Get("domain_id").(string)
368400
projectid := d.Get("projectid").(string)
369401

370402
// Create a new parameter struct
371403
p := cs.Limit.NewUpdateResourceLimitParams(resourcetype)
372404
if account != "" {
373405
p.SetAccount(account)
374406
}
375-
if domainid != "" {
376-
p.SetDomainid(domainid)
407+
if domain_id != "" {
408+
p.SetDomainid(domain_id)
377409
}
378410
if maxVal, ok := d.GetOk("max"); ok {
379411
maxIntVal := maxVal.(int)
380412
log.Printf("[DEBUG] Setting max value to %d", maxIntVal)
381413
p.SetMax(int64(maxIntVal))
414+
415+
// Store the original configured value for later reference
416+
// This helps the Read function distinguish between 0 and -1 when CloudStack returns -1
417+
log.Printf("[DEBUG] Storing configured max value in update: %d", maxIntVal)
418+
if err := d.Set("configured_max", maxIntVal); err != nil {
419+
return fmt.Errorf("error storing configured max value: %w", err)
420+
}
382421
}
383422
if projectid != "" {
384423
p.SetProjectid(projectid)
@@ -403,16 +442,16 @@ func resourceCloudStackLimitsDelete(d *schema.ResourceData, meta interface{}) er
403442
}
404443

405444
account := d.Get("account").(string)
406-
domainid := d.Get("domainid").(string)
445+
domain_id := d.Get("domain_id").(string)
407446
projectid := d.Get("projectid").(string)
408447

409448
// Create a new parameter struct
410449
p := cs.Limit.NewUpdateResourceLimitParams(resourcetype)
411450
if account != "" {
412451
p.SetAccount(account)
413452
}
414-
if domainid != "" {
415-
p.SetDomainid(domainid)
453+
if domain_id != "" {
454+
p.SetDomainid(domain_id)
416455
}
417456
if projectid != "" {
418457
p.SetProjectid(projectid)

0 commit comments

Comments
 (0)