Copilot commented on code in PR #167: URL: https://github.com/apache/cloudstack-terraform-provider/pull/167#discussion_r2256281128
########## cloudstack/resource_cloudstack_project.go: ########## @@ -0,0 +1,533 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package cloudstack + +import ( + "context" + "fmt" + "log" + "strings" + "time" + + "github.com/apache/cloudstack-go/v2/cloudstack" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func resourceCloudStackProject() *schema.Resource { + return &schema.Resource{ + Create: resourceCloudStackProjectCreate, + Read: resourceCloudStackProjectRead, + Update: resourceCloudStackProjectUpdate, + Delete: resourceCloudStackProjectDelete, + Importer: &schema.ResourceImporter{ + State: importStatePassthrough, + }, + + Schema: map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Required: true, + }, + + "display_text": { + Type: schema.TypeString, + Required: true, // Required for API version 4.18 and lower. TODO: Make this optional when support for API versions older than 4.18 is dropped. + }, + + "domain": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, + + "account": { + Type: schema.TypeString, + Optional: true, + }, + + "accountid": { + Type: schema.TypeString, + Optional: true, + }, + + "userid": { + Type: schema.TypeString, + Optional: true, + }, + }, + } +} + +func resourceCloudStackProjectCreate(d *schema.ResourceData, meta any) error { + cs := meta.(*cloudstack.CloudStackClient) + + // Get the name and display_text + name := d.Get("name").(string) + displaytext := d.Get("display_text").(string) + + // Get domain if provided + var domain string + if domainParam, ok := d.GetOk("domain"); ok { + domain = domainParam.(string) + } + + // Check if a project with this name already exists + existingProject, err := getProjectByName(cs, name, domain) + if err == nil { + // Project with this name already exists + log.Printf("[DEBUG] Project with name %s already exists, using existing project with ID: %s", name, existingProject.Id) + d.SetId(existingProject.Id) + + // Set the basic attributes to match the existing project + d.Set("name", existingProject.Name) + d.Set("display_text", existingProject.Displaytext) + d.Set("domain", existingProject.Domain) + + return resourceCloudStackProjectRead(d, meta) + } else if !strings.Contains(err.Error(), "not found") { + // If we got an error other than "not found", return it + return fmt.Errorf("error checking for existing project: %s", err) + } + + // Project doesn't exist, create a new one + + // The CloudStack API parameter order differs between versions: + // - In API 4.18 and lower: displaytext is the first parameter and name is the second + // - In API 4.19 and higher: name is the first parameter and displaytext is optional + // The CloudStack Go SDK uses the API 4.18 parameter order + p := cs.Project.NewCreateProjectParams(displaytext, name) + + // Set the domain if provided + if domain != "" { + domainid, e := retrieveID(cs, "domain", domain) + if e != nil { + return e.Error() Review Comment: The method `Error()` is being called on a `*retrieveError` type, but `retrieveError` likely implements the `error` interface directly. This should return `e` instead of `e.Error()`. ```suggestion return e ``` ########## cloudstack/resource_cloudstack_project.go: ########## @@ -0,0 +1,533 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package cloudstack + +import ( + "context" + "fmt" + "log" + "strings" + "time" + + "github.com/apache/cloudstack-go/v2/cloudstack" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func resourceCloudStackProject() *schema.Resource { + return &schema.Resource{ + Create: resourceCloudStackProjectCreate, + Read: resourceCloudStackProjectRead, + Update: resourceCloudStackProjectUpdate, + Delete: resourceCloudStackProjectDelete, + Importer: &schema.ResourceImporter{ + State: importStatePassthrough, + }, + + Schema: map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Required: true, + }, + + "display_text": { + Type: schema.TypeString, + Required: true, // Required for API version 4.18 and lower. TODO: Make this optional when support for API versions older than 4.18 is dropped. + }, + + "domain": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, + + "account": { + Type: schema.TypeString, + Optional: true, + }, + + "accountid": { + Type: schema.TypeString, + Optional: true, + }, + + "userid": { + Type: schema.TypeString, + Optional: true, + }, + }, + } +} + +func resourceCloudStackProjectCreate(d *schema.ResourceData, meta any) error { + cs := meta.(*cloudstack.CloudStackClient) + + // Get the name and display_text + name := d.Get("name").(string) + displaytext := d.Get("display_text").(string) + + // Get domain if provided + var domain string + if domainParam, ok := d.GetOk("domain"); ok { + domain = domainParam.(string) + } + + // Check if a project with this name already exists + existingProject, err := getProjectByName(cs, name, domain) + if err == nil { + // Project with this name already exists + log.Printf("[DEBUG] Project with name %s already exists, using existing project with ID: %s", name, existingProject.Id) + d.SetId(existingProject.Id) + + // Set the basic attributes to match the existing project + d.Set("name", existingProject.Name) + d.Set("display_text", existingProject.Displaytext) + d.Set("domain", existingProject.Domain) + + return resourceCloudStackProjectRead(d, meta) + } else if !strings.Contains(err.Error(), "not found") { + // If we got an error other than "not found", return it + return fmt.Errorf("error checking for existing project: %s", err) Review Comment: The project creation logic checks for existing projects by name, which could lead to unintended behavior. If a project with the same name exists in a different domain, this check may incorrectly reuse it. Consider adding domain validation to ensure projects are only reused within the intended scope. ```suggestion domainSet := false if domainParam, ok := d.GetOk("domain"); ok { domain = domainParam.(string) domainSet = true } // Only check for an existing project if domain is set if domainSet { existingProject, err := getProjectByName(cs, name, domain) if err == nil { // Project with this name and domain already exists log.Printf("[DEBUG] Project with name %s and domain %s already exists, using existing project with ID: %s", name, domain, existingProject.Id) d.SetId(existingProject.Id) // Set the basic attributes to match the existing project d.Set("name", existingProject.Name) d.Set("display_text", existingProject.Displaytext) d.Set("domain", existingProject.Domain) return resourceCloudStackProjectRead(d, meta) } else if !strings.Contains(err.Error(), "not found") { // If we got an error other than "not found", return it return fmt.Errorf("error checking for existing project: %s", err) } ``` -- 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: dev-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
