From 99148bc9855a100e07718d78e77e8fe2d3018d2c Mon Sep 17 00:00:00 2001 From: Jack Shaw Date: Thu, 22 Aug 2024 14:04:45 +0100 Subject: [PATCH] chore(constraints): ensure constraints are non-nil Fixes: https://github.com/juju/terraform-provider-juju/issues/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/resource_application.go | 16 +++- .../provider/resource_application_test.go | 87 +++++++++++++++++++ 2 files changed, 101 insertions(+), 2 deletions(-) diff --git a/internal/provider/resource_application.go b/internal/provider/resource_application.go index cf885ad0..90afc771 100644 --- a/internal/provider/resource_application.go +++ b/internal/provider/resource_application.go @@ -578,7 +578,15 @@ 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 - plan.Constraints = types.StringValue(readResp.Constraints.String()) + + // Constraints do not apply to subordinate applications. However, whilst juju + // may not distinguish between null constraints and empty-string constraints, + // terraform does not consider null to be a computed value. + if readResp.Principal { + plan.Constraints = types.StringValue(readResp.Constraints.String()) + } else { + plan.Constraints = types.StringValue("") + } plan.Placement = types.StringValue(readResp.Placement) plan.Principal = types.BoolNull() plan.ApplicationName = types.StringValue(createResp.AppName) @@ -708,9 +716,13 @@ func (r *applicationResource) Read(ctx context.Context, req resource.ReadRequest return } - // constraints do not apply to subordinate applications. + // Constraints do not apply to subordinate applications. However, whilst juju + // may not distinguish between null constraints and empty-string constraints, + // terraform does not consider null to be a computed value. if response.Principal { state.Constraints = types.StringValue(response.Constraints.String()) + } else { + state.Constraints = types.StringValue("") } exposeType := req.State.Schema.GetBlocks()[ExposeKey].(schema.ListNestedBlock).NestedObject.Type() if response.Expose != nil { diff --git a/internal/provider/resource_application_test.go b/internal/provider/resource_application_test.go index 93066573..5cbdf21e 100644 --- a/internal/provider/resource_application_test.go +++ b/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,75 @@ 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: "ubuntu", + ModelName: modelName, + CharmName: "ubuntu-lite", + Units: 1, + }) + if err != nil { + t.Fatal(err) + } + + _, err = TestClient.Applications.CreateApplication(ctx, &juju.CreateApplicationInput{ + ApplicationName: "telegraf", + ModelName: modelName, + CharmName: "telegraf", + CharmRevision: 73, + Units: 1, + }) + if err != nil { + t.Fatal(err) + } + + _, err = TestClient.Integrations.CreateIntegration(&juju.IntegrationInput{ + ModelName: modelName, + Apps: []string{"ubuntu", "telegraf"}, + }) + 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 +872,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" {