fix: fix tests

internal/juju/offers.go

@@ -403,6 +403,7 @@ func (c offersClient) RemoveRemoteOffer(input *RemoveRemoteOfferInput) []error {
 }
 
 // GrantOffer adds access to an offer managed by the access offer resource.
+// No action or error if the access was already granted to the user.
 func (c offersClient) GrantOffer(input *GrantRevokeOfferInput) error {
 	conn, err := c.GetConnection(nil)
 	if err != nil {
@@ -431,6 +432,7 @@ func (c offersClient) GrantOffer(input *GrantRevokeOfferInput) error {
 }
 
 // RevokeOffer revokes access to an offer managed by the access offer resource.
+// No action or error if the access was already revoked for the user.
 func (c offersClient) RevokeOffer(input *GrantRevokeOfferInput) error {
 	conn, err := c.GetConnection(nil)
 	if err != nil {

internal/provider/resource_access_offer.go

@@ -162,12 +162,12 @@ func (a *accessOfferResource) Create(ctx context.Context, req resource.CreateReq
 	}
 
 	// Call Offers.GrantOffer
-	users := make(map[permission.Access][]string)
-	users[permission.ConsumeAccess] = consumeUsers
-	users[permission.ReadAccess] = readUsers
-	users[permission.AdminAccess] = adminUsers
+	totalUsers := make(map[permission.Access][]string)
+	totalUsers[permission.ConsumeAccess] = consumeUsers
+	totalUsers[permission.ReadAccess] = readUsers
+	totalUsers[permission.AdminAccess] = adminUsers
 
-	for access, users := range users {
+	for access, users := range totalUsers {
 		err := a.client.Offers.GrantOffer(&juju.GrantRevokeOfferInput{
 			Users:    users,
 			Access:   string(access),
@@ -231,28 +231,34 @@ func (a *accessOfferResource) Read(ctx context.Context, req resource.ReadRequest
 
 		users[offerUserDetail.Access] = append(users[offerUserDetail.Access], offerUserDetail.UserName)
 	}
-
+	a.trace(fmt.Sprintf("read juju offer response %q", response))
 	// Save admin users to state
-	adminUsersSet, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, users[permission.AdminAccess])
-	resp.Diagnostics.Append(errDiag...)
-	if resp.Diagnostics.HasError() {
-		return
+	if len(users[permission.AdminAccess]) > 0 {
+		adminUsersSet, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, users[permission.AdminAccess])
+		resp.Diagnostics.Append(errDiag...)
+		if resp.Diagnostics.HasError() {
+			return
+		}
+		state.AdminUsers = adminUsersSet
 	}
-	state.AdminUsers = adminUsersSet
 	// Save consume users to state
-	consumeUsersSet, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, users[permission.ConsumeAccess])
-	resp.Diagnostics.Append(errDiag...)
-	if resp.Diagnostics.HasError() {
-		return
+	if len(users[permission.ConsumeAccess]) > 0 {
+		consumeUsersSet, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, users[permission.ConsumeAccess])
+		resp.Diagnostics.Append(errDiag...)
+		if resp.Diagnostics.HasError() {
+			return
+		}
+		state.ConsumeUsers = consumeUsersSet
 	}
-	state.ConsumeUsers = consumeUsersSet
 	// Save read users to state
-	readUsersSet, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, users[permission.ReadAccess])
-	resp.Diagnostics.Append(errDiag...)
-	if resp.Diagnostics.HasError() {
-		return
+	if len(users[permission.ReadAccess]) > 0 {
+		readUsersSet, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, users[permission.ReadAccess])
+		resp.Diagnostics.Append(errDiag...)
+		if resp.Diagnostics.HasError() {
+			return
+		}
+		state.ReadUsers = readUsersSet
 	}
-	state.ReadUsers = readUsersSet
 	// Set the plan onto the Terraform state
 	state.OfferURL = types.StringValue(offerURL)
 	resp.Diagnostics.Append(resp.State.Set(ctx, &state)...)
@@ -317,14 +323,14 @@ func (a *accessOfferResource) Update(ctx context.Context, req resource.UpdateReq
 	//                bob's permission will be revoked (revoke read) in the last update step
 
 	// scenario 1 - check for users added or moved
-	var adminStateUsers []string
-	if !state.AdminUsers.IsNull() {
-		resp.Diagnostics.Append(state.AdminUsers.ElementsAs(ctx, &adminStateUsers, false)...)
+	var readStateUsers []string
+	if !state.ReadUsers.IsNull() {
+		resp.Diagnostics.Append(state.ReadUsers.ElementsAs(ctx, &readStateUsers, false)...)
 		if resp.Diagnostics.HasError() {
 			return
 		}
 	}
-	err = processPermissionChanges(plan.OfferURL.ValueString(), string(permission.AdminAccess), adminUsers, adminStateUsers, a.client)
+	err = processPermissionChanges(plan.OfferURL.ValueString(), string(permission.ReadAccess), readUsers, readStateUsers, a.client)
 	if err != nil {
 		resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update offer access %s, got error: %s", state.ID.ValueString(), err))
 		return
@@ -343,14 +349,14 @@ func (a *accessOfferResource) Update(ctx context.Context, req resource.UpdateReq
 		return
 	}
 
-	var readStateUsers []string
-	if !state.ReadUsers.IsNull() {
-		resp.Diagnostics.Append(state.ReadUsers.ElementsAs(ctx, &readStateUsers, false)...)
+	var adminStateUsers []string
+	if !state.AdminUsers.IsNull() {
+		resp.Diagnostics.Append(state.AdminUsers.ElementsAs(ctx, &adminStateUsers, false)...)
 		if resp.Diagnostics.HasError() {
 			return
 		}
 	}
-	err = processPermissionChanges(plan.OfferURL.ValueString(), string(permission.ReadAccess), readUsers, readStateUsers, a.client)
+	err = processPermissionChanges(plan.OfferURL.ValueString(), string(permission.AdminAccess), adminUsers, adminStateUsers, a.client)
 	if err != nil {
 		resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update offer access %s, got error: %s", state.ID.ValueString(), err))
 		return
@@ -400,7 +406,60 @@ func (a *accessOfferResource) Update(ctx context.Context, req resource.UpdateReq
 
 // Delete remove access to the offer according to the resource.
 func (a *accessOfferResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {
-	// todo
+	// Check first if the client is configured
+	if a.client == nil {
+		addClientNotConfiguredError(&resp.Diagnostics, "access offer", "read")
+		return
+	}
+	var plan accessOfferResourceOffer
+
+	// Get the Terraform state from the request into the plan
+	resp.Diagnostics.Append(req.State.Get(ctx, &plan)...)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
+	// Get the users to grant admin
+	var adminUsers []string
+	if !plan.AdminUsers.IsNull() {
+		resp.Diagnostics.Append(plan.AdminUsers.ElementsAs(ctx, &adminUsers, false)...)
+		if resp.Diagnostics.HasError() {
+			return
+		}
+	}
+
+	// Get the users to grant consume
+	var consumeUsers []string
+	if !plan.ConsumeUsers.IsNull() {
+		resp.Diagnostics.Append(plan.ConsumeUsers.ElementsAs(ctx, &consumeUsers, false)...)
+		if resp.Diagnostics.HasError() {
+			return
+		}
+	}
+
+	// Get the users to grant read
+	var readUsers []string
+	if !plan.ReadUsers.IsNull() {
+		resp.Diagnostics.Append(plan.ReadUsers.ElementsAs(ctx, &readUsers, false)...)
+		if resp.Diagnostics.HasError() {
+			return
+		}
+	}
+
+	totalPlanUsers := append(adminUsers, consumeUsers...)
+	totalPlanUsers = append(totalPlanUsers, readUsers...)
+
+	// Revoking against "read" guarantees that the entire access will be removed
+	// instead of only decreasing the access level.
+	err := a.client.Offers.RevokeOffer(&juju.GrantRevokeOfferInput{
+		Users:    totalPlanUsers,
+		Access:   string(permission.ReadAccess),
+		OfferURL: plan.OfferURL.ValueString(),
+	})
+	if err != nil {
+		resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to destroy access offer resource, got error: %s", err))
+		return
+	}
 }
 
 // Configure sets the access offer resource with provider data.
@@ -449,6 +508,7 @@ func (a *accessOfferResource) trace(msg string, additionalFields ...map[string]i
 	tflog.SubsystemTrace(a.subCtx, LogResourceAccessOffer, msg, additionalFields...)
 }
 
+// Helpers
 func validateNoOverlaps(admin, consume, read []string) error {
 	sets := map[string]struct{}{}
 	for _, v := range consume {

internal/provider/resource_access_offer_test.go

@@ -15,44 +15,54 @@ import (
 
 func TestAcc_ResourceAccessOffer(t *testing.T) {
 	SkipJAAS(t)
-	userName := acctest.RandomWithPrefix("tfuser")
+	AdminUserName := acctest.RandomWithPrefix("tfadminuser")
+	ConsumeUserName := acctest.RandomWithPrefix("tfconsumeuser")
+	ReadUserName := acctest.RandomWithPrefix("tfreaduser")
 	userPassword := acctest.RandomWithPrefix("tf-test-user")
 	modelName := acctest.RandomWithPrefix("tf-access-model")
 	offerURL := fmt.Sprintf("admin/%s.appone", modelName)
-	access := "consume"
-	newAccess := "admin"
-	accessFail := "bogus"
 
 	resourceName := "juju_access_offer.access_appone_endpoint"
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:                 func() { testAccPreCheck(t) },
 		ProtoV6ProviderFactories: frameworkProviderFactories,
 		Steps: []resource.TestStep{
-			{ // Test access name validation
-				Config:      testAccResourceAccessOffer(userName, userPassword, modelName, accessFail),
-				ExpectError: regexp.MustCompile("Invalid Attribute Value Match.*"),
+			{ // Test username overlap validation
+				Config:      testAccResourceAccessOffer(AdminUserName, ConsumeUserName, ReadUserName, "admin", "admin", "", userPassword, modelName),
+				ExpectError: regexp.MustCompile("appears in.*"),
 			},
-			{ // Create the resource
-				Config: testAccResourceAccessOffer(userName, userPassword, modelName, access),
+			{ // Create the resource with user as admin
+				Config: testAccResourceAccessOffer(AdminUserName, ConsumeUserName, ReadUserName, "admin", "consume", "read", userPassword, modelName),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(resourceName, "offer_url", offerURL),
-					resource.TestCheckResourceAttr(resourceName, "access", access),
-					resource.TestCheckTypeSetElemAttr(resourceName, "users.*", userName),
+					resource.TestCheckTypeSetElemAttr(resourceName, "admin.*", AdminUserName),
+					resource.TestCheckTypeSetElemAttr(resourceName, "consume.*", ConsumeUserName),
+					resource.TestCheckTypeSetElemAttr(resourceName, "read.*", ReadUserName),
 				),
 			},
-			{ // Change access from consume to admin
-				Config: testAccResourceAccessOffer(userName, userPassword, modelName, newAccess),
+			{ // Change Admin to Consume and Consume to Admin
+				Config: testAccResourceAccessOffer(AdminUserName, ConsumeUserName, ReadUserName, "consume", "admin", "read", userPassword, modelName),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(resourceName, "offer_url", offerURL),
-					resource.TestCheckResourceAttr(resourceName, "access", newAccess),
-					resource.TestCheckTypeSetElemAttr(resourceName, "users.*", userName),
+					resource.TestCheckTypeSetElemAttr(resourceName, "admin.*", ConsumeUserName),
+					resource.TestCheckTypeSetElemAttr(resourceName, "consume.*", AdminUserName),
+					resource.TestCheckTypeSetElemAttr(resourceName, "read.*", ReadUserName),
+				),
+			},
+			{ // Remove user from read permission
+				Config: testAccResourceAccessOffer(AdminUserName, ConsumeUserName, ReadUserName, "consume", "admin", "", userPassword, modelName),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "offer_url", offerURL),
+					resource.TestCheckTypeSetElemAttr(resourceName, "admin.*", ConsumeUserName),
+					resource.TestCheckTypeSetElemAttr(resourceName, "consume.*", AdminUserName),
+					resource.TestCheckNoResourceAttr(resourceName, "read.*"),
 				),
 			},
 			{ // Destroy the resource and validate it can be imported correctly
 				Destroy:           true,
 				ImportStateVerify: true,
 				ImportState:       true,
-				ImportStateId:     fmt.Sprintf("%s:%s:%s", offerURL, newAccess, userName),
+				ImportStateId:     fmt.Sprintf("%s", offerURL),
 				ResourceName:      resourceName,
 			},
 		},
@@ -77,24 +87,39 @@ func TestAcc_ResourceAccessOffer_ErrorWhenUsedWithJAAS(t *testing.T) {
 func testAccResourceAccessOfferFixedUser() string {
 	return `
 resource "juju_access_offer" "test" {
-  access = "consume"
   offer_url = "admin/db.mysql"
-  users = ["bob"]
+  admin = ["bob"]
 }`
 }
 
-func testAccResourceAccessOffer(userName, userPassword, modelName, access string) string {
+func testAccResourceAccessOffer(AdminUserName, ConsumeUserName, ReadUserName, OfferAdmin, OfferConsume, OfferRead, userPassword, modelName string) string {
 	return internaltesting.GetStringFromTemplateWithData(
 		"testAccResourceAccessOffer",
 		`
 resource "juju_model" "{{.ModelName}}" {
 name = "{{.ModelName}}"
 }
 
-resource "juju_user" "operator" {
-  name = "{{.UserName}}"
+{{- if ne .AdminUserName "" }}
+resource "juju_user" "admin_operator" {
+  name = "{{.AdminUserName}}"
+  password = "{{.UserPassword}}"
+}
+{{- end }}
+
+{{- if ne .ConsumeUserName "" }}
+resource "juju_user" "consume_operator" {
+  name = "{{.ConsumeUserName}}"
+  password = "{{.UserPassword}}"
+}
+{{- end }}
+
+{{- if ne .ReadUserName "" }}
+resource "juju_user" "read_operator" {
+  name = "{{.ReadUserName}}"
   password = "{{.UserPassword}}"
 }
+{{- end }}
 
 resource "juju_application" "appone" {
   name  = "appone"
@@ -114,15 +139,30 @@ resource "juju_offer" "appone_endpoint" {
 
 resource "juju_access_offer" "access_appone_endpoint" {
     offer_url = juju_offer.appone_endpoint.url
-    users = [
-		juju_user.operator.name,
+	{{- if ne .OfferAdmin "" }}
+    admin = [
+		juju_user.{{.OfferAdmin}}_operator.name,
+	]
+	{{- end }}
+	{{- if ne .OfferConsume "" }}
+	consume = [
+		juju_user.{{.OfferConsume}}_operator.name,
+	]
+	{{- end }}
+	{{- if ne .OfferRead "" }}
+	read = [
+		juju_user.{{.OfferRead}}_operator.name,
 	]
-    access = "{{.Access}}"
+	{{- end }}
 }
 `, internaltesting.TemplateData{
-			"ModelName":    modelName,
-			"Access":       access,
-			"UserName":     userName,
-			"UserPassword": userPassword,
+			"ModelName":       modelName,
+			"AdminUserName":   AdminUserName,
+			"ConsumeUserName": ConsumeUserName,
+			"ReadUserName":    ReadUserName,
+			"OfferAdmin":      OfferAdmin,
+			"OfferConsume":    OfferConsume,
+			"OfferRead":       OfferRead,
+			"UserPassword":    userPassword,
 		})
 }