diff --git a/internal/juju/offers.go b/internal/juju/offers.go index 30c41c55..e47d4208 100644 --- a/internal/juju/offers.go +++ b/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 { diff --git a/internal/provider/resource_access_offer.go b/internal/provider/resource_access_offer.go index 0cf0b1dc..5e99cfcf 100644 --- a/internal/provider/resource_access_offer.go +++ b/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 { diff --git a/internal/provider/resource_access_offer_test.go b/internal/provider/resource_access_offer_test.go index 843ab3a6..e0665209 100644 --- a/internal/provider/resource_access_offer_test.go +++ b/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,13 +87,12 @@ 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", ` @@ -91,10 +100,26 @@ 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, }) }