Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Implements extra API Calls for AdvancedCluster TPF #2852

Merged
merged 10 commits into from
Dec 4, 2024
Merged

chore: Implements extra API Calls for AdvancedCluster TPF #2852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
22a1cfb
chore: Add support for ExtraAPIInfo in TFModel to find replication_sp…
EspenAlbert Dec 3, 2024
7dd94e0
feat: Initial /tenantUpgrade logic
EspenAlbert Dec 3, 2024
c4ef033
Merge branch 'master' into CLOUDP-286480_extra_api_calls
EspenAlbert Dec 4, 2024
91134ff
refactor: Fix zoneName default for legacy schema
EspenAlbert Dec 4, 2024
f29a751
address PR comments part 1
EspenAlbert Dec 4, 2024
9f14110
use explicit code instead of comments
EspenAlbert Dec 4, 2024
8329f46
chore: remove comments and fix suggested change commit
EspenAlbert Dec 4, 2024
d7f2c89
chore: remove comment
EspenAlbert Dec 4, 2024
e219aae
address PR comments 2
EspenAlbert Dec 4, 2024
f0603cc
test: hard code extra container response
EspenAlbert Dec 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions internal/common/constant/cloud_provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
package constant

const (
AWS = "AWS"
AZURE = "AZURE"
GCP = "GCP"
AWS = "AWS"
AZURE = "AZURE"
GCP = "GCP"
TENANT = "TENANT"
)
24 changes: 14 additions & 10 deletions internal/service/advancedclustertpf/model_ClusterDescription20240805.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,15 @@ type LegacySchemaInfo struct {
RootDiskSize *float64
}

