feat: tweaks to error messages and docstrings

- Remove globals for isJAAS placing them on the sharedClient struct instead.
- Improved error messages when Juju client calls fail.
- Use template strings in tests.

internal/juju/client.go

@@ -78,6 +78,9 @@ type sharedClient struct {
 
 	// subCtx is the context created with the new tflog subsystem for applications.
 	subCtx context.Context
+
+	checkJAASOnce sync.Once
+	isJAAS        bool
 }
 
 // NewClient returns a client which can talk to the juju controller

internal/provider/expect_recreated_resource_test.go

@@ -38,7 +38,7 @@ func (e expectRecreatedResource) CheckPlan(ctx context.Context, req plancheck.Ch
 	resp.Error = errors.Join(result...)
 }
 
-// expectRecreatedResource returns a plan check that asserts is a delete and create change present.
+// expectRecreatedResource returns a plan check that asserts a delete and create change are present.
 // All output and resource changes found will be aggregated and returned in a plan check error.
 func ExpectRecreatedResource(resourceName string) plancheck.PlanCheck {
 	return expectRecreatedResource{

internal/provider/resource_access_generic.go

@@ -24,8 +24,8 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/types"
 	"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
 	"github.com/hashicorp/terraform-plugin-log/tflog"
-
 	"github.com/juju/names/v5"
+
 	"github.com/juju/terraform-provider-juju/internal/juju"
 )
 
@@ -182,7 +182,7 @@ func (resource *genericJAASAccessResource) Create(ctx context.Context, req resou
 	// Make a call to create relations
 	err := resource.client.Jaas.AddRelations(tuples)
 	if err != nil {
-		resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create access relationships, got error: %s", err))
+		resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create access relationships for %s, got error: %s", targetTag.String(), err))
 		return
 	}
 	// Set the plan onto the Terraform state
@@ -196,7 +196,7 @@ func (resource *genericJAASAccessResource) Read(ctx context.Context, req resourc
 		addClientNotConfiguredError(&resp.Diagnostics, resource.resourceLogName, "read")
 		return
 	}
-	// Read Terraform configuration from the request into the plan
+	// Read Terraform configuration from the request into the resource model
 	state, targetTag := resource.info(ctx, req.State, &resp.Diagnostics)
 	if resp.Diagnostics.HasError() {
 		return
@@ -209,7 +209,7 @@ func (resource *genericJAASAccessResource) Read(ctx context.Context, req resourc
 	}
 	tuples, err := resource.client.Jaas.ReadRelations(ctx, &readTuple)
 	if err != nil {
-		resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to read access rules, got error: %s", err))
+		resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to read access rules for %s, got error: %s", targetTag.String(), err))
 		return
 	}
 	// Transform the tuples into an access model
@@ -266,7 +266,7 @@ func (resource *genericJAASAccessResource) Update(ctx context.Context, req resou
 	if len(addTuples) > 0 {
 		err := resource.client.Jaas.AddRelations(addTuples)
 		if err != nil {
-			resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to add access rules, got error: %s", err))
+			resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to add access rules for %s, got error: %s", targetTag.String(), err))
 			return
 		}
 	}
@@ -278,7 +278,7 @@ func (resource *genericJAASAccessResource) Update(ctx context.Context, req resou
 	if len(removeTuples) > 0 {
 		err := resource.client.Jaas.DeleteRelations(removeTuples)
 		if err != nil {
-			resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to remove access rules, got error: %s", err))
+			resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to remove access rules for %s, got error: %s", targetTag.String(), err))
 			return
 		}
 	}
@@ -307,11 +307,12 @@ func diffModels(plan, state genericJAASAccessModel, diag *diag.Diagnostics) (toA
 	return
 }
 
-func diffSet(current, desired basetypes.SetValue, diag *diag.Diagnostics) basetypes.SetValue {
+// diffSet returns the elements in the target set that are not present in the current set.
+func diffSet(current, target basetypes.SetValue, diag *diag.Diagnostics) basetypes.SetValue {
 	var diff []attr.Value
 	for _, source := range current.Elements() {
 		found := false
-		for _, target := range desired.Elements() {
+		for _, target := range target.Elements() {
 			if source.Equal(target) {
 				found = true
 			}
@@ -325,7 +326,7 @@ func diffSet(current, desired basetypes.SetValue, diag *diag.Diagnostics) basety
 	return newSet
 }
 
-// Delete defines how tuples for access control will be updated.
+// Delete defines how tuples for access control will be deleted.
 func (resource *genericJAASAccessResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {
 	// Check first if the client is configured
 	if resource.client == nil {
@@ -347,7 +348,7 @@ func (resource *genericJAASAccessResource) Delete(ctx context.Context, req resou
 	// Delete the tuples
 	err := resource.client.Jaas.DeleteRelations(tuples)
 	if err != nil {
-		resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to delete access rules, got error: %s", err))
+		resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to delete access rules for %s, got error: %s", targetTag.String(), err))
 		return
 	}
 }

internal/provider/resource_access_jaas_model_test.go

@@ -1,4 +1,4 @@
-// Copyright 2023 Canonical Ltd.
+// Copyright 2024 Canonical Ltd.
 // Licensed under the Apache License, Version 2.0, see LICENCE file for details.
 
 package provider
@@ -15,6 +15,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-testing/plancheck"
 	"github.com/hashicorp/terraform-plugin-testing/terraform"
 	"github.com/juju/names/v5"
+	internaltesting "github.com/juju/terraform-provider-juju/internal/testing"
 )
 
 func TestAcc_ResourceJaasAccessModel(t *testing.T) {
@@ -255,56 +256,88 @@ func TestAcc_ResourceJaasAccessModelServiceAccountAndUsers(t *testing.T) {
 // }
 
 func testAccResourceJaasAccessModelTwoUsers(modelName, access, userOne, userTwo string) string {
-	return fmt.Sprintf(`
+	return internaltesting.GetStringFromTemplateWithData(
+		"testAccResourceJaasAccessModelTwoUsers",
+		`
 resource "juju_model" "test-model" {
-  name = %q
+  name = "{{.ModelName}}"
 }
 
 resource "juju_jaas_access_model" "test" {
-  model_uuid   = juju_model.test-model.id
-  access       = %q
-  users        = [%q, %q]
-}`, modelName, access, userOne, userTwo)
+  model_uuid          = juju_model.test-model.id
+  access              = "{{.Access}}"
+  users               = ["{{.UserOne}}", "{{.UserTwo}}"]
+  service_accounts    = ["{{.SvcAccOne}}", "{{.SvcAccTwo}}"]
+}
+`, internaltesting.TemplateData{
+			"ModelName": modelName,
+			"Access":    access,
+			"UserOne":   userOne,
+			"UserTwo":   userTwo,
+		})
 }
 
-func testAccResourceJaasAccessModelOneUser(modelName, access, userOne string) string {
-	return fmt.Sprintf(`
+func testAccResourceJaasAccessModelOneUser(modelName, access, user string) string {
+	return internaltesting.GetStringFromTemplateWithData(
+		"testAccResourceJaasAccessModelOneUser",
+		`
 resource "juju_model" "test-model" {
-  name = %q
+  name = "{{.ModelName}}"
 }
 
 resource "juju_jaas_access_model" "test" {
-  model_uuid   = juju_model.test-model.id
-  access       = %q
-  users        = [%q]
-}`, modelName, access, userOne)
+  model_uuid          = juju_model.test-model.id
+  access              = "{{.Access}}"
+  users               = ["{{.User}}"]
+}
+`, internaltesting.TemplateData{
+			"ModelName": modelName,
+			"Access":    access,
+			"User":      user,
+		})
 }
 
-func testAccResourceJaasAccessModelOneSvcAccount(modelName, access, svcAccOne string) string {
-	return fmt.Sprintf(`
+func testAccResourceJaasAccessModelOneSvcAccount(modelName, access, svcAcc string) string {
+	return internaltesting.GetStringFromTemplateWithData(
+		"testAccResourceJaasAccessModelOneSvcAccount",
+		`
 resource "juju_model" "test-model" {
-  name = %q
+  name = "{{.ModelName}}"
 }
 
 resource "juju_jaas_access_model" "test" {
   model_uuid          = juju_model.test-model.id
-  access              = %q
-  service_accounts    = [%q]
-}`, modelName, access, svcAccOne)
+  access              = "{{.Access}}"
+  service_accounts    = ["{{.SvcAcc}}", "{{.SvcAcc}}"]
+}
+`, internaltesting.TemplateData{
+			"ModelName": modelName,
+			"Access":    access,
+			"SvcAcc":    svcAcc,
+		})
 }
 
 func testAccResourceJaasAccessModelSvcAccsAndUser(modelName, access, user, svcAccOne, svcAccTwo string) string {
-	return fmt.Sprintf(`
+	return internaltesting.GetStringFromTemplateWithData(
+		"testAccResourceJaasAccessModelSvcAccsAndUser",
+		`
 resource "juju_model" "test-model" {
-  name = %q
+  name = "{{.ModelName}}"
 }
 
 resource "juju_jaas_access_model" "test" {
   model_uuid          = juju_model.test-model.id
-  access              = %q
-  users               = [%q]
-  service_accounts    = [%q, %q]
-}`, modelName, access, user, svcAccOne, svcAccTwo)
+  access              = "{{.Access}}"
+  users               = ["{{.User}}"]
+  service_accounts    = ["{{.SvcAccOne}}", "{{.SvcAccTwo}}"]
+}
+`, internaltesting.TemplateData{
+			"ModelName": modelName,
+			"Access":    access,
+			"User":      user,
+			"SvcAccOne": svcAccOne,
+			"SvcAccTwo": svcAccTwo,
+		})
 }
 
 func testAccCheckModelUUIDNotEmpty(resourceName string, modelUUID *string) resource.TestCheckFunc {