fix(constraints): ensure constraints are non-nil

Fixes: #466

Juju does not distinguish between null constraints and empty-string
constraints. However, null is not considered by terraform to be a
computed value, whereas the empty string is.

Since the constraints schema marks constraints as a computed field, this
means if we insert null into state it will be considered uncomputed & on
the next apply terraform will attempt to compute the constraints by
setting them.

This leads to bugs since subordinate applications cannot have
constraints applied.

Since we know that subordinate applications cannot have constraints, on
Create or Read we can easily 'compute' the constraints ourselves as "".

internal/provider/provider_test.go

@@ -5,6 +5,7 @@ package provider
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"runtime"
 	"testing"
@@ -17,6 +18,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/types"
 	"github.com/hashicorp/terraform-plugin-go/tfprotov6"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/juju/terraform-provider-juju/internal/juju"
 )
@@ -172,9 +174,9 @@ func testAccPreCheck(t *testing.T) {
 		}
 	}
 	confResp := configureProvider(t, Provider)
-	assert.Equal(t, confResp.Diagnostics.HasError(), false)
+	require.Equal(t, confResp.Diagnostics.HasError(), false, fmt.Sprintf("provider configuration failed: %v", confResp.Diagnostics.Errors()))
 	testClient, ok := confResp.ResourceData.(*juju.Client)
-	assert.Truef(t, ok, "ResourceData, not of type juju client")
+	require.Truef(t, ok, "ResourceData, not of type juju client")
 	TestClient = testClient
 }
 

internal/provider/resource_application.go

@@ -578,6 +578,9 @@ func (r *applicationResource) Create(ctx context.Context, req resource.CreateReq
 	r.trace(fmt.Sprintf("read application resource %q", createResp.AppName))
 
 	// Save plan into Terraform state
+
+	// Constraints do not apply to subordinate applications. If the application
+	// is subordinate, the constraints will be set to the empty string.
 	plan.Constraints = types.StringValue(readResp.Constraints.String())
 	plan.Placement = types.StringValue(readResp.Placement)
 	plan.Principal = types.BoolNull()
@@ -688,6 +691,7 @@ func (r *applicationResource) Read(ctx context.Context, req resource.ReadRequest
 	state.ModelName = types.StringValue(modelName)
 
 	// Use the response to fill in state
+
 	state.Placement = types.StringValue(response.Placement)
 	state.Principal = types.BoolNull()
 	state.UnitCount = types.Int64Value(int64(response.Units))
@@ -708,10 +712,10 @@ func (r *applicationResource) Read(ctx context.Context, req resource.ReadRequest
 		return
 	}
 
-	// constraints do not apply to subordinate applications.
-	if response.Principal {
-		state.Constraints = types.StringValue(response.Constraints.String())
-	}
+	// Constraints do not apply to subordinate applications. If the application
+	// is subordinate, the constraints will be set to the empty string.
+	state.Constraints = types.StringValue(response.Constraints.String())
+
 	exposeType := req.State.Schema.GetBlocks()[ExposeKey].(schema.ListNestedBlock).NestedObject.Type()
 	if response.Expose != nil {
 		exp := parseNestedExpose(response.Expose)
@@ -986,7 +990,11 @@ func (r *applicationResource) Update(ctx context.Context, req resource.UpdateReq
 		}
 	}
 
-	if !plan.Constraints.Equal(state.Constraints) {
+	// Do not use .Equal() here as we should consider null constraints the same
+	// as empty-string constraints. Terraform considers them different, so will
+	// incorrectly attempt to update the constraints, which can cause trouble
+	// for subordinate applications.
+	if plan.Constraints.ValueString() != state.Constraints.ValueString() {
 		appConstraints, err := constraints.Parse(plan.Constraints.ValueString())
 		if err != nil {
 			resp.Diagnostics.AddError("Conversion", fmt.Sprintf("Unable to parse plan constraints, got error: %s", err))

internal/provider/resource_application_test.go

@@ -4,6 +4,7 @@
 package provider
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"regexp"
@@ -20,6 +21,7 @@ import (
 	"github.com/juju/juju/rpc/params"
 	"github.com/juju/names/v5"
 
+	"github.com/juju/terraform-provider-juju/internal/juju"
 	internaljuju "github.com/juju/terraform-provider-juju/internal/juju"
 	internaltesting "github.com/juju/terraform-provider-juju/internal/testing"
 )
@@ -156,6 +158,58 @@ func TestAcc_ResourceApplication_Updates(t *testing.T) {
 	})
 }
 
+func TestAcc_ResourceApplication_UpdateImportedSubordinate(t *testing.T) {
+	if testingCloud != LXDCloudTesting {
+		t.Skip(t.Name() + " only runs with LXD")
+	}
+
+	testAccPreCheck(t)
+
+	modelName := acctest.RandomWithPrefix("tf-test-application")
+
+	ctx := context.Background()
+
+	_, err := TestClient.Models.CreateModel(juju.CreateModelInput{
+		Name: modelName,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = TestClient.Applications.CreateApplication(ctx, &juju.CreateApplicationInput{
+		ApplicationName: "telegraf",
+		ModelName:       modelName,
+		CharmName:       "telegraf",
+		CharmChannel:    "latest/stable",
+		CharmRevision:   73,
+		Units:           0,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { testAccPreCheck(t) },
+		ProtoV6ProviderFactories: frameworkProviderFactories,
+		Steps: []resource.TestStep{
+			{
+				Config:             testAccResourceApplicationSubordinate(modelName, 73),
+				ImportState:        true,
+				ImportStateId:      fmt.Sprintf("%s:telegraf", modelName),
+				ImportStatePersist: true,
+				ResourceName:       "juju_application.telegraf",
+			},
+			{
+				Config: testAccResourceApplicationSubordinate(modelName, 75),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("juju_application.telegraf", "charm.0.name", "telegraf"),
+					resource.TestCheckResourceAttr("juju_application.telegraf", "charm.0.revision", "75"),
+				),
+			},
+		},
+	})
+}
+
 // TestAcc_ResourceApplication_UpdatesRevisionConfig will test the revision update that have new config parameters on
 // the charm. The test will check that the config is updated and the revision is updated as well.
 func TestAcc_ResourceApplication_UpdatesRevisionConfig(t *testing.T) {
@@ -801,6 +855,22 @@ resource "juju_application" "this" {
 	}
 }
 
+func testAccResourceApplicationSubordinate(modelName string, subordinateRevision int) string {
+	return fmt.Sprintf(`
+resource "juju_application" "telegraf" {
+  model = %q
+  name = "telegraf"
+
+  charm {
+    name = "telegraf"
+    revision = %d
+  }
+
+  units = 0
+}
+`, modelName, subordinateRevision)
+}
+
 func testAccResourceApplicationConstraintsSubordinate(modelName string, constraints string) string {
 	return fmt.Sprintf(`
 resource "juju_model" "this" {