fix: Provide err when error message populated in `mongodbatlas_privatelink_endpoint` and `mongodbatlas_privatelink_endpoint_service` #2803

cveticm · 2024-11-15T16:20:57Z

Description

Both mongodbatlas_privatelink_endpoint_service and mongodbatlas_privatelink_endpoint have a error_message computed attribute (present in GET response), however no error message is being created to make users aware of the error when it is present. This PR adds the necessary logic to inform the user when an error is present.

Within the mongodbatlas_privatelink_endpoint_service, a testcase, TestAccNetworkRSPrivateLinkEndpointServiceAWS_Failed, has been added to verify error is now received.
This test passes an invalid vcpe id to a mongodbatlas_privatelink_endpoint_service resource. Creation will succeed but GET requests will fail with Interface endpoint <invalid_vcpe_id> was not found
A testcase within mongodbatlas_privatelink_endpoint was not found and, hence, no test has been added.

Link to any related issue(s): CLOUDP-282564

Type of change:

Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR. A migration guide must be created or updated if the new feature will go in a major version.
Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR. A migration guide must be created or updated.
This change requires a documentation update
Documentation fix/enhancement

Required Checklist:

I have signed the MongoDB CLA
I have read the contributing guides
I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
I have added any necessary documentation (if appropriate)
I have run make fmt and formatted my code
If changes include deprecations or removals I have added appropriate changelog entries.
If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Further comments

cveticm · 2024-11-15T16:45:10Z

internal/service/privatelinkendpoint/resource_privatelink_endpoint.go

+	if privateEndpoint.GetErrorMessage() != "" {
+		return diag.FromErr(fmt.Errorf("privatelink endpoint is in a failed state: %s", privateEndpoint.GetErrorMessage()))
+	}


It is possible that this fix might not be touched ever. Given that only 3 fields (bar timeout) can be modified for this resource, the only failure I can conceive of which would allow for a successful POST and error in GET is an unexpected error from Atlas or the cloud provider. Further investigation would be needed to verify this.

As it's a simple fix which adds minimal computation time, I suggest keeping the fix in the case a scenario is revealed to trigger this code to save the user from a silent fail.

Uncertain about raising an error here, I think a warning might be more appropriate?
What is the UX if the error is raised? Is there anything they can do other than terraform state rm or terraform destroy to get to a working state again?
I think this might fail on every plan after the error_message is populated in backend response

To my knowledge, there isn't an alternative for UX other than destroying the resource as the API don't have a PATCH endpoint to update this resource. I think this might make an error appropriate. WDYT @EspenAlbert ?

cveticm · 2024-11-15T16:47:18Z

internal/service/privatelinkendpointservice/resource_privatelink_endpoint_service_test.go

+				Config: configFailAWS(
+					awsAccessKey, awsSecretKey, projectID, providerName, region, resourceSuffix,
+				),
+				Check:       resource.TestCheckResourceAttr(resourceName, "error_message", "privatelink endpoint is in a failed state: Interface endpoint vpce-11111111111111111 was not found."),


Check error_message field has been properly populated

if plan or apply fails, is it useful to have this attribute error_message that won't be able to be used?

@lantoli, I'm not sure I fully understand. This fix is to ensure error_message is populated in the case that creation is successful but the proceeding GET returns a fail state. In the case that plan fails or apply fails during creation, an alternative error is provided. Could you clarify your comment?

sorry, maybe I'm not understanding this test. Is the Check useful in the test when we also have a ExpectError? We are expecting an error, so for ExpectError to pass the plan or apply must fail, so in that case I think it doesn't matter what the attribute values are as it failed.

That's true, the check is more to outline that the error_message field has been correctly populated but ultimately isn't needed in this test since we expect an error. This check can be removed.

github-actions · 2024-11-15T16:47:56Z

APIx bot: a message has been sent to Docs Slack channel

andreaangiolillo

LGTM

AgustinBettati

LGTM, great that you managed to capture in a test 💯 Left follow up doubt in case we can leave it running in CI.

AgustinBettati · 2024-11-15T17:00:28Z

internal/service/privatelinkendpointservice/resource_privatelink_endpoint_service_test.go

@@ -17,6 +18,37 @@ func TestAccNetworkRSPrivateLinkEndpointServiceAWS_Complete(t *testing.T) {
 	resource.Test(t, *testCase)
 }

