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

Jump to bottom

INTMDB-263: Create Resource and Datasource for private_link_endpoint_adl #640

Merged
merged 11 commits into from
Jan 7, 2022
Merged

INTMDB-263: Create Resource and Datasource for private_link_endpoint_adl #640

merged 11 commits into from
Jan 7, 2022

Conversation

abner-dou
Copy link
Contributor

Description

A new resource and datasource was create for private link endpoints in datalake and online archive

Link to any related issue(s):INTMDB-263

Type of change:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Documentation fix/enhancement

Required Checklist:

  • I have signed the MongoDB CLA
  • I have read the Terraform contribution guidelines
  • 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

Further comments

TESTS:

--- PASS: TestAccResourceMongoDBAtlasPrivateLinkEndpointServiceADL_basic (6.90s)
PASS
coverage: 2.8% of statements
ok github.com/mongodb/terraform-provider-mongodbatlas/mongodbatlas 9.475s coverage: 2.8% of statements
? github.com/mongodb/terraform-provider-mongodbatlas/version [no test files]

--- PASS: TestAccResourceMongoDBAtlasPrivateLinkEndpointServiceADL_importBasic (4.23s)
PASS
coverage: 2.8% of statements
ok github.com/mongodb/terraform-provider-mongodbatlas/mongodbatlas 6.828s coverage: 2.8% of statements
? github.com/mongodb/terraform-provider-mongodbatlas/version [no test files]

--- PASS: TestAccDataSourceMongoDBAtlasPrivateLinkEndpointServiceADL_basic (4.33s)
PASS
coverage: 2.8% of statements
ok github.com/mongodb/terraform-provider-mongodbatlas/mongodbatlas 6.901s coverage: 2.8% of statements
? github.com/mongodb/terraform-provider-mongodbatlas/version [no test files]

--- PASS: TestAccDataSourceMongoDBAtlasPrivateLinkEndpointsServiceADL_basic (4.34s)
PASS
coverage: 3.0% of statements
ok github.com/mongodb/terraform-provider-mongodbatlas/mongodbatlas 6.976s coverage: 3.0% of statements
? github.com/mongodb/terraform-provider-mongodbatlas/version [no test files]

@abner-dou abner-dou self-assigned this Dec 16, 2021
@abner-dou
Copy link
Contributor Author

@themantissa There's still a problem with the API_KEYS that actually Edgar is resolving in another PR, that's why this PR is failing in this moment

andreaangiolillo
andreaangiolillo approved these changes Dec 20, 2021
View reviewed changes
Copy link
Collaborator

@andreaangiolillo andreaangiolillo left a comment

Choose a reason for hiding this comment

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

LGTM

mongodbatlas/data_source_mongodbatlas_privatelink_endpoint_service_adl.go Outdated
if err != nil {
// case 404
// deleted in the backend case
reset := strings.Contains(err.Error(), "404") && !d.IsNewResource()
Copy link
Collaborator

Choose a reason for hiding this comment

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

[q] Why did you add && !d.IsNewResource()? why getting 404 is not enough to remove the resource?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was added to avoid eventual inconsistency but in this case is not necessary

mongodbatlas/data_source_mongodbatlas_privatelink_endpoints_service_adl.go Outdated Show resolved Hide resolved
themantissa
themantissa requested changes Dec 21, 2021
View reviewed changes
Copy link
Collaborator

@themantissa themantissa left a comment

Choose a reason for hiding this comment

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

Error and docs suggestions/comments. Once edited I'd like our doc team to review before we merge to ensure that how we've presented this makes sense for both ADL and Online Archive.

mongodbatlas/data_source_mongodbatlas_privatelink_endpoints_service_adl.go Outdated Show resolved Hide resolved
mongodbatlas/data_source_mongodbatlas_privatelink_endpoint_service_adl.go Outdated Show resolved Hide resolved
mongodbatlas/resource_mongodbatlas_privatelink_endpoint_service_adl.go Outdated Show resolved Hide resolved
mongodbatlas/resource_mongodbatlas_privatelink_endpoint_service_adl.go Outdated Show resolved Hide resolved
mongodbatlas/resource_mongodbatlas_privatelink_endpoint_service_adl.go Outdated Show resolved Hide resolved
website/docs/d/privatelink_endpoints_service_adl.html.markdown Show resolved Hide resolved
website/docs/r/privatelink_endpoint_service_adl.html.markdown Outdated Show resolved Hide resolved
website/docs/r/privatelink_endpoint_service_adl.html.markdown Outdated Show resolved Hide resolved
website/docs/d/privatelink_endpoints_service_adl.html.markdown Outdated Show resolved Hide resolved
website/docs/d/privatelink_endpoints_service_adl.html.markdown Outdated Show resolved Hide resolved
@abner-dou abner-dou requested a review from themantissa January 3, 2022 20:31
themantissa
themantissa approved these changes Jan 6, 2022
View reviewed changes
Copy link
Collaborator

@themantissa themantissa left a comment

Choose a reason for hiding this comment

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

LGTM, will tag docs just for a quick check.

@themantissa
Copy link
Collaborator

@jwilliams-mongo can you review the docs portion of this for a check? Per guidence from the Data Lake team we are using the same resource for both Data Lake and Online Archive as it's the exact same API endpoint. In the Atlas Admin Doc it's described with two different endpoints, which makes it clear but wouldn't really work here. Does this seem clear enough to you?

@abner-dou abner-dou merged commit 474f8e4 into master Jan 7, 2022
@abner-dou abner-dou deleted the INTMDB-263-pel branch January 7, 2022 05:53
@jwilliams-mongo
Copy link
Collaborator

Sorry for the delay @themantissa I was OOTO. Would you still like me to take a look at this?

@rkistner rkistner mentioned this pull request Mar 7, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thetonymaster thetonymaster thetonymaster approved these changes

@andreaangiolillo andreaangiolillo andreaangiolillo approved these changes

@themantissa themantissa themantissa approved these changes

@coderGo93 coderGo93 Awaiting requested review from coderGo93

@jwilliams-mongo jwilliams-mongo Awaiting requested review from jwilliams-mongo

Assignees

@abner-dou abner-dou

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@abner-dou @themantissa @jwilliams-mongo @thetonymaster @andreaangiolillo