From aa8035f68a904bf07aa6d6b37928b75f9f12663e Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Tue, 19 Nov 2024 18:20:26 -0300 Subject: [PATCH 01/17] feat: add juju_access_offer resource --- internal/juju/offers.go | 28 +++ internal/provider/helpers.go | 1 + internal/provider/resource_access_offer.go | 201 +++++++++++++++++++++ 3 files changed, 230 insertions(+) create mode 100644 internal/provider/resource_access_offer.go diff --git a/internal/juju/offers.go b/internal/juju/offers.go index b8428417..da3800ac 100644 --- a/internal/juju/offers.go +++ b/internal/juju/offers.go @@ -74,6 +74,12 @@ type RemoveRemoteOfferInput struct { OfferURL string } +type GrantOfferInput struct { + User string + Access string + OfferURL string +} + func newOffersClient(sc SharedClient) *offersClient { return &offersClient{ SharedClient: sc, @@ -390,3 +396,25 @@ func (c offersClient) RemoveRemoteOffer(input *RemoveRemoteOfferInput) []error { return nil } + +// This function adds access to an offer +func (c offersClient) GrantOffer(input GrantOfferInput) error { + conn, err := c.GetConnection(nil) + if err != nil { + return err + } + defer func() { _ = conn.Close() }() + + client := applicationoffers.NewClient(conn) + _, err = client.ApplicationOffer(input.OfferURL) + if err != nil { + return err + } + + err = client.GrantOffer(input.User, input.Access, input.OfferURL) + if err != nil { + return err + } + + return nil +} diff --git a/internal/provider/helpers.go b/internal/provider/helpers.go index 0ac70790..e95df9e9 100644 --- a/internal/provider/helpers.go +++ b/internal/provider/helpers.go @@ -23,6 +23,7 @@ const ( LogResourceApplication = "resource-application" LogResourceAccessModel = "resource-access-model" + LogResourceAccessOffer = "resource-access-offer" LogResourceCredential = "resource-credential" LogResourceKubernetesCloud = "resource-kubernetes-cloud" LogResourceMachine = "resource-machine" diff --git a/internal/provider/resource_access_offer.go b/internal/provider/resource_access_offer.go new file mode 100644 index 00000000..0a99f046 --- /dev/null +++ b/internal/provider/resource_access_offer.go @@ -0,0 +1,201 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the Apache License, Version 2.0, see LICENCE file for details. + +package provider + +import ( + "context" + "fmt" + "strings" + + "github.com/hashicorp/terraform-plugin-framework-validators/setvalidator" + "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-log/tflog" + + "github.com/juju/juju/core/crossmodel" + "github.com/juju/names/v5" + "github.com/juju/terraform-provider-juju/internal/juju" +) + +// Ensure provider defined types fully satisfy framework interfaces. +var _ resource.Resource = &accessOfferResource{} +var _ resource.ResourceWithConfigure = &accessOfferResource{} +var _ resource.ResourceWithImportState = &accessOfferResource{} +var _ resource.ResourceWithConfigValidators = &accessOfferResource{} + +func NewAccessOfferResource() resource.Resource { + return &accessOfferResource{} +} + +type accessOfferResource struct { + client *juju.Client + + // subCtx is the context created with the new tflog subsystem for applications. + subCtx context.Context +} + +type accessOfferResourceOffer struct { + OfferURL types.String `tfsdk:"offer_url"` + Users types.List `tfsdk:"users"` + Access types.String `tfsdk:"access"` + + // ID required by the testing framework + ID types.String `tfsdk:"id"` +} + +// Metadata returns metadata about the access offer resource. +func (a *accessOfferResource) Metadata(_ context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) { + resp.TypeName = req.ProviderTypeName + "_access_offer" +} + +// Schema defines the schema for the access offer resource. +func (a *accessOfferResource) Schema(_ context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) { + resp.Schema = schema.Schema{ + Description: "A resource that represent a Juju Access Offer.", + Attributes: map[string]schema.Attribute{ + "access": schema.StringAttribute{ + Description: "Level of access to grant. Changing this value will replace the Terraform resource. Valid access levels are described at https://juju.is/docs/juju/manage-offers#control-access-to-an-offer", + Required: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.RequiresReplace(), + }, + Validators: []validator.String{ + stringvalidator.OneOf("admin", "read", "consume"), + }, + }, + "users": schema.SetAttribute{ + Description: "List of users to grant access.", + Optional: true, + ElementType: types.StringType, + Validators: []validator.Set{ + setvalidator.ValueStringsAre(ValidatorMatchString(names.IsValidUser, "user must be a valid Juju username")), + }, + }, + // ID required for imports + "id": schema.StringAttribute{ + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, + }, + "offer_url": schema.StringAttribute{ + Description: "The url of the offer for access management. If this is changed the resource will be deleted and a new resource will be created.", + Required: true, + Validators: []validator.String{ + ValidatorMatchString(func(s string) bool { + _, err := crossmodel.ParseOfferURL(s) + return err == nil + }, "offer_url must be a valid offer string."), + }, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.RequiresReplace(), + }, + }, + }, + } +} + +func (a *accessOfferResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { + // Check first if the client is configured + if a.client == nil { + addClientNotConfiguredError(&resp.Diagnostics, "access offer", "create") + return + } + var plan accessOfferResourceOffer + + // Read Terraform configuration from the request into the model + resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) + if resp.Diagnostics.HasError() { + return + } + + // Get the users + var users []string + resp.Diagnostics.Append(plan.Users.ElementsAs(ctx, &users, false)...) + if resp.Diagnostics.HasError() { + return + } + + // Get the offer + offerURLStr := plan.OfferURL.ValueString() + response, err := a.client.Offers.ReadOffer(&juju.ReadOfferInput{ + OfferURL: offerURLStr, + }) + if err != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create access offer resource, got error: %s", err)) + return + } + a.trace(fmt.Sprintf("read offer %q at %q", response.Name, response.OfferURL)) + + accessStr := plan.Access.ValueString() + // Call Offers.GrantOffer + for _, user := range users { + err := a.client.Offers.GrantOffer(juju.GrantOfferInput{ + User: user, + Access: accessStr, + OfferURL: offerURLStr, + }) + if err != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create access offer resource, got error: %s", err)) + return + } + } + plan.ID = types.StringValue(newAccessOfferIDFrom(offerURLStr, accessStr, users)) + + // Set the plan onto the Terraform state + resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) +} + +func (a *accessOfferResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { + // Read +} + +func (a *accessOfferResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { + // Update +} + +func (a *accessOfferResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { + // Delete +} + +func (a *accessOfferResource) Configure(ctx context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { + // Configure +} + +// ConfigValidators sets validators for the resource. +func (r *accessOfferResource) ConfigValidators(ctx context.Context) []resource.ConfigValidator { + // ConfigValidators + return nil +} + +func (a *accessOfferResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { + IDstr := req.ID + if len(strings.Split(IDstr, ":")) != 3 { + resp.Diagnostics.AddError( + "ImportState Failure", + fmt.Sprintf("Malformed AccessOffer ID %q, "+ + "please use format '::'", IDstr), + ) + return + } + resource.ImportStatePassthroughID(ctx, path.Root("id"), req, resp) +} + +func (a *accessOfferResource) trace(msg string, additionalFields ...map[string]interface{}) { + if a.subCtx == nil { + return + } + + tflog.SubsystemTrace(a.subCtx, LogResourceAccessOffer, msg, additionalFields...) +} + +func newAccessOfferIDFrom(offerURLStr string, accessStr string, users []string) string { + return fmt.Sprintf("%s:%s:%s", offerURLStr, accessStr, strings.Join(users, ",")) +} From 851a95c52f20b0a2d9075847be2bd071dc7216dc Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Thu, 21 Nov 2024 18:11:05 -0300 Subject: [PATCH 02/17] feat: add create and read to access_offer --- docs/resources/access_offer.md | 29 ++++++ internal/juju/offers.go | 2 + internal/provider/provider.go | 1 + internal/provider/resource_access_offer.go | 104 ++++++++++++++++++++- 4 files changed, 131 insertions(+), 5 deletions(-) create mode 100644 docs/resources/access_offer.md diff --git a/docs/resources/access_offer.md b/docs/resources/access_offer.md new file mode 100644 index 00000000..36920e70 --- /dev/null +++ b/docs/resources/access_offer.md @@ -0,0 +1,29 @@ +--- +# generated by https://github.com/hashicorp/terraform-plugin-docs +page_title: "juju_access_offer Resource - terraform-provider-juju" +subcategory: "" +description: |- + A resource that represent a Juju Access Offer. +--- + +# juju_access_offer (Resource) + +A resource that represent a Juju Access Offer. + + + + +## Schema + +### Required + +- `access` (String) Level of access to grant. Changing this value will replace the Terraform resource. Valid access levels are described at https://juju.is/docs/juju/manage-offers#control-access-to-an-offer +- `offer_url` (String) The url of the offer for access management. If this is changed the resource will be deleted and a new resource will be created. + +### Optional + +- `users` (Set of String) List of users to grant access. + +### Read-Only + +- `id` (String) The ID of this resource. diff --git a/internal/juju/offers.go b/internal/juju/offers.go index da3800ac..675fe5ef 100644 --- a/internal/juju/offers.go +++ b/internal/juju/offers.go @@ -54,6 +54,7 @@ type ReadOfferResponse struct { ModelName string Name string OfferURL string + Users []crossmodel.OfferUserDetails } type DestroyOfferInput struct { @@ -176,6 +177,7 @@ func (c offersClient) ReadOffer(input *ReadOfferInput) (*ReadOfferResponse, erro response.ApplicationName = result.ApplicationName response.OfferURL = result.OfferURL response.Endpoint = result.Endpoints[0].Name + response.Users = result.Users //no model name is returned but it can be parsed from the resulting offer URL to ensure parity //TODO: verify if we can fetch information another way diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 4507dd78..2b5ecc44 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -361,6 +361,7 @@ func getJujuProviderModel(ctx context.Context, req provider.ConfigureRequest) (j func (p *jujuProvider) Resources(_ context.Context) []func() resource.Resource { return []func() resource.Resource{ func() resource.Resource { return NewAccessModelResource() }, + func() resource.Resource { return NewAccessOfferResource() }, func() resource.Resource { return NewApplicationResource() }, func() resource.Resource { return NewCredentialResource() }, func() resource.Resource { return NewIntegrationResource() }, diff --git a/internal/provider/resource_access_offer.go b/internal/provider/resource_access_offer.go index 0a99f046..535a3c4c 100644 --- a/internal/provider/resource_access_offer.go +++ b/internal/provider/resource_access_offer.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework-validators/setvalidator" "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" + "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" @@ -17,6 +18,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" "github.com/hashicorp/terraform-plugin-framework/schema/validator" "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/juju/core/crossmodel" @@ -43,7 +45,7 @@ type accessOfferResource struct { type accessOfferResourceOffer struct { OfferURL types.String `tfsdk:"offer_url"` - Users types.List `tfsdk:"users"` + Users types.Set `tfsdk:"users"` Access types.String `tfsdk:"access"` // ID required by the testing framework @@ -154,7 +156,55 @@ func (a *accessOfferResource) Create(ctx context.Context, req resource.CreateReq } func (a *accessOfferResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { - // Read + // 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 + } + + // Retrieve information from the plan + offerURL, access, stateUsers := retrieveAccessOfferDataFromID(ctx, plan.ID, plan.Users, &resp.Diagnostics) + if resp.Diagnostics.HasError() { + return + } + + // Get user/access info from Offer + response, err := a.client.Offers.ReadOffer(&juju.ReadOfferInput{ + OfferURL: offerURL, + }) + if err != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to read offer, got error: %s", err)) + return + } + a.trace(fmt.Sprintf("read juju offer %q", offerURL)) + + // Add to state the ones in the state and set in the offer + plan.OfferURL = types.StringValue(offerURL) + plan.Access = types.StringValue(access) + var users []string + for _, user := range stateUsers { + for _, offerUserDetail := range response.Users { + if user == offerUserDetail.UserName && string(offerUserDetail.Access) == access { + users = append(users, offerUserDetail.UserName) + } + } + } + uss, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, users) + plan.Users = uss + resp.Diagnostics.Append(errDiag...) + if resp.Diagnostics.HasError() { + return + } + + // Set the plan onto the Terraform state + resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) } func (a *accessOfferResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { @@ -166,13 +216,30 @@ func (a *accessOfferResource) Delete(ctx context.Context, req resource.DeleteReq } func (a *accessOfferResource) Configure(ctx context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { - // Configure + // Prevent panic if the provider has not been configured. + if req.ProviderData == nil { + return + } + + client, ok := req.ProviderData.(*juju.Client) + if !ok { + resp.Diagnostics.AddError( + "Unexpected Resource Configure Type", + fmt.Sprintf("Expected *juju.Client, got: %T. Please report this issue to the provider developers.", req.ProviderData), + ) + return + } + a.client = client + // Create the local logging subsystem here, using the TF context when creating it. + a.subCtx = tflog.NewSubsystem(ctx, LogResourceAccessOffer) } // ConfigValidators sets validators for the resource. func (r *accessOfferResource) ConfigValidators(ctx context.Context) []resource.ConfigValidator { - // ConfigValidators - return nil + // JAAS users should use juju_jaas_access_offer instead. + return []resource.ConfigValidator{ + NewAvoidJAASValidator(r.client, "juju_jaas_access_offer"), + } } func (a *accessOfferResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { @@ -199,3 +266,30 @@ func (a *accessOfferResource) trace(msg string, additionalFields ...map[string]i func newAccessOfferIDFrom(offerURLStr string, accessStr string, users []string) string { return fmt.Sprintf("%s:%s:%s", offerURLStr, accessStr, strings.Join(users, ",")) } + +func retrieveAccessOfferDataFromID(ctx context.Context, ID types.String, users types.Set, diag *diag.Diagnostics) (string, string, + []string) { + resID := strings.Split(ID.ValueString(), ":") + if len(resID) < 2 { + diag.AddError("Malformed ID", fmt.Sprintf("AccessOffer ID %q is malformed, "+ + "please use the format '::'", resID)) + return "", "", nil + } + stateUsers := []string{} + if len(resID) == 3 { + stateUsers = strings.Split(resID[2], ",") + } else { + // Note: Is this still valid? + // In 0.8.0 sdk2 version of the provider, the implementation of the access model + // resource had a bug where it didn't contain the users. So we accommodate upgrades + // from that by attempting to get the users from the state if the ID doesn't contain + // any users (which happens only when coming from the previous version because the + // ID is a computed field). + diag.Append(users.ElementsAs(ctx, &stateUsers, false)...) + if diag.HasError() { + return "", "", nil + } + } + + return resID[0], resID[1], stateUsers +} From cd7da1a58e39cd3caa2f257136a5501316dcf0a6 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Fri, 22 Nov 2024 16:58:04 -0300 Subject: [PATCH 03/17] feat: add delete --- internal/juju/offers.go | 26 ++++++++++++++-- internal/provider/resource_access_offer.go | 36 ++++++++++++++++++++-- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/internal/juju/offers.go b/internal/juju/offers.go index 675fe5ef..274b94ea 100644 --- a/internal/juju/offers.go +++ b/internal/juju/offers.go @@ -75,7 +75,7 @@ type RemoveRemoteOfferInput struct { OfferURL string } -type GrantOfferInput struct { +type GrantRevokeOfferInput struct { User string Access string OfferURL string @@ -400,7 +400,7 @@ func (c offersClient) RemoveRemoteOffer(input *RemoveRemoteOfferInput) []error { } // This function adds access to an offer -func (c offersClient) GrantOffer(input GrantOfferInput) error { +func (c offersClient) GrantOffer(input *GrantRevokeOfferInput) error { conn, err := c.GetConnection(nil) if err != nil { return err @@ -420,3 +420,25 @@ func (c offersClient) GrantOffer(input GrantOfferInput) error { return nil } + +// This function revokes access to an offer. +func (c offersClient) RevokeOffer(input *GrantRevokeOfferInput) error { + conn, err := c.GetConnection(nil) + if err != nil { + return err + } + defer func() { _ = conn.Close() }() + + client := applicationoffers.NewClient(conn) + _, err = client.ApplicationOffer(input.OfferURL) + if err != nil { + return err + } + + err = client.RevokeOffer(input.User, input.Access, input.OfferURL) + if err != nil { + return err + } + + return nil +} diff --git a/internal/provider/resource_access_offer.go b/internal/provider/resource_access_offer.go index 535a3c4c..e93c8b55 100644 --- a/internal/provider/resource_access_offer.go +++ b/internal/provider/resource_access_offer.go @@ -139,7 +139,7 @@ func (a *accessOfferResource) Create(ctx context.Context, req resource.CreateReq accessStr := plan.Access.ValueString() // Call Offers.GrantOffer for _, user := range users { - err := a.client.Offers.GrantOffer(juju.GrantOfferInput{ + err := a.client.Offers.GrantOffer(&juju.GrantRevokeOfferInput{ User: user, Access: accessStr, OfferURL: offerURLStr, @@ -212,7 +212,39 @@ func (a *accessOfferResource) Update(ctx context.Context, req resource.UpdateReq } func (a *accessOfferResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { - // Delete + // 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 + var users []string + resp.Diagnostics.Append(plan.Users.ElementsAs(ctx, &users, false)...) + if resp.Diagnostics.HasError() { + return + } + + // Revoking against "read" guarantees that the entire access will be removed + // instead of only decreasing the access level. + for _, user := range users { + err := a.client.Offers.RevokeOffer(&juju.GrantRevokeOfferInput{ + User: user, + Access: "read", + 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 + } + } } func (a *accessOfferResource) Configure(ctx context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { From 540c520820e7d3bbe2bf1f59e51c852613cbad41 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Fri, 22 Nov 2024 17:40:35 -0300 Subject: [PATCH 04/17] feat: add update --- internal/provider/resource_access_offer.go | 88 +++++++++++++++++++++- 1 file changed, 84 insertions(+), 4 deletions(-) diff --git a/internal/provider/resource_access_offer.go b/internal/provider/resource_access_offer.go index e93c8b55..8c2a275b 100644 --- a/internal/provider/resource_access_offer.go +++ b/internal/provider/resource_access_offer.go @@ -6,6 +6,7 @@ package provider import ( "context" "fmt" + "slices" "strings" "github.com/hashicorp/terraform-plugin-framework-validators/setvalidator" @@ -65,9 +66,6 @@ func (a *accessOfferResource) Schema(_ context.Context, req resource.SchemaReque "access": schema.StringAttribute{ Description: "Level of access to grant. Changing this value will replace the Terraform resource. Valid access levels are described at https://juju.is/docs/juju/manage-offers#control-access-to-an-offer", Required: true, - PlanModifiers: []planmodifier.String{ - stringplanmodifier.RequiresReplace(), - }, Validators: []validator.String{ stringvalidator.OneOf("admin", "read", "consume"), }, @@ -208,7 +206,89 @@ func (a *accessOfferResource) Read(ctx context.Context, req resource.ReadRequest } func (a *accessOfferResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { - // Update + // 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 + } + + // Retrieve information from the plan + offerURL, access, stateUsers := retrieveAccessOfferDataFromID(ctx, plan.ID, plan.Users, &resp.Diagnostics) + if resp.Diagnostics.HasError() { + return + } + + // Get user/access info from Offer + response, err := a.client.Offers.ReadOffer(&juju.ReadOfferInput{ + OfferURL: offerURL, + }) + if err != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to read offer, got error: %s", err)) + return + } + a.trace(fmt.Sprintf("read juju offer %q", offerURL)) + offerUsers := response.Users + // Check if a user was removed and delete access (revoke read) + // Create map to be used in the next step for adding new users + offerUserNames := make(map[string]struct{}) + for _, offerUser := range offerUsers { + offerUserNames[offerUser.UserName] = struct{}{} + if !slices.Contains(stateUsers, offerUser.UserName) { + err := a.client.Offers.RevokeOffer(&juju.GrantRevokeOfferInput{ + User: offerUser.UserName, + Access: "read", + OfferURL: offerURL, + }) + if err != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update access offer resource, got error: %s", err)) + return + } + } + } + // Check if is a new user and grant access + // If not, revoke all access and grant the right one + for _, stateUser := range stateUsers { + if _, ok := offerUserNames[stateUser]; !ok { + // user has no access yet so we need to grant + err := a.client.Offers.GrantOffer(&juju.GrantRevokeOfferInput{ + User: stateUser, + Access: access, + OfferURL: offerURL, + }) + if err != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create access offer resource, got error: %s", err)) + return + } + continue + } + // revoke read (remove all) + err := a.client.Offers.RevokeOffer(&juju.GrantRevokeOfferInput{ + User: stateUser, + Access: "read", + OfferURL: offerURL, + }) + if err != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update access offer resource, got error: %s", err)) + return + } + // grant the right permission + err = a.client.Offers.GrantOffer(&juju.GrantRevokeOfferInput{ + User: stateUser, + Access: access, + OfferURL: offerURL, + }) + if err != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update access offer resource, got error: %s", err)) + return + } + } } func (a *accessOfferResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { From 0ccd3796b779627e4a006c3419e443f61ce10dc7 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Thu, 28 Nov 2024 18:19:08 -0300 Subject: [PATCH 05/17] feat: add tests --- internal/provider/resource_access_offer.go | 83 +++++++---- .../provider/resource_access_offer_test.go | 130 ++++++++++++++++++ 2 files changed, 186 insertions(+), 27 deletions(-) create mode 100644 internal/provider/resource_access_offer_test.go diff --git a/internal/provider/resource_access_offer.go b/internal/provider/resource_access_offer.go index 8c2a275b..7a691c44 100644 --- a/internal/provider/resource_access_offer.go +++ b/internal/provider/resource_access_offer.go @@ -66,6 +66,9 @@ func (a *accessOfferResource) Schema(_ context.Context, req resource.SchemaReque "access": schema.StringAttribute{ Description: "Level of access to grant. Changing this value will replace the Terraform resource. Valid access levels are described at https://juju.is/docs/juju/manage-offers#control-access-to-an-offer", Required: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.RequiresReplace(), + }, Validators: []validator.String{ stringvalidator.OneOf("admin", "read", "consume"), }, @@ -159,16 +162,16 @@ func (a *accessOfferResource) Read(ctx context.Context, req resource.ReadRequest addClientNotConfiguredError(&resp.Diagnostics, "access offer", "read") return } - var plan accessOfferResourceOffer + var state accessOfferResourceOffer - // Get the Terraform state from the request into the plan - resp.Diagnostics.Append(req.State.Get(ctx, &plan)...) + // Get the Terraform state + resp.Diagnostics.Append(req.State.Get(ctx, &state)...) if resp.Diagnostics.HasError() { return } - // Retrieve information from the plan - offerURL, access, stateUsers := retrieveAccessOfferDataFromID(ctx, plan.ID, plan.Users, &resp.Diagnostics) + // Get information from ID + offerURL, _, _ := retrieveAccessOfferDataFromID(ctx, state.ID, state.Users, &resp.Diagnostics) if resp.Diagnostics.HasError() { return } @@ -178,31 +181,37 @@ func (a *accessOfferResource) Read(ctx context.Context, req resource.ReadRequest OfferURL: offerURL, }) if err != nil { - resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to read offer, got error: %s", err)) + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to read offer %s, got error: %s", offerURL, err)) return } a.trace(fmt.Sprintf("read juju offer %q", offerURL)) // Add to state the ones in the state and set in the offer - plan.OfferURL = types.StringValue(offerURL) - plan.Access = types.StringValue(access) - var users []string - for _, user := range stateUsers { - for _, offerUserDetail := range response.Users { - if user == offerUserDetail.UserName && string(offerUserDetail.Access) == access { - users = append(users, offerUserDetail.UserName) - } + users := []string{} + var access string + for _, offerUserDetail := range response.Users { + if offerUserDetail.UserName == "everyone@external" || offerUserDetail.UserName == "admin" { + continue + } + users = append(users, offerUserDetail.UserName) + if access != "" && access != string(offerUserDetail.Access) { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("User %s has different access %s", offerUserDetail.UserName, offerUserDetail.Access)) + return } + access = string(offerUserDetail.Access) } - uss, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, users) - plan.Users = uss + newStateUsers, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, users) + state.Users = newStateUsers resp.Diagnostics.Append(errDiag...) if resp.Diagnostics.HasError() { return } // Set the plan onto the Terraform state - resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) + state.ID = types.StringValue(newAccessOfferIDFrom(offerURL, access, users)) + state.Access = types.StringValue(access) + state.OfferURL = types.StringValue(offerURL) + resp.Diagnostics.Append(resp.State.Set(ctx, &state)...) } func (a *accessOfferResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { @@ -213,15 +222,18 @@ func (a *accessOfferResource) Update(ctx context.Context, req resource.UpdateReq } var plan accessOfferResourceOffer - // Get the Terraform state from the request into the plan - resp.Diagnostics.Append(req.State.Get(ctx, &plan)...) + // Get the plan + resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) if resp.Diagnostics.HasError() { return } // Retrieve information from the plan - offerURL, access, stateUsers := retrieveAccessOfferDataFromID(ctx, plan.ID, plan.Users, &resp.Diagnostics) - if resp.Diagnostics.HasError() { + offerURL := plan.OfferURL.ValueString() + access := plan.Access.ValueString() + var planUsers []string + if err := plan.Users.ElementsAs(ctx, &planUsers, false); err != nil { + resp.Diagnostics.AddError("Plan error", fmt.Sprintf("Unable to read users from the plan, got error: %s", err)) return } @@ -235,12 +247,17 @@ func (a *accessOfferResource) Update(ctx context.Context, req resource.UpdateReq } a.trace(fmt.Sprintf("read juju offer %q", offerURL)) offerUsers := response.Users + // Check if a user was removed and delete access (revoke read) // Create map to be used in the next step for adding new users offerUserNames := make(map[string]struct{}) for _, offerUser := range offerUsers { + if offerUser.UserName == "everyone@external" || offerUser.UserName == "admin" { + continue + } offerUserNames[offerUser.UserName] = struct{}{} - if !slices.Contains(stateUsers, offerUser.UserName) { + if !slices.Contains(planUsers, offerUser.UserName) { + a.trace(fmt.Sprintf("revoke user %q for offer %q", offerUser.UserName, offerURL)) err := a.client.Offers.RevokeOffer(&juju.GrantRevokeOfferInput{ User: offerUser.UserName, Access: "read", @@ -254,11 +271,12 @@ func (a *accessOfferResource) Update(ctx context.Context, req resource.UpdateReq } // Check if is a new user and grant access // If not, revoke all access and grant the right one - for _, stateUser := range stateUsers { - if _, ok := offerUserNames[stateUser]; !ok { + for _, planUser := range planUsers { + if _, ok := offerUserNames[planUser]; !ok { // user has no access yet so we need to grant + a.trace(fmt.Sprintf("grant permission %q to user %q for offer %q", access, planUser, offerURL)) err := a.client.Offers.GrantOffer(&juju.GrantRevokeOfferInput{ - User: stateUser, + User: planUser, Access: access, OfferURL: offerURL, }) @@ -269,8 +287,9 @@ func (a *accessOfferResource) Update(ctx context.Context, req resource.UpdateReq continue } // revoke read (remove all) + a.trace(fmt.Sprintf("revoke user %q for offer %q", planUser, offerURL)) err := a.client.Offers.RevokeOffer(&juju.GrantRevokeOfferInput{ - User: stateUser, + User: planUser, Access: "read", OfferURL: offerURL, }) @@ -279,8 +298,9 @@ func (a *accessOfferResource) Update(ctx context.Context, req resource.UpdateReq return } // grant the right permission + a.trace(fmt.Sprintf("grant permission %q to user %q for offer %q", access, planUser, offerURL)) err = a.client.Offers.GrantOffer(&juju.GrantRevokeOfferInput{ - User: stateUser, + User: planUser, Access: access, OfferURL: offerURL, }) @@ -289,6 +309,15 @@ func (a *accessOfferResource) Update(ctx context.Context, req resource.UpdateReq return } } + plan.ID = types.StringValue(newAccessOfferIDFrom(offerURL, access, planUsers)) + plan.Access = types.StringValue(access) + newStateUsers, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, planUsers) + plan.Users = newStateUsers + resp.Diagnostics.Append(errDiag...) + if resp.Diagnostics.HasError() { + return + } + resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) } func (a *accessOfferResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { diff --git a/internal/provider/resource_access_offer_test.go b/internal/provider/resource_access_offer_test.go new file mode 100644 index 00000000..f735cde0 --- /dev/null +++ b/internal/provider/resource_access_offer_test.go @@ -0,0 +1,130 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the Apache License, Version 2.0, see LICENCE file for details. + +package provider + +import ( + "fmt" + "regexp" + "testing" + + "github.com/hashicorp/terraform-plugin-testing/helper/acctest" + "github.com/hashicorp/terraform-plugin-testing/helper/resource" + internaltesting "github.com/juju/terraform-provider-juju/internal/testing" +) + +func TestAcc_ResourceAccessOffer(t *testing.T) { + SkipJAAS(t) + userName := acctest.RandomWithPrefix("tfuser") + userPassword := acctest.RandomWithPrefix("tf-test-user") + modelName := acctest.RandomWithPrefix("tf-access-model") + offerURL := fmt.Sprintf("admin/%s.prometheus-k8s", modelName) + access := "consume" + newAccess := "admin" + accessFail := "bogus" + + resourceName := "juju_access_offer.access_prometheus_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.*"), + }, + { // Create the resource + Config: testAccResourceAccessOffer(userName, userPassword, modelName, access), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "offer_url", offerURL), + resource.TestCheckResourceAttr(resourceName, "access", access), + resource.TestCheckTypeSetElemAttr(resourceName, "users.*", userName), + ), + }, + { // Change access from consume to admin + Config: testAccResourceAccessOffer(userName, userPassword, modelName, newAccess), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "offer_url", offerURL), + resource.TestCheckResourceAttr(resourceName, "access", newAccess), + resource.TestCheckTypeSetElemAttr(resourceName, "users.*", userName), + ), + }, + { // 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), + ResourceName: resourceName, + }, + }, + }) +} + +func TestAcc_ResourceAccessOffer_ErrorWhenUsedWithJAAS(t *testing.T) { + OnlyTestAgainstJAAS(t) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: frameworkProviderFactories, + Steps: []resource.TestStep{ + { + Config: testAccResourceAccessOfferFixedUser(), + ExpectError: regexp.MustCompile("This resource is not supported with JAAS"), + }, + }, + }) +} + +func testAccResourceAccessOfferFixedUser() string { + return ` +resource "juju_access_offer" "test" { + access = "write" + offer_url = "foo" + users = ["bob"] +}` +} + +func testAccResourceAccessOffer(userName, userPassword, modelName, access string) string { + return internaltesting.GetStringFromTemplateWithData( + "testAccResourceAccessOffer", + ` +resource "juju_model" "{{.ModelName}}" { +name = "{{.ModelName}}" +} + +resource "juju_user" "operator" { + name = "{{.UserName}}" + password = "{{.UserPassword}}" +} + +resource "juju_application" "prometheus" { +name = "prometheus-k8s" +model = juju_model.{{.ModelName}}.name + +charm { +name = "prometheus-k8s" +channel = "latest/stable" +} + +units = 1 +} + +resource "juju_offer" "prometheus_endpoint" { + model = juju_model.{{.ModelName}}.name + application_name = juju_application.prometheus.name + endpoint = "receive-remote-write" +} + +resource "juju_access_offer" "access_prometheus_endpoint" { + offer_url = juju_offer.prometheus_endpoint.url + users = [ + juju_user.operator.name, + ] + access = "{{.Access}}" +} +`, internaltesting.TemplateData{ + "ModelName": modelName, + "Access": access, + "UserName": userName, + "UserPassword": userPassword, + }) +} From 2119bba18f6a3d7c3b95a3814a1de8b35ba77399 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Mon, 2 Dec 2024 16:17:12 -0300 Subject: [PATCH 06/17] fix: fix jaas test --- internal/provider/resource_access_offer_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/provider/resource_access_offer_test.go b/internal/provider/resource_access_offer_test.go index f735cde0..f1c0e097 100644 --- a/internal/provider/resource_access_offer_test.go +++ b/internal/provider/resource_access_offer_test.go @@ -77,8 +77,8 @@ func TestAcc_ResourceAccessOffer_ErrorWhenUsedWithJAAS(t *testing.T) { func testAccResourceAccessOfferFixedUser() string { return ` resource "juju_access_offer" "test" { - access = "write" - offer_url = "foo" + access = "consume" + offer_url = "admin/db.mysql" users = ["bob"] }` } From 42cd2c6727cbca79f4af9f8b02460a4156299754 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Mon, 2 Dec 2024 17:59:55 -0300 Subject: [PATCH 07/17] fix: replace prometheus charm for juju-qa-dummy-source --- .../provider/resource_access_offer_test.go | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/internal/provider/resource_access_offer_test.go b/internal/provider/resource_access_offer_test.go index f1c0e097..772c16ec 100644 --- a/internal/provider/resource_access_offer_test.go +++ b/internal/provider/resource_access_offer_test.go @@ -96,26 +96,24 @@ resource "juju_user" "operator" { password = "{{.UserPassword}}" } -resource "juju_application" "prometheus" { -name = "prometheus-k8s" -model = juju_model.{{.ModelName}}.name +resource "juju_application" "appone" { + name = "appone" + model = juju_model.{{.ModelName}}.name -charm { -name = "prometheus-k8s" -channel = "latest/stable" + charm { + name = "juju-qa-dummy-source" + base = "ubuntu@22.04" + } } -units = 1 -} - -resource "juju_offer" "prometheus_endpoint" { +resource "juju_offer" "appone_endpoint" { model = juju_model.{{.ModelName}}.name - application_name = juju_application.prometheus.name - endpoint = "receive-remote-write" + application_name = juju_application.appone.name + endpoint = "sink" } -resource "juju_access_offer" "access_prometheus_endpoint" { - offer_url = juju_offer.prometheus_endpoint.url +resource "juju_access_offer" "access_appone_endpoint" { + offer_url = juju_offer.appone_endpoint.url users = [ juju_user.operator.name, ] From ee944cd2b08df3868f78f26774bfdb1164f4aa52 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Tue, 3 Dec 2024 10:37:45 -0300 Subject: [PATCH 08/17] fix: fix resourcename and offerurl --- internal/provider/resource_access_offer_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/provider/resource_access_offer_test.go b/internal/provider/resource_access_offer_test.go index 772c16ec..843ab3a6 100644 --- a/internal/provider/resource_access_offer_test.go +++ b/internal/provider/resource_access_offer_test.go @@ -18,12 +18,12 @@ func TestAcc_ResourceAccessOffer(t *testing.T) { userName := acctest.RandomWithPrefix("tfuser") userPassword := acctest.RandomWithPrefix("tf-test-user") modelName := acctest.RandomWithPrefix("tf-access-model") - offerURL := fmt.Sprintf("admin/%s.prometheus-k8s", modelName) + offerURL := fmt.Sprintf("admin/%s.appone", modelName) access := "consume" newAccess := "admin" accessFail := "bogus" - resourceName := "juju_access_offer.access_prometheus_endpoint" + resourceName := "juju_access_offer.access_appone_endpoint" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, ProtoV6ProviderFactories: frameworkProviderFactories, From c21ac36eb58ca3f1dda09ef170410f1e2f5e8099 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Wed, 11 Dec 2024 10:07:30 -0300 Subject: [PATCH 09/17] fix: add godocs --- internal/juju/offers.go | 11 +++++++---- internal/provider/resource_access_offer.go | 7 +++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/internal/juju/offers.go b/internal/juju/offers.go index 274b94ea..a09d53b1 100644 --- a/internal/juju/offers.go +++ b/internal/juju/offers.go @@ -87,6 +87,7 @@ func newOffersClient(sc SharedClient) *offersClient { } } +// CreateOffer creates offer managed by the offer resource. func (c offersClient) CreateOffer(input *CreateOfferInput) (*CreateOfferResponse, []error) { var errs []error @@ -159,6 +160,7 @@ func (c offersClient) CreateOffer(input *CreateOfferInput) (*CreateOfferResponse return &resp, nil } +// ReadOffer reads offer managed by the offer resource. func (c offersClient) ReadOffer(input *ReadOfferInput) (*ReadOfferResponse, error) { conn, err := c.GetConnection(nil) if err != nil { @@ -190,6 +192,7 @@ func (c offersClient) ReadOffer(input *ReadOfferInput) (*ReadOfferResponse, erro return &response, nil } +// DestroyOffer destroys offer managed by the offer resource. func (c offersClient) DestroyOffer(input *DestroyOfferInput) error { conn, err := c.GetConnection(nil) if err != nil { @@ -257,7 +260,7 @@ func parseModelFromURL(url string) (result string, success bool) { return result, true } -// This function allows the integration resource to consume the offers managed by the offer resource +// ConsumeRemoteOffer allows the integration resource to consume the offers managed by the offer resource. func (c offersClient) ConsumeRemoteOffer(input *ConsumeRemoteOfferInput) (*ConsumeRemoteOfferResponse, error) { modelConn, err := c.GetConnection(&input.ModelName) if err != nil { @@ -338,7 +341,7 @@ func (c offersClient) ConsumeRemoteOffer(input *ConsumeRemoteOfferInput) (*Consu return &response, nil } -// This function allows the integration resource to destroy the offers managed by the offer resource +// RemoveRemoteOffer allows the integration resource to destroy the offers managed by the offer resource. func (c offersClient) RemoveRemoteOffer(input *RemoveRemoteOfferInput) []error { var errors []error conn, err := c.GetConnection(&input.ModelName) @@ -399,7 +402,7 @@ func (c offersClient) RemoveRemoteOffer(input *RemoveRemoteOfferInput) []error { return nil } -// This function adds access to an offer +// GrantOffer adds access to an offer managed by the access offer resource. func (c offersClient) GrantOffer(input *GrantRevokeOfferInput) error { conn, err := c.GetConnection(nil) if err != nil { @@ -421,7 +424,7 @@ func (c offersClient) GrantOffer(input *GrantRevokeOfferInput) error { return nil } -// This function revokes access to an offer. +// RevokeOffer revokes access to an offer managed by the access offer resource. 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 7a691c44..c594e65b 100644 --- a/internal/provider/resource_access_offer.go +++ b/internal/provider/resource_access_offer.go @@ -33,6 +33,7 @@ var _ resource.ResourceWithConfigure = &accessOfferResource{} var _ resource.ResourceWithImportState = &accessOfferResource{} var _ resource.ResourceWithConfigValidators = &accessOfferResource{} +// NewAccessOfferResource returns a new instance of the Access Offer resource. func NewAccessOfferResource() resource.Resource { return &accessOfferResource{} } @@ -105,6 +106,7 @@ func (a *accessOfferResource) Schema(_ context.Context, req resource.SchemaReque } } +// Create attempts to grant access to the offer. func (a *accessOfferResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { // Check first if the client is configured if a.client == nil { @@ -156,6 +158,7 @@ func (a *accessOfferResource) Create(ctx context.Context, req resource.CreateReq resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) } +// Read reads users and permissions granted to the offer func (a *accessOfferResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { // Check first if the client is configured if a.client == nil { @@ -214,6 +217,7 @@ func (a *accessOfferResource) Read(ctx context.Context, req resource.ReadRequest resp.Diagnostics.Append(resp.State.Set(ctx, &state)...) } +// Update attempts to update the access to the offer. func (a *accessOfferResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { // Check first if the client is configured if a.client == nil { @@ -320,6 +324,7 @@ func (a *accessOfferResource) Update(ctx context.Context, req resource.UpdateReq resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) } +// Delete remove access to the offer according to the resource. func (a *accessOfferResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { // Check first if the client is configured if a.client == nil { @@ -356,6 +361,7 @@ func (a *accessOfferResource) Delete(ctx context.Context, req resource.DeleteReq } } +// Configure sets the access offer resource with provider data. func (a *accessOfferResource) Configure(ctx context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { // Prevent panic if the provider has not been configured. if req.ProviderData == nil { @@ -383,6 +389,7 @@ func (r *accessOfferResource) ConfigValidators(ctx context.Context) []resource.C } } +// ImportState import existing resource to the state. func (a *accessOfferResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { IDstr := req.ID if len(strings.Split(IDstr, ":")) != 3 { From 35e236ae6039449fe56f505be9fa73d1bc0c04ca Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Thu, 12 Dec 2024 17:39:57 -0300 Subject: [PATCH 10/17] feat: change the schema (proposal) --- internal/juju/offers.go | 18 +- internal/provider/resource_access_offer.go | 304 ++++++--------------- 2 files changed, 97 insertions(+), 225 deletions(-) diff --git a/internal/juju/offers.go b/internal/juju/offers.go index a09d53b1..679c23f4 100644 --- a/internal/juju/offers.go +++ b/internal/juju/offers.go @@ -76,7 +76,7 @@ type RemoveRemoteOfferInput struct { } type GrantRevokeOfferInput struct { - User string + Users []string Access string OfferURL string } @@ -416,9 +416,11 @@ func (c offersClient) GrantOffer(input *GrantRevokeOfferInput) error { return err } - err = client.GrantOffer(input.User, input.Access, input.OfferURL) - if err != nil { - return err + for _, user := range input.Users { + err = client.GrantOffer(user, input.Access, input.OfferURL) + if err != nil { + return err + } } return nil @@ -438,9 +440,11 @@ func (c offersClient) RevokeOffer(input *GrantRevokeOfferInput) error { return err } - err = client.RevokeOffer(input.User, input.Access, input.OfferURL) - if err != nil { - return err + for _, user := range input.Users { + err = client.RevokeOffer(user, input.Access, input.OfferURL) + if err != nil { + return err + } } return nil diff --git a/internal/provider/resource_access_offer.go b/internal/provider/resource_access_offer.go index c594e65b..2322f52e 100644 --- a/internal/provider/resource_access_offer.go +++ b/internal/provider/resource_access_offer.go @@ -7,11 +7,9 @@ import ( "context" "fmt" "slices" - "strings" + "github.com/hashicorp/terraform-plugin-framework-validators/resourcevalidator" "github.com/hashicorp/terraform-plugin-framework-validators/setvalidator" - "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" - "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" @@ -23,6 +21,7 @@ import ( "github.com/hashicorp/terraform-plugin-log/tflog" "github.com/juju/juju/core/crossmodel" + "github.com/juju/juju/core/permission" "github.com/juju/names/v5" "github.com/juju/terraform-provider-juju/internal/juju" ) @@ -46,9 +45,10 @@ type accessOfferResource struct { } type accessOfferResourceOffer struct { - OfferURL types.String `tfsdk:"offer_url"` - Users types.Set `tfsdk:"users"` - Access types.String `tfsdk:"access"` + OfferURL types.String `tfsdk:"offer_url"` + AdminUsers types.Set `tfsdk:"admin"` + ConsumeUsers types.Set `tfsdk:"consume"` + ReadUsers types.Set `tfsdk:"read"` // ID required by the testing framework ID types.String `tfsdk:"id"` @@ -62,20 +62,26 @@ func (a *accessOfferResource) Metadata(_ context.Context, req resource.MetadataR // Schema defines the schema for the access offer resource. func (a *accessOfferResource) Schema(_ context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) { resp.Schema = schema.Schema{ - Description: "A resource that represent a Juju Access Offer.", + Description: "A resource that represent a Juju Access Offer. Warning: Do not repeat users across different access levels.", Attributes: map[string]schema.Attribute{ - "access": schema.StringAttribute{ - Description: "Level of access to grant. Changing this value will replace the Terraform resource. Valid access levels are described at https://juju.is/docs/juju/manage-offers#control-access-to-an-offer", - Required: true, - PlanModifiers: []planmodifier.String{ - stringplanmodifier.RequiresReplace(), + string(permission.AdminAccess): schema.SetAttribute{ + Description: "List of users to grant admin access.", + Optional: true, + ElementType: types.StringType, + Validators: []validator.Set{ + setvalidator.ValueStringsAre(ValidatorMatchString(names.IsValidUser, "user must be a valid Juju username")), }, - Validators: []validator.String{ - stringvalidator.OneOf("admin", "read", "consume"), + }, + string(permission.ConsumeAccess): schema.SetAttribute{ + Description: "List of users to grant consume access.", + Optional: true, + ElementType: types.StringType, + Validators: []validator.Set{ + setvalidator.ValueStringsAre(ValidatorMatchString(names.IsValidUser, "user must be a valid Juju username")), }, }, - "users": schema.SetAttribute{ - Description: "List of users to grant access.", + string(permission.ReadAccess): schema.SetAttribute{ + Description: "List of users to grant read access.", Optional: true, ElementType: types.StringType, Validators: []validator.Set{ @@ -121,13 +127,38 @@ func (a *accessOfferResource) Create(ctx context.Context, req resource.CreateReq return } - // Get the users - var users []string - resp.Diagnostics.Append(plan.Users.ElementsAs(ctx, &users, false)...) + // Get the users to grant admin + var adminUsers []string + resp.Diagnostics.Append(plan.AdminUsers.ElementsAs(ctx, &adminUsers, false)...) + if resp.Diagnostics.HasError() { + return + } + + // Get the users to grant consume + var consumeUsers []string + resp.Diagnostics.Append(plan.ConsumeUsers.ElementsAs(ctx, &consumeUsers, false)...) + if resp.Diagnostics.HasError() { + return + } + + // Get the users to grant read + var readUsers []string + resp.Diagnostics.Append(plan.ReadUsers.ElementsAs(ctx, &readUsers, false)...) if resp.Diagnostics.HasError() { return } + // Validate if there are repeated user + combinedUsers := append(append(adminUsers, consumeUsers...), readUsers...) + slices.Sort(combinedUsers) + originalCount := len(combinedUsers) + compactedUsers := slices.Compact(combinedUsers) + compactedCount := len(compactedUsers) + if originalCount != compactedCount { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create access offer resource, got same user in different access.")) + return + } + // Get the offer offerURLStr := plan.OfferURL.ValueString() response, err := a.client.Offers.ReadOffer(&juju.ReadOfferInput{ @@ -139,12 +170,16 @@ func (a *accessOfferResource) Create(ctx context.Context, req resource.CreateReq } a.trace(fmt.Sprintf("read offer %q at %q", response.Name, response.OfferURL)) - accessStr := plan.Access.ValueString() // Call Offers.GrantOffer - for _, user := range users { + users := make(map[permission.Access][]string) + users[permission.ConsumeAccess] = consumeUsers + users[permission.ReadAccess] = readUsers + users[permission.AdminAccess] = adminUsers + + for access, users := range users { err := a.client.Offers.GrantOffer(&juju.GrantRevokeOfferInput{ - User: user, - Access: accessStr, + Users: users, + Access: string(access), OfferURL: offerURLStr, }) if err != nil { @@ -152,7 +187,7 @@ func (a *accessOfferResource) Create(ctx context.Context, req resource.CreateReq return } } - plan.ID = types.StringValue(newAccessOfferIDFrom(offerURLStr, accessStr, users)) + plan.ID = types.StringValue(response.OfferURL) // Set the plan onto the Terraform state resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) @@ -174,10 +209,7 @@ func (a *accessOfferResource) Read(ctx context.Context, req resource.ReadRequest } // Get information from ID - offerURL, _, _ := retrieveAccessOfferDataFromID(ctx, state.ID, state.Users, &resp.Diagnostics) - if resp.Diagnostics.HasError() { - return - } + offerURL := state.ID.ValueString() // Get user/access info from Offer response, err := a.client.Offers.ReadOffer(&juju.ReadOfferInput{ @@ -189,176 +221,47 @@ func (a *accessOfferResource) Read(ctx context.Context, req resource.ReadRequest } a.trace(fmt.Sprintf("read juju offer %q", offerURL)) - // Add to state the ones in the state and set in the offer - users := []string{} - var access string + // Create the map + users := make(map[permission.Access][]string) + users[permission.ConsumeAccess] = []string{} + users[permission.ReadAccess] = []string{} + users[permission.AdminAccess] = []string{} for _, offerUserDetail := range response.Users { if offerUserDetail.UserName == "everyone@external" || offerUserDetail.UserName == "admin" { continue } - users = append(users, offerUserDetail.UserName) - if access != "" && access != string(offerUserDetail.Access) { - resp.Diagnostics.AddError("Client Error", fmt.Sprintf("User %s has different access %s", offerUserDetail.UserName, offerUserDetail.Access)) + + if _, ok := users[offerUserDetail.Access]; !ok { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("User %s has unexpected access %s", offerUserDetail.UserName, offerUserDetail.Access)) return } - access = string(offerUserDetail.Access) + + users[offerUserDetail.Access] = append(users[offerUserDetail.Access], offerUserDetail.UserName) } - newStateUsers, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, users) - state.Users = newStateUsers - resp.Diagnostics.Append(errDiag...) - if resp.Diagnostics.HasError() { - return + + // Save found users to state + for access, user := range users { + stateUsers, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, users) + resp.Diagnostics.Append(errDiag...) + if resp.Diagnostics.HasError() { + return + } + state.Users = newStateUsers } // Set the plan onto the Terraform state - state.ID = types.StringValue(newAccessOfferIDFrom(offerURL, access, users)) - state.Access = types.StringValue(access) state.OfferURL = types.StringValue(offerURL) resp.Diagnostics.Append(resp.State.Set(ctx, &state)...) } // Update attempts to update the access to the offer. func (a *accessOfferResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { - // Check first if the client is configured - if a.client == nil { - addClientNotConfiguredError(&resp.Diagnostics, "access offer", "read") - return - } - var plan accessOfferResourceOffer - - // Get the plan - resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) - if resp.Diagnostics.HasError() { - return - } - - // Retrieve information from the plan - offerURL := plan.OfferURL.ValueString() - access := plan.Access.ValueString() - var planUsers []string - if err := plan.Users.ElementsAs(ctx, &planUsers, false); err != nil { - resp.Diagnostics.AddError("Plan error", fmt.Sprintf("Unable to read users from the plan, got error: %s", err)) - return - } - - // Get user/access info from Offer - response, err := a.client.Offers.ReadOffer(&juju.ReadOfferInput{ - OfferURL: offerURL, - }) - if err != nil { - resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to read offer, got error: %s", err)) - return - } - a.trace(fmt.Sprintf("read juju offer %q", offerURL)) - offerUsers := response.Users - - // Check if a user was removed and delete access (revoke read) - // Create map to be used in the next step for adding new users - offerUserNames := make(map[string]struct{}) - for _, offerUser := range offerUsers { - if offerUser.UserName == "everyone@external" || offerUser.UserName == "admin" { - continue - } - offerUserNames[offerUser.UserName] = struct{}{} - if !slices.Contains(planUsers, offerUser.UserName) { - a.trace(fmt.Sprintf("revoke user %q for offer %q", offerUser.UserName, offerURL)) - err := a.client.Offers.RevokeOffer(&juju.GrantRevokeOfferInput{ - User: offerUser.UserName, - Access: "read", - OfferURL: offerURL, - }) - if err != nil { - resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update access offer resource, got error: %s", err)) - return - } - } - } - // Check if is a new user and grant access - // If not, revoke all access and grant the right one - for _, planUser := range planUsers { - if _, ok := offerUserNames[planUser]; !ok { - // user has no access yet so we need to grant - a.trace(fmt.Sprintf("grant permission %q to user %q for offer %q", access, planUser, offerURL)) - err := a.client.Offers.GrantOffer(&juju.GrantRevokeOfferInput{ - User: planUser, - Access: access, - OfferURL: offerURL, - }) - if err != nil { - resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create access offer resource, got error: %s", err)) - return - } - continue - } - // revoke read (remove all) - a.trace(fmt.Sprintf("revoke user %q for offer %q", planUser, offerURL)) - err := a.client.Offers.RevokeOffer(&juju.GrantRevokeOfferInput{ - User: planUser, - Access: "read", - OfferURL: offerURL, - }) - if err != nil { - resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update access offer resource, got error: %s", err)) - return - } - // grant the right permission - a.trace(fmt.Sprintf("grant permission %q to user %q for offer %q", access, planUser, offerURL)) - err = a.client.Offers.GrantOffer(&juju.GrantRevokeOfferInput{ - User: planUser, - Access: access, - OfferURL: offerURL, - }) - if err != nil { - resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update access offer resource, got error: %s", err)) - return - } - } - plan.ID = types.StringValue(newAccessOfferIDFrom(offerURL, access, planUsers)) - plan.Access = types.StringValue(access) - newStateUsers, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, planUsers) - plan.Users = newStateUsers - resp.Diagnostics.Append(errDiag...) - if resp.Diagnostics.HasError() { - return - } - resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) + // todo } // Delete remove access to the offer according to the resource. func (a *accessOfferResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { - // 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 - var users []string - resp.Diagnostics.Append(plan.Users.ElementsAs(ctx, &users, false)...) - if resp.Diagnostics.HasError() { - return - } - - // Revoking against "read" guarantees that the entire access will be removed - // instead of only decreasing the access level. - for _, user := range users { - err := a.client.Offers.RevokeOffer(&juju.GrantRevokeOfferInput{ - User: user, - Access: "read", - 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 - } - } + // todo } // Configure sets the access offer resource with provider data. @@ -386,20 +289,16 @@ func (r *accessOfferResource) ConfigValidators(ctx context.Context) []resource.C // JAAS users should use juju_jaas_access_offer instead. return []resource.ConfigValidator{ NewAvoidJAASValidator(r.client, "juju_jaas_access_offer"), + resourcevalidator.AtLeastOneOf( + path.MatchRoot(string(permission.AdminAccess)), + path.MatchRoot(string(permission.ConsumeAccess)), + path.MatchRoot(string(permission.ReadAccess)), + ), } } // ImportState import existing resource to the state. func (a *accessOfferResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { - IDstr := req.ID - if len(strings.Split(IDstr, ":")) != 3 { - resp.Diagnostics.AddError( - "ImportState Failure", - fmt.Sprintf("Malformed AccessOffer ID %q, "+ - "please use format '::'", IDstr), - ) - return - } resource.ImportStatePassthroughID(ctx, path.Root("id"), req, resp) } @@ -410,34 +309,3 @@ func (a *accessOfferResource) trace(msg string, additionalFields ...map[string]i tflog.SubsystemTrace(a.subCtx, LogResourceAccessOffer, msg, additionalFields...) } - -func newAccessOfferIDFrom(offerURLStr string, accessStr string, users []string) string { - return fmt.Sprintf("%s:%s:%s", offerURLStr, accessStr, strings.Join(users, ",")) -} - -func retrieveAccessOfferDataFromID(ctx context.Context, ID types.String, users types.Set, diag *diag.Diagnostics) (string, string, - []string) { - resID := strings.Split(ID.ValueString(), ":") - if len(resID) < 2 { - diag.AddError("Malformed ID", fmt.Sprintf("AccessOffer ID %q is malformed, "+ - "please use the format '::'", resID)) - return "", "", nil - } - stateUsers := []string{} - if len(resID) == 3 { - stateUsers = strings.Split(resID[2], ",") - } else { - // Note: Is this still valid? - // In 0.8.0 sdk2 version of the provider, the implementation of the access model - // resource had a bug where it didn't contain the users. So we accommodate upgrades - // from that by attempting to get the users from the state if the ID doesn't contain - // any users (which happens only when coming from the previous version because the - // ID is a computed field). - diag.Append(users.ElementsAs(ctx, &stateUsers, false)...) - if diag.HasError() { - return "", "", nil - } - } - - return resID[0], resID[1], stateUsers -} From 138874f5dc34081eedc3535a296c99c720dcf9b8 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Fri, 13 Dec 2024 18:01:44 -0300 Subject: [PATCH 11/17] fix: add configvalidator --- docs/resources/access_offer.md | 9 +- internal/provider/resource_access_offer.go | 145 ++++++++++++++------- 2 files changed, 103 insertions(+), 51 deletions(-) diff --git a/docs/resources/access_offer.md b/docs/resources/access_offer.md index 36920e70..11fb896d 100644 --- a/docs/resources/access_offer.md +++ b/docs/resources/access_offer.md @@ -3,12 +3,12 @@ page_title: "juju_access_offer Resource - terraform-provider-juju" subcategory: "" description: |- - A resource that represent a Juju Access Offer. + A resource that represent a Juju Access Offer. Warning: Do not repeat users across different access levels. --- # juju_access_offer (Resource) -A resource that represent a Juju Access Offer. +A resource that represent a Juju Access Offer. Warning: Do not repeat users across different access levels. @@ -17,12 +17,13 @@ A resource that represent a Juju Access Offer. ### Required -- `access` (String) Level of access to grant. Changing this value will replace the Terraform resource. Valid access levels are described at https://juju.is/docs/juju/manage-offers#control-access-to-an-offer - `offer_url` (String) The url of the offer for access management. If this is changed the resource will be deleted and a new resource will be created. ### Optional -- `users` (Set of String) List of users to grant access. +- `admin` (Set of String) List of users to grant admin access. "admin" user is not allowed. +- `consume` (Set of String) List of users to grant consume access. "admin" user is not allowed. +- `read` (Set of String) List of users to grant read access. "admin" user is not allowed. ### Read-Only diff --git a/internal/provider/resource_access_offer.go b/internal/provider/resource_access_offer.go index 2322f52e..c8ec81c7 100644 --- a/internal/provider/resource_access_offer.go +++ b/internal/provider/resource_access_offer.go @@ -31,6 +31,7 @@ var _ resource.Resource = &accessOfferResource{} var _ resource.ResourceWithConfigure = &accessOfferResource{} var _ resource.ResourceWithImportState = &accessOfferResource{} var _ resource.ResourceWithConfigValidators = &accessOfferResource{} +var _ resource.ResourceWithValidateConfig = &accessOfferResource{} // NewAccessOfferResource returns a new instance of the Access Offer resource. func NewAccessOfferResource() resource.Resource { @@ -65,7 +66,7 @@ func (a *accessOfferResource) Schema(_ context.Context, req resource.SchemaReque Description: "A resource that represent a Juju Access Offer. Warning: Do not repeat users across different access levels.", Attributes: map[string]schema.Attribute{ string(permission.AdminAccess): schema.SetAttribute{ - Description: "List of users to grant admin access.", + Description: "List of users to grant admin access. \"admin\" user is not allowed.", Optional: true, ElementType: types.StringType, Validators: []validator.Set{ @@ -73,7 +74,7 @@ func (a *accessOfferResource) Schema(_ context.Context, req resource.SchemaReque }, }, string(permission.ConsumeAccess): schema.SetAttribute{ - Description: "List of users to grant consume access.", + Description: "List of users to grant consume access. \"admin\" user is not allowed.", Optional: true, ElementType: types.StringType, Validators: []validator.Set{ @@ -81,7 +82,7 @@ func (a *accessOfferResource) Schema(_ context.Context, req resource.SchemaReque }, }, string(permission.ReadAccess): schema.SetAttribute{ - Description: "List of users to grant read access.", + Description: "List of users to grant read access. \"admin\" user is not allowed.", Optional: true, ElementType: types.StringType, Validators: []validator.Set{ @@ -129,46 +130,30 @@ func (a *accessOfferResource) Create(ctx context.Context, req resource.CreateReq // Get the users to grant admin var adminUsers []string - resp.Diagnostics.Append(plan.AdminUsers.ElementsAs(ctx, &adminUsers, false)...) - if resp.Diagnostics.HasError() { - return + 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 - resp.Diagnostics.Append(plan.ConsumeUsers.ElementsAs(ctx, &consumeUsers, false)...) - if resp.Diagnostics.HasError() { - return + 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 - resp.Diagnostics.Append(plan.ReadUsers.ElementsAs(ctx, &readUsers, false)...) - if resp.Diagnostics.HasError() { - return - } - - // Validate if there are repeated user - combinedUsers := append(append(adminUsers, consumeUsers...), readUsers...) - slices.Sort(combinedUsers) - originalCount := len(combinedUsers) - compactedUsers := slices.Compact(combinedUsers) - compactedCount := len(compactedUsers) - if originalCount != compactedCount { - resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create access offer resource, got same user in different access.")) - return - } - - // Get the offer - offerURLStr := plan.OfferURL.ValueString() - response, err := a.client.Offers.ReadOffer(&juju.ReadOfferInput{ - OfferURL: offerURLStr, - }) - if err != nil { - resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create access offer resource, got error: %s", err)) - return + if !plan.ReadUsers.IsNull() { + resp.Diagnostics.Append(plan.ReadUsers.ElementsAs(ctx, &readUsers, false)...) + if resp.Diagnostics.HasError() { + return + } } - a.trace(fmt.Sprintf("read offer %q at %q", response.Name, response.OfferURL)) // Call Offers.GrantOffer users := make(map[permission.Access][]string) @@ -180,14 +165,16 @@ func (a *accessOfferResource) Create(ctx context.Context, req resource.CreateReq err := a.client.Offers.GrantOffer(&juju.GrantRevokeOfferInput{ Users: users, Access: string(access), - OfferURL: offerURLStr, + OfferURL: plan.OfferURL.ValueString(), }) if err != nil { resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create access offer resource, got error: %s", err)) return } } - plan.ID = types.StringValue(response.OfferURL) + + // Set ID as the offer URL + plan.ID = plan.OfferURL // Set the plan onto the Terraform state resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) @@ -239,16 +226,27 @@ func (a *accessOfferResource) Read(ctx context.Context, req resource.ReadRequest users[offerUserDetail.Access] = append(users[offerUserDetail.Access], offerUserDetail.UserName) } - // Save found users to state - for access, user := range users { - stateUsers, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, users) - resp.Diagnostics.Append(errDiag...) - if resp.Diagnostics.HasError() { - return - } - state.Users = newStateUsers + // Save admin users to state + adminUsersSet, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, users[permission.AdminAccess]) + resp.Diagnostics.Append(errDiag...) + if resp.Diagnostics.HasError() { + return } - + 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 + } + 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 + } + state.ReadUsers = readUsersSet // Set the plan onto the Terraform state state.OfferURL = types.StringValue(offerURL) resp.Diagnostics.Append(resp.State.Set(ctx, &state)...) @@ -285,10 +283,10 @@ func (a *accessOfferResource) Configure(ctx context.Context, req resource.Config } // ConfigValidators sets validators for the resource. -func (r *accessOfferResource) ConfigValidators(ctx context.Context) []resource.ConfigValidator { +func (a *accessOfferResource) ConfigValidators(ctx context.Context) []resource.ConfigValidator { // JAAS users should use juju_jaas_access_offer instead. return []resource.ConfigValidator{ - NewAvoidJAASValidator(r.client, "juju_jaas_access_offer"), + NewAvoidJAASValidator(a.client, "juju_jaas_access_offer"), resourcevalidator.AtLeastOneOf( path.MatchRoot(string(permission.AdminAccess)), path.MatchRoot(string(permission.ConsumeAccess)), @@ -297,6 +295,59 @@ func (r *accessOfferResource) ConfigValidators(ctx context.Context) []resource.C } } +func (a *accessOfferResource) ValidateConfig(ctx context.Context, req resource.ValidateConfigRequest, resp *resource.ValidateConfigResponse) { + // TODO this validation does not work in case user name depends on the output of other resource + var configData accessOfferResourceOffer + + // Read Terraform configuration from the request into the model + resp.Diagnostics.Append(req.Config.Get(ctx, &configData)...) + if resp.Diagnostics.HasError() { + return + } + + // Get the users to grant admin + var adminUsers []string + if !configData.AdminUsers.IsNull() { + resp.Diagnostics.Append(configData.AdminUsers.ElementsAs(ctx, &adminUsers, false)...) + if resp.Diagnostics.HasError() { + return + } + } + + // Get the users to grant consume + var consumeUsers []string + if !configData.ConsumeUsers.IsNull() { + resp.Diagnostics.Append(configData.ConsumeUsers.ElementsAs(ctx, &consumeUsers, false)...) + if resp.Diagnostics.HasError() { + return + } + } + + // Get the users to grant read + var readUsers []string + if !configData.ReadUsers.IsNull() { + resp.Diagnostics.Append(configData.ReadUsers.ElementsAs(ctx, &readUsers, false)...) + if resp.Diagnostics.HasError() { + return + } + } + + combinedUsers := append(append(adminUsers, consumeUsers...), readUsers...) + // Validate if there are repeated user + if slices.Contains(combinedUsers, "admin") { + resp.Diagnostics.AddAttributeError(path.Root("offer_url"), "Attribute Error", "\"admin\" user is not allowed") + } + // Validate if there are repeated user + slices.Sort(combinedUsers) + originalCount := len(combinedUsers) + compactedUsers := slices.Compact(combinedUsers) + compactedCount := len(compactedUsers) + if originalCount != compactedCount { + resp.Diagnostics.AddAttributeError(path.Root("offer_url"), "Attribute Error", "do not repeat users across different access levels") + } + +} + // ImportState import existing resource to the state. func (a *accessOfferResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { resource.ImportStatePassthroughID(ctx, path.Root("id"), req, resp) From a94e1537bd68296cdb3c1c90761b92619d28b31d Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Mon, 16 Dec 2024 18:37:18 -0300 Subject: [PATCH 12/17] fix: remove configvalidator and implement update --- internal/juju/offers.go | 8 + internal/provider/resource_access_offer.go | 248 ++++++++++++++++----- 2 files changed, 205 insertions(+), 51 deletions(-) diff --git a/internal/juju/offers.go b/internal/juju/offers.go index 679c23f4..30c41c55 100644 --- a/internal/juju/offers.go +++ b/internal/juju/offers.go @@ -419,6 +419,10 @@ func (c offersClient) GrantOffer(input *GrantRevokeOfferInput) error { for _, user := range input.Users { err = client.GrantOffer(user, input.Access, input.OfferURL) if err != nil { + // ignore if user was already granted + if strings.Contains(err.Error(), "user already has") { + continue + } return err } } @@ -443,6 +447,10 @@ func (c offersClient) RevokeOffer(input *GrantRevokeOfferInput) error { for _, user := range input.Users { err = client.RevokeOffer(user, input.Access, input.OfferURL) if err != nil { + // ignorer if user was already revoked + if strings.Contains(err.Error(), "not found") { + continue + } return err } } diff --git a/internal/provider/resource_access_offer.go b/internal/provider/resource_access_offer.go index c8ec81c7..0cf0b1dc 100644 --- a/internal/provider/resource_access_offer.go +++ b/internal/provider/resource_access_offer.go @@ -6,7 +6,6 @@ package provider import ( "context" "fmt" - "slices" "github.com/hashicorp/terraform-plugin-framework-validators/resourcevalidator" "github.com/hashicorp/terraform-plugin-framework-validators/setvalidator" @@ -31,7 +30,6 @@ var _ resource.Resource = &accessOfferResource{} var _ resource.ResourceWithConfigure = &accessOfferResource{} var _ resource.ResourceWithImportState = &accessOfferResource{} var _ resource.ResourceWithConfigValidators = &accessOfferResource{} -var _ resource.ResourceWithValidateConfig = &accessOfferResource{} // NewAccessOfferResource returns a new instance of the Access Offer resource. func NewAccessOfferResource() resource.Resource { @@ -155,6 +153,14 @@ func (a *accessOfferResource) Create(ctx context.Context, req resource.CreateReq } } + // validate if there are overlaps + // validation is done here considering dynamic (juju_user resource) and static values for users + err := validateNoOverlaps(adminUsers, consumeUsers, readUsers) + if err != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create access offer resource, got error: %s", err)) + return + } + // Call Offers.GrantOffer users := make(map[permission.Access][]string) users[permission.ConsumeAccess] = consumeUsers @@ -254,7 +260,142 @@ func (a *accessOfferResource) Read(ctx context.Context, req resource.ReadRequest // Update attempts to update the access to the offer. func (a *accessOfferResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { - // todo + // Check first if the client is configured + if a.client == nil { + addClientNotConfiguredError(&resp.Diagnostics, "access offer", "update") + return + } + var plan, state accessOfferResourceOffer + + // Read Terraform configuration from the request into the model + resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) + resp.Diagnostics.Append(req.State.Get(ctx, &state)...) + 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 + } + } + + // validate if there are overlaps + // validation is done here considering dynamic (juju_user resource) and static values for users + err := validateNoOverlaps(adminUsers, consumeUsers, readUsers) + if err != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create access offer resource, got error: %s", err)) + return + } + + // If bob is in the plan but not in state = grant + // if bob is in the state but not in the plan = revoke + // scenario 1 (users added/moved): bob was moved from admin to consume + // bob will be 're-granted' consume permission in further steps + // scenario 2 (users removed): bob was removed from the resource (user was in state but not in the plan anymore) + // 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)...) + if resp.Diagnostics.HasError() { + return + } + } + 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 + } + + var consumeStateUsers []string + if !state.ConsumeUsers.IsNull() { + resp.Diagnostics.Append(state.ConsumeUsers.ElementsAs(ctx, &consumeStateUsers, false)...) + if resp.Diagnostics.HasError() { + return + } + } + err = processPermissionChanges(plan.OfferURL.ValueString(), string(permission.ConsumeAccess), consumeUsers, consumeStateUsers, 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 + } + + 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.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 + } + + // scenario 2 - check for users removed from the resource (they are in state but not in plan anymore) + totalStateUsers := append(adminStateUsers, consumeStateUsers...) + totalStateUsers = append(totalStateUsers, readStateUsers...) + totalPlanUsers := append(adminUsers, consumeUsers...) + totalPlanUsers = append(totalPlanUsers, readUsers...) + removeUsers := diffUsers(totalStateUsers, totalPlanUsers) + if len(removeUsers) > 0 { + err := a.client.Offers.RevokeOffer(&juju.GrantRevokeOfferInput{ + Users: removeUsers, + 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 + } + } + + // Save admin users to state + adminUsersSet, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, adminUsers) + resp.Diagnostics.Append(errDiag...) + if resp.Diagnostics.HasError() { + return + } + plan.AdminUsers = adminUsersSet + // Save consume users to state + consumeUsersSet, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, consumeUsers) + resp.Diagnostics.Append(errDiag...) + if resp.Diagnostics.HasError() { + return + } + plan.ConsumeUsers = consumeUsersSet + // Save read users to state + readUsersSet, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, readUsers) + resp.Diagnostics.Append(errDiag...) + if resp.Diagnostics.HasError() { + return + } + plan.ReadUsers = readUsersSet + resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) } // Delete remove access to the offer according to the resource. @@ -295,68 +436,73 @@ func (a *accessOfferResource) ConfigValidators(ctx context.Context) []resource.C } } -func (a *accessOfferResource) ValidateConfig(ctx context.Context, req resource.ValidateConfigRequest, resp *resource.ValidateConfigResponse) { - // TODO this validation does not work in case user name depends on the output of other resource - var configData accessOfferResourceOffer +// ImportState import existing resource to the state. +func (a *accessOfferResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { + resource.ImportStatePassthroughID(ctx, path.Root("id"), req, resp) +} - // Read Terraform configuration from the request into the model - resp.Diagnostics.Append(req.Config.Get(ctx, &configData)...) - if resp.Diagnostics.HasError() { +func (a *accessOfferResource) trace(msg string, additionalFields ...map[string]interface{}) { + if a.subCtx == nil { return } - // Get the users to grant admin - var adminUsers []string - if !configData.AdminUsers.IsNull() { - resp.Diagnostics.Append(configData.AdminUsers.ElementsAs(ctx, &adminUsers, false)...) - if resp.Diagnostics.HasError() { - return - } - } + tflog.SubsystemTrace(a.subCtx, LogResourceAccessOffer, msg, additionalFields...) +} - // Get the users to grant consume - var consumeUsers []string - if !configData.ConsumeUsers.IsNull() { - resp.Diagnostics.Append(configData.ConsumeUsers.ElementsAs(ctx, &consumeUsers, false)...) - if resp.Diagnostics.HasError() { - return - } +func validateNoOverlaps(admin, consume, read []string) error { + sets := map[string]struct{}{} + for _, v := range consume { + sets[v] = struct{}{} } - - // Get the users to grant read - var readUsers []string - if !configData.ReadUsers.IsNull() { - resp.Diagnostics.Append(configData.ReadUsers.ElementsAs(ctx, &readUsers, false)...) - if resp.Diagnostics.HasError() { - return + for _, v := range read { + if _, exists := sets[v]; exists { + return fmt.Errorf("user '%s' appears in both 'consume' and 'read'", v) } + sets[v] = struct{}{} } - - combinedUsers := append(append(adminUsers, consumeUsers...), readUsers...) - // Validate if there are repeated user - if slices.Contains(combinedUsers, "admin") { - resp.Diagnostics.AddAttributeError(path.Root("offer_url"), "Attribute Error", "\"admin\" user is not allowed") - } - // Validate if there are repeated user - slices.Sort(combinedUsers) - originalCount := len(combinedUsers) - compactedUsers := slices.Compact(combinedUsers) - compactedCount := len(compactedUsers) - if originalCount != compactedCount { - resp.Diagnostics.AddAttributeError(path.Root("offer_url"), "Attribute Error", "do not repeat users across different access levels") + for _, v := range admin { + if _, exists := sets[v]; exists { + return fmt.Errorf("user '%s' appears in multiple roles (e.g., 'consume', 'read', 'admin')", v) + } } + return nil } -// ImportState import existing resource to the state. -func (a *accessOfferResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { - resource.ImportStatePassthroughID(ctx, path.Root("id"), req, resp) +func processPermissionChanges(offerURL, permissionType string, planUsers, stateUsers []string, jujuClient *juju.Client) error { + toGrant := diffUsers(planUsers, stateUsers) + toRevoke := diffUsers(stateUsers, planUsers) + err := jujuClient.Offers.RevokeOffer(&juju.GrantRevokeOfferInput{ + Users: toRevoke, + Access: permissionType, + OfferURL: offerURL, + }) + if err != nil { + return err + } + + err = jujuClient.Offers.GrantOffer(&juju.GrantRevokeOfferInput{ + Users: toGrant, + Access: permissionType, + OfferURL: offerURL, + }) + if err != nil { + return err + } + return nil } -func (a *accessOfferResource) trace(msg string, additionalFields ...map[string]interface{}) { - if a.subCtx == nil { - return +func diffUsers(a, b []string) []string { + set := make(map[string]struct{}) + for _, v := range b { + set[v] = struct{}{} } - tflog.SubsystemTrace(a.subCtx, LogResourceAccessOffer, msg, additionalFields...) + var diff []string + for _, v := range a { + if _, found := set[v]; !found { + diff = append(diff, v) + } + } + return diff } From 9a2253ce6f0200acab51fd4aae0309e8b5a55a74 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Tue, 17 Dec 2024 14:25:27 -0300 Subject: [PATCH 13/17] fix: fix tests --- internal/juju/offers.go | 2 + internal/provider/resource_access_offer.go | 120 +++++++++++++----- .../provider/resource_access_offer_test.go | 96 ++++++++++---- 3 files changed, 160 insertions(+), 58 deletions(-) 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, }) } From d5d94e2a106da2f4e8cb23f6caeaba2029d73900 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Tue, 17 Dec 2024 14:49:00 -0300 Subject: [PATCH 14/17] docs: add doc usage --- docs/resources/access_offer.md | 20 +++++++++++++++++++ .../resources/juju_access_offer/import.sh | 6 ++++++ .../resources/juju_access_offer/resource.tf | 4 ++++ 3 files changed, 30 insertions(+) create mode 100644 examples/resources/juju_access_offer/import.sh create mode 100644 examples/resources/juju_access_offer/resource.tf diff --git a/docs/resources/access_offer.md b/docs/resources/access_offer.md index 11fb896d..13cec545 100644 --- a/docs/resources/access_offer.md +++ b/docs/resources/access_offer.md @@ -10,7 +10,14 @@ description: |- A resource that represent a Juju Access Offer. Warning: Do not repeat users across different access levels. +## Example Usage +```terraform +resource "juju_access_offer" "this" { + offer_url = juju_offer.my_application_offer.url + consume = [juju_user.dev.name] +} +``` ## Schema @@ -28,3 +35,16 @@ A resource that represent a Juju Access Offer. Warning: Do not repeat users acro ### Read-Only - `id` (String) The ID of this resource. + +## Import + +Import is supported using the following syntax: + +```shell +# Access Offers can be imported by using the Offer URL as in the juju show-offers output. +# Example: +# $juju show-offer mysql +# Store URL Access Description Endpoint Interface Role +# mycontroller admin/db.mysql admin MariaDB Server is one of the most ... mysql mysql provider +$ terraform import juju_access_offer.db admin/db.mysql +``` diff --git a/examples/resources/juju_access_offer/import.sh b/examples/resources/juju_access_offer/import.sh new file mode 100644 index 00000000..094b0170 --- /dev/null +++ b/examples/resources/juju_access_offer/import.sh @@ -0,0 +1,6 @@ +# Access Offers can be imported by using the Offer URL as in the juju show-offers output. +# Example: +# $juju show-offer mysql +# Store URL Access Description Endpoint Interface Role +# mycontroller admin/db.mysql admin MariaDB Server is one of the most ... mysql mysql provider +$ terraform import juju_access_offer.db admin/db.mysql diff --git a/examples/resources/juju_access_offer/resource.tf b/examples/resources/juju_access_offer/resource.tf new file mode 100644 index 00000000..265ee378 --- /dev/null +++ b/examples/resources/juju_access_offer/resource.tf @@ -0,0 +1,4 @@ +resource "juju_access_offer" "this" { + offer_url = juju_offer.my_application_offer.url + consume = [juju_user.dev.name] +} From e36c04a0571089d9b9eab1ba23dfc39d3128194f Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Tue, 17 Dec 2024 14:58:11 -0300 Subject: [PATCH 15/17] fix: remove reading offer --- internal/juju/offers.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/internal/juju/offers.go b/internal/juju/offers.go index e47d4208..f054e8d0 100644 --- a/internal/juju/offers.go +++ b/internal/juju/offers.go @@ -412,10 +412,6 @@ func (c offersClient) GrantOffer(input *GrantRevokeOfferInput) error { defer func() { _ = conn.Close() }() client := applicationoffers.NewClient(conn) - _, err = client.ApplicationOffer(input.OfferURL) - if err != nil { - return err - } for _, user := range input.Users { err = client.GrantOffer(user, input.Access, input.OfferURL) @@ -441,10 +437,6 @@ func (c offersClient) RevokeOffer(input *GrantRevokeOfferInput) error { defer func() { _ = conn.Close() }() client := applicationoffers.NewClient(conn) - _, err = client.ApplicationOffer(input.OfferURL) - if err != nil { - return err - } for _, user := range input.Users { err = client.RevokeOffer(user, input.Access, input.OfferURL) From e4f848e827516bc0d22a509cfe4593414d955130 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Tue, 17 Dec 2024 15:13:53 -0300 Subject: [PATCH 16/17] fix: validate admin user --- internal/provider/resource_access_offer.go | 19 ++++++-- .../provider/resource_access_offer_test.go | 46 +++++++++++++++++++ 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/internal/provider/resource_access_offer.go b/internal/provider/resource_access_offer.go index 5e99cfcf..7d01f515 100644 --- a/internal/provider/resource_access_offer.go +++ b/internal/provider/resource_access_offer.go @@ -153,9 +153,9 @@ func (a *accessOfferResource) Create(ctx context.Context, req resource.CreateReq } } - // validate if there are overlaps + // validate if there are overlaps or admin user // validation is done here considering dynamic (juju_user resource) and static values for users - err := validateNoOverlaps(adminUsers, consumeUsers, readUsers) + err := validateNoOverlapsNoAdmin(adminUsers, consumeUsers, readUsers) if err != nil { resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create access offer resource, got error: %s", err)) return @@ -307,9 +307,9 @@ func (a *accessOfferResource) Update(ctx context.Context, req resource.UpdateReq } } - // validate if there are overlaps + // validate if there are overlaps or admin user // validation is done here considering dynamic (juju_user resource) and static values for users - err := validateNoOverlaps(adminUsers, consumeUsers, readUsers) + err := validateNoOverlapsNoAdmin(adminUsers, consumeUsers, readUsers) if err != nil { resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create access offer resource, got error: %s", err)) return @@ -509,18 +509,27 @@ func (a *accessOfferResource) trace(msg string, additionalFields ...map[string]i } // Helpers -func validateNoOverlaps(admin, consume, read []string) error { +func validateNoOverlapsNoAdmin(admin, consume, read []string) error { sets := map[string]struct{}{} for _, v := range consume { + if v == "admin" { + return fmt.Errorf("user admin is not allowed") + } sets[v] = struct{}{} } for _, v := range read { + if v == "admin" { + return fmt.Errorf("user admin is not allowed") + } if _, exists := sets[v]; exists { return fmt.Errorf("user '%s' appears in both 'consume' and 'read'", v) } sets[v] = struct{}{} } for _, v := range admin { + if v == "admin" { + return fmt.Errorf("user admin is not allowed") + } if _, exists := sets[v]; exists { return fmt.Errorf("user '%s' appears in multiple roles (e.g., 'consume', 'read', 'admin')", v) } diff --git a/internal/provider/resource_access_offer_test.go b/internal/provider/resource_access_offer_test.go index e0665209..b95bdb9b 100644 --- a/internal/provider/resource_access_offer_test.go +++ b/internal/provider/resource_access_offer_test.go @@ -84,6 +84,23 @@ func TestAcc_ResourceAccessOffer_ErrorWhenUsedWithJAAS(t *testing.T) { }) } +func TestAcc_ResourceAccessOffer_ErrorWhenUsedWithAdmin(t *testing.T) { + SkipJAAS(t) + + modelNameAdminTest := acctest.RandomWithPrefix("tf-access-admin-model") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: frameworkProviderFactories, + Steps: []resource.TestStep{ + { // Test username admin validation + Config: testAccResourceAccessOfferAdminUser(modelNameAdminTest), + ExpectError: regexp.MustCompile("user admin is not allowed.*"), + }, + }, + }) +} + func testAccResourceAccessOfferFixedUser() string { return ` resource "juju_access_offer" "test" { @@ -92,6 +109,35 @@ resource "juju_access_offer" "test" { }` } +func testAccResourceAccessOfferAdminUser(modelName string) string { + return internaltesting.GetStringFromTemplateWithData("testAccResourceAccessOfferAdminUser", ` +resource "juju_model" "{{.ModelName}}" { +name = "{{.ModelName}}" +} + +resource "juju_application" "appone" { + name = "appone" + model = juju_model.{{.ModelName}}.name + + charm { + name = "juju-qa-dummy-source" + base = "ubuntu@22.04" + } +} + +resource "juju_offer" "appone_endpoint" { + model = juju_model.{{.ModelName}}.name + application_name = juju_application.appone.name + endpoint = "sink" +} + +resource "juju_access_offer" "test" { + offer_url = juju_offer.appone_endpoint.url + admin = ["admin"] +}`, internaltesting.TemplateData{ + "ModelName": modelName}) +} + func testAccResourceAccessOffer(AdminUserName, ConsumeUserName, ReadUserName, OfferAdmin, OfferConsume, OfferRead, userPassword, modelName string) string { return internaltesting.GetStringFromTemplateWithData( "testAccResourceAccessOffer", From 81dff48a8bb0ad90ff605d44f1ce3ae75741e4cc Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Tue, 17 Dec 2024 15:22:21 -0300 Subject: [PATCH 17/17] fix: fix lint error --- internal/provider/resource_access_offer_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/provider/resource_access_offer_test.go b/internal/provider/resource_access_offer_test.go index b95bdb9b..737762e5 100644 --- a/internal/provider/resource_access_offer_test.go +++ b/internal/provider/resource_access_offer_test.go @@ -62,7 +62,7 @@ func TestAcc_ResourceAccessOffer(t *testing.T) { Destroy: true, ImportStateVerify: true, ImportState: true, - ImportStateId: fmt.Sprintf("%s", offerURL), + ImportStateId: offerURL, ResourceName: resourceName, }, },