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

Add support for is_secret_data_base64 in secret_version and secret_version_access datasources for global and regional stacks #11877

Merged
merged 8 commits into from
Oct 11, 2024
Merged

Add support for is_secret_data_base64 in secret_version and secret_version_access datasources for global and regional stacks #11877

merged 8 commits into from
Oct 11, 2024

Conversation

abheda-crest
Copy link
Contributor

Add support for is_secret_data_base64 in secret_version and secret_version_access datasources for global and regional stacks. I've also removed the redundant testAccCheckDatasourceSecretManagerSecretVersionAccess function and used the existing testAccCheckDatasourceSecretManagerSecretVersion function.
fixes hashicorp/terraform-provider-google#17321

Release Note Template for Downstream PRs (will be copied)

secretmanager: added `is_secret_data_base64` field to `google_secret_manager_secret_version` and `google_secret_manager_secret_version_access` datasources

secretmanagerregional: added `is_secret_data_base64` field to `google_secret_manager_regional_secret_version` and `google_secret_manager_regional_secret_version_access` datasources

@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Oct 1, 2024
@abheda-crest abheda-crest marked this pull request as ready for review October 1, 2024 07:07
@github-actions github-actions bot requested a review from rileykarson October 1, 2024 07:07
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 1, 2024

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

@rileykarson, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

gptSanyam
gptSanyam approved these changes Oct 3, 2024
View reviewed changes
Copy link
Contributor

@gptSanyam gptSanyam left a comment

Choose a reason for hiding this comment

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

LGTM

@abheda-crest
Copy link
Contributor Author

@rileykarson Could you please review this PR?

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 4, 2024

@rileykarson This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 8, 2024

@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 1 week. Please take a look! Use the label disable-review-reminders to disable these notifications.

@abheda-crest
Copy link
Contributor Author

@rileykarson Could you please review this PR?

rileykarson
rileykarson reviewed Oct 11, 2024
View reviewed changes
Copy link
Member

@rileykarson rileykarson left a comment

Choose a reason for hiding this comment

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

Just a few small nits, see inline.

mmv1/third_party/terraform/services/secretmanager/data_source_secret_manager_secret_version.go Outdated
if err != nil {
return fmt.Errorf("Error decoding secret manager secret version data: %s", err.Error())
var secretData string
dIsSecretDataBase64, ok := d.Get("is_secret_data_base64").(bool)
Copy link
Member

Choose a reason for hiding this comment

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

nit: You can omit the ok- the return of d.Get is guaranteed to be a bool by the schema, as the type is TypeBool.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I got your point and I've removed the checks as discussed. However, I originally included them to enhance the defensiveness of the type assertions based on the review comments I received on PR #11754.

Out of curiosity, are there specific scenarios—perhaps during string type assertions—where it would be advisable to retain such checks?

...ird_party/terraform/services/secretmanager/data_source_secret_manager_secret_version_test.go Outdated
@@ -74,6 +98,43 @@ func testAccCheckDatasourceSecretManagerSecretVersion(n, expected string) resour
}
}

