From 6ffb42d0a466bf16965a2ca761e03a638998aae9 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Fri, 16 Aug 2024 15:59:42 +0200 Subject: [PATCH] feat: implement crud for the generic access resource --- internal/juju/client.go | 10 +- internal/juju/jaas.go | 22 +- internal/juju/jaas_test.go | 1 + .../expect_recreated_resource_test.go | 47 +++ internal/provider/provider.go | 1 + internal/provider/provider_test.go | 10 + internal/provider/resource_access_generic.go | 310 ++++++++++++++- .../provider/resource_access_jaas_model.go | 36 +- .../resource_access_jaas_model_test.go | 366 ++++++++++++++++++ internal/provider/resource_offer_test.go | 21 +- 10 files changed, 780 insertions(+), 44 deletions(-) create mode 100644 internal/provider/expect_recreated_resource_test.go create mode 100644 internal/provider/resource_access_jaas_model_test.go diff --git a/internal/juju/client.go b/internal/juju/client.go index c8ba7dbe..85448b0d 100644 --- a/internal/juju/client.go +++ b/internal/juju/client.go @@ -10,6 +10,7 @@ import ( "sync" "time" + jaasApi "github.com/canonical/jimm-go-sdk/v3/api" "github.com/hashicorp/terraform-plugin-log/tflog" "github.com/juju/errors" "github.com/juju/juju/api" @@ -125,13 +126,18 @@ var isJAAS bool // IsJAAS uses a synchronisation object to only perform the check once and return the same result. func (sc *sharedClient) IsJAAS(defaultVal bool) bool { checkJAASOnce.Do(func() { + isJAAS = defaultVal conn, err := sc.GetConnection(nil) if err != nil { - isJAAS = defaultVal return } defer conn.Close() - isJAAS = conn.BestFacadeVersion("JIMM") != 0 + jc := jaasApi.NewClient(conn) + _, err = jc.ListControllers() + if err == nil { + isJAAS = true + return + } }) return isJAAS } diff --git a/internal/juju/jaas.go b/internal/juju/jaas.go index de8d5e7a..5d29672c 100644 --- a/internal/juju/jaas.go +++ b/internal/juju/jaas.go @@ -52,6 +52,22 @@ func toAPITuple(tuple JaasTuple) params.RelationshipTuple { } } +func toJaasTuples(tuples []params.RelationshipTuple) []JaasTuple { + out := make([]JaasTuple, 0, len(tuples)) + for _, tuple := range tuples { + out = append(out, toJaasTuple(tuple)) + } + return out +} + +func toJaasTuple(tuple params.RelationshipTuple) JaasTuple { + return JaasTuple{ + Object: tuple.Object, + Relation: tuple.Relation, + Target: tuple.TargetObject, + } +} + // AddRelations attempts to create the provided slice of relationship tuples. // An empty slice of tuples will return an error. func (jc *jaasClient) AddRelations(tuples []JaasTuple) error { @@ -90,7 +106,7 @@ func (jc *jaasClient) DeleteRelations(tuples []JaasTuple) error { // ReadRelations attempts to read relations that match the criteria defined by `tuple`. // An nil tuple pointer is invalid and will return an error. -func (jc *jaasClient) ReadRelations(ctx context.Context, tuple *JaasTuple) ([]params.RelationshipTuple, error) { +func (jc *jaasClient) ReadRelations(ctx context.Context, tuple *JaasTuple) ([]JaasTuple, error) { if tuple == nil { return nil, errors.New("read relation tuple is nil") } @@ -102,7 +118,7 @@ func (jc *jaasClient) ReadRelations(ctx context.Context, tuple *JaasTuple) ([]pa defer func() { _ = conn.Close() }() client := jc.getJaasApiClient(conn) - relations := make([]params.RelationshipTuple, 0) + relations := make([]JaasTuple, 0) req := ¶ms.ListRelationshipTuplesRequest{Tuple: toAPITuple(*tuple)} for { resp, err := client.ListRelationshipTuples(req) @@ -114,7 +130,7 @@ func (jc *jaasClient) ReadRelations(ctx context.Context, tuple *JaasTuple) ([]pa jc.Errorf(err, "call to ListRelationshipTuples contained error(s)") return nil, errors.New(resp.Errors[0]) } - relations = append(relations, resp.Tuples...) + relations = append(relations, toJaasTuples(resp.Tuples)...) if resp.ContinuationToken == "" { return relations, nil } diff --git a/internal/juju/jaas_test.go b/internal/juju/jaas_test.go index a46b026b..74fefffb 100644 --- a/internal/juju/jaas_test.go +++ b/internal/juju/jaas_test.go @@ -120,6 +120,7 @@ func (s *JaasSuite) TestReadRelations() { relations, err := client.ReadRelations(context.Background(), &tuple) s.Require().NoError(err) s.Require().Len(relations, 2) + s.Require().Equal(relations, []JaasTuple{tuple, tuple}) } func (s *JaasSuite) TestReadRelationsEmptyTuple() { diff --git a/internal/provider/expect_recreated_resource_test.go b/internal/provider/expect_recreated_resource_test.go new file mode 100644 index 00000000..0edf2c1f --- /dev/null +++ b/internal/provider/expect_recreated_resource_test.go @@ -0,0 +1,47 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package provider + +import ( + "context" + "errors" + "fmt" + + tfjson "github.com/hashicorp/terraform-json" + "github.com/hashicorp/terraform-plugin-testing/plancheck" +) + +var _ plancheck.PlanCheck = expectRecreatedResource{} + +type expectRecreatedResource struct { + resourceName string +} + +// CheckPlan implements the plan check logic. +func (e expectRecreatedResource) CheckPlan(ctx context.Context, req plancheck.CheckPlanRequest, resp *plancheck.CheckPlanResponse) { + var result []error + + for _, rc := range req.Plan.ResourceChanges { + if rc.Address == e.resourceName { + changes := rc.Change.Actions + if len(changes) != 2 { + result = append(result, fmt.Errorf("2 changes for resource %s (delete and create): %d found", rc.Address, len(changes))) + continue + } + if changes[0] != tfjson.ActionDelete && changes[1] != tfjson.ActionCreate { + result = append(result, fmt.Errorf("expected delete then create for resource %s, but found planned action(s): %v", rc.Address, rc.Change.Actions)) + } + } + } + + resp.Error = errors.Join(result...) +} + +// expectRecreatedResource returns a plan check that asserts is a delete and create change present. +// All output and resource changes found will be aggregated and returned in a plan check error. +func ExpectRecreatedResource(resourceName string) plancheck.PlanCheck { + return expectRecreatedResource{ + resourceName: resourceName, + } +} diff --git a/internal/provider/provider.go b/internal/provider/provider.go index fd3ce2be..9d91f928 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -378,6 +378,7 @@ func (p *jujuProvider) Resources(_ context.Context) []func() resource.Resource { func() resource.Resource { return NewUserResource() }, func() resource.Resource { return NewSecretResource() }, func() resource.Resource { return NewAccessSecretResource() }, + func() resource.Resource { return NewJAASAccessModelResource() }, } } diff --git a/internal/provider/provider_test.go b/internal/provider/provider_test.go index 341be882..736c3a40 100644 --- a/internal/provider/provider_test.go +++ b/internal/provider/provider_test.go @@ -266,3 +266,13 @@ func TestFrameworkProviderSchema(t *testing.T) { assert.Equal(t, resp.Diagnostics.HasError(), false) assert.Len(t, resp.Schema.Attributes, 6) } + +func expectedResourceOwner() string { + // Only 1 field is expected to be populated. + username := os.Getenv(JujuUsernameEnvKey) + clientId := os.Getenv(JujuClientIDEnvKey) + if clientId != "" { + clientId = clientId + "@serviceaccount" + } + return username + clientId +} diff --git a/internal/provider/resource_access_generic.go b/internal/provider/resource_access_generic.go index eb0c033d..730ff920 100644 --- a/internal/provider/resource_access_generic.go +++ b/internal/provider/resource_access_generic.go @@ -7,11 +7,13 @@ import ( "context" "fmt" "regexp" + "strings" jimmnames "github.com/canonical/jimm-go-sdk/v3/names" "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/attr" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" @@ -20,9 +22,10 @@ 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/names/v5" + "github.com/juju/names/v5" "github.com/juju/terraform-provider-juju/internal/juju" ) @@ -37,10 +40,17 @@ type Getter interface { Get(ctx context.Context, target interface{}) diag.Diagnostics } -// resourceInfo defines how the [genericJAASAccessResource] can query for information +// Setter is used to set details on a state object. +// Implemented by Terraform's [State] type. +type Setter interface { + Set(ctx context.Context, target interface{}) diag.Diagnostics +} + +// resourcer defines how the [genericJAASAccessResource] can query/save for information // on the target object. -type resourceInfo interface { - Identity(ctx context.Context, getter Getter, diag *diag.Diagnostics) string +type resourcer interface { + Info(ctx context.Context, getter Getter, diag *diag.Diagnostics) (genericJAASAccessModel, names.Tag) + Save(ctx context.Context, setter Setter, info genericJAASAccessModel, tag names.Tag) diag.Diagnostics } // genericJAASAccessResource is a generic resource that can be used for creating access rules with JAAS. @@ -49,7 +59,7 @@ type resourceInfo interface { // The embedded struct requires a targetInfo interface to enable fetching the target object in the relation. type genericJAASAccessResource struct { client *juju.Client - targetInfo resourceInfo + targetResource resourcer resourceLogName string // subCtx is the context created with the new tflog subsystem for applications. @@ -64,9 +74,6 @@ type genericJAASAccessModel struct { ServiceAccounts types.Set `tfsdk:"service_accounts"` Groups types.Set `tfsdk:"groups"` Access types.String `tfsdk:"access"` - - // ID required by the testing framework - ID types.String `tfsdk:"id"` } // ConfigValidators sets validators for the resource. @@ -116,7 +123,15 @@ func (r *genericJAASAccessResource) partialAccessSchema() map[string]schema.Attr // service accounts are treated as users but defined separately // for different validation and logic in the provider. Validators: []validator.Set{ - setvalidator.ValueStringsAre(ValidatorMatchString(names.IsValidUser, "service account must be valid Juju usernames")), + setvalidator.ValueStringsAre(ValidatorMatchString( + func(s string) bool { + // Use EnsureValidServiceAccountId instead of IsValidServiceAccountId + // because we avoid requiring the user to add @serviceaccount for service accounts + // and opt to add that in the provide code. EnsureValidServiceAccountId adds the + // @serviceaccount domain before verifying the string is a valid service account ID. + _, err := jimmnames.EnsureValidServiceAccountId(s) + return err == nil + }, "service account ID must be a valid Juju username")), setvalidator.ValueStringsAre(stringvalidator.RegexMatches(avoidAtSymbolRe, "service account should not contain an @ symbol")), }, }, @@ -126,7 +141,7 @@ func (r *genericJAASAccessResource) partialAccessSchema() map[string]schema.Attr // Configure enables provider-level data or clients to be set in the // provider-defined DataSource type. It is separately executed for each // ReadDataSource RPC. -func (a *genericJAASAccessResource) Configure(ctx context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { +func (resource *genericJAASAccessResource) Configure(ctx context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { // Prevent panic if the provider has not been configured. if req.ProviderData == nil { return @@ -140,27 +155,290 @@ func (a *genericJAASAccessResource) Configure(ctx context.Context, req resource. ) return } - a.client = client + resource.client = client // Create the local logging subsystem here, using the TF context when creating it. - a.subCtx = tflog.NewSubsystem(ctx, a.resourceLogName) + resource.subCtx = tflog.NewSubsystem(ctx, resource.resourceLogName) } // Create defines how tuples for access control will be created. -func (a *genericJAASAccessResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { +func (resource *genericJAASAccessResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { + // Check first if the client is configured + if resource.client == nil { + addClientNotConfiguredError(&resp.Diagnostics, resource.resourceLogName, "create") + return + } + + // Read Terraform configuration from the request into the model + plan, targetTag := resource.info(ctx, req.Plan, &resp.Diagnostics) + if resp.Diagnostics.HasError() { + return + } + // Create tuples to create from the plan + tuples := modelToTuples(ctx, targetTag, plan, &resp.Diagnostics) + if resp.Diagnostics.HasError() { + return + } + // Make a call to create relations + err := resource.client.Jaas.AddRelations(tuples) + if err != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create access relationships, got error: %s", err)) + return + } + // Set the plan onto the Terraform state + resp.Diagnostics.Append(resource.targetResource.Save(ctx, &resp.State, plan, targetTag)...) } // Read defines how tuples for access control will be read. -func (a *genericJAASAccessResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { +func (resource *genericJAASAccessResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { + // Check first if the client is configured + if resource.client == nil { + addClientNotConfiguredError(&resp.Diagnostics, resource.resourceLogName, "read") + return + } + // Read Terraform configuration from the request into the plan + state, targetTag := resource.info(ctx, req.State, &resp.Diagnostics) + if resp.Diagnostics.HasError() { + return + } + // Create a tuple that defines what relations we are interested in + readTuple := juju.JaasTuple{ + Target: targetTag.String(), + Relation: state.Access.ValueString(), + } + tuples, err := resource.client.Jaas.ReadRelations(ctx, &readTuple) + if err != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to read access rules, got error: %s", err)) + return + } + // Transform the tuples into an access model + newModel := tuplesToModel(ctx, tuples, &resp.Diagnostics) + if resp.Diagnostics.HasError() { + return + } + state.Users = newModel.Users + state.Groups = newModel.Groups + state.ServiceAccounts = newModel.ServiceAccounts + resp.Diagnostics.Append(resource.targetResource.Save(ctx, &resp.State, state, targetTag)...) } // Update defines how tuples for access control will be updated. -func (a *genericJAASAccessResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { +func (resource *genericJAASAccessResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { + // Check first if the client is configured + if resource.client == nil { + addClientNotConfiguredError(&resp.Diagnostics, resource.resourceLogName, "update") + return + } + + // Note: We only need to read the targetID from either the plan or the state. + // If it changed, the resource should be replaced rather than updated. + + // Read Terraform configuration from the state + state, targetTag := resource.info(ctx, req.State, &resp.Diagnostics) + if resp.Diagnostics.HasError() { + return + } + + // Read Terraform configuration from the plan + plan, _ := resource.info(ctx, req.Plan, &resp.Diagnostics) + if resp.Diagnostics.HasError() { + return + } + + // Get a diff of the plan vs. state to know what relations to add/remove + modelAdd, modelRemove := diffModels(plan, state, &resp.Diagnostics) + if resp.Diagnostics.HasError() { + return + } + + // Create a list of tuples to add and tuples to remove + addTuples := modelToTuples(ctx, targetTag, modelAdd, &resp.Diagnostics) + if resp.Diagnostics.HasError() { + return + } + removeTuples := modelToTuples(ctx, targetTag, modelRemove, &resp.Diagnostics) + if resp.Diagnostics.HasError() { + return + } + + // Add new relations + if len(addTuples) > 0 { + err := resource.client.Jaas.AddRelations(addTuples) + if err != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to add access rules, got error: %s", err)) + return + } + } + + // TODO: Consider updating the state here to reflect the newly added tuples before removing tuples in case the next removal fails. + // Would require an intermediate state. + // Delete removed relations + if len(removeTuples) > 0 { + err := resource.client.Jaas.DeleteRelations(removeTuples) + if err != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to remove access rules, got error: %s", err)) + return + } + } + + // Set the desired plan onto the Terraform state after all updates have taken place. + resp.Diagnostics.Append(resource.save(ctx, &resp.State, plan, targetTag)...) +} + +func diffModels(plan, state genericJAASAccessModel, diag *diag.Diagnostics) (toAdd, toRemove genericJAASAccessModel) { + newUsers := diffSet(plan.Users, state.Users, diag) + newGroups := diffSet(plan.Groups, state.Groups, diag) + newServiceAccounts := diffSet(plan.ServiceAccounts, state.ServiceAccounts, diag) + toAdd.Users = newUsers + toAdd.Groups = newGroups + toAdd.ServiceAccounts = newServiceAccounts + toAdd.Access = plan.Access + + removedUsers := diffSet(state.Users, plan.Users, diag) + removedGroups := diffSet(state.Groups, plan.Groups, diag) + removedServiceAccounts := diffSet(state.ServiceAccounts, plan.ServiceAccounts, diag) + toRemove.Users = removedUsers + toRemove.Groups = removedGroups + toRemove.ServiceAccounts = removedServiceAccounts + toRemove.Access = plan.Access + + return +} + +func diffSet(current, desired basetypes.SetValue, diag *diag.Diagnostics) basetypes.SetValue { + var diff []attr.Value + for _, source := range current.Elements() { + found := false + for _, target := range desired.Elements() { + if source.Equal(target) { + found = true + } + } + if !found { + diff = append(diff, source) + } + } + newSet, diags := basetypes.NewSetValue(current.ElementType(context.Background()), diff) + diag.Append(diags...) + return newSet } // Delete defines how tuples for access control will be updated. -func (a *genericJAASAccessResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { +func (resource *genericJAASAccessResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { + // Check first if the client is configured + if resource.client == nil { + addClientNotConfiguredError(&resp.Diagnostics, "access model", "delete") + return + } + + // Read Terraform configuration from the state + state, targetTag := resource.info(ctx, req.State, &resp.Diagnostics) + if resp.Diagnostics.HasError() { + return + } + + // Create the tuples to delete + tuples := modelToTuples(ctx, targetTag, state, &resp.Diagnostics) + if resp.Diagnostics.HasError() { + return + } + // Delete the tuples + err := resource.client.Jaas.DeleteRelations(tuples) + if err != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to delete access rules, got error: %s", err)) + return + } +} + +// modelToTuples return a list of tuples based on the access model provided. +func modelToTuples(ctx context.Context, targetTag names.Tag, model genericJAASAccessModel, diag *diag.Diagnostics) []juju.JaasTuple { + var users []string + var groups []string + var serviceAccounts []string + diag.Append(model.Users.ElementsAs(ctx, &users, false)...) + diag.Append(model.Groups.ElementsAs(ctx, &groups, false)...) + diag.Append(model.ServiceAccounts.ElementsAs(ctx, &serviceAccounts, false)...) + if diag.HasError() { + return []juju.JaasTuple{} + } + baseTuple := juju.JaasTuple{ + Target: targetTag.String(), + Relation: model.Access.ValueString(), + } + var tuples []juju.JaasTuple + userNameToTagf := func(s string) string { return names.NewUserTag(s).String() } + groupIDToTagf := func(s string) string { return jimmnames.NewGroupTag(s).String() } + // Note that service accounts are treated as users but with an @serviceaccount domain. + // We add the @serviceaccount domain by calling `EnsureValidServiceAccountId` so that the user writing the plan doesn't have to. + // We can ignore the error below because the inputs have already gone through validation. + serviceAccIDToTagf := func(s string) string { + r, _ := jimmnames.EnsureValidServiceAccountId(s) + return names.NewUserTag(r).String() + } + tuples = append(tuples, assignTupleObject(baseTuple, users, userNameToTagf)...) + tuples = append(tuples, assignTupleObject(baseTuple, groups, groupIDToTagf)...) + tuples = append(tuples, assignTupleObject(baseTuple, serviceAccounts, serviceAccIDToTagf)...) + return tuples +} + +// tuplesToModel does the reverse of planToTuples converting a slice of tuples to an access model. +func tuplesToModel(ctx context.Context, tuples []juju.JaasTuple, diag *diag.Diagnostics) genericJAASAccessModel { + var users []string + var groups []string + var serviceAccounts []string + for _, tuple := range tuples { + tag, err := jimmnames.ParseTag(tuple.Object) + if err != nil { + diag.AddError("failed to parse relation tag", fmt.Sprintf("error parsing %s:%s", tuple.Object, err.Error())) + continue + } + switch tag.Kind() { + case names.UserTagKind: + userTag := tag.(names.UserTag) + if jimmnames.IsValidServiceAccountId(userTag.Id()) { + // Remove the domain so it matches the plan. + svcAccount := userTag.Id() + domainStart := strings.IndexRune(userTag.Id(), '@') + if domainStart != -1 { + svcAccount = svcAccount[:domainStart] + } + serviceAccounts = append(serviceAccounts, svcAccount) + } else { + users = append(users, userTag.Id()) + } + case jimmnames.GroupTagKind: + groups = append(groups, tag.Id()) + } + } + userSet, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, users) + diag.Append(errDiag...) + groupSet, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, groups) + diag.Append(errDiag...) + serviceAccountSet, errDiag := basetypes.NewSetValueFrom(ctx, types.StringType, serviceAccounts) + diag.Append(errDiag...) + var model genericJAASAccessModel + model.Users = userSet + model.Groups = groupSet + model.ServiceAccounts = serviceAccountSet + return model +} + +func assignTupleObject(baseTuple juju.JaasTuple, items []string, idToTag func(string) string) []juju.JaasTuple { + tuples := make([]juju.JaasTuple, 0, len(items)) + for _, item := range items { + t := baseTuple + t.Object = idToTag(item) + tuples = append(tuples, t) + } + return tuples +} + +func (a *genericJAASAccessResource) info(ctx context.Context, getter Getter, diags *diag.Diagnostics) (genericJAASAccessModel, names.Tag) { + return a.targetResource.Info(ctx, getter, diags) +} +func (a *genericJAASAccessResource) save(ctx context.Context, setter Setter, info genericJAASAccessModel, tag names.Tag) diag.Diagnostics { + return a.targetResource.Save(ctx, setter, info, tag) } diff --git a/internal/provider/resource_access_jaas_model.go b/internal/provider/resource_access_jaas_model.go index de371380..b394e136 100644 --- a/internal/provider/resource_access_jaas_model.go +++ b/internal/provider/resource_access_jaas_model.go @@ -13,6 +13,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/juju/names/v5" ) @@ -23,18 +24,36 @@ var _ resource.ResourceWithConfigure = &jaasAccessModelResource{} // NewJAASAccessModelResource returns a new resource for JAAS model access. func NewJAASAccessModelResource() resource.Resource { return &jaasAccessModelResource{genericJAASAccessResource: genericJAASAccessResource{ - targetInfo: modelInfo{}, + targetResource: modelInfo{}, resourceLogName: LogResourceJAASAccessModel, }} } type modelInfo struct{} -// Identity implements the [resourceInfo] interface, used to extract the model UUID from the Terraform plan/state. -func (j modelInfo) Identity(ctx context.Context, plan Getter, diag *diag.Diagnostics) string { +// Info implements the [resourceInfo] interface, used to extract the info from a Terraform plan/state. +func (j modelInfo) Info(ctx context.Context, getter Getter, diag *diag.Diagnostics) (genericJAASAccessModel, names.Tag) { modelAccess := jaasAccessModelResourceModel{} - diag.Append(plan.Get(ctx, &modelAccess)...) - return names.NewModelTag(modelAccess.ModelUUID.String()).String() + diag.Append(getter.Get(ctx, &modelAccess)...) + accessModel := genericJAASAccessModel{ + Users: modelAccess.Users, + Groups: modelAccess.Groups, + ServiceAccounts: modelAccess.ServiceAccounts, + Access: modelAccess.Access, + } + return accessModel, names.NewModelTag(modelAccess.ModelUUID.ValueString()) +} + +// Save implements the [resourceInfo] interface, used to save info on Terraform's state. +func (j modelInfo) Save(ctx context.Context, setter Setter, info genericJAASAccessModel, tag names.Tag) diag.Diagnostics { + modelAccess := jaasAccessModelResourceModel{ + ModelUUID: basetypes.NewStringValue(tag.Id()), + Users: info.Users, + Groups: info.Groups, + ServiceAccounts: info.ServiceAccounts, + Access: info.Access, + } + return setter.Set(ctx, modelAccess) } type jaasAccessModelResource struct { @@ -42,8 +61,11 @@ type jaasAccessModelResource struct { } type jaasAccessModelResourceModel struct { - ModelUUID types.String `tfsdk:"model_uuid"` - genericJAASAccessModel + ModelUUID types.String `tfsdk:"model_uuid"` + Users types.Set `tfsdk:"users"` + ServiceAccounts types.Set `tfsdk:"service_accounts"` + Groups types.Set `tfsdk:"groups"` + Access types.String `tfsdk:"access"` } // Metadata returns metadata about the JAAS model access resource. diff --git a/internal/provider/resource_access_jaas_model_test.go b/internal/provider/resource_access_jaas_model_test.go new file mode 100644 index 00000000..7742f079 --- /dev/null +++ b/internal/provider/resource_access_jaas_model_test.go @@ -0,0 +1,366 @@ +// Copyright 2023 Canonical Ltd. +// Licensed under the Apache License, Version 2.0, see LICENCE file for details. + +package provider + +import ( + "fmt" + "regexp" + "testing" + + "github.com/canonical/jimm-go-sdk/v3/api" + "github.com/canonical/jimm-go-sdk/v3/api/params" + "github.com/hashicorp/terraform-plugin-testing/helper/acctest" + "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" + "github.com/hashicorp/terraform-plugin-testing/terraform" + "github.com/juju/names/v5" +) + +func TestAcc_ResourceJaasAccessModel(t *testing.T) { + OnlyTestAgainstJAAS(t) + modelName := acctest.RandomWithPrefix("tf-jaas-access-model") + accessSuccess := "writer" + accessFail := "bogus" + userOne := "foo@domain.com" + userTwo := "bar@domain.com" + var modelUUID string + + resourceName := "juju_jaas_access_model.test" + + // Test 0: Test an invalid access string. + // Test 1: Test adding a valid set of users. + // Test 2: Test updating the users to remove 1 user. + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: frameworkProviderFactories, + CheckDestroy: resource.ComposeTestCheckFunc( + testAccCheckJaasModelAccess(userOne, accessSuccess, &modelUUID, false), + ), + Steps: []resource.TestStep{ + { + Config: testAccResourceJaasAccessModelTwoUsers(modelName, accessFail, userOne, userTwo), + ExpectError: regexp.MustCompile(fmt.Sprintf("unknown relation %s", accessFail)), + }, + { + Config: testAccResourceJaasAccessModelTwoUsers(modelName, accessSuccess, userOne, userTwo), + Check: resource.ComposeTestCheckFunc( + testAccCheckModelUUIDNotEmpty(resourceName, &modelUUID), + testAccCheckJaasModelAccess(userOne, accessSuccess, &modelUUID, true), + testAccCheckJaasModelAccess(userTwo, accessSuccess, &modelUUID, true), + resource.TestCheckResourceAttr(resourceName, "access", accessSuccess), + resource.TestCheckTypeSetElemAttr(resourceName, "users.*", "foo@domain.com"), + resource.TestCheckTypeSetElemAttr(resourceName, "users.*", "bar@domain.com"), + resource.TestCheckResourceAttr(resourceName, "users.#", "2"), + ), + }, + { + Config: testAccResourceJaasAccessModelOneUser(modelName, accessSuccess, userOne), + Check: resource.ComposeTestCheckFunc( + testAccCheckJaasModelAccess(userOne, accessSuccess, &modelUUID, true), + testAccCheckJaasModelAccess(userTwo, accessSuccess, &modelUUID, false), + resource.TestCheckResourceAttr(resourceName, "access", accessSuccess), + resource.TestCheckTypeSetElemAttr(resourceName, "users.*", "foo@domain.com"), + resource.TestCheckResourceAttr(resourceName, "users.#", "1"), + ), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPostRefresh: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, + }, + }, + }) +} + +// TestAcc_ResourceJaasAccessModelAdmin verifies behaviour when setting admin access. +// When a model is created, it is expected that the model owner is also a model admin. +// Test that the refresh plan is not empty if the model owner is not included and verify +// that the model owner has access to the model. +func TestAcc_ResourceJaasAccessModelAdmin(t *testing.T) { + OnlyTestAgainstJAAS(t) + expectedResourceOwner() + modelName := acctest.RandomWithPrefix("tf-jaas-access-model") + accessAdmin := "administrator" + userOne := "foo@domain.com" + var modelUUID string + + resourceName := "juju_jaas_access_model.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: frameworkProviderFactories, + CheckDestroy: resource.ComposeTestCheckFunc( + testAccCheckJaasModelAccess(userOne, accessAdmin, &modelUUID, false), + // TODO(Kian): The owner keeps access to the model after the destroy model command is + // issued so that they can monitor the progress. Determine if there is a way to ensure + // that relation is also eventually removed. + // testAccCheckJaasModelAccess(expectedResourceOwner(), accessAdmin, &modelUUID, false), + ), + Steps: []resource.TestStep{ + { + Config: testAccResourceJaasAccessModelOneUser(modelName, accessAdmin, userOne), + Check: resource.ComposeTestCheckFunc( + testAccCheckModelUUIDNotEmpty(resourceName, &modelUUID), + testAccCheckJaasModelAccess(userOne, accessAdmin, &modelUUID, true), + testAccCheckJaasModelAccess(expectedResourceOwner(), accessAdmin, &modelUUID, true), + resource.TestCheckResourceAttr(resourceName, "access", accessAdmin), + resource.TestCheckTypeSetElemAttr(resourceName, "users.*", "foo@domain.com"), + resource.TestCheckResourceAttr(resourceName, "users.#", "1"), + ), + ExpectError: regexp.MustCompile(`.*the refresh plan was not empty\.`), + }, + }, + }) +} + +func TestAcc_ResourceJaasAccessModelChangingAccessReplacesResource(t *testing.T) { + OnlyTestAgainstJAAS(t) + modelName := acctest.RandomWithPrefix("tf-jaas-access-model") + accessWriter := "writer" + accessReader := "reader" + userOne := "foo@domain.com" + var modelUUID string + + resourceName := "juju_jaas_access_model.test" + + // Test 1: Test adding a valid user. + // Test 2: Test updating model access string and see the resource will be replaced. + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: frameworkProviderFactories, + CheckDestroy: resource.ComposeTestCheckFunc( + testAccCheckJaasModelAccess(userOne, accessWriter, &modelUUID, false), + ), + Steps: []resource.TestStep{ + { + Config: testAccResourceJaasAccessModelOneUser(modelName, accessWriter, userOne), + Check: resource.ComposeTestCheckFunc( + testAccCheckModelUUIDNotEmpty(resourceName, &modelUUID), + testAccCheckJaasModelAccess(userOne, accessWriter, &modelUUID, true), + resource.TestCheckResourceAttr(resourceName, "access", accessWriter), + resource.TestCheckTypeSetElemAttr(resourceName, "users.*", "foo@domain.com"), + resource.TestCheckResourceAttr(resourceName, "users.#", "1"), + ), + }, + { + Config: testAccResourceJaasAccessModelOneUser(modelName, accessReader, userOne), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + ExpectRecreatedResource(resourceName), + }, + }, + }, + }, + }) +} + +func TestAcc_ResourceJaasAccessModelServiceAccountAndUsers(t *testing.T) { + OnlyTestAgainstJAAS(t) + modelName := acctest.RandomWithPrefix("tf-jaas-access-model") + accessSuccess := "writer" + svcAccountOne := "foo-1" + svcAccountTwo := "foo-2" + user := "bob@domain.com" + svcAccountOneWithDomain := svcAccountOne + "@serviceaccount" + svcAccountTwoWithDomain := svcAccountTwo + "@serviceaccount" + var modelUUID string + + resourceName := "juju_jaas_access_model.test" + + // Test 0: Test adding an invalid service account tag + // Test 0: Test adding a valid service account. + // Test 1: Test adding an additional service account and user. + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: frameworkProviderFactories, + CheckDestroy: resource.ComposeTestCheckFunc( + testAccCheckJaasModelAccess(svcAccountOneWithDomain, accessSuccess, &modelUUID, false), + testAccCheckJaasModelAccess(svcAccountTwoWithDomain, accessSuccess, &modelUUID, false), + testAccCheckJaasModelAccess(user, accessSuccess, &modelUUID, false), + ), + Steps: []resource.TestStep{ + { + Config: testAccResourceJaasAccessModelOneSvcAccount(modelName, accessSuccess, "##invalid-svc-acc-id##"), + // The regex below may break because of changes in formatting/line breaks in the TF output. + ExpectError: regexp.MustCompile(".*ID must be a valid Juju username.*"), + }, + { + Config: testAccResourceJaasAccessModelOneSvcAccount(modelName, accessSuccess, svcAccountOne), + Check: resource.ComposeTestCheckFunc( + testAccCheckModelUUIDNotEmpty(resourceName, &modelUUID), + testAccCheckJaasModelAccess(svcAccountOneWithDomain, accessSuccess, &modelUUID, true), + resource.TestCheckResourceAttr(resourceName, "access", accessSuccess), + resource.TestCheckTypeSetElemAttr(resourceName, "service_accounts.*", svcAccountOne), + resource.TestCheckResourceAttr(resourceName, "service_accounts.#", "1"), + ), + }, + { + Config: testAccResourceJaasAccessModelSvcAccsAndUser(modelName, accessSuccess, user, svcAccountOne, svcAccountTwo), + Check: resource.ComposeTestCheckFunc( + testAccCheckModelUUIDNotEmpty(resourceName, &modelUUID), + testAccCheckJaasModelAccess(user, accessSuccess, &modelUUID, true), + testAccCheckJaasModelAccess(svcAccountOneWithDomain, accessSuccess, &modelUUID, true), + testAccCheckJaasModelAccess(svcAccountTwoWithDomain, accessSuccess, &modelUUID, true), + resource.TestCheckResourceAttr(resourceName, "access", accessSuccess), + resource.TestCheckTypeSetElemAttr(resourceName, "users.*", user), + resource.TestCheckTypeSetElemAttr(resourceName, "service_accounts.*", svcAccountOne), + resource.TestCheckTypeSetElemAttr(resourceName, "service_accounts.*", svcAccountTwo), + resource.TestCheckResourceAttr(resourceName, "users.#", "1"), + resource.TestCheckResourceAttr(resourceName, "service_accounts.#", "2"), + ), + }, + }, + }) +} + +// TODO(Kian): Add the test below after a stable release of the provider that includes jaas resources. + +// func TestAcc_ResourceJaasAccessModel_UpgradeProvider(t *testing.T) { +// OnlyTestAgainstJAAS(t) +// if testingCloud != LXDCloudTesting { +// t.Skip(t.Name() + " only runs with LXD") +// } + +// modelName := acctest.RandomWithPrefix("tf-jaas-access-model") +// accessSuccess := "writer" + +// resourceName := "juju_access_model.test" +// resource.ParallelTest(t, resource.TestCase{ +// PreCheck: func() { testAccPreCheck(t) }, + +// Steps: []resource.TestStep{ +// { +// ExternalProviders: map[string]resource.ExternalProvider{ +// "juju": { +// VersionConstraint: TestProviderStableVersion, +// Source: "juju/juju", +// }, +// }, +// Config: testAccResourceJaasAccessModel(modelName, accessSuccess), +// Check: resource.ComposeTestCheckFunc( +// resource.TestMatchResourceAttr(resourceName, "model_uuid", regexp.MustCompile(".+")), +// resource.TestCheckResourceAttr(resourceName, "access", accessSuccess), +// resource.TestCheckTypeSetElemAttr(resourceName, "users.*", "foo@domain.com"), +// resource.TestCheckTypeSetElemAttr(resourceName, "users.*", "bar@domain.com"), +// ), +// }, +// { +// ProtoV6ProviderFactories: frameworkProviderFactories, +// Config: testAccResourceJaasAccessModel(modelName, accessSuccess), +// PlanOnly: true, +// }, +// }, +// }) +// } + +func testAccResourceJaasAccessModelTwoUsers(modelName, access, userOne, userTwo string) string { + return fmt.Sprintf(` +resource "juju_model" "test-model" { + name = %q +} + +resource "juju_jaas_access_model" "test" { + model_uuid = juju_model.test-model.id + access = %q + users = [%q, %q] +}`, modelName, access, userOne, userTwo) +} + +func testAccResourceJaasAccessModelOneUser(modelName, access, userOne string) string { + return fmt.Sprintf(` +resource "juju_model" "test-model" { + name = %q +} + +resource "juju_jaas_access_model" "test" { + model_uuid = juju_model.test-model.id + access = %q + users = [%q] +}`, modelName, access, userOne) +} + +func testAccResourceJaasAccessModelOneSvcAccount(modelName, access, svcAccOne string) string { + return fmt.Sprintf(` +resource "juju_model" "test-model" { + name = %q +} + +resource "juju_jaas_access_model" "test" { + model_uuid = juju_model.test-model.id + access = %q + service_accounts = [%q] +}`, modelName, access, svcAccOne) +} + +func testAccResourceJaasAccessModelSvcAccsAndUser(modelName, access, user, svcAccOne, svcAccTwo string) string { + return fmt.Sprintf(` +resource "juju_model" "test-model" { + name = %q +} + +resource "juju_jaas_access_model" "test" { + model_uuid = juju_model.test-model.id + access = %q + users = [%q] + service_accounts = [%q, %q] +}`, modelName, access, user, svcAccOne, svcAccTwo) +} + +func testAccCheckModelUUIDNotEmpty(resourceName string, modelUUID *string) resource.TestCheckFunc { + return func(s *terraform.State) error { + // retrieve the resource by name from state + rs, ok := s.RootModule().Resources[resourceName] + if !ok { + return fmt.Errorf("Not found: %s", resourceName) + } + + val, ok := rs.Primary.Attributes["model_uuid"] + if !ok { + return fmt.Errorf("Model UUID is not set") + } + if val == "" { + return fmt.Errorf("Model UUID is empty") + } + if modelUUID == nil { + return fmt.Errorf("cannot set model UUID, nil poiner") + } + *modelUUID = val + return nil + } +} + +func testAccCheckJaasModelAccess(user, relation string, modelUUID *string, expectedAccess bool) resource.TestCheckFunc { + return func(s *terraform.State) error { + if modelUUID == nil { + return fmt.Errorf("no model UUID set") + } + conn, err := TestClient.Models.GetConnection(nil) + if err != nil { + return err + } + defer func() { _ = conn.Close() }() + jc := api.NewClient(conn) + req := params.CheckRelationRequest{ + Tuple: params.RelationshipTuple{ + Object: names.NewUserTag(user).String(), + Relation: relation, + TargetObject: names.NewModelTag(*modelUUID).String(), + }, + } + resp, err := jc.CheckRelation(&req) + if err != nil { + return err + } + if resp.Allowed != expectedAccess { + var access string + if expectedAccess { + access = "access" + } else { + access = "no access" + } + return fmt.Errorf("expected %s for user %s as %s to model (%s), but access is %t", access, user, relation, *modelUUID, resp.Allowed) + } + return nil + } +} diff --git a/internal/provider/resource_offer_test.go b/internal/provider/resource_offer_test.go index b9e11c5d..1958188d 100644 --- a/internal/provider/resource_offer_test.go +++ b/internal/provider/resource_offer_test.go @@ -5,7 +5,6 @@ package provider import ( "fmt" - "os" "testing" "github.com/hashicorp/terraform-plugin-testing/helper/acctest" @@ -28,8 +27,8 @@ func TestAcc_ResourceOffer(t *testing.T) { Config: testAccResourceOffer(modelName, "base = \"ubuntu@22.04\""), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("juju_offer.this", "model", modelName), - resource.TestCheckResourceAttr("juju_offer.this", "url", fmt.Sprintf("%v/%v.%v", expectedOfferUser(), modelName, "this")), - resource.TestCheckResourceAttr("juju_offer.this", "id", fmt.Sprintf("%v/%v.%v", expectedOfferUser(), modelName, "this")), + resource.TestCheckResourceAttr("juju_offer.this", "url", fmt.Sprintf("%v/%v.%v", expectedResourceOwner(), modelName, "this")), + resource.TestCheckResourceAttr("juju_offer.this", "id", fmt.Sprintf("%v/%v.%v", expectedResourceOwner(), modelName, "this")), ), }, { @@ -41,7 +40,7 @@ func TestAcc_ResourceOffer(t *testing.T) { map[string]string{"name": "apptwo", "endpoint": "db", "offer_url": ""}), resource.TestCheckTypeSetElemNestedAttrs("juju_integration.int", "application.*", - map[string]string{"name": "", "endpoint": "", "offer_url": fmt.Sprintf("%v/%v.%v", expectedOfferUser(), + map[string]string{"name": "", "endpoint": "", "offer_url": fmt.Sprintf("%v/%v.%v", expectedResourceOwner(), modelName2, "appone")}), ), }, @@ -125,8 +124,8 @@ func TestAcc_ResourceOffer_UpgradeProvider(t *testing.T) { Config: testAccResourceOffer(modelName, "series = \"focal\""), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("juju_offer.this", "model", modelName), - resource.TestCheckResourceAttr("juju_offer.this", "url", fmt.Sprintf("%v/%v.%v", expectedOfferUser(), modelName, "this")), - resource.TestCheckResourceAttr("juju_offer.this", "id", fmt.Sprintf("%v/%v.%v", expectedOfferUser(), modelName, "this")), + resource.TestCheckResourceAttr("juju_offer.this", "url", fmt.Sprintf("%v/%v.%v", expectedResourceOwner(), modelName, "this")), + resource.TestCheckResourceAttr("juju_offer.this", "id", fmt.Sprintf("%v/%v.%v", expectedResourceOwner(), modelName, "this")), ), }, { @@ -162,13 +161,3 @@ resource "juju_offer" "this" { } `, modelName, os) } - -func expectedOfferUser() string { - // Only 1 field is expected to be populated. - username := os.Getenv(JujuUsernameEnvKey) - clientId := os.Getenv(JujuClientIDEnvKey) - if clientId != "" { - clientId = clientId + "@serviceaccount" - } - return username + clientId -}