+func TestAccNetworkRSPrivateLinkEndpointServiceAWS_Failed(t *testing.T) {
+	acc.SkipTestForCI(t) // needs AWS configuration


nit: Is it possible this test can be enabled in the CI? Don't see any particular pre-requisite AWS resources being used.

Yes, thanks for pointing out AWS provider was not needed! Removed CI skip 👍

Test now runs successfully in CI

great that it runs in CI 👏

internal/service/privatelinkendpointservice/resource_privatelink_endpoint_service_test.go

.changelog/2803.txt

carriecwk

LGTM!

cveticm added 3 commits November 15, 2024 15:07

fix and testing to mongodbatlas_privatelink_endpoint_service

2c0679b

fix in mongodbatlas_privatelink_endpoint

0c76026

adjusting error msg for clarify of resources

d933807

github-actions bot added the bug label Nov 15, 2024

added changelog

43b43b7

cveticm added enhancement and removed bug labels Nov 15, 2024

github-actions bot added bug and removed enhancement labels Nov 15, 2024

fix to changelog

ae3679f

cveticm commented Nov 15, 2024

View reviewed changes

cveticm marked this pull request as ready for review November 15, 2024 16:47

cveticm requested review from a team as code owners November 15, 2024 16:47

andreaangiolillo approved these changes Nov 15, 2024

View reviewed changes

AgustinBettati approved these changes Nov 15, 2024

View reviewed changes

cveticm added 2 commits November 15, 2024 17:11

removed aws provider from testcase and enabled running in CI

4ec2987

fix to precheck and project used

952b9c4

lantoli reviewed Nov 18, 2024

View reviewed changes

internal/service/privatelinkendpointservice/resource_privatelink_endpoint_service_test.go Outdated Show resolved Hide resolved

lantoli reviewed Nov 18, 2024

View reviewed changes

internal/service/privatelinkendpointservice/resource_privatelink_endpoint_service_test.go Show resolved Hide resolved

lantoli approved these changes Nov 18, 2024

View reviewed changes

lantoli reviewed Nov 18, 2024

View reviewed changes

.changelog/2803.txt Outdated Show resolved Hide resolved

lantoli reviewed Nov 18, 2024

View reviewed changes

.changelog/2803.txt Outdated Show resolved Hide resolved

Implementing feedback

9e04ae6

carriecwk approved these changes Nov 19, 2024

View reviewed changes

removed unneeded check from test

8f133b6

cveticm merged commit 5080ffa into master Nov 20, 2024
38 checks passed

cveticm deleted the CLOUDP-282564-provide-err-when-error_message-populated branch November 20, 2024 10:36

svc-apix-Bot added a commit that referenced this pull request Nov 20, 2024

chore: Updates CHANGELOG.md for #2803

2a8e2ab

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Provide err when error message populated in `mongodbatlas_privatelink_endpoint` and `mongodbatlas_privatelink_endpoint_service` #2803

fix: Provide err when error message populated in `mongodbatlas_privatelink_endpoint` and `mongodbatlas_privatelink_endpoint_service` #2803

cveticm commented Nov 15, 2024 •

edited

Loading

cveticm Nov 15, 2024 •

edited

Loading

EspenAlbert Nov 18, 2024 •

edited

Loading

cveticm Nov 18, 2024

cveticm Nov 15, 2024

lantoli Nov 18, 2024 •

edited

Loading

cveticm Nov 18, 2024

lantoli Nov 19, 2024 •

edited

Loading

cveticm Nov 19, 2024

github-actions bot commented Nov 15, 2024

andreaangiolillo left a comment

AgustinBettati left a comment

AgustinBettati Nov 15, 2024 •

edited

Loading

cveticm Nov 15, 2024

cveticm Nov 18, 2024

lantoli Nov 18, 2024

carriecwk left a comment

fix: Provide err when error message populated in mongodbatlas_privatelink_endpoint and mongodbatlas_privatelink_endpoint_service #2803

fix: Provide err when error message populated in mongodbatlas_privatelink_endpoint and mongodbatlas_privatelink_endpoint_service #2803

Conversation

cveticm commented Nov 15, 2024 • edited Loading

Description

Type of change:

Required Checklist:

Further comments

cveticm Nov 15, 2024 • edited Loading

Choose a reason for hiding this comment

EspenAlbert Nov 18, 2024 • edited Loading

Choose a reason for hiding this comment

cveticm Nov 18, 2024

Choose a reason for hiding this comment

cveticm Nov 15, 2024

Choose a reason for hiding this comment

lantoli Nov 18, 2024 • edited Loading

Choose a reason for hiding this comment

cveticm Nov 18, 2024

Choose a reason for hiding this comment

lantoli Nov 19, 2024 • edited Loading

Choose a reason for hiding this comment

cveticm Nov 19, 2024

Choose a reason for hiding this comment

github-actions bot commented Nov 15, 2024

andreaangiolillo left a comment

Choose a reason for hiding this comment

AgustinBettati left a comment

Choose a reason for hiding this comment

AgustinBettati Nov 15, 2024 • edited Loading

Choose a reason for hiding this comment

cveticm Nov 15, 2024

Choose a reason for hiding this comment

cveticm Nov 18, 2024

Choose a reason for hiding this comment

lantoli Nov 18, 2024

Choose a reason for hiding this comment

carriecwk left a comment

Choose a reason for hiding this comment

fix: Provide err when error message populated in `mongodbatlas_privatelink_endpoint` and `mongodbatlas_privatelink_endpoint_service` #2803

fix: Provide err when error message populated in `mongodbatlas_privatelink_endpoint` and `mongodbatlas_privatelink_endpoint_service` #2803

cveticm commented Nov 15, 2024 •

edited

Loading

cveticm Nov 15, 2024 •

edited

Loading

EspenAlbert Nov 18, 2024 •

edited

Loading

lantoli Nov 18, 2024 •

edited

Loading

lantoli Nov 19, 2024 •

edited

Loading

AgustinBettati Nov 15, 2024 •

edited

Loading