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: Correctly handle GCP updates of mongodbatlas_network_peering #2306

Merged
merged 5 commits into from
May 28, 2024
Merged

fix: Correctly handle GCP updates of mongodbatlas_network_peering #2306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3804645
correctly handle GCP updates of network peering
oarbusi May 24, 2024
cc9bc58
changelog
oarbusi May 27, 2024
b2056ef
fix changelog
oarbusi May 27, 2024
5bde3d2
remove unnecessary comment
oarbusi May 27, 2024
d55f505
Update .changelog/2306.txt
oarbusi May 28, 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/2306.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: Correctly handles GCP updates of mongodbatlas_network_peering
```
10 changes: 3 additions & 7 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 @@ -135,11 +135,13 @@ func Resource() *schema.Resource {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
},
"network_name": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
},
"atlas_gcp_project_id": {
Type: schema.TypeString,
Expand Down Expand Up @@ -410,13 +412,7 @@ func resourceUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.
ContainerId: conversion.GetEncodedID(d.Get("container_id").(string), "container_id"),
}

// Updating any of the attributes for Azure Network Peering forces a recreation of the network peering.
// Need to check if GCP and AWS have the same behavior
switch peer.GetProviderName() {
case "GCP":
peer.SetGcpProjectId(d.Get("gcp_project_id").(string))
oarbusi marked this conversation as resolved.
Show resolved Hide resolved
peer.SetNetworkName(d.Get("network_name").(string))
default: // AWS by default
if peer.GetProviderName() == "AWS" {
region, _ := conversion.ValRegion(d.Get("accepter_region_name"), "network_peering")
peer.SetAccepterRegionName(region)
peer.SetAwsAccountId(d.Get("aws_account_id").(string))
Expand Down
46 changes: 46 additions & 0 deletions 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 @@ -142,6 +142,52 @@ func TestAccNetworkRSNetworkPeering_basicGCP(t *testing.T) {
})
}

func TestAccNetworkRSNetworkPeering_updateBasicGCP(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

just to confirm, with the previous implementation this test was failing in the patch request right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes exactly. It was failing because update is not possible if the Status is Available

acc.SkipTestForCI(t) // needs GCP configuration
Copy link
Member

Choose a reason for hiding this comment

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

is it doable to enable in CI or it's very complicated?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have run the tests but I have done it using my GCP account. If we are able to have a generic account for the team we could do it, but I didn't want to use my account credentials to run the tests in CI.

Copy link
Member

Choose a reason for hiding this comment

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

that's for sure. maybe we can create a triage ticket to have test credentials in GCP

Copy link
Collaborator Author

@oarbusi oarbusi May 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I will create the ticket! Thanks
edit: created CLOUDP-250829


var (
projectID = os.Getenv("MONGODB_ATLAS_PROJECT_ID")
providerName = "GCP"
gcpProjectID = os.Getenv("GCP_PROJECT_ID")
networkName = acc.RandomName()
updatedNetworkName = acc.RandomName()
)

resource.Test(t, resource.TestCase{
PreCheck: func() { acc.PreCheck(t); acc.PreCheckPeeringEnvGCP(t) },
ProtoV6ProviderFactories: acc.TestAccProviderV6Factories,
CheckDestroy: acc.CheckDestroyNetworkPeering,
Steps: []resource.TestStep{
{
Config: configGCP(projectID, providerName, gcpProjectID, networkName),
Check: resource.ComposeTestCheckFunc(
checkExists(resourceName),
resource.TestCheckResourceAttrSet(resourceName, "project_id"),
resource.TestCheckResourceAttrSet(resourceName, "container_id"),
resource.TestCheckResourceAttrSet(resourceName, "network_name"),

resource.TestCheckResourceAttr(resourceName, "provider_name", providerName),
resource.TestCheckResourceAttr(resourceName, "gcp_project_id", gcpProjectID),
resource.TestCheckResourceAttr(resourceName, "network_name", networkName),
),
},
{
Config: configGCP(projectID, providerName, gcpProjectID, updatedNetworkName),
Check: resource.ComposeTestCheckFunc(
checkExists(resourceName),
resource.TestCheckResourceAttrSet(resourceName, "project_id"),
resource.TestCheckResourceAttrSet(resourceName, "container_id"),
resource.TestCheckResourceAttrSet(resourceName, "network_name"),

resource.TestCheckResourceAttr(resourceName, "provider_name", providerName),
resource.TestCheckResourceAttr(resourceName, "gcp_project_id", gcpProjectID),
resource.TestCheckResourceAttr(resourceName, "network_name", updatedNetworkName),
),
},
},
})
}

func TestAccNetworkRSNetworkPeering_AWSDifferentRegionName(t *testing.T) {
var (
vpcID = os.Getenv("AWS_VPC_ID")
Expand Down