feat: allow to purge additional_disk_space

internal/schemautil/custom_diff.go

@@ -2,15 +2,20 @@ package schemautil
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 
 	"github.com/aiven/aiven-go-client/v2"
+	avngen "github.com/aiven/go-client-codegen"
+	"github.com/aiven/go-client-codegen/handler/service"
 	"github.com/docker/go-units"
 	"github.com/hashicorp/go-cty/cty"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"golang.org/x/exp/slices"
+
+	"github.com/aiven/terraform-provider-aiven/internal/common"
 )
 
 func CustomizeDiffGenericService(serviceType string) schema.CustomizeDiffFunc {
@@ -27,7 +32,10 @@ func CustomizeDiffGenericService(serviceType string) schema.CustomizeDiffFunc {
 		),
 		customdiff.IfValueChange("additional_disk_space",
 			ShouldNotBeEmpty,
-			CustomizeDiffCheckDiskSpace,
+			customdiff.Sequence(
+				CustomizeDiffCheckDiskSpace,
+				CustomizeDiffAdditionalDiskSpace,
+			),
 		),
 		customdiff.IfValueChange("service_integrations",
 			ShouldNotBeEmpty,
@@ -254,3 +262,61 @@ func checkForMultipleValues(v cty.Value) error {
 
 	return nil
 }
+
+var ErrAutoscalerConflict = errors.New("autoscaler integration is enabled, additional_disk_space cannot be set")
+
+// CustomizeDiffAdditionalDiskSpace
+// 1. checks that additional_disk_space is not set if autoscaler is enabled
+// 2. outputs a diff for a computed field, which otherwise would be suppressed when removed
+func CustomizeDiffAdditionalDiskSpace(ctx context.Context, diff *schema.ResourceDiff, _ interface{}) error {
+	client, err := common.GenClient()
+	if err != nil {
+		return err
+	}
+
+	s, err := client.ServiceGet(ctx, diff.Get("project").(string), diff.Get("service_name").(string))
+	if avngen.IsNotFound(err) {
+		// The service does not exist, so we cannot check if autoscaler is enabled
+		return nil
+	}
+
+	if err != nil {
+		return err
+	}
+
+	isAutoscalerEnabled := false
+	for _, i := range s.ServiceIntegrations {
+		if i.IntegrationType == service.IntegrationTypeAutoscaler {
+			isAutoscalerEnabled = true
+			break
+		}
+	}
+
+	k := "additional_disk_space"
+
+	// There are three possible sources of additional_disk_space value:
+	// 1. It is explicitly set in config file
+	// 2. Computed: disk_space - plan.disk_space = additional_disk_space
+	// 3. Computed: autoscaler is enabled, so additional_disk_space is managed by the autoscaler
+	if HasConfigValue(diff, k) || HasConfigValue(diff, "disk_space") {
+		if isAutoscalerEnabled {
+			// Autoscaler is enabled, so we cannot set additional_disk_space
+			return ErrAutoscalerConflict
+		}
+
+		// It is in the config file, lets TF handle it
+		return nil
+	}
+
+	if isAutoscalerEnabled {
+		// If the autoscaler is enabled, we don't need to manage the field,
+		// it will change its value automatically
+		return nil
+	}
+
+	// It is not set but has a value (ShouldNotBeEmpty proves it has).
+	// That means the value is being removed.
+	// We must output a diff for the computed field,
+	// which otherwise TF will suppress it
+	return diff.SetNew(k, "0B")
+}

internal/schemautil/helpers.go

@@ -8,15 +8,22 @@ import (
 	"github.com/aiven/aiven-go-client/v2"
 	"github.com/aiven/go-client-codegen/handler/service"
 	"github.com/docker/go-units"
+	"github.com/hashicorp/go-cty/cty"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 )
 
 // ResourceStateOrResourceDiff either *schema.ResourceState or *schema.ResourceDiff
 type ResourceStateOrResourceDiff interface {
+	GetRawConfig() cty.Value
 	GetOk(key string) (interface{}, bool)
 	Get(key string) interface{}
 }
 
+func HasConfigValue(d ResourceStateOrResourceDiff, key string) bool {
+	c := d.GetRawConfig()
+	return !(c.IsNull() || c.AsValueMap()[key].IsNull())
+}
+
 // PlanParameters service plan aparameters
 type PlanParameters struct {
 	DiskSizeMBDefault int

internal/schemautil/service.go

@@ -56,8 +56,6 @@ const (
 	ServiceTypeValkey           = "valkey"
 )
 
-var ErrAutoscalerConflict = errors.New("autoscaler integration is enabled, additional_disk_space cannot be set")
-
 var TechEmailsResourceSchema = &schema.Resource{
 	Schema: map[string]*schema.Schema{
 		"email": {
@@ -474,13 +472,6 @@ func ResourceServiceUpdate(ctx context.Context, d *schema.ResourceData, m interf
 		}
 	}
 
-	// On service update, we send a default disc space value for a common
-	// if the TF user does not specify it
-	diskSpace, err := getDiskSpaceFromStateOrDiff(ctx, d, client)
-	if err != nil {
-		return diag.Errorf("error getting default disc space: %s", err)
-	}
-
 	projectName, serviceName, err := SplitResourceID2(d.Id())
 	if err != nil {
 		return diag.Errorf("error splitting service id (%s): %s", d.Id(), err)
@@ -516,26 +507,31 @@ func ResourceServiceUpdate(ctx context.Context, d *schema.ResourceData, m interf
 		return diag.FromErr(err)
 	}
 
-	// Note: Only service_integrations are needed here, but no specific API exists yet.
-	s, err := avnGen.ServiceGet(ctx, projectName, serviceName, service.ServiceGetIncludeSecrets(true))
-	if err != nil {
-		return diag.Errorf("unable to GET service %s: %s", d.Id(), err)
-	}
-
-	autoscalerIntegrations := getIntegrationsForTerraform(s.ServiceIntegrations, service.IntegrationTypeAutoscaler)
-	if _, ok := d.GetOk("additional_disk_space"); ok && len(autoscalerIntegrations) > 0 {
-		return diag.FromErr(ErrAutoscalerConflict)
-	}
-
 	cloud := d.Get("cloud_name").(string)
 	plan := d.Get("plan").(string)
 	powered := true
 	terminationProtection := d.Get("termination_protection").(bool)
 
+	// Sends disk size only when there is no scaler enabled
 	var diskSpaceMb *int
-	if diskSpace > 0 {
-		diskSpaceMb = &diskSpace
+	s, err := avnGen.ServiceGet(ctx, projectName, serviceName)
+	if err != nil {
+		return nil
+	}
+
+	if len(getIntegrationsForTerraform(s.ServiceIntegrations, service.IntegrationTypeAutoscaler)) == 0 {
+		// On service update, we send a default disc space value for a common
+		// if the TF user does not specify it
+		diskSpace, err := getDiskSpaceFromStateOrDiff(ctx, d, client)
+		if err != nil {
+			return diag.Errorf("error getting default disc space: %s", err)
+		}
+
+		if diskSpace > 0 {
+			diskSpaceMb = &diskSpace
+		}
 	}
+
 	serviceUpdate := &service.ServiceUpdateIn{
 		Cloud:                 &cloud,
 		Plan:                  &plan,
@@ -578,27 +574,28 @@ func ResourceServiceUpdate(ctx context.Context, d *schema.ResourceData, m interf
 	return ResourceServiceRead(ctx, d, m)
 }
 
+// getDiskSpaceFromStateOrDiff three cases:
+// 1. disk_space is set
+// 2. plan disk space
+// 3. plan disk space + additional_disk_space
 func getDiskSpaceFromStateOrDiff(ctx context.Context, d ResourceStateOrResourceDiff, client *aiven.Client) (int, error) {
-	var diskSpace int
+	if v, ok := d.GetOk("disk_space"); ok {
+		return ConvertToDiskSpaceMB(v.(string)), nil
+	}
 
 	// Get service plan specific defaults
-	servicePlanParams, err := GetServicePlanParametersFromSchema(ctx, client, d)
+	plan, err := GetServicePlanParametersFromSchema(ctx, client, d)
 	if err != nil {
 		if aiven.IsNotFound(err) {
 			return 0, nil
 		}
 		return 0, fmt.Errorf("unable to get service plan parameters: %w", err)
 	}
 
-	// Use `additional_disk_space` if set
-	if ads, ok := d.GetOk("additional_disk_space"); ok {
-		diskSpace = servicePlanParams.DiskSizeMBDefault + ConvertToDiskSpaceMB(ads.(string))
-	} else if ds, ok := d.GetOk("disk_space"); ok {
-		// Use `disk_space` if set...
-		diskSpace = ConvertToDiskSpaceMB(ds.(string))
-	} else {
-		// ... otherwise, use the default disk space
-		diskSpace = servicePlanParams.DiskSizeMBDefault
+	// Adds additional_disk_space only if it is in the config
+	diskSpace := plan.DiskSizeMBDefault
+	if HasConfigValue(d, "additional_disk_space") {
+		diskSpace += ConvertToDiskSpaceMB(d.Get("additional_disk_space").(string))
 	}
 
 	return diskSpace, nil
@@ -691,31 +688,17 @@ func copyServicePropertiesFromAPIResponseToTerraform(
 	if s.DiskSpaceMb != nil {
 		diskSpace = *s.DiskSpaceMb
 	}
-	additionalDiskSpace := diskSpace - servicePlanParams.DiskSizeMBDefault
-
-	_, isAdditionalDiskSpaceSet := d.GetOk("additional_disk_space")
-	_, isDiskSpaceSet := d.GetOk("disk_space")
 
-	// Handles two different cases:
-	//
-	// 1. During import when neither `additional_disk_space` nor `disk_space` are set
-	// 2. During create / update when `additional_disk_space` is set
-	if additionalDiskSpace > 0 && (!isDiskSpaceSet || isAdditionalDiskSpaceSet) {
-		if err := d.Set("additional_disk_space", HumanReadableByteSize(additionalDiskSpace*units.MiB)); err != nil {
-			return err
-		}
-		if err := d.Set("disk_space", nil); err != nil {
-			return err
-		}
+	additionalDiskSpace := diskSpace - servicePlanParams.DiskSizeMBDefault
+	if err := d.Set("additional_disk_space", HumanReadableByteSize(additionalDiskSpace*units.MiB)); err != nil {
+		return err
 	}
 
+	_, isDiskSpaceSet := d.GetOk("disk_space")
 	if isDiskSpaceSet && diskSpace > 0 {
 		if err := d.Set("disk_space", HumanReadableByteSize(diskSpace*units.MiB)); err != nil {
 			return err
 		}
-		if err := d.Set("additional_disk_space", nil); err != nil {
-			return err
-		}
 	}
 
 	if err := d.Set("disk_space_used", HumanReadableByteSize(diskSpace*units.MiB)); err != nil {

internal/sdkprovider/service/pg/pg_test.go

@@ -265,7 +265,7 @@ func TestAccAivenPG_deleting_additional_disk_size(t *testing.T) {
 					resource.TestCheckResourceAttr(resourceName, "maintenance_window_dow", "monday"),
 					resource.TestCheckResourceAttr(resourceName, "maintenance_window_time", "10:00:00"),
 					resource.TestCheckResourceAttr(resourceName, "disk_space_default", "80GiB"),
-					resource.TestCheckResourceAttr(resourceName, "disk_space_used", "100GiB"),
+					resource.TestCheckResourceAttr(resourceName, "disk_space_used", "80GiB"),
 					resource.TestCheckResourceAttr(resourceName, "termination_protection", "false"),
 				),
 			},