func testAccCheckDatasourceSecretManagerSecretVersionSecretData(datasource, resource string) resource.TestCheckFunc {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
func testAccCheckDatasourceSecretManagerSecretVersionSecretData(datasource, resource string) resource.TestCheckFunc {
func testAccCheckSecretManagerSecretVersionSecretDataDatasourceMatchesResource(datasource, resource string) resource.TestCheckFunc {

(+ callers)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the function name wherever required.

...rm/services/secretmanagerregional/data_source_secret_manager_regional_secret_version_test.go Outdated
@@ -201,3 +250,40 @@ func testAccCheckDataSourceSecretManagerRegionalRegionalSecretVersion(n, expecte
return nil
}
}

func testAccCheckDataSourceSecretManagerRegionalRegionalSecretVersionSecretData(datasource, resource string) resource.TestCheckFunc {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
func testAccCheckDataSourceSecretManagerRegionalRegionalSecretVersionSecretData(datasource, resource string) resource.TestCheckFunc {
func testAccCheckSecretManagerRegionalRegionalSecretVersionSecretDataDatasourceMatchesResource(datasource, resource string) resource.TestCheckFunc {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the function name wherever required.

@modular-magician modular-magician added service/secretmanager and removed awaiting-approval Pull requests that need reviewer's approval to run presubmit tests labels Oct 11, 2024
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 12 files changed, 354 insertions(+), 40 deletions(-))
google-beta provider: Diff ( 12 files changed, 354 insertions(+), 40 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 84
Passed tests: 76
Skipped tests: 4
Affected tests: 4

Click here to see the affected service packages
  • secretmanager
  • secretmanagerregional

Action taken

Found 4 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccDatasourceSecretManagerSecretVersionAccess_withBase64SecretData
  • TestAccDatasourceSecretManagerSecretVersion_withBase64SecretData
  • TestAccDataSourceSecretManagerRegionalRegionalSecretVersionAccess_withBase64SecretData
  • TestAccDataSourceSecretManagerRegionalRegionalSecretVersion_withBase64SecretData

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🟢 Tests passed during RECORDING mode:
TestAccDataSourceSecretManagerRegionalRegionalSecretVersionAccess_withBase64SecretData[Debug log]
TestAccDataSourceSecretManagerRegionalRegionalSecretVersion_withBase64SecretData[Debug log]
TestAccDatasourceSecretManagerSecretVersionAccess_withBase64SecretData[Debug log]
TestAccDatasourceSecretManagerSecretVersion_withBase64SecretData[Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.

🟢 All tests passed!

View the build log or the debug log for each test

@github-actions github-actions bot requested a review from rileykarson October 11, 2024 17:14
@modular-magician modular-magician added awaiting-approval Pull requests that need reviewer's approval to run presubmit tests and removed awaiting-approval Pull requests that need reviewer's approval to run presubmit tests labels Oct 11, 2024
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 12 files changed, 338 insertions(+), 40 deletions(-))
google-beta provider: Diff ( 12 files changed, 338 insertions(+), 40 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 84
Passed tests: 80
Skipped tests: 4
Affected tests: 0

Click here to see the affected service packages
  • secretmanager
  • secretmanagerregional

🟢 All tests passed!

View the build log

rileykarson
rileykarson approved these changes Oct 11, 2024
View reviewed changes
Copy link
Member

@rileykarson rileykarson left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@rileykarson rileykarson merged commit c7e7579 into GoogleCloudPlatform:main Oct 11, 2024
10 checks passed
This was referenced Oct 11, 2024
Merged
Merged
Merged
gontech pushed a commit to gontech/magic-modules that referenced this pull request Oct 16, 2024
@abheda-crest @gontech
Add support for is_secret_data_base64 in secret_version and secret_…
9d3a5d6 
…version_access datasources for global and regional stacks (GoogleCloudPlatform#11877)
BBBmau pushed a commit to BBBmau/magic-modules that referenced this pull request Oct 23, 2024
@abheda-crest @BBBmau
Add support for is_secret_data_base64 in secret_version and secret_…
91c34d3 
…version_access datasources for global and regional stacks (GoogleCloudPlatform#11877)
BBBmau pushed a commit to BBBmau/magic-modules that referenced this pull request Oct 24, 2024
@abheda-crest @BBBmau
Add support for is_secret_data_base64 in secret_version and secret_…
30bcf32 
…version_access datasources for global and regional stacks (GoogleCloudPlatform#11877)
BBBmau pushed a commit to BBBmau/magic-modules that referenced this pull request Nov 5, 2024
@abheda-crest @BBBmau
Add support for is_secret_data_base64 in secret_version and secret_…
bca7997 
…version_access datasources for global and regional stacks (GoogleCloudPlatform#11877)
akshat-jindal-nit pushed a commit to akshat-jindal-nit/magic-modules that referenced this pull request Nov 18, 2024
@abheda-crest @akshat-jindal-nit
Add support for is_secret_data_base64 in secret_version and secret_…
69820e8 
…version_access datasources for global and regional stacks (GoogleCloudPlatform#11877)
amanMahendroo pushed a commit to amanMahendroo/magic-modules that referenced this pull request Dec 17, 2024
@abheda-crest @amanMahendroo
Add support for is_secret_data_base64 in secret_version and secret_…
51ba361 
…version_access datasources for global and regional stacks (GoogleCloudPlatform#11877)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gptSanyam gptSanyam gptSanyam approved these changes

@rileykarson rileykarson rileykarson approved these changes

Assignees
No one assigned
Labels
service/secretmanager
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support to data source "google_secret_manager_secret_version" for binary secret values
4 participants
@abheda-crest @modular-magician @rileykarson @gptSanyam