chore: Enable migration tests for clusters in legacy schema #2975
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,12 +3,12 @@ package advancedclustertpf | |
| import ( | ||
| "context" | ||
| "fmt" | ||
| "math/big" | ||
| "strings" | ||
|
|
||
| "github.com/hashicorp/terraform-plugin-framework-timeouts/resource/timeouts" | ||
| "github.com/hashicorp/terraform-plugin-framework/attr" | ||
| "github.com/hashicorp/terraform-plugin-framework/diag" | ||
| "github.com/hashicorp/terraform-plugin-framework/resource" | ||
| "github.com/hashicorp/terraform-plugin-framework/tfsdk" | ||
| "github.com/hashicorp/terraform-plugin-framework/types" | ||
|
|
@@ -18,10 +18,12 @@ import ( | |
| "go.mongodb.org/atlas-sdk/v20241113004/admin" | ||
| ) | ||
|
|
||
| // MoveState is used with moved block to upgrade from cluster to adv_cluster | ||
| func (r *rs) MoveState(context.Context) []resource.StateMover { | ||
| return []resource.StateMover{{StateMover: stateMover}} | ||
| } | ||
|
|
||
| // UpgradeState is used to upgrade from adv_cluster schema v1 (SDKv2) to v2 (TPF) | ||
| func (r *rs) UpgradeState(ctx context.Context) map[int64]resource.StateUpgrader { | ||
| return map[int64]resource.StateUpgrader{ | ||
| 1: {StateUpgrader: stateUpgraderFromV1}, | ||
|
|
@@ -39,101 +41,149 @@ func stateUpgraderFromV1(ctx context.Context, req resource.UpgradeStateRequest, | |
| setStateResponse(ctx, &resp.Diagnostics, req.RawState, &resp.State) | ||
| } | ||
|
|
||
| // Minimum attributes needed from source schema. Read will fill in the rest | ||
|
There was a problem hiding this comment. non-blocking comment: in the future where new attributes will be added, will there be some compile-time or test failure if that field needs to be specified here? There was a problem hiding this comment. in line 69 , we're using: IgnoreUndefinedAttributes: true} so the schema doesn't make it fail if they some attribute doesn't exist, later when the value is tried to be read it will be null There was a problem hiding this comment.
There was a problem hiding this comment. if the previous version/resource have them and we don't set them, then there will a plan change, e.g. if timeouts or retain_backups_enabled is defined in SDKv2 and want to migrate to TPF, users will get a plan change saying that timeouts/retain_backups_enabled will be deleted. in the case that will come with moving from flex cluster to adv_cluster for example, they don't exist so it's ok not to send them. There was a problem hiding this comment. it's to avoid plan changes by filling attributes that Read can't do |
||
| var stateAttrs = map[string]tftypes.Type{ | ||
| "project_id": tftypes.String, // project_id and name to identify the cluster | ||
| "name": tftypes.String, | ||
| "retain_backups_enabled": tftypes.Bool, // TF specific so can't be got in Read | ||
| "mongo_db_major_version": tftypes.String, // Has special logic in overrideAttributesWithPrevStateValue that needs the previous state | ||
| "timeouts": tftypes.Object{ // TF specific so can't be got in Read | ||
| AttributeTypes: map[string]tftypes.Type{ | ||
| "create": tftypes.String, | ||
| "update": tftypes.String, | ||
| "delete": tftypes.String, | ||
| }, | ||
| }, | ||
| "replication_specs": tftypes.List{ // Needed to check if some num_shards are > 1 so we need to force legacy schema | ||
| ElementType: tftypes.Object{ | ||
| AttributeTypes: map[string]tftypes.Type{ | ||
| "num_shards": tftypes.Number, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| func setStateResponse(ctx context.Context, diags *diag.Diagnostics, stateIn *tfprotov6.RawState, stateOut *tfsdk.State) { | ||
| rawStateValue, err := stateIn.UnmarshalWithOpts(tftypes.Object{ | ||
| AttributeTypes: stateAttrs, | ||
| }, tfprotov6.UnmarshalOpts{ValueFromJSONOpts: tftypes.ValueFromJSONOpts{IgnoreUndefinedAttributes: true}}) | ||
| if err != nil { | ||
| diags.AddError("Unable to Unmarshal state", err.Error()) | ||
| return | ||
| } | ||
| var stateObj map[string]tftypes.Value | ||
| if err := rawStateValue.As(&stateObj); err != nil { | ||
| diags.AddError("Unable to Parse state", err.Error()) | ||
| return | ||
| } | ||
| projectID, name := getProjectIDNameFromStateObj(diags, stateObj) | ||
| if diags.HasError() { | ||
| return | ||
| } | ||
| model := NewTFModel(ctx, &admin.ClusterDescription20240805{ | ||
| GroupId: projectID, | ||
| Name: name, | ||
| }, getTimeoutFromStateObj(stateObj), diags, ExtraAPIInfo{}) | ||
| if diags.HasError() { | ||
| return | ||
| } | ||
| AddAdvancedConfig(ctx, model, nil, nil, diags) | ||
| if diags.HasError() { | ||
| return | ||
| } | ||
| setOptionalModelAttrs(stateObj, model) | ||
| diags.Append(stateOut.Set(ctx, model)...) | ||
| } | ||
|
|
||
| func getAttrFromStateObj[T any](rawState map[string]tftypes.Value, attrName string) *T { | ||
| var ret *T | ||
| if err := rawState[attrName].As(&ret); err != nil { | ||
| return nil | ||
| } | ||
| return ret | ||
| } | ||
|
|
||
| func getProjectIDNameFromStateObj(diags *diag.Diagnostics, stateObj map[string]tftypes.Value) (projectID, name *string) { | ||
| projectID = getAttrFromStateObj[string](stateObj, "project_id") | ||
| name = getAttrFromStateObj[string](stateObj, "name") | ||
| if !conversion.IsStringPresent(projectID) || !conversion.IsStringPresent(name) { | ||
| diags.AddError("Unable to read project_id or name from state", fmt.Sprintf("project_id: %s, name: %s", | ||
| conversion.SafeString(projectID), conversion.SafeString(name))) | ||
| return | ||
| } | ||
| return projectID, name | ||
| } | ||
|
|
||
| func getTimeoutFromStateObj(stateObj map[string]tftypes.Value) timeouts.Value { | ||
| attrTypes := map[string]attr.Type{ | ||
| "create": types.StringType, | ||
| "update": types.StringType, | ||
| "delete": types.StringType, | ||
| } | ||
| nullObj := timeouts.Value{Object: types.ObjectNull(attrTypes)} | ||
| timeoutState := getAttrFromStateObj[map[string]tftypes.Value](stateObj, "timeouts") | ||
| if timeoutState == nil { | ||
| return nullObj | ||
| } | ||
| timeoutMap := make(map[string]attr.Value) | ||
| for action := range attrTypes { | ||
| actionTimeout := getAttrFromStateObj[string](*timeoutState, action) | ||
| if actionTimeout == nil { | ||
| timeoutMap[action] = types.StringNull() | ||
| } else { | ||
| timeoutMap[action] = types.StringPointerValue(actionTimeout) | ||
| } | ||
| } | ||
| obj, d := types.ObjectValue(attrTypes, timeoutMap) | ||
| if d.HasError() { | ||
| return nullObj | ||
| } | ||
| return timeouts.Value{Object: obj} | ||
| } | ||
|
|
||
| func setOptionalModelAttrs(stateObj map[string]tftypes.Value, model *TFModel) { | ||
| if retainBackupsEnabled := getAttrFromStateObj[bool](stateObj, "retain_backups_enabled"); retainBackupsEnabled != nil { | ||
| model.RetainBackupsEnabled = types.BoolPointerValue(retainBackupsEnabled) | ||
| } | ||
| if mongoDBMajorVersion := getAttrFromStateObj[string](stateObj, "mongo_db_major_version"); mongoDBMajorVersion != nil { | ||
| model.MongoDBMajorVersion = types.StringPointerValue(mongoDBMajorVersion) | ||
| } | ||
| if isLegacySchemaState(stateObj) { | ||
| sendLegacySchemaRequestToRead(model) | ||
| } | ||
| } | ||
|
|
||
| func isLegacySchemaState(stateObj map[string]tftypes.Value) bool { | ||
| one := big.NewFloat(1.0) | ||
| specsVal := getAttrFromStateObj[[]tftypes.Value](stateObj, "replication_specs") | ||
| if specsVal == nil { | ||
| return false | ||
| } | ||
| for _, specVal := range *specsVal { | ||
| var specObj map[string]tftypes.Value | ||
| if err := specVal.As(&specObj); err != nil { | ||
| return false | ||
| } | ||
| numShardsVal := specObj["num_shards"] | ||
| var numShards *big.Float | ||
| if err := numShardsVal.As(&numShards); err != nil || numShards == nil { | ||
| return false | ||
| } | ||
| if numShards.Cmp(one) > 0 { // legacy schema if numShards > 1 | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| // sendLegacySchemaRequestToRead sets ClusterID to a special value so Read can know whether it must use legacy schema. | ||
| // private state can't be used here because it's not available in Move Upgrader. | ||
| // ClusterID is computed (not optional) so the value will be overridden in Read and the special value won't ever appear in the state file. | ||
|
There was a problem hiding this comment. let me know if there is any question about why we use ClusterID as a side channel to communicate between State Move / Upgrader and Read. Also if you have a better idea, please let me know There was a problem hiding this comment. if we go with this option, do you think we can add a check of ClusterID != There was a problem hiding this comment. not using ClusterID any more |
||
| func sendLegacySchemaRequestToRead(model *TFModel) { | ||
| model.ClusterID = types.StringValue("forceLegacySchema") | ||
| } | ||
|
There was a problem hiding this comment. As an alternative, is it too complex to populate the replication spec list only with the bare minimum so our existing logic detects the legacy sharding config (objects with num_shards)? With this approach we would avoid There was a problem hiding this comment. I was a bit reluctant in case we needed to pass a lot of structure but at the end we only need to pass the num_shards in the replications_configs, changed There was a problem hiding this comment. correct, looks good. If we can leave a comment give context of why we add replication specs that would be good for future reference There was a problem hiding this comment. i did here: 8fe7cd6 would you add something else to the latest PR doc? |
||
|
|
||
| // receivedLegacySchemaRequestInRead checks if Read has to use the legacy schema because a State Move or Upgrader happened just before. | ||
| func receivedLegacySchemaRequestInRead(model *TFModel) bool { | ||
| return model.ClusterID.ValueString() == "forceLegacySchema" | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the last mig test to enable for TPF