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.

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

Added missing fields in info_type_transformations data_loss_prevention_deidentify_template resource #7912

Conversation

JayS-crest
Copy link
Contributor

@JayS-crest JayS-crest commented May 10, 2023

added support for redactConfig, fixedSizeBucketingConfig, bucketingConfig, timePartConfig and dateShiftConfig in infoTypeTransformations in dlp data_loss_prevention_deidentify_template resource

fixes hashicorp/terraform-provider-google#11378

If this PR is for Terraform, I acknowledge that I have:

  • Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
  • Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
  • Generated Terraform providers, and ran make test and make lint in the generated providers to ensure it passes unit and linter tests.
  • Ran relevant acceptance tests using my own Google Cloud project and credentials (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
  • Read the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

dlp: added `redact_config`, `fixed_size_bucketing_config`, `bucketing_config`, `time_part_config` and `date_shift_config`  fields to `google_data_loss_prevention_deidentify_template` resource

@modular-magician
Copy link
Collaborator

Hello! I am a robot who works on Magic Modules PRs.

I've detected that you're a community contributor. @rileykarson, a repository maintainer, has been assigned to assist you and help review your changes.

❓ First time contributing? Click here for more details

Your assigned reviewer will help review your code by:

  • Ensuring it's backwards compatible, covers common error cases, etc.
  • Summarizing the change into a user-facing changelog note.
  • Passes tests, either our "VCR" suite, a set of presubmit tests, or with manual test runs.

You can help make sure that review is quick by running local tests and ensuring they're passing in between each push you make to your PR's branch. Also, try to leave a comment with each push you make, as pushes generally don't generate emails.

If your reviewer doesn't get back to you within a week after your most recent change, please feel free to leave a comment on the issue asking them to take a look! In the absence of a dedicated review dashboard most maintainers manage their pending reviews through email, and those will sometimes get lost in their inbox.


@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 May 10, 2023
@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.

Terraform GA: Diff ( 3 files changed, 7640 insertions(+), 3389 deletions(-))
Terraform Beta: Diff ( 3 files changed, 7640 insertions(+), 3389 deletions(-))
TF Conversion: Diff ( 3 files changed, 1284 insertions(+), 3 deletions(-))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_data_loss_prevention_deidentify_template (16 total tests)
Untested fields: deidentify_config.info_type_transformations.transformations.primitive_transformation.bucketing_config.buckets.replacement_value.day_of_week_value, deidentify_config.info_type_transformations.transformations.primitive_transformation.bucketing_config.buckets.max.day_of_week_value, deidentify_config.info_type_transformations.transformations.primitive_transformation.bucketing_config.buckets.min.day_of_week_value, deidentify_config.info_type_transformations.transformations.primitive_transformation.date_shift_config.crypto_key.kms_wrapped.crypto_key_name, deidentify_config.info_type_transformations.transformations.primitive_transformation.date_shift_config.crypto_key.kms_wrapped.wrapped_key, deidentify_config.info_type_transformations.transformations.primitive_transformation.date_shift_config.crypto_key.unwrapped.key, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.lower_bound.time_value.minutes, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.lower_bound.time_value.nanos, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.lower_bound.time_value.seconds, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.lower_bound.time_value.hours, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.lower_bound.timestamp_value, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.lower_bound.boolean_value, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.lower_bound.date_value.day, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.lower_bound.date_value.month, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.lower_bound.date_value.year, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.lower_bound.day_of_week_value, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.lower_bound.float_value, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.lower_bound.string_value, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.upper_bound.day_of_week_value, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.upper_bound.float_value, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.upper_bound.string_value, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.upper_bound.time_value.nanos, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.upper_bound.time_value.seconds, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.upper_bound.time_value.hours, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.upper_bound.time_value.minutes, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.upper_bound.timestamp_value, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.upper_bound.boolean_value, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.upper_bound.date_value.day, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.upper_bound.date_value.month, deidentify_config.info_type_transformations.transformations.primitive_transformation.fixed_size_bucketing_config.upper_bound.date_value.year

Please add acceptance tests which include these fields.

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 0
Passed tests 0
Skipped tests: 0
Affected tests: 0

Errors occurred during REPLAYING mode. Please fix them to complete your PR
View the build log

@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label May 11, 2023
@JayS-crest
Copy link
Contributor Author

I've added the necessary tests for missing attributes. Seems like that would work now.

@JayS-crest
Copy link
Contributor Author

Hey @rileykarson, Could you please start the tests? It is awaiting approval on your end. Thanks!

@modular-magician modular-magician removed the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label May 17, 2023
@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.

Terraform GA: Diff ( 3 files changed, 7599 insertions(+), 4057 deletions(-))
Terraform Beta: Diff ( 3 files changed, 7599 insertions(+), 4057 deletions(-))
TF Conversion: Diff ( 3 files changed, 966 insertions(+), 3 deletions(-))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_data_loss_prevention_deidentify_template (20 total tests)
Untested fields: deidentify_config.info_type_transformations.transformations.primitive_transformation.bucketing_config.buckets.replacement_value.day_of_week_value, deidentify_config.info_type_transformations.transformations.primitive_transformation.bucketing_config.buckets.max.day_of_week_value, deidentify_config.info_type_transformations.transformations.primitive_transformation.bucketing_config.buckets.min.day_of_week_value

Please add acceptance tests which include these fields.

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 12
Passed tests 12
Skipped tests: 0
Affected tests: 0

Errors occurred during REPLAYING mode. Please fix them to complete your PR
View the build log

@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label May 18, 2023
@JayS-crest
Copy link
Contributor Author

JayS-crest commented May 18, 2023

Hi, I mistakenly named the attribute day_of_the_week_value instead of day_of_week_value in the test. I've renamed the fields in all the three cases and looks like the tests will run now.

@modular-magician modular-magician removed the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label May 18, 2023
@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.

Terraform GA: Diff ( 3 files changed, 7601 insertions(+), 4060 deletions(-))
Terraform Beta: Diff ( 3 files changed, 7601 insertions(+), 4060 deletions(-))
TF Conversion: Diff ( 3 files changed, 966 insertions(+), 3 deletions(-))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_data_loss_prevention_deidentify_template (20 total tests)
Untested fields: deidentify_config.info_type_transformations.transformations.primitive_transformation.bucketing_config.buckets.min.day_of_week_value, deidentify_config.info_type_transformations.transformations.primitive_transformation.bucketing_config.buckets.replacement_value.day_of_week_value, deidentify_config.info_type_transformations.transformations.primitive_transformation.bucketing_config.buckets.max.day_of_week_value

Please add acceptance tests which include these fields.

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 12
Passed tests 12
Skipped tests: 0
Affected tests: 0

Errors occurred during REPLAYING mode. Please fix them to complete your PR
View the build log

@rileykarson
Copy link
Member

I'm continuing to get build errors- you should see them in the GH action.

@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label May 18, 2023
@JayS-crest
Copy link
Contributor Author

I tried checking GH actions but got Forbidden Error 403. But while manually checking the test case, I found that the name of the testcase function was different as it contained day_of_the_week_value. Hence, I've updated that and the fields would be detected now as the case is part for tests.

@rileykarson
Copy link
Member

@ScottSuarez can you investigate I thought that was the intention behind using GHA.

@rileykarson
Copy link
Member

@JayS-crest are you able to successfully run your test locally?

@JayS-crest
Copy link
Contributor Author

@rileykarson Apart from that, I've noticed that if the Vcr Tests fail, we cannot see the failing test or logs. It shows permission denied there as well. Is there a way to check that ? And yes, the tests here will run fine now as the fields will be detected so whenever you can approve that. Thanks!

@ScottSuarez
Copy link
Contributor

@ScottSuarez can you investigate I thought that was the intention behind using GHA.

GHA's are visible to all users. They should of been able to access the results. You can open these links with incognito to verify.

@rileykarson
Copy link
Member

rileykarson commented May 18, 2023

I manually checked each config for tests and they were passing

Can you please share output from the 899c52c (#7912) commit? The code that was pushed here could not have worked, due to the compilation error you just resolved. I'm not sure why that worked locally for you, given that.

@modular-magician modular-magician removed the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label May 18, 2023
@JayS-crest
Copy link
Contributor Author

@ScottSuarez can you investigate I thought that was the intention behind using GHA.

GHA's are visible to all users. They should of been able to access the results. You can open these links with incognito to verify.

When I tried opening the build log, I was shown Forbidden Error. Even now if I try to open link, it is showing the same error. Any probable reason why it isn't working for me?

@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.

Terraform GA: Diff ( 3 files changed, 7601 insertions(+), 4060 deletions(-))
Terraform Beta: Diff ( 3 files changed, 7601 insertions(+), 4060 deletions(-))
TF Conversion: Diff ( 3 files changed, 966 insertions(+), 3 deletions(-))

@JayS-crest
Copy link
Contributor Author

I manually checked each config for tests and they were passing

Can you please share output from the 899c52c (#7912) commit? The code that was pushed here could not have worked, due to the compilation error you just resolved. I'm not sure why that worked locally for you, given that.

So, as per what I think, I tried running different configs manually and not the test cases(functions). Hence, that was probably the reason why I was not able to catch the errors. Currently, when you asked me for config logs, I ran the Tests and that is when I found the errors in build-and-unit-test and resolved that.

@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label May 18, 2023
@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 12
Passed tests 12
Skipped tests: 0
Affected tests: 0

Errors occurred during REPLAYING mode. Please fix them to complete your PR
View the build log

@rileykarson
Copy link
Member

The GitHub actions are the build and test steps! @ScottSuarez we may need to make the part external contributors can/can't interact with more clear.

@modular-magician modular-magician removed the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label May 18, 2023
@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.

Terraform GA: Diff ( 3 files changed, 7601 insertions(+), 4060 deletions(-))
Terraform Beta: Diff ( 3 files changed, 7601 insertions(+), 4060 deletions(-))
TF Conversion: Diff ( 3 files changed, 966 insertions(+), 3 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 2747
Passed tests 2450
Skipped tests: 283
Affected tests: 14

Action taken

Found 14 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccDataLossPreventionDeidentifyTemplate_dlpDeidentifyTemplateSkipCharactersExample|TestAccDataLossPreventionDeidentifyTemplate_dlpDeidentifyTemplateBasicExample|TestAccDataLossPreventionDeidentifyTemplate_dlpDeidentifyTemplate_infoTypeTransformationsUpdate|TestAccDataLossPreventionDeidentifyTemplate_dlpDeidentifyTemplate_infoTypeTransformations_primitiveTransformations_dateShiftConfig|TestAccDataLossPreventionDeidentifyTemplate_dlpDeidentifyTemplate_infoTypeTransformations_primitiveTransformations_fixedSizeBucketingConfig|TestAccDataLossPreventionDeidentifyTemplate_dlpDeidentifyTemplate_infoTypeTransformations_primitiveTransformations_bucketingConfig|TestAccDataLossPreventionDeidentifyTemplate_dlpDeidentifyTemplate_infoTypeTransformations_primitiveTransformations|TestAccComputeFirewallPolicyRule_multipleRules|TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample|TestAccAlloydbCluster_missingLocation|TestAccApigeeKeystoresAliasesPkcs12_ApigeeKeystoresAliasesPkcs12Example|TestAccAlloydbBackup_missingLocation|TestAccDataSourceGoogleFirebaseAndroidAppConfig|TestAccDataSourceAlloydbLocations_basic

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

Tests passed during RECORDING mode:
TestAccDataLossPreventionDeidentifyTemplate_dlpDeidentifyTemplateSkipCharactersExample[Debug log]
TestAccDataLossPreventionDeidentifyTemplate_dlpDeidentifyTemplateBasicExample[Debug log]
TestAccDataLossPreventionDeidentifyTemplate_dlpDeidentifyTemplate_infoTypeTransformationsUpdate[Debug log]
TestAccDataLossPreventionDeidentifyTemplate_dlpDeidentifyTemplate_infoTypeTransformations_primitiveTransformations_dateShiftConfig[Debug log]
TestAccDataLossPreventionDeidentifyTemplate_dlpDeidentifyTemplate_infoTypeTransformations_primitiveTransformations_fixedSizeBucketingConfig[Debug log]
TestAccDataLossPreventionDeidentifyTemplate_dlpDeidentifyTemplate_infoTypeTransformations_primitiveTransformations_bucketingConfig[Debug log]
TestAccDataLossPreventionDeidentifyTemplate_dlpDeidentifyTemplate_infoTypeTransformations_primitiveTransformations[Debug log]
TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample[Debug log]
TestAccAlloydbCluster_missingLocation[Debug log]
TestAccApigeeKeystoresAliasesPkcs12_ApigeeKeystoresAliasesPkcs12Example[Debug log]
TestAccAlloydbBackup_missingLocation[Debug log]
TestAccDataSourceAlloydbLocations_basic[Debug log]

Tests failed during RECORDING mode:
TestAccComputeFirewallPolicyRule_multipleRules[Error message] [Debug log]
TestAccDataSourceGoogleFirebaseAndroidAppConfig[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

@ScottSuarez
Copy link
Contributor

@ScottSuarez can you investigate I thought that was the intention behind using GHA.

GHA's are visible to all users. They should of been able to access the results. You can open these links with incognito to verify.

When I tried opening the build log, I was shown Forbidden Error. Even now if I try to open link, it is showing the same error. Any probable reason why it isn't working for me?

correct. you cannot access the cloud build links. You can access the status check for build-and-unit-tests

@ScottSuarez
Copy link
Contributor

ScottSuarez commented May 18, 2023

The GitHub actions are the build and test steps! @ScottSuarez we may need to make the part external contributors can/can't interact with more clear.

Could you make an issue so we can track? There is messaging we can definitely modify. Ideally we will move everything to github actions eventually but I agree maybe there is some stopgap we can apply in the meantime.

@rileykarson
Copy link
Member

Everything would be impossible- VCR cannot move to GHA

@ScottSuarez
Copy link
Contributor

ScottSuarez commented May 18, 2023

Everything would be impossible- VCR cannot move to GHA

Yes, everything except VCR or anything that specifically needs our GCP env. Lets talk outside this thread if need. Not clear on vision yet.

@JayS-crest
Copy link
Contributor Author

Hi @rileykarson, as the tests have passed now, could you please merge this PR? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for missing infoTypeTransformations in google_data_loss_prevention_deidentify_template
4 participants