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

fix: Sets all attributes of Azure mongodbatlas_network_peering as ForceNew, forcing recreation of the resource when updating #2299

Merged
merged 9 commits into from
May 24, 2024
Merged

fix: Sets all attributes of Azure mongodbatlas_network_peering as ForceNew, forcing recreation of the resource when updating #2299

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
74d4665
set all necessary attributes for update
oarbusi May 23, 2024
b8fca6c
changelog
oarbusi May 23, 2024
864a354
force new when updating azure network peering
oarbusi May 23, 2024
0e84e27
Update .changelog/2299.txt
oarbusi May 24, 2024
0733f53
run tests in CI
oarbusi May 24, 2024
b25818d
%[]q instead of "%[]s" for parameters of configuration
oarbusi May 24, 2024
728bb6d
PreCheckBasic
oarbusi May 24, 2024
83c50d6
remove azure from update
oarbusi May 24, 2024
efabd9b
mention force new in changelog
oarbusi May 24, 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
3 changes: 3 additions & 0 deletions .changelog/2299.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/mongodbatlas_network_peering: Sets all necessary attributes for update of Azure network peering
```
30 changes: 15 additions & 15 deletions internal/service/networkpeering/resource_network_peering.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,21 +103,25 @@ func Resource() *schema.Resource {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
},
"azure_subscription_id": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
},
"resource_group_name": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
},
"vnet_name": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
AgustinBettati marked this conversation as resolved.
Show resolved Hide resolved
},
"error_state": {
Type: schema.TypeString,
Expand Down Expand Up @@ -411,28 +415,24 @@ func resourceUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.
peer.SetGcpProjectId(d.Get("gcp_project_id").(string))
peer.SetNetworkName(d.Get("network_name").(string))
case "AZURE":
if d.HasChange("azure_directory_id") {
peer.SetAzureDirectoryId(d.Get("azure_directory_id").(string))
}

if d.HasChange("azure_subscription_id") {
peer.SetAzureSubscriptionId(d.Get("azure_subscription_id").(string))
}

if d.HasChange("resource_group_name") {
peer.SetResourceGroupName(d.Get("resource_group_name").(string))
}

if d.HasChange("vnet_name") {
peer.SetVnetName(d.Get("vnet_name").(string))
}
peer.SetAzureDirectoryId(d.Get("azure_directory_id").(string))
peer.SetAzureSubscriptionId(d.Get("azure_subscription_id").(string))
peer.SetResourceGroupName(d.Get("resource_group_name").(string))
peer.SetVnetName(d.Get("vnet_name").(string))
default: // AWS by default
region, _ := conversion.ValRegion(d.Get("accepter_region_name"), "network_peering")
peer.SetAccepterRegionName(region)
peer.SetAwsAccountId(d.Get("aws_account_id").(string))
peer.SetRouteTableCidrBlock(d.Get("route_table_cidr_block").(string))
peer.SetVpcId(d.Get("vpc_id").(string))
}
peerConn, resp, getErr := conn.NetworkPeeringApi.GetPeeringConnection(ctx, projectID, peerID).Execute()
if getErr != nil {
if resp != nil && resp.StatusCode == 404 {
return nil
}
}
fmt.Print(peerConn.GetStatus())

_, _, err := conn.NetworkPeeringApi.UpdatePeeringConnection(ctx, projectID, peerID, peer).Execute()
if err != nil {
Expand Down
46 changes: 45 additions & 1 deletion internal/service/networkpeering/resource_network_peering_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,50 @@ func TestAccNetworkRSNetworkPeering_basicAzure(t *testing.T) {
})
}

func TestAccNetworkRSNetworkPeering_updateBasicAzure(t *testing.T) {
acc.SkipTestForCI(t) // needs Azure configuration
Copy link
Collaborator

Choose a reason for hiding this comment

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

:( is it hard to have this information in our CI setup so that we can actually have it running?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Main issue I found while running them locally is the limitation of network_container that in AWS and Azure there can only be one container per cloud provider region and that you can't delete a network peering container if your project contains clusters. I'll work around that so that we can have the tests run in the CI

Copy link
Member

Choose a reason for hiding this comment

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

in case it can help, we improved cleanup process to delete endpoints: https://github.com/mongodb/terraform-provider-mongodbatlas/blob/master/scripts/cleanup-test-env.sh#L19
we could improve it to delete these projects as well if needed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done! Now the tests run in the CI

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you very much!


var (
projectID = os.Getenv("MONGODB_ATLAS_PROJECT_ID")
directoryID = os.Getenv("AZURE_DIRECTORY_ID")
subscriptionID = os.Getenv("AZURE_SUBSCRIPTION_ID")
resourceGroupName = os.Getenv("AZURE_RESOURCE_GROUP_NAME")
vNetName = os.Getenv("AZURE_VNET_NAME")
updatedvNetName = os.Getenv("AZURE_VNET_NAME_UPDATED")
providerName = "AZURE"
)

resource.Test(t, resource.TestCase{
PreCheck: func() { acc.PreCheck(t); acc.PreCheckPeeringEnvAzure(t) },
ProtoV6ProviderFactories: acc.TestAccProviderV6Factories,
CheckDestroy: acc.CheckDestroyNetworkPeering,
Steps: []resource.TestStep{
{
Config: configAzure(projectID, providerName, directoryID, subscriptionID, resourceGroupName, vNetName),
Check: resource.ComposeTestCheckFunc(
checkExists(resourceName),
resource.TestCheckResourceAttrSet(resourceName, "project_id"),
resource.TestCheckResourceAttrSet(resourceName, "container_id"),
resource.TestCheckResourceAttr(resourceName, "provider_name", providerName),
resource.TestCheckResourceAttr(resourceName, "vnet_name", vNetName),
resource.TestCheckResourceAttr(resourceName, "azure_directory_id", directoryID),
),
},
{
Config: configAzure(projectID, providerName, directoryID, subscriptionID, resourceGroupName, updatedvNetName),
Check: resource.ComposeTestCheckFunc(
checkExists(resourceName),
resource.TestCheckResourceAttrSet(resourceName, "project_id"),
resource.TestCheckResourceAttrSet(resourceName, "container_id"),
resource.TestCheckResourceAttr(resourceName, "provider_name", providerName),
resource.TestCheckResourceAttr(resourceName, "vnet_name", updatedvNetName),
resource.TestCheckResourceAttr(resourceName, "azure_directory_id", directoryID),
),
},
},
})
}

func TestAccNetworkRSNetworkPeering_basicGCP(t *testing.T) {
acc.SkipTestForCI(t) // needs GCP configuration

Expand Down Expand Up @@ -275,7 +319,7 @@ func configAzure(projectID, providerName, directoryID, subscriptionID, resourceG
azure_directory_id = "%[3]s"
azure_subscription_id = "%[4]s"
resource_group_name = "%[5]s"
vnet_name = "%[6]s"
vnet_name = "%[6]s"
oarbusi marked this conversation as resolved.
Show resolved Hide resolved
}
`, projectID, providerName, directoryID, subscriptionID, resourceGroupName, vNetName)
}
Expand Down