func NewTFModel(ctx context.Context, input *admin.ClusterDescription20240805, timeout timeouts.Value, diags *diag.Diagnostics, legacyInfo *LegacySchemaInfo) *TFModel {
type ExtraAPIInfo struct {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although we could define only containerIDs map[string]string as a parameter, I think this is more "future-proof".
(Couldn't use LegacySchemaInfo, as that is only meant for numShards > 1 configurations)

Copy link
Member

@lantoli lantoli Dec 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have mixed feelings about anticipating future needs as it can create overcomplexity but it's ok here.

ContainerIDs map[string]string
}

func NewTFModel(ctx context.Context, input *admin.ClusterDescription20240805, timeout timeouts.Value, diags *diag.Diagnostics, legacyInfo *LegacySchemaInfo, apiInfo ExtraAPIInfo) *TFModel {
biConnector := NewBiConnectorConfigObjType(ctx, input.BiConnector, diags)
connectionStrings := NewConnectionStringsObjType(ctx, input.ConnectionStrings, diags)
labels := NewLabelsObjType(ctx, input.Labels, diags)
replicationSpecs := NewReplicationSpecsObjType(ctx, input.ReplicationSpecs, diags, legacyInfo)
replicationSpecs := NewReplicationSpecsObjType(ctx, input.ReplicationSpecs, diags, legacyInfo, apiInfo)
tags := NewTagsObjType(ctx, input.Tags, diags)
if diags.HasError() {
return nil
Expand Down Expand Up @@ -112,15 +116,15 @@ func NewLabelsObjType(ctx context.Context, input *[]admin.ComponentLabel, diags
return setType
}

func NewReplicationSpecsObjType(ctx context.Context, input *[]admin.ReplicationSpec20240805, diags *diag.Diagnostics, legacyInfo *LegacySchemaInfo) types.List {
func NewReplicationSpecsObjType(ctx context.Context, input *[]admin.ReplicationSpec20240805, diags *diag.Diagnostics, legacyInfo *LegacySchemaInfo, apiInfo ExtraAPIInfo) types.List {
if input == nil {
return types.ListNull(ReplicationSpecsObjType)
}
var tfModels *[]TFReplicationSpecsModel
if legacyInfo == nil {
tfModels = convertReplicationSpecs(ctx, input, diags)
tfModels = convertReplicationSpecs(ctx, input, diags, apiInfo)
} else {
tfModels = convertReplicationSpecsLegacy(ctx, input, diags, legacyInfo)
tfModels = convertReplicationSpecsLegacy(ctx, input, diags, legacyInfo, apiInfo)
}
if diags.HasError() {
return types.ListNull(ReplicationSpecsObjType)
Expand All @@ -130,15 +134,15 @@ func NewReplicationSpecsObjType(ctx context.Context, input *[]admin.ReplicationS
return listType
}

func convertReplicationSpecs(ctx context.Context, input *[]admin.ReplicationSpec20240805, diags *diag.Diagnostics) *[]TFReplicationSpecsModel {
func convertReplicationSpecs(ctx context.Context, input *[]admin.ReplicationSpec20240805, diags *diag.Diagnostics, apiInfo ExtraAPIInfo) *[]TFReplicationSpecsModel {
tfModels := make([]TFReplicationSpecsModel, len(*input))
for i, item := range *input {
regionConfigs := NewRegionConfigsObjType(ctx, item.RegionConfigs, diags)
tfModels[i] = TFReplicationSpecsModel{
Id: types.StringPointerValue(item.Id),
ExternalId: types.StringPointerValue(item.Id),
NumShards: types.Int64Value(1), // TODO: Static
ContainerId: conversion.ToTFMapOfString(ctx, diags, nil), // TODO: Static
NumShards: types.Int64Value(1), // TODO: Static
EspenAlbert marked this conversation as resolved.
Show resolved Hide resolved
ContainerId: conversion.ToTFMapOfString(ctx, diags, &apiInfo.ContainerIDs),
RegionConfigs: regionConfigs,
ZoneId: types.StringPointerValue(item.ZoneId),
ZoneName: types.StringPointerValue(item.ZoneName),
Expand All @@ -147,7 +151,7 @@ func convertReplicationSpecs(ctx context.Context, input *[]admin.ReplicationSpec
return &tfModels
}

func convertReplicationSpecsLegacy(ctx context.Context, input *[]admin.ReplicationSpec20240805, diags *diag.Diagnostics, legacyInfo *LegacySchemaInfo) *[]TFReplicationSpecsModel {
func convertReplicationSpecsLegacy(ctx context.Context, input *[]admin.ReplicationSpec20240805, diags *diag.Diagnostics, legacyInfo *LegacySchemaInfo, apiInfo ExtraAPIInfo) *[]TFReplicationSpecsModel {
tfModels := []TFReplicationSpecsModel{}
tfModelsSkipIndexes := []int{}
for i, item := range *input {
Expand Down Expand Up @@ -179,7 +183,7 @@ func convertReplicationSpecsLegacy(ctx context.Context, input *[]admin.Replicati
}
}
tfModels = append(tfModels, TFReplicationSpecsModel{
ContainerId: conversion.ToTFMapOfString(ctx, diags, nil), // TODO: Static
ContainerId: conversion.ToTFMapOfString(ctx, diags, &apiInfo.ContainerIDs),
ExternalId: types.StringPointerValue(item.Id),
Id: types.StringValue(legacyID),
RegionConfigs: regionConfigs,
Expand Down
13 changes: 12 additions & 1 deletion internal/service/advancedclustertpf/model_to_ClusterDescription20240805.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"go.mongodb.org/atlas-sdk/v20241113001/admin"
)

const defaultZoneName = "ZoneName managed by Terraform"

func NewAtlasReq(ctx context.Context, input *TFModel, diags *diag.Diagnostics) *admin.ClusterDescription20240805 {
acceptDataRisksAndForceReplicaSetReconfig, ok := conversion.StringPtrToTimePtr(input.AcceptDataRisksAndForceReplicaSetReconfig.ValueStringPointer())
if !ok {
Expand Down Expand Up @@ -93,11 +95,20 @@ func newReplicationSpec20240805(ctx context.Context, input types.List, diags *di
Id: conversion.NilForUnknown(item.Id, item.Id.ValueStringPointer()),
ZoneId: conversion.NilForUnknown(item.ZoneId, item.ZoneId.ValueStringPointer()),
RegionConfigs: newCloudRegionConfig20240805(ctx, item.RegionConfigs, diags),
ZoneName: item.ZoneName.ValueStringPointer(),
ZoneName: conversion.StringPtr(resolveZoneNameOrUseDefault(item)),
}
}
return &resp
}

func resolveZoneNameOrUseDefault(item *TFReplicationSpecsModel) string {
zoneName := conversion.NilForUnknown(item.ZoneName, item.ZoneName.ValueStringPointer())
if zoneName == nil {
return defaultZoneName
}
return *zoneName
}

func newResourceTag(ctx context.Context, input types.Set, diags *diag.Diagnostics) *[]admin.ResourceTag {
if input.IsUnknown() || input.IsNull() {
return nil
Expand Down
2 changes: 1 addition & 1 deletion internal/service/advancedclustertpf/move_state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func setMoveStateResponse(ctx context.Context, projectID, clusterName string, re
model := NewTFModel(ctx, &admin.ClusterDescription20240805{
GroupId: conversion.StringPtr(projectID),
Name: conversion.StringPtr(clusterName),
}, validTimeout, &resp.Diagnostics, nil)
}, validTimeout, &resp.Diagnostics, nil, ExtraAPIInfo{})
if resp.Diagnostics.HasError() {
return
}
Expand Down
52 changes: 38 additions & 14 deletions internal/service/advancedclustertpf/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,24 @@ func (r *rs) Update(ctx context.Context, req resource.UpdateRequest, resp *resou
if diags.HasError() {
return
}
cluster := r.applyClusterChanges(ctx, diags, &state, &plan)
stateReq := normalizeFromTFModel(ctx, &state, diags, false)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refactored some applyClusterChanges logic here, to have the distinction between tenantUpgrade/ClusterChanges more clear

planReq := normalizeFromTFModel(ctx, &plan, diags, false)
if diags.HasError() {
return
}
normalizePatchState(stateReq)
patchReq, err := update.PatchPayload(stateReq, planReq)
if err != nil {
diags.AddError("errorPatchPayload", err.Error())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more human-friendly summary

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will handle as part of next ticket

return
}
upgradeRequest := getTenantUpgradeRequest(stateReq, patchReq)
var cluster *admin.ClusterDescription20240805
if upgradeRequest != nil {
cluster = r.applyTenantUpgrade(ctx, &plan, upgradeRequest, diags)
} else {
cluster = r.applyClusterChanges(ctx, diags, &state, &plan, patchReq)
}
if diags.HasError() {
return
}
Expand Down Expand Up @@ -233,18 +250,7 @@ func (r *rs) applyAdvancedConfigurationChanges(ctx context.Context, diags *diag.
return legacyAdvConfig, advConfig
}

func (r *rs) applyClusterChanges(ctx context.Context, diags *diag.Diagnostics, state, plan *TFModel) *admin.ClusterDescription20240805 {
stateReq := normalizeFromTFModel(ctx, state, diags, false)
planReq := normalizeFromTFModel(ctx, plan, diags, false)
if diags.HasError() {
return nil
}
normalizePatchState(stateReq)
patchReq, err := update.PatchPayload(stateReq, planReq)
if err != nil {
diags.AddError("errorPatchPayload", err.Error())
return nil
}
func (r *rs) applyClusterChanges(ctx context.Context, diags *diag.Diagnostics, state, plan *TFModel, patchReq *admin.ClusterDescription20240805) *admin.ClusterDescription20240805 {
var cluster *admin.ClusterDescription20240805
if usingLegacySchema(ctx, plan.ReplicationSpecs, diags) {
// Only updates of replication specs will be done with legacy API
Expand Down Expand Up @@ -344,6 +350,19 @@ func (r *rs) updateAndWaitLegacy(ctx context.Context, patchReq *admin20240805.Cl
return AwaitChanges(ctx, r.Client.AtlasV2.ClustersApi, &tfModel.Timeouts, diags, projectID, clusterName, changeReasonUpdate)
}

func (r *rs) applyTenantUpgrade(ctx context.Context, plan *TFModel, upgradeRequest *admin.LegacyAtlasTenantClusterUpgradeRequest, diags *diag.Diagnostics) *admin.ClusterDescription20240805 {
api := r.Client.AtlasV2.ClustersApi
oarbusi marked this conversation as resolved.
Show resolved Hide resolved
projectID := plan.ProjectID.ValueString()
clusterName := plan.Name.ValueString()
upgradeRequest.Name = clusterName
_, _, err := api.UpgradeSharedCluster(ctx, projectID, upgradeRequest).Execute()
if err != nil {
diags.AddError("errorTenantUpgrade", fmt.Sprintf(errorUpdate, clusterName, err.Error()))
return nil
}
return AwaitChanges(ctx, api, &plan.Timeouts, diags, projectID, clusterName, changeReasonUpdate)
}

func (r *rs) convertClusterAddAdvConfig(ctx context.Context, legacyAdvConfig *admin20240530.ClusterDescriptionProcessArgs, advConfig *admin.ClusterDescriptionProcessArgs20240805, cluster *admin.ClusterDescription20240805, modelIn *TFModel, diags *diag.Diagnostics) *TFModel {
api := r.Client.AtlasV2.ClustersApi
api20240530 := r.Client.AtlasV220240530.ClustersApi
Expand All @@ -365,7 +384,12 @@ func (r *rs) convertClusterAddAdvConfig(ctx context.Context, legacyAdvConfig *ad
}
}
legacyInfo := resolveLegacyInfo(ctx, modelIn, diags, cluster, api20240530)
modelOut := NewTFModel(ctx, cluster, modelIn.Timeouts, diags, legacyInfo)
apiInfo, err := resolveExtraAPIInfo(ctx, projectID, cluster, r.Client.AtlasV2.NetworkPeeringApi)
if err != nil {
diags.AddError("errorExtraApiInfo", err.Error())
return nil
}
modelOut := NewTFModel(ctx, cluster, modelIn.Timeouts, diags, legacyInfo, *apiInfo)
if diags.HasError() {
return nil
}
Expand Down
47 changes: 47 additions & 0 deletions internal/service/advancedclustertpf/resource_compatibility_reuse.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"strings"

"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/constant"
"github.com/spf13/cast"
admin20240530 "go.mongodb.org/atlas-sdk/v20240530005/admin"
"go.mongodb.org/atlas-sdk/v20241113001/admin"
Expand All @@ -17,6 +18,52 @@ func FormatMongoDBMajorVersion(version string) string {
return fmt.Sprintf("%.1f", cast.ToFloat32(version))
}

// based on flattenAdvancedReplicationSpecRegionConfigs in model_advanced_cluster.go
func resolveExtraAPIInfo(ctx context.Context, projectID string, cluster *admin.ClusterDescription20240805, api admin.NetworkPeeringApi) (*ExtraAPIInfo, error) {
containerIDs := map[string]string{}
for _, spec := range cluster.GetReplicationSpecs() {
for _, regionConfig := range spec.GetRegionConfigs() {
providerName := regionConfig.GetProviderName()
if providerName == constant.TENANT {
continue
}
params := &admin.ListPeeringContainerByCloudProviderApiParams{
GroupId: projectID,
ProviderName: &providerName,
}
containerIDKey := fmt.Sprintf("%s:%s", providerName, regionConfig.GetRegionName())
if _, ok := containerIDs[containerIDKey]; ok {
continue
}
containers, _, err := api.ListPeeringContainerByCloudProviderWithParams(ctx, params).Execute()
if err != nil {
return nil, err
}
if results := getAdvancedClusterContainerID(containers.GetResults(), &regionConfig); results != "" {
EspenAlbert marked this conversation as resolved.
Show resolved Hide resolved
containerIDs[containerIDKey] = results
} else {
return nil, fmt.Errorf("container id not found for %s", containerIDKey)
}
}
}
return &ExtraAPIInfo{
ContainerIDs: containerIDs,
}, nil
}

// copied from model_advanced_cluster.go
func getAdvancedClusterContainerID(containers []admin.CloudProviderContainer, cluster *admin.CloudRegionConfig20240805) string {
for i, container := range containers {
gpc := cluster.GetProviderName() == constant.GCP
azure := container.GetProviderName() == cluster.GetProviderName() && container.GetRegion() == cluster.GetRegionName()
aws := container.GetRegionName() == cluster.GetRegionName()
if gpc || azure || aws {
return containers[i].GetId()
}
}
return ""
}

func getReplicationSpecIDsFromOldAPI(ctx context.Context, projectID, clusterName string, api admin20240530.ClustersApi) (map[string]string, error) {
clusterOldAPI, _, err := api.GetCluster(ctx, projectID, clusterName).Execute()
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion internal/service/advancedclustertpf/resource_compatiblity.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@ func numShardsMap(ctx context.Context, input types.List, diags *diag.Diagnostics
counts := map[string]int64{}
for i := range elements {
e := elements[i]
counts[e.ZoneName.ValueString()] = e.NumShards.ValueInt64()
zoneName := resolveZoneNameOrUseDefault(&e)
counts[zoneName] = e.NumShards.ValueInt64()
}
return counts
}
Expand Down
14 changes: 3 additions & 11 deletions internal/service/advancedclustertpf/resource_schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64default"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/setdefault"
Copy link
Collaborator Author

@EspenAlbert EspenAlbert Dec 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes to the resource_schema.go are due to TPF strictness and using the /tenantUpgrade endpoint, which would raise errors:

  | Error: Provider produced inconsistent result after apply
  | 
  | When applying changes to mongodbatlas_advanced_cluster.test, provider
  | "provider[\"registry.terraform.io/hashicorp/mongodbatlas\"]" produced an
  | unexpected new value: .create_date: was
  | cty.StringVal("2024-12-03T16:54:26Z"), but now
  | cty.StringVal("2024-12-03T16:57:44Z").
  | 
  | This is a bug in the provider, which should be reported in the provider's own
  | issue tracker.
  | 
  | Error: Provider produced inconsistent result after apply
  | 
  | When applying changes to mongodbatlas_advanced_cluster.test, provider
  | "provider[\"registry.terraform.io/hashicorp/mongodbatlas\"]" produced an
  | unexpected new value: .replication_specs[0].zone_name: was
  | cty.StringVal("ZoneName managed by Terraform"), but now cty.StringVal("Zone
  | 1").
  | 
  | This is a bug in the provider, which should be reported in the provider's own
  | issue tracker.
  | 
  | Error: Provider produced inconsistent result after apply
  | 
  | When applying changes to mongodbatlas_advanced_cluster.test, provider
  | "provider[\"registry.terraform.io/hashicorp/mongodbatlas\"]" produced an
  | unexpected new value: .cluster_id: was
  | cty.StringVal("674f37c288864967c85f6b47"), but now
  | cty.StringVal("674f388843ca6e2ab9803e84").

"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/schemafunc"
Expand Down Expand Up @@ -128,10 +127,7 @@ func ResourceSchema(ctx context.Context) schema.Schema {
},
},
"create_date": schema.StringAttribute{
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it makes sense to remove UseStateForUnknown as this is intended to use only when we're sure that value won't change

},
Computed: true,
MarkdownDescription: "Date and time when MongoDB Cloud created this cluster. This parameter expresses its value in ISO 8601 format in UTC.",
},
"encryption_at_rest_provider": schema.StringAttribute{
Expand All @@ -149,10 +145,7 @@ func ResourceSchema(ctx context.Context) schema.Schema {
MarkdownDescription: "Unique 24-hexadecimal digit string that identifies your project. Use the [/groups](#tag/Projects/operation/listProjects) endpoint to retrieve all projects to which the authenticated user has access.\n\n**NOTE**: Groups and projects are synonymous terms. Your group id is the same as your project id. For existing groups, your group/project id remains the same. The resource and corresponding endpoints use the term groups.",
},
"cluster_id": schema.StringAttribute{ // TODO: was generated as id
EspenAlbert marked this conversation as resolved.
Show resolved Hide resolved
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
Computed: true,
MarkdownDescription: "Unique 24-hexadecimal digit string that identifies the cluster.",
},
"labels": schema.SetNestedAttribute{
Expand Down Expand Up @@ -276,9 +269,8 @@ func ResourceSchema(ctx context.Context) schema.Schema {
MarkdownDescription: "Unique 24-hexadecimal digit string that identifies the zone in a Global Cluster. This value can be used to configure Global Cluster backup policies.",
},
"zone_name": schema.StringAttribute{
Computed: true, // must be computed to have a Default
Computed: true,
Optional: true,
Default: stringdefault.StaticString("ZoneName managed by Terraform"), // TODO: as in current resource
MarkdownDescription: "Human-readable label that describes the zone this shard belongs to in a Global Cluster. Provide this value only if \"clusterType\" : \"GEOSHARDED\" but not \"selfManagedSharding\" : true.",
},
},
Expand Down
Loading
Loading