-
Notifications
You must be signed in to change notification settings - Fork 187
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.
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
feat: Implements new mongodbatlas_encryption_at_rest
singular data source & adds valid
attribute for cloud provider configs in the resource
#2538
Conversation
@@ -148,6 +151,11 @@ func (r *encryptionAtRestRS) Schema(ctx context.Context, req resource.SchemaRequ | |||
Description: "Unique 24-hexadecimal digit string that identifies an Amazon Web Services (AWS) Identity and Access Management (IAM) role. This IAM role has the permissions required to manage your AWS customer master key.", | |||
MarkdownDescription: "Unique 24-hexadecimal digit string that identifies an Amazon Web Services (AWS) Identity and Access Management (IAM) role. This IAM role has the permissions required to manage your AWS customer master key.", | |||
}, | |||
"valid": schema.BoolAttribute{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This attribute was missing, added to also keep alignment with the data source
mongodbatlas_encryption_at_rest
singular data sourcemongodbatlas_encryption_at_rest
singular data source & add valid
attribute for cloud provider configs in the resource
mongodbatlas_encryption_at_rest
singular data source & add valid
attribute for cloud provider configs in the resourcemongodbatlas_encryption_at_rest
singular data source & adds valid
attribute for cloud provider configs in the resource
@@ -10,7 +10,7 @@ import ( | |||
"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/conversion" | |||
) | |||
|
|||
func NewTfEncryptionAtRestRSModel(ctx context.Context, projectID string, encryptionResp *admin.EncryptionAtRest) *TfEncryptionAtRestRSModel { | |||
func NewTFEncryptionAtRestRSModel(ctx context.Context, projectID string, encryptionResp *admin.EncryptionAtRest) *TfEncryptionAtRestRSModel { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor renaming from Tf
to TF
APIx bot: a message has been sent to Docs Slack channel |
assume_role test currently failing on master as well |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice! Left a few refactoring suggestions.
BTW: Maybe a stretch goal of the epic is refacstoring the resource_migration_test.go
? Looks like we can get rid of some duplication.
type TFEncryptionAtRestDSModel struct { | ||
AzureKeyVaultConfig *TFAzureKeyVaultConfigModel `tfsdk:"azure_key_vault_config"` | ||
AwsKmsConfig *TFAwsKmsConfigModel `tfsdk:"aws_kms_config"` | ||
GoogleCloudKmsConfig *TFGcpKmsConfigModel `tfsdk:"google_cloud_kms_config"` | ||
ID types.String `tfsdk:"id"` | ||
ProjectID types.String `tfsdk:"project_id"` | ||
} | ||
|
||
func NewTFEncryptionAtRestDSModel(projectID string, encryptionResp *admin.EncryptionAtRest) *TFEncryptionAtRestDSModel { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This model is identical to TFEncryptionAtRestRSModel right? We could use the same one and avoid defining this new conversion method as well. In this case we could rename to TFEncryptionAtRestModel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're not identical. The difference is that TFEncryptionAtRestRSModel has array objects for nested AWS, Azure, GCP configs. That's because the resource was migrated from TF SDK to TPF so we had to keep some behavior to avoid non-empty plans after that migration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohh I see, thanks for clarifying. Have my doubts if we prefer to have the schemas aligned, but understand the current data source aligns better with the API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'd prefer to keep it as is, not only due to closer alignment with the API but also because having those objects as lists in the resource was something we had to do in the past because of TF SDK limitations but now that we have TPF I'd want to stay on the correct implementation which is having these as singular objects.
I have tried to keep them as aligned as possible but would not like to carry forward unwanted/unnecessary complexities, if that makes sense.
Description: "Flag that indicates whether someone enabled encryption at rest for the specified project through Amazon Web Services (AWS) Key Management Service (KMS). To disable encryption at rest using customer key management and remove the configuration details, pass only this parameter with a value of `false`.", | ||
MarkdownDescription: "Flag that indicates whether someone enabled encryption at rest for the specified project through Amazon Web Services (AWS) Key Management Service (KMS). To disable encryption at rest using customer key management and remove the configuration details, pass only this parameter with a value of `false`.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description: "Flag that indicates whether someone enabled encryption at rest for the specified project through Amazon Web Services (AWS) Key Management Service (KMS). To disable encryption at rest using customer key management and remove the configuration details, pass only this parameter with a value of `false`.", | |
MarkdownDescription: "Flag that indicates whether someone enabled encryption at rest for the specified project through Amazon Web Services (AWS) Key Management Service (KMS). To disable encryption at rest using customer key management and remove the configuration details, pass only this parameter with a value of `false`.", | |
Description: "Flag that indicates whether encryption at rest is enabled for the specified project through Amazon Web Services (AWS) Key Management Service (KMS). To disable encryption at rest using customer key management and remove the configuration details, pass only this parameter with a value of `false`.", | |
MarkdownDescription: "Flag that indicates whether encryption at rest is enabled for the specified project through Amazon Web Services (AWS) Key Management Service (KMS). To disable encryption at rest using customer key management and remove the configuration details, pass only this parameter with a value of `false`.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As shared in the slack thread since this is being fetched directly from the API spec, would like to keep these in alignment, have applied the necessary comments though, please lmk if you still have any concerns:)
Description: "Flag that indicates whether someone enabled encryption at rest for the specified project. To disable encryption at rest using customer key management and remove the configuration details, pass only this parameter with a value of `false`.", | ||
MarkdownDescription: "Flag that indicates whether someone enabled encryption at rest for the specified project. To disable encryption at rest using customer key management and remove the configuration details, pass only this parameter with a value of `false`.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description: "Flag that indicates whether someone enabled encryption at rest for the specified project. To disable encryption at rest using customer key management and remove the configuration details, pass only this parameter with a value of `false`.", | |
MarkdownDescription: "Flag that indicates whether someone enabled encryption at rest for the specified project. To disable encryption at rest using customer key management and remove the configuration details, pass only this parameter with a value of `false`.", | |
Description: "Flag that indicates whether encryption at rest is enabled for the specified project. To disable encryption at rest using customer key management and remove the configuration details, pass only this parameter with a value of `false`.", | |
MarkdownDescription: "Flag that indicates whether enabled encryption at rest is enabled for the specified project. To disable encryption at rest using customer key management and remove the configuration details, pass only this parameter with a value of `false`.", |
Description: "Private data that you need secured and that belongs to the specified Azure Key Vault (AKV) tenant (**azureKeyVault.tenantID**). This data can include any type of sensitive data such as passwords, database connection strings, API keys, and the like. AKV stores this information as encrypted binary data.", | ||
MarkdownDescription: "Private data that you need secured and that belongs to the specified Azure Key Vault (AKV) tenant (**azureKeyVault.tenantID**). This data can include any type of sensitive data such as passwords, database connection strings, API keys, and the like. AKV stores this information as encrypted binary data.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description: "Private data that you need secured and that belongs to the specified Azure Key Vault (AKV) tenant (**azureKeyVault.tenantID**). This data can include any type of sensitive data such as passwords, database connection strings, API keys, and the like. AKV stores this information as encrypted binary data.", | |
MarkdownDescription: "Private data that you need secured and that belongs to the specified Azure Key Vault (AKV) tenant (**azureKeyVault.tenantID**). This data can include any type of sensitive data such as passwords, database connection strings, API keys, and the like. AKV stores this information as encrypted binary data.", | |
Description: "Private data to secure that belongs to the specified Azure Key Vault (AKV) tenant (**azureKeyVault.tenantID**). AKV stores this information as encrypted binary data.", | |
MarkdownDescription: "Private data to secure that belongs to the specified Azure Key Vault (AKV) tenant (**azureKeyVault.tenantID**). AKV stores this information as encrypted binary data.", |
I don't think we need to prescribe use-cases for this field. The description of what it does is sufficiently suggestive to users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, as shared in the slack thread since this is being fetched directly from the API spec, would like to keep these in alignment, have applied the necessary comments though, please lmk if you still have any concerns:)
Description: "Flag that indicates whether someone enabled encryption at rest for the specified project. To disable encryption at rest using customer key management and remove the configuration details, pass only this parameter with a value of `false`.", | ||
MarkdownDescription: "Flag that indicates whether someone enabled encryption at rest for the specified project. To disable encryption at rest using customer key management and remove the configuration details, pass only this parameter with a value of `false`.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description: "Flag that indicates whether someone enabled encryption at rest for the specified project. To disable encryption at rest using customer key management and remove the configuration details, pass only this parameter with a value of `false`.", | |
MarkdownDescription: "Flag that indicates whether someone enabled encryption at rest for the specified project. To disable encryption at rest using customer key management and remove the configuration details, pass only this parameter with a value of `false`.", | |
Description: "Flag that indicates whether encryption at rest is enabled for the specified project. To disable encryption at rest using customer key management and remove the configuration details, pass only this parameter with a value of `false`.", | |
MarkdownDescription: "Flag that indicates whether encryption at rest is enabled for the specified project. To disable encryption at rest using customer key management and remove the configuration details, pass only this parameter with a value of `false`.", |
Co-authored-by: lmkerbey-mdb <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Moving forward with merge as relevant doc comments have been addressed to not deviate too much from API spec docs and adding overrides. Failing test is not related. |
…o master (#2569) * update sdk dev (#2490) * chore: Creates TF models & interfaces for new `mongodbatlas_encryption_at_rest_private_endpoint` resource (#2493) * chore: Creates TF models & interfaces for new `mongodbatlas_encryption_at_rest_private_endpoint` data source (#2500) * feat: Updates `mongodbatlas_encryption_at_rest` resource to use new `azure_key_vault_config.require_private_networking` field (#2509) * chore: Creates TF models & interfaces for `mongodbatlas_encryption_at_rest_private_endpoints` plural data source (#2502) * feat: Implements `mongodbatlas_encryption_at_rest_private_endpoint` resource (#2512) * wip - implementing CRUD * include changelog entry * small adjustments * supporting state transition logic * implement acceptance test * add unit testing for state transitions * handle return error message if failed status is present * add acceptance test transitioning for public to private network * improve messaging for failed status * fix prechecks * use global const for resource name * avoid hardcoded value * adjust state transition logic for delete * adjusting target version in migration test to 1.19.0 * adjust default refresh to 30 seconds for quicker response * feat: Implements `mongodbatlas_encryption_at_rest_private_endpoint` singular data source (#2527) * implement singular data source * including changelog entry * doc: Updates existing documentation for `mongodbatlas_encryption_at_rest` resource to be auto-generated (#2529) * doc: Include example for new `mongodbatlas_encryption_at_rest_private_endpoint` resource (#2540) * Include example for ear with private endpoint * fix example * adjust readme * Update examples/mongodbatlas_encryption_at_rest_private_endpoint/azure/README.md Co-authored-by: maastha <[email protected]> * Update examples/mongodbatlas_encryption_at_rest_private_endpoint/azure/README.md Co-authored-by: maastha <[email protected]> * add example cli command * make use of variables to make value of resource id more compact --------- Co-authored-by: maastha <[email protected]> * feat: Implements new `mongodbatlas_encryption_at_rest_private_endpoints` data source (#2536) * temporary change to cloud provider access and getting latest sdk * implements plural data source * adapted cloud provider access with latest changes from dev preview * fix unit test * adding changelog entry * add changes to verify plural data source in basic test case * doc adjust to cloud_provider attribute * feat: Implements new `mongodbatlas_encryption_at_rest` singular data source & adds `valid` attribute for cloud provider configs in the resource (#2538) * fix: Adds error message handling to `mongodbatlas_encryption_at_rest_private_endpoint` resource (#2544) * doc: Adds documentation for new `encryption_at_rest_private_endpoint` resource and data sources (#2547) * adding documentation for encryption_at_rest_private_endpoint resource and data sources * align generated docs * minor typo fix * Adjust description of project_id to make it more concise * align note stating feature is available by request as defined in general docs * chore: Adopt latest changes from master into ear private endpoint dev branch to adopt latest SDK (#2549) * test: Reduce instance size and use of provisioned disk iops for test that verifies transition for symmetric to asymmetric configuration (#2503) * doc: Include changelog entries to mention 2 new guides (#2506) * add entry for 2 new guides * add link * chore: Updates examples link in index.md for v1.18.0 release * chore: Updates CHANGELOG.md header for v1.18.0 release * doc: Update Atlas SP db_role_to_execute info. (#2508) * (DOCSP-41590) Updating Atlas SP db_role_to_execute info. * Update docs/resources/stream_connection.md Co-authored-by: kanchana-mongodb <[email protected]> --------- Co-authored-by: kanchana-mongodb <[email protected]> * doc: Contributing Guidelines Updates (#2494) * Contributing Guidelines Updates * Update README.md * Update README.md * Update contributing/README.md Co-authored-by: kyuan-mongodb <[email protected]> --------- Co-authored-by: kyuan-mongodb <[email protected]> * test: Simply migration test checks after 1.18.0 release and adjust version constraint in advanced_cluster examples uing new schema (#2510) * doc: Add references to the terraform modules in the resources documentations (#2513) * add references to the modules in the resources documentations * fix pr comments * chore: Bump hashicorp/setup-terraform from 3.1.1 to 3.1.2 (#2515) Bumps [hashicorp/setup-terraform](https://github.com/hashicorp/setup-terraform) from 3.1.1 to 3.1.2. - [Release notes](https://github.com/hashicorp/setup-terraform/releases) - [Changelog](https://github.com/hashicorp/setup-terraform/blob/main/CHANGELOG.md) - [Commits](hashicorp/setup-terraform@651471c...b9cd54a) --- updated-dependencies: - dependency-name: hashicorp/setup-terraform dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore: Add mention of support ticket when opening a pull request (#2507) * Add mention of creating support ticket when opening PR * rephrasing to avoid mention of priority * including suggestion * doc: Updates`mongodbatlas_advanced_cluster` ISS migration guide & resource doc with expected 500 error on update (#2525) * chore: Updates mongodbatlas_advanced_cluster tests to expect temporary SERVICE_UNAVAILABLE error when migrating from old to new schema (#2523) * doc: Fixes wordings in the new advanced_cluster sharding guide. (#2524) * chore: Updates examples link in index.md for v1.18.1 release * chore: Updates CHANGELOG.md header for v1.18.1 release * chore: upgrades go SDK from `v20240805001` to `v20240805002` (#2534) * chore: Updates to Go 1.23 (#2535) * update asdf TF version * update to Go 1.23 * update linter * update golang-ci linter * disable Go telemetry * revert TF change * chore: Bump go.mongodb.org/atlas from 0.36.0 to 0.37.0 (#2532) Bumps [go.mongodb.org/atlas](https://github.com/mongodb/go-client-mongodb-atlas) from 0.36.0 to 0.37.0. - [Release notes](https://github.com/mongodb/go-client-mongodb-atlas/releases) - [Changelog](https://github.com/mongodb/go-client-mongodb-atlas/blob/master/CHANGELOG.md) - [Commits](mongodb/go-client-mongodb-atlas@v0.36.0...v0.37.0) --- updated-dependencies: - dependency-name: go.mongodb.org/atlas dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore: Bump github.com/hashicorp/hcl/v2 from 2.21.0 to 2.22.0 (#2530) Bumps [github.com/hashicorp/hcl/v2](https://github.com/hashicorp/hcl) from 2.21.0 to 2.22.0. - [Release notes](https://github.com/hashicorp/hcl/releases) - [Changelog](https://github.com/hashicorp/hcl/blob/main/CHANGELOG.md) - [Commits](hashicorp/hcl@v2.21.0...v2.22.0) --- updated-dependencies: - dependency-name: github.com/hashicorp/hcl/v2 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * update asdf TF version to 1.9.5 (#2537) * chore: Changes deprecation message for labels attribute (#2542) * chore: Upgrades go SDK from `v20240805002` to `v20240805003` (#2545) * major version update calling gomajor tool * manual change to reincorporate v20240530005 * reverts temp changes in cloud provider resources, fixes sdk versions in new implementations --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: svc-apix-bot <[email protected]> Co-authored-by: lmkerbey-mdb <[email protected]> Co-authored-by: kanchana-mongodb <[email protected]> Co-authored-by: Zuhair Ahmed <[email protected]> Co-authored-by: kyuan-mongodb <[email protected]> Co-authored-by: rubenVB01 <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: maastha <[email protected]> Co-authored-by: Marco Suma <[email protected]> Co-authored-by: Espen Albert <[email protected]> Co-authored-by: Leo Antoli <[email protected]> Co-authored-by: Oriol <[email protected]> * doc: Adds documentation & examples for `mongodbatlas_encryption_at_rest` singular data source (#2543) * chore: Enables `mongodbatlas_encryption_at_rest` (Azure) tests to run in CI (#2551) * chore: Adds `mongodbatlas_encryption_at_rest_private_endpoint` acceptance test using azapi to approve private endpoint & check ACTIVE status (#2558) * doc: Add user journey considerations in current resource and example documentation (#2559) * minor typo fix * improve initial description in ear * adjust ear docs with mention of azure private link * private link doc adjustments * improve example * improve example * add mention in ear examples about policies * add note on update operation * link adjustments and add header for handling existing clusters * add note on private endpoint * add note in data sources * Update docs/resources/encryption_at_rest_private_endpoint.md Co-authored-by: maastha <[email protected]> * add clarification of preview flag for data sources --------- Co-authored-by: maastha <[email protected]> * update project_ip_addresses action * address doc comment --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Agustin Bettati <[email protected]> Co-authored-by: svc-apix-bot <[email protected]> Co-authored-by: lmkerbey-mdb <[email protected]> Co-authored-by: kanchana-mongodb <[email protected]> Co-authored-by: Zuhair Ahmed <[email protected]> Co-authored-by: kyuan-mongodb <[email protected]> Co-authored-by: rubenVB01 <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Marco Suma <[email protected]> Co-authored-by: Espen Albert <[email protected]> Co-authored-by: Leo Antoli <[email protected]> Co-authored-by: Oriol <[email protected]>
Description
Implements new
mongodbatlas_encryption_at_rest
singular data source and addsvalid
attribute to the resource.Link to any related issue(s): CLOUDP-267667
Type of change:
Required Checklist:
Further comments