diff --git a/docs/resources/model.md b/docs/resources/model.md index f37b0fe1..e7f593f3 100644 --- a/docs/resources/model.md +++ b/docs/resources/model.md @@ -34,7 +34,7 @@ resource "juju_model" "this" { ### Required -- `name` (String) The name to be assigned to the model +- `name` (String) The name to be assigned to the model. Changing this value will require the model to be destroyed and recreated by terraform. ### Optional @@ -47,6 +47,7 @@ resource "juju_model" "this" { - `id` (String) The ID of this resource. - `type` (String) Type of the model. Set by the Juju's API server +- `uuid` (String) The uuid of the model ### Nested Schema for `cloud` diff --git a/internal/juju/client.go b/internal/juju/client.go index 9ad85187..f6085140 100644 --- a/internal/juju/client.go +++ b/internal/juju/client.go @@ -17,6 +17,7 @@ import ( "github.com/juju/juju/api/client/modelmanager" "github.com/juju/juju/api/connector" "github.com/juju/juju/core/model" + "github.com/juju/names/v5" ) const ( @@ -63,12 +64,12 @@ func (c Client) IsJAAS() bool { } type jujuModel struct { - uuid string + name string modelType model.ModelType } func (j jujuModel) String() string { - return fmt.Sprintf("uuid(%s) type(%s)", j.uuid, j.modelType.String()) + return fmt.Sprintf("uuid(%s) type(%s)", j.name, j.modelType.String()) } type sharedClient struct { @@ -145,12 +146,14 @@ func (sc *sharedClient) IsJAAS(defaultVal bool) bool { } // GetConnection returns a juju connection for use creating juju -// api clients given the provided model name. -func (sc *sharedClient) GetConnection(modelName *string) (api.Connection, error) { +// api clients given the provided model uuid, name, or neither. +// Allowing a model name is a fallback behavior until the name +// used by most resources has been removed in favor of the UUID. +func (sc *sharedClient) GetConnection(modelIdentifier *string) (api.Connection, error) { var modelUUID string - if modelName != nil { + if modelIdentifier != nil { var err error - modelUUID, err = sc.ModelUUID(*modelName) + modelUUID, err = sc.ModelUUID(*modelIdentifier) if err != nil { return nil, err } @@ -184,7 +187,16 @@ func (sc *sharedClient) GetConnection(modelName *string) (api.Connection, error) return conn, nil } -func (sc *sharedClient) ModelUUID(modelName string) (string, error) { +// ModelUUID returns the model uuid for the provided modelIdentifier. +// The modelIdentifier can be a model name or model uuid. If the modelIdentifier +// is a uuid, return that without verification. If a name is provided, first +// search the modelUUIDCache for the uuid. If it's not found, fill the model +// cache and try again. +func (sc *sharedClient) ModelUUID(modelIdentifier string) (string, error) { + if names.IsValidModel(modelIdentifier) { + return modelIdentifier, nil + } + modelName := modelIdentifier sc.modelUUIDmu.Lock() defer sc.modelUUIDmu.Unlock() dataMap := make(map[string]interface{}) @@ -193,16 +205,20 @@ func (sc *sharedClient) ModelUUID(modelName string) (string, error) { dataMap[k] = v.String() } sc.Tracef(fmt.Sprintf("ModelUUID cache looking for %q", modelName), dataMap) - if modelWithName, ok := sc.modelUUIDcache[modelName]; ok { - sc.Tracef(fmt.Sprintf("Found uuid for %q in cache", modelName)) - return modelWithName.uuid, nil + for uuid, m := range sc.modelUUIDcache { + if m.name == modelName { + sc.Tracef(fmt.Sprintf("Found uuid for %q in cache", modelName)) + return uuid, nil + } } if err := sc.fillModelCache(); err != nil { return "", err } - if modelWithName, ok := sc.modelUUIDcache[modelName]; ok { - sc.Tracef(fmt.Sprintf("Found uuid for %q in cache on 2nd attempt", modelName)) - return modelWithName.uuid, nil + for uuid, m := range sc.modelUUIDcache { + if m.name == modelName { + sc.Tracef(fmt.Sprintf("Found uuid for %q in cache on 2nd attempt", modelName)) + return uuid, nil + } } return "", errors.NotFoundf("model %q", modelName) } @@ -226,47 +242,65 @@ func (sc *sharedClient) fillModelCache() error { return err } for _, modelSummary := range modelSummaries { - modelWithName := jujuModel{ - uuid: modelSummary.UUID, + modelWithUUID := jujuModel{ + name: modelSummary.Name, modelType: modelSummary.Type, } - sc.modelUUIDcache[modelSummary.Name] = modelWithName + sc.modelUUIDcache[modelSummary.UUID] = modelWithUUID } return nil } -func (sc *sharedClient) ModelType(modelName string) (model.ModelType, error) { +// ModelType returns the model type for the provided modelIdentifier from +// the cache of model data. The modelIdentifier can be a name or UUID. +func (sc *sharedClient) ModelType(modelIdentifier string) (model.ModelType, error) { sc.modelUUIDmu.Lock() defer sc.modelUUIDmu.Unlock() - if modelWithName, ok := sc.modelUUIDcache[modelName]; ok { - return modelWithName.modelType, nil + if names.IsValidModel(modelIdentifier) { + if modelWithUUID, ok := sc.modelUUIDcache[modelIdentifier]; ok { + return modelWithUUID.modelType, nil + } } - return model.ModelType(""), errors.NotFoundf("type for model %q", modelName) + for _, m := range sc.modelUUIDcache { + if m.name == modelIdentifier { + return m.modelType, nil + } + } + + return "", errors.NotFoundf("type for model %q", modelIdentifier) } +// RemoveModel deletes the model with the given UUID from the cache of +// model data. func (sc *sharedClient) RemoveModel(modelUUID string) { sc.modelUUIDmu.Lock() - var modelName string - for k, v := range sc.modelUUIDcache { - if v.uuid == modelUUID { - modelName = k - break - } - } - if modelName != "" { - delete(sc.modelUUIDcache, modelName) - } + delete(sc.modelUUIDcache, modelUUID) sc.modelUUIDmu.Unlock() } +// AddModel adds a model to the cache of model data. If any of the three required +// pieces of data are empty, nothing is added to the cache of model data. If the UUID +// already exists in the cache, do nothing. func (sc *sharedClient) AddModel(modelName, modelUUID string, modelType model.ModelType) { + if modelName == "" || !names.IsValidModel(modelUUID) || modelType.String() == "" { + sc.Tracef("Missing data, failed to add to the cache.", map[string]interface{}{ + "modelName": modelName, "modelUUID": modelUUID, "modelType": modelType.String()}) + return + } + sc.modelUUIDmu.Lock() - sc.modelUUIDcache[modelName] = jujuModel{ - uuid: modelUUID, + defer sc.modelUUIDmu.Unlock() + if m, ok := sc.modelUUIDcache[modelUUID]; ok { + sc.Warnf("Attempting to add an existing model to the cache.", map[string]interface{}{ + "existing model in cache": m, "new modelName": modelName, "new modelUUID": modelUUID, + "new modelType": modelType.String()}) + return + } + sc.modelUUIDcache[modelUUID] = jujuModel{ + name: modelName, modelType: modelType, } - sc.modelUUIDmu.Unlock() } // module names for logging diff --git a/internal/juju/models.go b/internal/juju/models.go index 33358114..bbe41dc4 100644 --- a/internal/juju/models.go +++ b/internal/juju/models.go @@ -11,6 +11,7 @@ import ( "github.com/juju/juju/api/client/modelconfig" "github.com/juju/juju/api/client/modelmanager" "github.com/juju/juju/core/constraints" + "github.com/juju/juju/core/model" "github.com/juju/juju/rpc/params" "github.com/juju/names/v5" ) @@ -34,12 +35,6 @@ type modelsClient struct { SharedClient } -type GrantModelInput struct { - User string - Access string - ModelName string -} - type CreateModelInput struct { Name string CloudName string @@ -65,6 +60,7 @@ type ReadModelResponse struct { type UpdateModelInput struct { Name string + UUID string CloudName string Config map[string]string Unset []string @@ -72,22 +68,28 @@ type UpdateModelInput struct { Credential string } -type UpdateAccessModelInput struct { - ModelName string - OldAccess string - Grant []string - Revoke []string - Access string -} - type DestroyModelInput struct { UUID string } +type GrantModelInput struct { + User string + Access string + ModelIdentifier string +} + +type UpdateAccessModelInput struct { + ModelIdentifier string + OldAccess string + Grant []string + Revoke []string + Access string +} + type DestroyAccessModelInput struct { - ModelName string - Revoke []string - Access string + ModelIdentifier string + Revoke []string + Access string } func newModelsClient(sc SharedClient) *modelsClient { @@ -124,6 +126,8 @@ func (c *modelsClient) GetModelByName(name string) (*params.ModelInfo, error) { modelInfo := results[0].Result + c.AddModel(modelInfo.Name, modelUUID, model.ModelType(modelInfo.Type)) + c.Tracef(fmt.Sprintf("Retrieved model info: %s, %+v", name, modelInfo)) return modelInfo, nil } @@ -256,7 +260,7 @@ func (c *modelsClient) ReadModel(name string) (*ReadModelResponse, error) { } func (c *modelsClient) UpdateModel(input UpdateModelInput) error { - conn, err := c.GetConnection(&input.Name) + conn, err := c.GetConnection(&input.UUID) if err != nil { return err } @@ -350,7 +354,7 @@ func (c *modelsClient) GrantModel(input GrantModelInput) error { client := modelmanager.NewClient(conn) - modelUUID, err := c.ModelUUID(input.ModelName) + modelUUID, err := c.ModelUUID(input.ModelIdentifier) if err != nil { return err } @@ -367,7 +371,7 @@ func (c *modelsClient) GrantModel(input GrantModelInput) error { // If a user has had `write`, then removing that access would decrease their // access to `read` and the user will remain part of the model access. func (c *modelsClient) UpdateAccessModel(input UpdateAccessModelInput) error { - model := input.ModelName + model := input.ModelIdentifier access := input.OldAccess uuid, err := c.ModelUUID(model) @@ -412,7 +416,7 @@ func (c *modelsClient) DestroyAccessModel(input DestroyAccessModelInput) error { client := modelmanager.NewClient(conn) - uuid, err := c.ModelUUID(input.ModelName) + uuid, err := c.ModelUUID(input.ModelIdentifier) if err != nil { return err } diff --git a/internal/provider/resource_access_jaas_model_test.go b/internal/provider/resource_access_jaas_model_test.go index 71458cf3..8a0e253a 100644 --- a/internal/provider/resource_access_jaas_model_test.go +++ b/internal/provider/resource_access_jaas_model_test.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/terraform-plugin-testing/plancheck" "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/juju/names/v5" + internaltesting "github.com/juju/terraform-provider-juju/internal/testing" ) @@ -349,7 +350,7 @@ resource "juju_model" "test-model" { } resource "juju_jaas_access_model" "test" { - model_uuid = juju_model.test-model.id + model_uuid = juju_model.test-model.uuid access = "{{.Access}}" users = ["{{.UserOne}}", "{{.UserTwo}}"] } @@ -370,7 +371,7 @@ resource "juju_model" "test-model" { } resource "juju_jaas_access_model" "test" { - model_uuid = juju_model.test-model.id + model_uuid = juju_model.test-model.uuid access = "{{.Access}}" users = ["{{.User}}"] } @@ -394,7 +395,7 @@ resource "juju_jaas_group" "test" { } resource "juju_jaas_access_model" "test" { - model_uuid = juju_model.test-model.id + model_uuid = juju_model.test-model.uuid access = "{{.Access}}" users = ["{{.User}}"] groups = [juju_jaas_group.test.uuid] @@ -418,7 +419,7 @@ resource "juju_model" "test-model" { } resource "juju_jaas_access_model" "test" { - model_uuid = juju_model.test-model.id + model_uuid = juju_model.test-model.uuid access = "{{.Access}}" service_accounts = ["{{.SvcAcc}}"] } @@ -438,7 +439,7 @@ resource "juju_model" "test-model" { } resource "juju_jaas_access_model" "test" { - model_uuid = juju_model.test-model.id + model_uuid = juju_model.test-model.uuid access = "{{.Access}}" users = ["{{.User}}"] service_accounts = ["{{.SvcAccOne}}", "{{.SvcAccTwo}}"] diff --git a/internal/provider/resource_access_model.go b/internal/provider/resource_access_model.go index 7f78aef5..1fd8a05d 100644 --- a/internal/provider/resource_access_model.go +++ b/internal/provider/resource_access_model.go @@ -142,9 +142,9 @@ func (a *accessModelResource) Create(ctx context.Context, req resource.CreateReq // Call Models.GrantModel for _, user := range users { err := a.client.Models.GrantModel(juju.GrantModelInput{ - User: user, - Access: accessStr, - ModelName: modelNameStr, + User: user, + Access: accessStr, + ModelIdentifier: modelNameStr, }) if err != nil { resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create access model resource, got error: %s", err)) @@ -281,11 +281,11 @@ func (a *accessModelResource) Update(ctx context.Context, req resource.UpdateReq } err := a.client.Models.UpdateAccessModel(juju.UpdateAccessModelInput{ - ModelName: modelName, - OldAccess: oldAccess, - Grant: addedUserList, - Revoke: missingUserList, - Access: access, + ModelIdentifier: modelName, + OldAccess: oldAccess, + Grant: addedUserList, + Revoke: missingUserList, + Access: access, }) if err != nil { resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update access model resource, got error: %s", err)) @@ -321,9 +321,9 @@ func (a *accessModelResource) Delete(ctx context.Context, req resource.DeleteReq } err := a.client.Models.DestroyAccessModel(juju.DestroyAccessModelInput{ - ModelName: plan.Model.ValueString(), - Revoke: stateUsers, - Access: plan.Access.ValueString(), + ModelIdentifier: plan.Model.ValueString(), + Revoke: stateUsers, + Access: plan.Access.ValueString(), }) if err != nil { resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to delete access model resource, got error: %s", err)) diff --git a/internal/provider/resource_model.go b/internal/provider/resource_model.go index 442f8710..ea09f972 100644 --- a/internal/provider/resource_model.go +++ b/internal/provider/resource_model.go @@ -52,6 +52,7 @@ type modelResourceModel struct { Constraints types.String `tfsdk:"constraints"` Credential types.String `tfsdk:"credential"` Type types.String `tfsdk:"type"` + UUID types.String `tfsdk:"uuid"` // ID required by the testing framework ID types.String `tfsdk:"id"` } @@ -67,12 +68,20 @@ func (r *modelResource) Schema(_ context.Context, _ resource.SchemaRequest, resp Description: "A resource that represent a Juju Model.", Attributes: map[string]schema.Attribute{ "name": schema.StringAttribute{ - Description: "The name to be assigned to the model", - Required: true, + Description: "The name to be assigned to the model. Changing this value will" + + " require the model to be destroyed and recreated by terraform.", + Required: true, PlanModifiers: []planmodifier.String{ stringplanmodifier.RequiresReplace(), }, }, + "uuid": schema.StringAttribute{ + Description: "The uuid of the model", + Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, + }, "config": schema.MapAttribute{ Description: "Override default model configuration", Optional: true, @@ -224,7 +233,7 @@ func (r *modelResource) Create(ctx context.Context, req resource.CreateRequest, Credential: credential, }) if err != nil { - resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create model, got error: %s", err)) + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create model %q, got error: %s", plan.Name, err)) return } r.trace(fmt.Sprintf("model created : %q", modelName)) @@ -246,6 +255,7 @@ func (r *modelResource) Create(ctx context.Context, req resource.CreateRequest, plan.Credential = types.StringValue(response.CloudCredentialName) plan.Type = types.StringValue(response.Type) + plan.UUID = types.StringValue(response.UUID) plan.ID = types.StringValue(response.UUID) r.trace(fmt.Sprintf("model resource created: %q", modelName)) @@ -366,6 +376,7 @@ func (r *modelResource) Read(ctx context.Context, req resource.ReadRequest, resp state.Name = types.StringValue(modelName) state.Type = types.StringValue(response.ModelInfo.Type) state.Credential = types.StringValue(credential) + state.UUID = types.StringValue(response.ModelInfo.UUID) state.ID = types.StringValue(response.ModelInfo.UUID) r.trace(fmt.Sprintf("Read model resource for: %v", modelName)) @@ -455,7 +466,7 @@ func (r *modelResource) Update(ctx context.Context, req resource.UpdateRequest, } err = r.client.Models.UpdateModel(juju.UpdateModelInput{ - Name: plan.Name.ValueString(), + UUID: plan.UUID.ValueString(), CloudName: cloudNameInput, Config: configMap, Unset: unsetConfigKeys, @@ -486,9 +497,14 @@ func (r *modelResource) Delete(ctx context.Context, req resource.DeleteRequest, return } - err := r.client.Models.DestroyModel(juju.DestroyModelInput{ - UUID: state.ID.ValueString(), - }) + modelUUID := state.UUID.ValueString() + if modelUUID == "" { + modelUUID = state.ID.ValueString() + } + arg := juju.DestroyModelInput{ + UUID: modelUUID, + } + err := r.client.Models.DestroyModel(arg) if err != nil { resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to delete model, got error: %s", err)) return diff --git a/internal/provider/resource_model_test.go b/internal/provider/resource_model_test.go index 20c38d01..7daa1475 100644 --- a/internal/provider/resource_model_test.go +++ b/internal/provider/resource_model_test.go @@ -5,6 +5,7 @@ package provider import ( "fmt" + "regexp" "testing" "github.com/hashicorp/terraform-plugin-testing/helper/acctest" @@ -14,6 +15,8 @@ import ( "github.com/juju/juju/rpc/params" ) +var validUUID = regexp.MustCompile(`[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}`) + func TestAcc_ResourceModel(t *testing.T) { modelName := acctest.RandomWithPrefix("tf-test-model") logLevelInfo := "INFO" @@ -29,6 +32,7 @@ func TestAcc_ResourceModel(t *testing.T) { Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(resourceName, "name", modelName), resource.TestCheckResourceAttr(resourceName, "config.logging-config", fmt.Sprintf("=%s", logLevelInfo)), + resource.TestMatchResourceAttr(resourceName, "uuid", validUUID), ), }, {