Copilot commented on code in PR #138: URL: https://github.com/apache/cloudstack-terraform-provider/pull/138#discussion_r2314208068
########## cloudstack/service_offering_util.go: ########## @@ -0,0 +1,277 @@ +// 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" + "strings" + + "github.com/apache/cloudstack-go/v2/cloudstack" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +// ------------------------------------------------------------------------------------------------------------------------------ +// Common update methods +// - +func (state *serviceOfferingCommonResourceModel) commonUpdate(ctx context.Context, cs *cloudstack.UpdateServiceOfferingResponse) { + if cs.Displaytext != "" { + state.DisplayText = types.StringValue(cs.Displaytext) + } + if cs.Domainid != "" { + state.DomainIds, _ = types.SetValueFrom(ctx, types.StringType, strings.Split(cs.Domainid, ",")) Review Comment: Error from types.SetValueFrom is ignored. This could lead to silent failures when converting domain IDs. The error should be handled properly or at least logged. ########## cloudstack/service_offering_util.go: ########## @@ -0,0 +1,277 @@ +// 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" + "strings" + + "github.com/apache/cloudstack-go/v2/cloudstack" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +// ------------------------------------------------------------------------------------------------------------------------------ +// Common update methods +// - +func (state *serviceOfferingCommonResourceModel) commonUpdate(ctx context.Context, cs *cloudstack.UpdateServiceOfferingResponse) { + if cs.Displaytext != "" { + state.DisplayText = types.StringValue(cs.Displaytext) + } + if cs.Domainid != "" { + state.DomainIds, _ = types.SetValueFrom(ctx, types.StringType, strings.Split(cs.Domainid, ",")) + } + if cs.Hosttags != "" { + state.HostTags = types.StringValue(cs.Hosttags) + } + if cs.Name != "" { + state.Name = types.StringValue(cs.Name) + } + if cs.Zoneid != "" { + state.ZoneIds, _ = types.SetValueFrom(ctx, types.StringType, strings.Split(cs.Zoneid, ",")) Review Comment: Error from types.SetValueFrom is ignored. This could lead to silent failures when converting zone IDs. The error should be handled properly or at least logged. ########## cloudstack/service_offering_util.go: ########## @@ -0,0 +1,277 @@ +// 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" + "strings" + + "github.com/apache/cloudstack-go/v2/cloudstack" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +// ------------------------------------------------------------------------------------------------------------------------------ +// Common update methods +// - +func (state *serviceOfferingCommonResourceModel) commonUpdate(ctx context.Context, cs *cloudstack.UpdateServiceOfferingResponse) { + if cs.Displaytext != "" { + state.DisplayText = types.StringValue(cs.Displaytext) + } + if cs.Domainid != "" { + state.DomainIds, _ = types.SetValueFrom(ctx, types.StringType, strings.Split(cs.Domainid, ",")) + } + if cs.Hosttags != "" { + state.HostTags = types.StringValue(cs.Hosttags) + } + if cs.Name != "" { + state.Name = types.StringValue(cs.Name) + } + if cs.Zoneid != "" { + state.ZoneIds, _ = types.SetValueFrom(ctx, types.StringType, strings.Split(cs.Zoneid, ",")) + } +} + +func (plan *serviceOfferingCommonResourceModel) commonUpdateParams(ctx context.Context, p *cloudstack.UpdateServiceOfferingParams) *cloudstack.UpdateServiceOfferingParams { + if !plan.DisplayText.IsNull() { + p.SetDisplaytext(plan.DisplayText.ValueString()) + } + if !plan.DomainIds.IsNull() { + p.SetDomainid(plan.DomainIds.String()) + } + if !plan.HostTags.IsNull() { + p.SetHosttags(plan.HostTags.ValueString()) + } + if !plan.Name.IsNull() { + p.SetName(plan.Name.ValueString()) + } + if !plan.ZoneIds.IsNull() && len(plan.ZoneIds.Elements()) > 0 { + p.SetZoneid(plan.ZoneIds.String()) + } else { + p.SetZoneid("all") + } + + return p + +} + +// ------------------------------------------------------------------------------------------------------------------------------ +// common Read methods +// - +func (state *serviceOfferingCommonResourceModel) commonRead(ctx context.Context, cs *cloudstack.ServiceOffering) { + state.Id = types.StringValue(cs.Id) + + if cs.Deploymentplanner != "" { + state.DeploymentPlanner = types.StringValue(cs.Deploymentplanner) + } + if cs.Diskofferingid != "" { + state.DiskOfferingId = types.StringValue(cs.Diskofferingid) + } + if cs.Displaytext != "" { + state.DisplayText = types.StringValue(cs.Displaytext) + } + if cs.Domainid != "" { + state.DomainIds, _ = types.SetValueFrom(ctx, types.StringType, strings.Split(cs.Domainid, ",")) Review Comment: Error from types.SetValueFrom is ignored. This could lead to silent failures when converting domain IDs. The error should be handled properly or at least logged. ########## cloudstack/service_offering_util.go: ########## @@ -0,0 +1,277 @@ +// 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" + "strings" + + "github.com/apache/cloudstack-go/v2/cloudstack" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +// ------------------------------------------------------------------------------------------------------------------------------ +// Common update methods +// - +func (state *serviceOfferingCommonResourceModel) commonUpdate(ctx context.Context, cs *cloudstack.UpdateServiceOfferingResponse) { + if cs.Displaytext != "" { + state.DisplayText = types.StringValue(cs.Displaytext) + } + if cs.Domainid != "" { + state.DomainIds, _ = types.SetValueFrom(ctx, types.StringType, strings.Split(cs.Domainid, ",")) + } + if cs.Hosttags != "" { + state.HostTags = types.StringValue(cs.Hosttags) + } + if cs.Name != "" { + state.Name = types.StringValue(cs.Name) + } + if cs.Zoneid != "" { + state.ZoneIds, _ = types.SetValueFrom(ctx, types.StringType, strings.Split(cs.Zoneid, ",")) + } +} + +func (plan *serviceOfferingCommonResourceModel) commonUpdateParams(ctx context.Context, p *cloudstack.UpdateServiceOfferingParams) *cloudstack.UpdateServiceOfferingParams { + if !plan.DisplayText.IsNull() { + p.SetDisplaytext(plan.DisplayText.ValueString()) + } + if !plan.DomainIds.IsNull() { + p.SetDomainid(plan.DomainIds.String()) + } + if !plan.HostTags.IsNull() { + p.SetHosttags(plan.HostTags.ValueString()) + } + if !plan.Name.IsNull() { + p.SetName(plan.Name.ValueString()) + } + if !plan.ZoneIds.IsNull() && len(plan.ZoneIds.Elements()) > 0 { + p.SetZoneid(plan.ZoneIds.String()) + } else { + p.SetZoneid("all") + } + + return p + +} + +// ------------------------------------------------------------------------------------------------------------------------------ +// common Read methods +// - +func (state *serviceOfferingCommonResourceModel) commonRead(ctx context.Context, cs *cloudstack.ServiceOffering) { + state.Id = types.StringValue(cs.Id) + + if cs.Deploymentplanner != "" { + state.DeploymentPlanner = types.StringValue(cs.Deploymentplanner) + } + if cs.Diskofferingid != "" { + state.DiskOfferingId = types.StringValue(cs.Diskofferingid) + } + if cs.Displaytext != "" { + state.DisplayText = types.StringValue(cs.Displaytext) + } + if cs.Domainid != "" { + state.DomainIds, _ = types.SetValueFrom(ctx, types.StringType, strings.Split(cs.Domainid, ",")) + } + if cs.Hosttags != "" { + state.HostTags = types.StringValue(cs.Hosttags) + } + if cs.Name != "" { + state.Name = types.StringValue(cs.Name) + } + if cs.Networkrate > 0 { + state.NetworkRate = types.Int32Value(int32(cs.Networkrate)) + } + if cs.Zoneid != "" { + state.ZoneIds, _ = types.SetValueFrom(ctx, types.StringType, strings.Split(cs.Zoneid, ",")) + } + + state.DynamicScalingEnabled = types.BoolValue(cs.Dynamicscalingenabled) + state.IsVolatile = types.BoolValue(cs.Isvolatile) + state.LimitCpuUse = types.BoolValue(cs.Limitcpuuse) + state.OfferHa = types.BoolValue(cs.Offerha) + +} + +func (state *ServiceOfferingDiskQosHypervisor) commonRead(ctx context.Context, cs *cloudstack.ServiceOffering) { + if cs.DiskBytesReadRate > 0 { + state.DiskBytesReadRate = types.Int64Value(cs.DiskBytesReadRate) + } + if cs.DiskBytesReadRateMax > 0 { + state.DiskBytesReadRateMax = types.Int64Value(cs.DiskBytesReadRateMax) + } + if cs.DiskBytesReadRateMaxLength > 0 { + state.DiskBytesReadRateMaxLength = types.Int64Value(cs.DiskBytesReadRateMaxLength) + } + if cs.DiskBytesWriteRate > 0 { + state.DiskBytesWriteRate = types.Int64Value(cs.DiskBytesWriteRate) + } + if cs.DiskBytesWriteRateMax > 0 { + state.DiskBytesWriteRateMax = types.Int64Value(cs.DiskBytesWriteRateMax) + } + if cs.DiskBytesWriteRateMaxLength > 0 { + state.DiskBytesWriteRateMaxLength = types.Int64Value(cs.DiskBytesWriteRateMaxLength) + } + +} + +func (state *ServiceOfferingDiskOffering) commonRead(ctx context.Context, cs *cloudstack.ServiceOffering) { + + if cs.CacheMode != "" { + state.CacheMode = types.StringValue(cs.CacheMode) + } + if cs.Diskofferingstrictness { + state.DiskOfferingStrictness = types.BoolValue(cs.Diskofferingstrictness) + } + if cs.Provisioningtype != "" { + state.ProvisionType = types.StringValue(cs.Provisioningtype) + } + if cs.Rootdisksize > 0 { + state.RootDiskSize = types.Int64Value(cs.Rootdisksize) + } + if cs.Storagetype != "" { + state.StorageType = types.StringValue(cs.Storagetype) + } + if cs.Storagetags != "" { + state.StorageTags = types.StringValue(cs.Storagetags) + } +} + +func (state *ServiceOfferingDiskQosStorage) commonRead(ctx context.Context, cs *cloudstack.ServiceOffering) { + if cs.Iscustomizediops { + state.CustomizedIops = types.BoolValue(cs.Iscustomizediops) + } + if cs.Hypervisorsnapshotreserve > 0 { + state.HypervisorSnapshotReserve = types.Int32Value(int32(cs.Hypervisorsnapshotreserve)) + } + if cs.Maxiops > 0 { + state.MaxIops = types.Int64Value(cs.Maxiops) + } + if cs.Miniops > 0 { + state.MinIops = types.Int64Value(cs.Miniops) + } + +} + +// ------------------------------------------------------------------------------------------------------------------------------ +// common Create methods +// - +func (plan *serviceOfferingCommonResourceModel) commonCreateParams(ctx context.Context, p *cloudstack.CreateServiceOfferingParams) *cloudstack.CreateServiceOfferingParams { + if !plan.DeploymentPlanner.IsNull() && !plan.DeploymentPlanner.IsUnknown() { + p.SetDeploymentplanner(plan.DeploymentPlanner.ValueString()) + } else { + plan.DeploymentPlanner = types.StringNull() + } + if !plan.DiskOfferingId.IsNull() { + p.SetDiskofferingid(plan.DiskOfferingId.ValueString()) + } + if !plan.DomainIds.IsNull() { + domainids := make([]string, len(plan.DomainIds.Elements())) + plan.DomainIds.ElementsAs(ctx, &domainids, false) + p.SetDomainid(domainids) + } + if !plan.DynamicScalingEnabled.IsNull() { + p.SetDynamicscalingenabled(plan.DynamicScalingEnabled.ValueBool()) + } + if !plan.HostTags.IsNull() { + p.SetHosttags(plan.HostTags.ValueString()) + } + if !plan.IsVolatile.IsNull() { + p.SetIsvolatile(plan.IsVolatile.ValueBool()) + } + if !plan.LimitCpuUse.IsNull() { + p.SetLimitcpuuse(plan.LimitCpuUse.ValueBool()) + } + if !plan.NetworkRate.IsNull() { + p.SetNetworkrate(int(plan.NetworkRate.ValueInt32())) + } + if !plan.OfferHa.IsNull() { + p.SetOfferha(plan.OfferHa.ValueBool()) + } + if !plan.ZoneIds.IsNull() { + zoneIds := make([]string, len(plan.ZoneIds.Elements())) + plan.ZoneIds.ElementsAs(ctx, &zoneIds, false) + p.SetZoneid(zoneIds) + } + + return p + +} +func (plan *ServiceOfferingDiskQosHypervisor) commonCreateParams(ctx context.Context, p *cloudstack.CreateServiceOfferingParams) *cloudstack.CreateServiceOfferingParams { + if !plan.DiskBytesReadRate.IsNull() { + p.SetBytesreadrate(plan.DiskBytesReadRate.ValueInt64()) + } + if !plan.DiskBytesReadRateMax.IsNull() { + p.SetBytesreadratemax(plan.DiskBytesReadRateMax.ValueInt64()) + } + if !plan.DiskBytesReadRateMaxLength.IsNull() { + p.SetBytesreadratemaxlength(plan.DiskBytesReadRateMaxLength.ValueInt64()) + } + if !plan.DiskBytesWriteRate.IsNull() { + p.SetByteswriterate(plan.DiskBytesWriteRate.ValueInt64()) + } + if !plan.DiskBytesWriteRateMax.IsNull() { + p.SetByteswriteratemax(plan.DiskBytesWriteRateMax.ValueInt64()) + } + if !plan.DiskBytesWriteRateMaxLength.IsNull() { + p.SetByteswriteratemaxlength(plan.DiskBytesWriteRateMaxLength.ValueInt64()) + } + + return p +} + +func (plan *ServiceOfferingDiskOffering) commonCreateParams(ctx context.Context, p *cloudstack.CreateServiceOfferingParams) *cloudstack.CreateServiceOfferingParams { + + if !plan.CacheMode.IsNull() { + p.SetCachemode(plan.CacheMode.ValueString()) + } + if !plan.DiskOfferingStrictness.IsNull() { + p.SetDiskofferingstrictness(plan.DiskOfferingStrictness.ValueBool()) + } + if !plan.ProvisionType.IsNull() { + p.SetProvisioningtype(plan.ProvisionType.ValueString()) + } + if !plan.RootDiskSize.IsNull() { + p.SetRootdisksize(plan.RootDiskSize.ValueInt64()) + } + if !plan.StorageType.IsNull() { + p.SetStoragetype(plan.StorageType.ValueString()) + } + if !plan.StorageTags.IsNull() { + p.SetTags(plan.StorageTags.ValueString()) + } + + return p + +} + +func (plan *ServiceOfferingDiskQosStorage) commonCreateParams(ctx context.Context, p *cloudstack.CreateServiceOfferingParams) *cloudstack.CreateServiceOfferingParams { + if !plan.CustomizedIops.IsNull() { + p.SetCustomizediops(plan.CustomizedIops.ValueBool()) + } + if !plan.HypervisorSnapshotReserve.IsNull() { + p.SetHypervisorsnapshotreserve(int(plan.HypervisorSnapshotReserve.ValueInt32())) + } + if !plan.MaxIops.IsNull() { + p.SetMaxiops(int64(plan.MaxIops.ValueInt64())) Review Comment: Redundant type conversion from int64 to int64. The ValueInt64() method already returns int64, so the conversion is unnecessary. ```suggestion p.SetMaxiops(plan.MaxIops.ValueInt64()) ``` ########## cloudstack/service_offering_constrained_resource.go: ########## @@ -0,0 +1,313 @@ +// +// 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" + "strconv" + + "github.com/apache/cloudstack-go/v2/cloudstack" + "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/int32planmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-framework/types/basetypes" +) + +var ( + _ resource.Resource = &serviceOfferingConstrainedResource{} + _ resource.ResourceWithConfigure = &serviceOfferingConstrainedResource{} +) + +func NewserviceOfferingConstrainedResource() resource.Resource { + return &serviceOfferingConstrainedResource{} +} + +type serviceOfferingConstrainedResource struct { + client *cloudstack.CloudStackClient +} + +func (r *serviceOfferingConstrainedResource) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) { + resp.Schema = schema.Schema{ + Attributes: serviceOfferingMergeCommonSchema(map[string]schema.Attribute{ + "cpu_speed": schema.Int32Attribute{ + Description: "VMware and Xen based hypervisors this is the CPU speed of the service offering in MHz. For the KVM hypervisor the values of the parameters cpuSpeed and cpuNumber will be used to calculate the `shares` value. This value is used by the KVM hypervisor to calculate how much time the VM will have access to the host's CPU. The `shares` value does not have a unit, and its purpose is being a weight value for the host to compare between its guest VMs. For more information, see https://libvirt.org/formatdomain.html#cpu-tuning.";, + Required: true, + PlanModifiers: []planmodifier.Int32{ + int32planmodifier.RequiresReplace(), + }, + }, + "max_cpu_number": schema.Int32Attribute{ + Description: "The maximum number of CPUs to be set with Custom Compute Offering", + Required: true, + PlanModifiers: []planmodifier.Int32{ + int32planmodifier.RequiresReplace(), + }, + }, + "max_memory": schema.Int32Attribute{ + Description: "The maximum memory size of the custom service offering in MB", + Required: true, + PlanModifiers: []planmodifier.Int32{ + int32planmodifier.RequiresReplace(), + }, + }, + "min_cpu_number": schema.Int32Attribute{ + Description: "The minimum number of CPUs to be set with Custom Compute Offering", + Required: true, + PlanModifiers: []planmodifier.Int32{ + int32planmodifier.RequiresReplace(), + }, + }, + "min_memory": schema.Int32Attribute{ + Description: "The minimum memory size of the custom service offering in MB", + Required: true, + PlanModifiers: []planmodifier.Int32{ + int32planmodifier.RequiresReplace(), + }, + }, + }), + } +} + +func (r *serviceOfferingConstrainedResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { + var plan serviceOfferingConstrainedResourceModel + var planDiskQosHypervisor ServiceOfferingDiskQosHypervisor + var planDiskOffering ServiceOfferingDiskOffering + var planDiskQosStorage ServiceOfferingDiskQosStorage + + resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) + if !plan.ServiceOfferingDiskQosHypervisor.IsNull() { + resp.Diagnostics.Append(plan.ServiceOfferingDiskQosHypervisor.As(ctx, &planDiskQosHypervisor, basetypes.ObjectAsOptions{})...) + } + if !plan.ServiceOfferingDiskOffering.IsNull() { + resp.Diagnostics.Append(plan.ServiceOfferingDiskOffering.As(ctx, &planDiskOffering, basetypes.ObjectAsOptions{})...) + } + if !plan.ServiceOfferingDiskQosStorage.IsNull() { + resp.Diagnostics.Append(plan.ServiceOfferingDiskQosStorage.As(ctx, &planDiskQosStorage, basetypes.ObjectAsOptions{})...) + } + if resp.Diagnostics.HasError() { + return + } + + // common params + params := r.client.ServiceOffering.NewCreateServiceOfferingParams(plan.DisplayText.ValueString(), plan.Name.ValueString()) + plan.commonCreateParams(ctx, params) + planDiskQosHypervisor.commonCreateParams(ctx, params) + planDiskOffering.commonCreateParams(ctx, params) + planDiskQosStorage.commonCreateParams(ctx, params) + + // resource specific params + if !plan.CpuSpeed.IsNull() { + params.SetCpuspeed(int(plan.CpuSpeed.ValueInt32())) + } Review Comment: Missing validation that CpuSpeed is required. The schema shows this field as required, but there's a null check here. Either remove the null check or make the schema field optional. ```suggestion params.SetCpuspeed(int(plan.CpuSpeed.ValueInt32())) ``` ########## cloudstack/service_offering_util.go: ########## @@ -0,0 +1,277 @@ +// 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" + "strings" + + "github.com/apache/cloudstack-go/v2/cloudstack" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +// ------------------------------------------------------------------------------------------------------------------------------ +// Common update methods +// - +func (state *serviceOfferingCommonResourceModel) commonUpdate(ctx context.Context, cs *cloudstack.UpdateServiceOfferingResponse) { + if cs.Displaytext != "" { + state.DisplayText = types.StringValue(cs.Displaytext) + } + if cs.Domainid != "" { + state.DomainIds, _ = types.SetValueFrom(ctx, types.StringType, strings.Split(cs.Domainid, ",")) + } + if cs.Hosttags != "" { + state.HostTags = types.StringValue(cs.Hosttags) + } + if cs.Name != "" { + state.Name = types.StringValue(cs.Name) + } + if cs.Zoneid != "" { + state.ZoneIds, _ = types.SetValueFrom(ctx, types.StringType, strings.Split(cs.Zoneid, ",")) + } +} + +func (plan *serviceOfferingCommonResourceModel) commonUpdateParams(ctx context.Context, p *cloudstack.UpdateServiceOfferingParams) *cloudstack.UpdateServiceOfferingParams { + if !plan.DisplayText.IsNull() { + p.SetDisplaytext(plan.DisplayText.ValueString()) + } + if !plan.DomainIds.IsNull() { + p.SetDomainid(plan.DomainIds.String()) Review Comment: Using plan.DomainIds.String() directly may not provide the correct format for the API. This will likely result in a string representation like '[\"id1\", \"id2\"]' instead of a comma-separated list. Consider extracting the actual string values and joining them with commas. ########## cloudstack/service_offering_util.go: ########## @@ -0,0 +1,277 @@ +// 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" + "strings" + + "github.com/apache/cloudstack-go/v2/cloudstack" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +// ------------------------------------------------------------------------------------------------------------------------------ +// Common update methods +// - +func (state *serviceOfferingCommonResourceModel) commonUpdate(ctx context.Context, cs *cloudstack.UpdateServiceOfferingResponse) { + if cs.Displaytext != "" { + state.DisplayText = types.StringValue(cs.Displaytext) + } + if cs.Domainid != "" { + state.DomainIds, _ = types.SetValueFrom(ctx, types.StringType, strings.Split(cs.Domainid, ",")) + } + if cs.Hosttags != "" { + state.HostTags = types.StringValue(cs.Hosttags) + } + if cs.Name != "" { + state.Name = types.StringValue(cs.Name) + } + if cs.Zoneid != "" { + state.ZoneIds, _ = types.SetValueFrom(ctx, types.StringType, strings.Split(cs.Zoneid, ",")) + } +} + +func (plan *serviceOfferingCommonResourceModel) commonUpdateParams(ctx context.Context, p *cloudstack.UpdateServiceOfferingParams) *cloudstack.UpdateServiceOfferingParams { + if !plan.DisplayText.IsNull() { + p.SetDisplaytext(plan.DisplayText.ValueString()) + } + if !plan.DomainIds.IsNull() { + p.SetDomainid(plan.DomainIds.String()) + } + if !plan.HostTags.IsNull() { + p.SetHosttags(plan.HostTags.ValueString()) + } + if !plan.Name.IsNull() { + p.SetName(plan.Name.ValueString()) + } + if !plan.ZoneIds.IsNull() && len(plan.ZoneIds.Elements()) > 0 { + p.SetZoneid(plan.ZoneIds.String()) Review Comment: Using plan.ZoneIds.String() directly may not provide the correct format for the API. This will likely result in a string representation like '[\"id1\", \"id2\"]' instead of a comma-separated list. Consider extracting the actual string values and joining them with commas. ```suggestion zoneIds := plan.ZoneIds.Elements() zoneIdStrs := make([]string, len(zoneIds)) for i, v := range zoneIds { zoneIdStrs[i] = v.(types.String).ValueString() } p.SetZoneid(strings.Join(zoneIdStrs, ",")) ``` ########## cloudstack/service_offering_constrained_resource_test.go: ########## @@ -0,0 +1,319 @@ +// +// 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 ( + "testing" + + "github.com/hashicorp/terraform-plugin-testing/helper/resource" +) + +func TestAccServiceOfferingConstrained(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccMuxProvider, + Steps: []resource.TestStep{ + { + Config: testAccServiceOfferingCustomConstrained1, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("cloudstack_service_offering_constrained.constrained1", "name", "constrained1"), + ), + }, + { + Config: testAccServiceOfferingCustomConstrained1ZoneAll, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("cloudstack_service_offering_constrained.constrained1", "name", "constrained1"), + ), + }, + { + Config: testAccServiceOfferingCustomConstrained2, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("cloudstack_service_offering_constrained.constrained2", "name", "constrained2"), + ), + }, + { + Config: testAccServiceOfferingCustomConstrained2_update, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("cloudstack_service_offering_constrained.constrained2", "name", "constrained2update"), + ), + }, + { + Config: testAccServiceOfferingCustomConstrained_disk, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("cloudstack_service_offering_constrained.constrained1", "name", "constrained1"), + ), + }, + { + Config: testAccServiceOfferingCustomConstrained_disk_hypervisor, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("cloudstack_service_offering_constrained.disk_hypervisor", "name", "disk_hypervisor"), + ), + }, + { + Config: testAccServiceOfferingCustomConstrained_disk_storage, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("cloudstack_service_offering_constrained.disk_storage", "name", "disk_storage"), + ), + }, + }, + }) +} + +const testAccServiceOfferingCustomConstrained1 = ` +resource "cloudstack_zone" "test" { + name = "acctest" + dns1 = "8.8.8.8" + internal_dns1 = "8.8.4.4" + network_type = "Advanced" +} + +resource "cloudstack_service_offering_constrained" "constrained1" { + display_text = "constrained1" + name = "constrained1" + + // compute + cpu_speed = 2500 + max_cpu_number = 10 + min_cpu_number = 2 + + // memory + max_memory = 4096 + min_memory = 1024 + + // other + host_tags = "test0101,test0202" + network_rate = 1024 + deployment_planner = "UserDispersingPlanner" + + // Feature flags + dynamic_scaling_enabled = false + is_volatile = false + limit_cpu_use = false + offer_ha = false + zone_ids = [cloudstack_zone.test.id] + +} +` + +const testAccServiceOfferingCustomConstrained1ZoneAll = ` +resource "cloudstack_service_offering_constrained" "constrained1" { + display_text = "constrained11" + name = "constrained1" + + // compute + cpu_speed = 2500 + max_cpu_number = 10 + min_cpu_number = 2 + + // memory + max_memory = 4096 + min_memory = 1024 + + // other + host_tags = "test0101,test0202" + network_rate = 1024 + deployment_planner = "UserDispersingPlanner" + + // Feature flags + dynamic_scaling_enabled = false + is_volatile = false + limit_cpu_use = false + offer_ha = false + zone_ids = [] +} +` + +const testAccServiceOfferingCustomConstrained2 = ` +resource "cloudstack_service_offering_constrained" "constrained2" { + display_text = "constrained2" + name = "constrained2" + + // compute + cpu_speed = 2500 + max_cpu_number = 10 + min_cpu_number = 2 + + // memory + max_memory = 4096 + min_memory = 1024 + + // other + host_tags = "test0101,test0202" + network_rate = 1024 + deployment_planner = "UserDispersingPlanner" + + // Feature flags + dynamic_scaling_enabled = true + is_volatile = true + limit_cpu_use = true + offer_ha = true +} +` +const testAccServiceOfferingCustomConstrained2_update = ` +resource "cloudstack_service_offering_constrained" "constrained2" { + display_text = "constrained2update" + name = "constrained2update" + + // compute + cpu_speed = 2500 + max_cpu_number = 10 + min_cpu_number = 2 + + // memory + max_memory = 4096 + min_memory = 1024 + + // other + host_tags = "test0101,test0202" + network_rate = 1024 + deployment_planner = "UserDispersingPlanner" + + // Feature flags + dynamic_scaling_enabled = true + is_volatile = true + limit_cpu_use = true + offer_ha = true +} +` + +const testAccServiceOfferingCustomConstrained_disk = ` +resource "cloudstack_service_offering_constrained" "constrained1" { + display_text = "constrained1" + name = "constrained1" + + // compute + cpu_speed = 2500 + max_cpu_number = 10 + min_cpu_number = 2 + + // memory + max_memory = 4096 + min_memory = 1024 + + // other + host_tags = "test0101,test0202" + network_rate = 1024 + deployment_planner = "UserDispersingPlanner" + + // Feature flags + dynamic_scaling_enabled = false + is_volatile = false + limit_cpu_use = false + offer_ha = false + + disk_offering = { + storage_type = "local" + sdfjklsdf = "sdfjks" Review Comment: This appears to be test data with a nonsensical attribute name 'sdfjklsdf'. This will likely cause the test to fail as this is not a valid schema attribute. This should be removed or replaced with a valid attribute. ```suggestion ``` ########## cloudstack/service_offering_util.go: ########## @@ -0,0 +1,277 @@ +// 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" + "strings" + + "github.com/apache/cloudstack-go/v2/cloudstack" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +// ------------------------------------------------------------------------------------------------------------------------------ +// Common update methods +// - +func (state *serviceOfferingCommonResourceModel) commonUpdate(ctx context.Context, cs *cloudstack.UpdateServiceOfferingResponse) { + if cs.Displaytext != "" { + state.DisplayText = types.StringValue(cs.Displaytext) + } + if cs.Domainid != "" { + state.DomainIds, _ = types.SetValueFrom(ctx, types.StringType, strings.Split(cs.Domainid, ",")) + } + if cs.Hosttags != "" { + state.HostTags = types.StringValue(cs.Hosttags) + } + if cs.Name != "" { + state.Name = types.StringValue(cs.Name) + } + if cs.Zoneid != "" { + state.ZoneIds, _ = types.SetValueFrom(ctx, types.StringType, strings.Split(cs.Zoneid, ",")) + } +} + +func (plan *serviceOfferingCommonResourceModel) commonUpdateParams(ctx context.Context, p *cloudstack.UpdateServiceOfferingParams) *cloudstack.UpdateServiceOfferingParams { + if !plan.DisplayText.IsNull() { + p.SetDisplaytext(plan.DisplayText.ValueString()) + } + if !plan.DomainIds.IsNull() { + p.SetDomainid(plan.DomainIds.String()) + } + if !plan.HostTags.IsNull() { + p.SetHosttags(plan.HostTags.ValueString()) + } + if !plan.Name.IsNull() { + p.SetName(plan.Name.ValueString()) + } + if !plan.ZoneIds.IsNull() && len(plan.ZoneIds.Elements()) > 0 { + p.SetZoneid(plan.ZoneIds.String()) + } else { + p.SetZoneid("all") + } + + return p + +} + +// ------------------------------------------------------------------------------------------------------------------------------ +// common Read methods +// - +func (state *serviceOfferingCommonResourceModel) commonRead(ctx context.Context, cs *cloudstack.ServiceOffering) { + state.Id = types.StringValue(cs.Id) + + if cs.Deploymentplanner != "" { + state.DeploymentPlanner = types.StringValue(cs.Deploymentplanner) + } + if cs.Diskofferingid != "" { + state.DiskOfferingId = types.StringValue(cs.Diskofferingid) + } + if cs.Displaytext != "" { + state.DisplayText = types.StringValue(cs.Displaytext) + } + if cs.Domainid != "" { + state.DomainIds, _ = types.SetValueFrom(ctx, types.StringType, strings.Split(cs.Domainid, ",")) + } + if cs.Hosttags != "" { + state.HostTags = types.StringValue(cs.Hosttags) + } + if cs.Name != "" { + state.Name = types.StringValue(cs.Name) + } + if cs.Networkrate > 0 { + state.NetworkRate = types.Int32Value(int32(cs.Networkrate)) + } + if cs.Zoneid != "" { + state.ZoneIds, _ = types.SetValueFrom(ctx, types.StringType, strings.Split(cs.Zoneid, ",")) Review Comment: Error from types.SetValueFrom is ignored. This could lead to silent failures when converting zone IDs. The error should be handled properly or at least logged. ########## cloudstack/service_offering_unconstrained_resource_test.go: ########## @@ -0,0 +1,207 @@ +// // +// // 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. +// // Review Comment: The license header has extra comment slashes. Lines 1-18 have double comment prefixes ('//' instead of a single '//'). This should be consistent with other files in the codebase. ```suggestion // // 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. // ``` -- 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
