Skip to content

Commit

Permalink
chore(constraints): ensure constraints are non-nil
Browse files Browse the repository at this point in the history
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 "".
  • Loading branch information
jack-w-shaw committed Aug 22, 2024
1 parent 16dee9a commit 4a47025
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 3 deletions.
5 changes: 4 additions & 1 deletion internal/provider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,10 @@ func testAccPreCheck(t *testing.T) {
}
}
confResp := configureProvider(t, Provider)
assert.Equal(t, confResp.Diagnostics.HasError(), false)
errs := confResp.Diagnostics.Errors()
if len(errs) > 0 {
t.Fatalf("provider configuration failed: %v", errs)
}
testClient, ok := confResp.ResourceData.(*juju.Client)
assert.Truef(t, ok, "ResourceData, not of type juju client")
TestClient = testClient
Expand Down
16 changes: 14 additions & 2 deletions internal/provider/resource_application.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
70 changes: 70 additions & 0 deletions internal/provider/resource_application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package provider

import (
"context"
"fmt"
"os"
"regexp"
Expand All @@ -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"
)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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" {
Expand Down

0 comments on commit 4a47025

Please sign in to comment.