Add aws_opensearchserverless_security_policy data source #32226

joshjluo · 2023-06-26T18:13:04Z

Description

Adding new data source for OpenSearch Serverless Security Policy

Relations

References

https://docs.aws.amazon.com/opensearch-service/latest/ServerlessAPIReference/API_Operations.html
https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/opensearchserverless_security_policy

Output from Acceptance Testing

$ make testacc TESTS=TestAccOpenSearchServerlessSecurityPolicyDataSource PKG=opensearchserverless
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/opensearchserverless/... -v -count 1 -parallel 20 -run='TestAccOpenSearchServerlessSecurityPolicyDataSource'  -timeout 180m
=== RUN   TestAccOpenSearchServerlessSecurityPolicyDataSource_basic
=== PAUSE TestAccOpenSearchServerlessSecurityPolicyDataSource_basic
=== CONT  TestAccOpenSearchServerlessSecurityPolicyDataSource_basic
--- PASS: TestAccOpenSearchServerlessSecurityPolicyDataSource_basic (48.83s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/opensearchserverless       53.033s

github-actions · 2023-06-26T18:13:18Z

Community Note

Voting for Prioritization

Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
Please see our prioritization guide for information on how we prioritize.
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

For Submitters

Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
For new resources and data sources, use skaff to generate scaffolding with comments detailing common expectations.
Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

github-actions

Welcome @joshjluo 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTOR guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

AdamTylerLynch

Requesting a few changes and asking for hashi guidance on the marshalling.

AdamTylerLynch · 2023-06-27T15:09:29Z

internal/service/opensearchserverless/security_policy_data_source.go

+	return &schema.Resource{
+		ReadWithoutTimeout: dataSourceSecurityPolicyRead,
+
+		Schema: map[string]*schema.Schema{


The API has createdDate and lastModifiedDate in the response elements. What were your thoughts on omitting these from the DS?

Cite: https://docs.aws.amazon.com/opensearch-service/latest/ServerlessAPIReference/API_GetSecurityPolicy.html

The Resource Contribution Guidelines suggested that timestamps should be skipped. Do you think they should still be included here?

Skips Timestamp Attributes: Generally, creation and modification dates from the API should be omitted from the schema.

Cite: https://hashicorp.github.io/terraform-provider-aws/raising-a-pull-request/#resource-contribution-guidelines

@joshjluo we do avoid timestamps in resources because of the high potential to cause continuous drift, but they can be included in data sources

AdamTylerLynch · 2023-06-27T15:14:05Z

internal/service/opensearchserverless/security_policy_data_source.go

+				Type:     schema.TypeString,
+				Computed: true,
+			},
+			"name": {


The API documentation has stated length and patterns can you please include those here on schema validation?

You can see an example here: https://github.com/hashicorp/terraform-provider-aws/blob/8f461fec79149dc1031ee843250ead0da9030483/internal/service/opensearchserverless/collection.go#L104C8-L104C8

Updated to provide validation on both length and patterns.

AdamTylerLynch · 2023-06-27T15:19:12Z

internal/service/opensearchserverless/security_policy_data_source.go

+				Type:     schema.TypeString,
+				Computed: true,
+			},
+			"type": {


The API has two allowed values (encryption or network), and those values are defined in the AWS SDK. Can you please add validation as such?

You can see an example here:
https://github.com/hashicorp/terraform-provider-aws/blob/8f461fec79149dc1031ee843250ead0da9030483/internal/service/opensearchserverless/security_policy.go#L86C12-L86C12

Added validation for the type

AdamTylerLynch · 2023-06-27T15:27:52Z

internal/service/opensearchserverless/security_policy_data_source.go

+		return sdkdiag.AppendErrorf(diags, "reading SecurityPolicy with name (%s) and type (%s): %s", securityPolicyName, securityPolicyType, err)
+	}
+
+	policyBytes, err := securityPolicy.Policy.MarshalSmithyDocument()


@jar-b have you seen this method before? I have not seen Smith document marshaling prior, to want to verify.

Yes, the OpenSearch Serverless Access Policy was just merged today and uses a similar pattern:

terraform-provider-aws/internal/service/opensearchserverless/access_policy_data_source.go

Line 88 in 2402e64

policyBytes, err := out.Policy.MarshalSmithyDocument()

AdamTylerLynch · 2023-06-27T15:28:43Z

internal/service/opensearchserverless/security_policy_data_source_test.go

+	collection := fmt.Sprintf("collection/%s", rName)
+	return fmt.Sprintf(`
+resource "aws_opensearchserverless_security_policy" "test" {
+	name = %[1]q


We use spaces (not tabs) in the embedded Terraform/HCL string.

Updated to use spaces

joshjluo · 2023-06-27T22:55:11Z

internal/service/opensearchserverless/security_policy_data_source.go

+				Type:     schema.TypeString,
+				Computed: true,
+			},
+			"name": {


Updated to provide validation on both length and patterns.

joshjluo · 2023-06-27T22:56:19Z

internal/service/opensearchserverless/security_policy_data_source.go

+				Type:     schema.TypeString,
+				Computed: true,
+			},
+			"type": {


Added validation for the type

joshjluo · 2023-06-27T23:00:26Z

internal/service/opensearchserverless/security_policy_data_source_test.go

+		},
+		ErrorCheck:               acctest.ErrorCheck(t, names.OpenSearchServerlessEndpointID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		CheckDestroy:             testAccCheckSecurityPolicyDestroy(ctx),


Added a CheckDestroy here as suggested by the documentation: https://hashicorp.github.io/terraform-provider-aws/running-and-writing-acceptance-tests/#data-source-acceptance-testing

joshjluo

Saw #32231 got merged in earlier today. Should I update this PR to follow a similar pattern to keep things consistent within the package?

…ata-source

…d dates

johnsonaj

LGTM 🚀

$ make testacc TESTARGS='-run=TestAccOpenSearchServerlessSecurityPolicyDataSource_' PKG=opensearchserverless

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/opensearchserverless/... -v -count 1 -parallel 20  -run=TestAccOpenSearchServerlessSecurityPolicyDataSource_ -timeout 180m
--- PASS: TestAccOpenSearchServerlessSecurityPolicyDataSource_basic (17.97s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/opensearchserverless	21.316s

johnsonaj · 2023-06-30T19:02:13Z

@joshjluo thank you for the contribution! 🎉

github-actions · 2023-07-07T15:49:04Z

This functionality has been released in v5.7.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

github-actions · 2023-08-07T02:10:36Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

Add aws_opensearchserverless_security_policy data source

322977a

github-actions bot reviewed Jun 26, 2023

View reviewed changes

justinretzolk added new-data-source Introduces a new data source. and removed needs-triage Waiting for first response or review from a maintainer. labels Jun 26, 2023

joshjluo changed the title ~~Add aws_opensearchserverless_security_policy data source~~ [WIP]: Add aws_opensearchserverless_security_policy data source Jun 27, 2023

AdamTylerLynch suggested changes Jun 27, 2023

View reviewed changes

AdamTylerLynch added the partner Contribution from a partner. label Jun 27, 2023

joshjluo force-pushed the f-aws_opensearchserverless_security_policy-data-source branch from 6240c9b to 92601f0 Compare June 27, 2023 22:50

joshjluo commented Jun 27, 2023

View reviewed changes

joshjluo changed the title ~~[WIP]: Add aws_opensearchserverless_security_policy data source~~ Add aws_opensearchserverless_security_policy data source Jun 27, 2023

joshjluo marked this pull request as ready for review June 27, 2023 23:04

joshjluo commented Jun 28, 2023

View reviewed changes

joshjluo requested a review from AdamTylerLynch June 28, 2023 00:02

joshjluo force-pushed the f-aws_opensearchserverless_security_policy-data-source branch from 92601f0 to 86a4d09 Compare June 28, 2023 15:15

Add aws_opensearchserverless_security_policy data source

304e3fb

joshjluo force-pushed the f-aws_opensearchserverless_security_policy-data-source branch from 86a4d09 to 304e3fb Compare June 28, 2023 15:51

johnsonaj self-assigned this Jun 29, 2023

johnsonaj added 4 commits June 30, 2023 13:06

Merge branch 'main' into f-aws_opensearchserverless_security_policy-d…

c39a199

…ata-source

aws_opensearchserverless_security_policy: remove id attribute

3f05e64

aws_opensearchserverless_security_policy: add created and lastModifie…

146a72d

…d dates

aws_opensearchserverless_security_policy: update documentation

eaa8ea9

johnsonaj approved these changes Jun 30, 2023

View reviewed changes

johnsonaj merged commit 5925a10 into hashicorp:main Jun 30, 2023

github-actions bot added this to the v5.7.0 milestone Jun 30, 2023

github-actions bot locked as resolved and limited conversation to collaborators Aug 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add aws_opensearchserverless_security_policy data source #32226

Add aws_opensearchserverless_security_policy data source #32226

joshjluo commented Jun 26, 2023 •

edited

Loading

github-actions bot commented Jun 26, 2023

github-actions bot left a comment

AdamTylerLynch left a comment

AdamTylerLynch Jun 27, 2023

joshjluo Jun 27, 2023

johnsonaj Jun 29, 2023

AdamTylerLynch Jun 27, 2023

joshjluo Jun 27, 2023

AdamTylerLynch Jun 27, 2023

joshjluo Jun 27, 2023

AdamTylerLynch Jun 27, 2023

jar-b Jun 27, 2023

AdamTylerLynch Jun 27, 2023

joshjluo Jun 27, 2023

joshjluo Jun 27, 2023

joshjluo Jun 27, 2023

joshjluo Jun 27, 2023

joshjluo left a comment •

edited

Loading

johnsonaj left a comment

johnsonaj commented Jun 30, 2023

github-actions bot commented Jul 7, 2023

github-actions bot commented Aug 7, 2023

Add aws_opensearchserverless_security_policy data source #32226

Add aws_opensearchserverless_security_policy data source #32226

Conversation

joshjluo commented Jun 26, 2023 • edited Loading

Description

Relations

References

Output from Acceptance Testing

github-actions bot commented Jun 26, 2023

Community Note

github-actions bot left a comment

Choose a reason for hiding this comment

AdamTylerLynch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshjluo left a comment • edited Loading

Choose a reason for hiding this comment

johnsonaj left a comment

Choose a reason for hiding this comment

johnsonaj commented Jun 30, 2023

github-actions bot commented Jul 7, 2023

github-actions bot commented Aug 7, 2023

joshjluo commented Jun 26, 2023 •

edited

Loading

joshjluo left a comment •

edited

Loading