kristofer-atlas commented on code in PR #285:
URL:
https://github.com/apache/cloudstack-terraform-provider/pull/285#discussion_r2914737645
##########
cloudstack/resource_cloudstack_domain.go:
##########
@@ -89,9 +89,62 @@ func resourceCloudStackDomainCreate(d *schema.ResourceData,
meta interface{}) er
return resourceCloudStackDomainRead(d, meta)
}
-func resourceCloudStackDomainRead(d *schema.ResourceData, meta interface{})
error { return nil }
+func resourceCloudStackDomainRead(d *schema.ResourceData, meta interface{})
error {
+ cs := meta.(*cloudstack.CloudStackClient)
+
+ log.Printf("[DEBUG] Reading Domain %s", d.Id())
+
+ p := cs.Domain.NewListDomainsParams()
+ p.SetId(d.Id())
+
+ domains, err := cs.Domain.ListDomains(p)
+ if err != nil {
+ return fmt.Errorf("Error reading Domain %s: %s", d.Id(), err)
+ }
+
+ if domains.Count == 0 {
+ log.Printf("[DEBUG] Domain %s does no longer exist", d.Id())
+ d.SetId("")
+ return nil
+ }
+
+ domain := domains.Domains[0]
+ log.Printf("[DEBUG] Domain %s found: %s", d.Id(), domain.Name)
+
+ d.Set("name", domain.Name)
+ d.Set("domain_id", domain.Id)
+ d.Set("network_domain", domain.Networkdomain)
Review Comment:
Fixed in 744dbe8. Made `domain_id` and `parent_domain_id` both `Computed:
true` (in addition to `Optional`) and `ForceNew: true`. This tells Terraform
the provider may populate these values server-side even when the user does not
specify them in config, eliminating the perpetual diff. ForceNew is appropriate
since neither can be changed after domain creation.
##########
cloudstack/resource_cloudstack_disk_offering.go:
##########
@@ -72,8 +73,59 @@ func resourceCloudStackDiskOfferingCreate(d
*schema.ResourceData, meta interface
return resourceCloudStackDiskOfferingRead(d, meta)
}
-func resourceCloudStackDiskOfferingRead(d *schema.ResourceData, meta
interface{}) error { return nil }
+func resourceCloudStackDiskOfferingRead(d *schema.ResourceData, meta
interface{}) error {
+ cs := meta.(*cloudstack.CloudStackClient)
+
+ log.Printf("[DEBUG] Retrieving disk offering %s",
d.Get("name").(string))
+
+ offering, count, err := cs.DiskOffering.GetDiskOfferingByID(d.Id())
+ if err != nil {
+ if count == 0 {
+ log.Printf("[DEBUG] Disk offering %s does no longer
exist", d.Get("name").(string))
+ d.SetId("")
+ return nil
+ }
+ return fmt.Errorf("Error retrieving disk offering %s: %s",
d.Id(), err)
+ }
+
+ d.Set("name", offering.Name)
+ d.Set("display_text", offering.Displaytext)
+ d.Set("disk_size", offering.Disksize)
+
+ return nil
+}
+
+func resourceCloudStackDiskOfferingUpdate(d *schema.ResourceData, meta
interface{}) error {
+ cs := meta.(*cloudstack.CloudStackClient)
+
+ p := cs.DiskOffering.NewUpdateDiskOfferingParams(d.Id())
+
+ if d.HasChange("name") {
+ p.SetName(d.Get("name").(string))
+ }
+ if d.HasChange("display_text") {
+ p.SetDisplaytext(d.Get("display_text").(string))
+ }
Review Comment:
Fixed in 744dbe8. Marked `disk_size` as `ForceNew: true` since CloudStack's
`updateDiskOffering` API does not support changing disk size after creation.
##########
cloudstack/resource_cloudstack_user.go:
##########
@@ -88,6 +88,28 @@ func resourceCloudStackUserCreate(d *schema.ResourceData,
meta interface{}) erro
}
func resourceCloudStackUserUpdate(d *schema.ResourceData, meta interface{})
error {
+ cs := meta.(*cloudstack.CloudStackClient)
+
+ p := cs.User.NewUpdateUserParams(d.Id())
+
+ if d.HasChange("email") {
+ p.SetEmail(d.Get("email").(string))
+ }
+ if d.HasChange("first_name") {
+ p.SetFirstname(d.Get("first_name").(string))
+ }
+ if d.HasChange("last_name") {
+ p.SetLastname(d.Get("last_name").(string))
+ }
+ if d.HasChange("password") {
+ p.SetPassword(d.Get("password").(string))
+ }
Review Comment:
Fixed in 744dbe8. Marked both `account` and `username` as `ForceNew: true`.
The account a user belongs to cannot be changed after creation, and while
`username` is technically updatable via `updateUser`, forcing recreation is the
safer and more predictable behavior.
##########
cloudstack/resource_cloudstack_user.go:
##########
@@ -88,6 +88,28 @@ func resourceCloudStackUserCreate(d *schema.ResourceData,
meta interface{}) erro
}
func resourceCloudStackUserUpdate(d *schema.ResourceData, meta interface{})
error {
+ cs := meta.(*cloudstack.CloudStackClient)
+
+ p := cs.User.NewUpdateUserParams(d.Id())
+
+ if d.HasChange("email") {
+ p.SetEmail(d.Get("email").(string))
+ }
+ if d.HasChange("first_name") {
+ p.SetFirstname(d.Get("first_name").(string))
+ }
+ if d.HasChange("last_name") {
+ p.SetLastname(d.Get("last_name").(string))
+ }
+ if d.HasChange("password") {
+ p.SetPassword(d.Get("password").(string))
+ }
+
+ _, err := cs.User.UpdateUser(p)
+ if err != nil {
+ return err
+ }
Review Comment:
Fixed in 744dbe8. Error is now wrapped with user ID context:
`fmt.Errorf("Error updating User %s: %s", d.Id(), err)`
##########
cloudstack/resource_cloudstack_account.go:
##########
@@ -37,26 +37,32 @@ func resourceCloudStackAccount() *schema.Resource {
"email": {
Type: schema.TypeString,
Required: true,
+ ForceNew: true,
},
"first_name": {
Type: schema.TypeString,
Required: true,
+ ForceNew: true,
},
"last_name": {
Type: schema.TypeString,
Required: true,
+ ForceNew: true,
},
"password": {
Type: schema.TypeString,
Required: true,
+ ForceNew: true,
},
Review Comment:
Fixed in 744dbe8. Added `Sensitive: true` to `password` on both the
`cloudstack_account` and `cloudstack_user` resources.
##########
tests/comprehensive/versions.tf:
##########
@@ -0,0 +1,9 @@
+terraform {
+ required_providers {
+ cloudstack = {
+ source = "local/cloudstack/cloudstack"
+ version = "0.5.0"
+ }
Review Comment:
Moot — the entire `tests/` directory has been removed in 744dbe8. These
manual TF configs were redundant with the existing Go acceptance tests and were
causing Apache RAT check failures due to missing license headers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
