move `framework_validators.go` to new `fwvalidators` package #12268

BBBmau · 2024-11-07T01:04:42Z

This PR would make it possible to add validators being used by EphemeralResources.

When attempting to move validators to framework_provider.go in fwprovider we end up getting an import cycle because of fwprovider importing resourcemanager in order to support ephermeral resources and data sources under plugin-framework.

You can view how service_account_token ephemeral would look with this being merged here

The validators can be seen being used in this resource where they are added in the same reosurce file itself instead of being placed in the validator file. #12140

When we merg this we can perform the following:

merge use fwvalidators to fix import cycle error BBBmau/magic-modules#24 into service_account_token resource supprt
merge ephemeral: add ephemeral_google_service_account_token #12140 - tests will be passing as i've ran locally with the fwvalidators package. This would also allow other resources to add the necessary validators into fwvalidators package.

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.

modular-magician · 2024-11-07T01:15:07Z

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 ( 3 files changed, 288 insertions(+), 120 deletions(-))
google-beta provider: Diff ( 3 files changed, 288 insertions(+), 120 deletions(-))

modular-magician · 2024-11-07T01:17:57Z

Tests analytics

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

Click here to see the affected service packages

🔴 Errors occurred during REPLAYING mode. Please fix them to complete your PR.

View the build log

SarahFrench

Please update how we reference the validators in the fwprovider package now that they've been moved out of there; currently there are build errors:

BBBmau · 2024-11-07T16:40:39Z

odd that i didn't run into this error when building. Seemed to work just fine.

Latest commit references explicitly from fwvalidators

modular-magician · 2024-11-07T16:50:25Z

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 ( 4 files changed, 297 insertions(+), 128 deletions(-))
google-beta provider: Diff ( 4 files changed, 297 insertions(+), 128 deletions(-))

modular-magician · 2024-11-07T16:53:17Z

Tests analytics

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

Click here to see the affected service packages

🔴 Errors occurred during REPLAYING mode. Please fix them to complete your PR.

View the build log

mmv1/third_party/terraform/fwvalidators/framework_validators.go

BBBmau · 2024-11-07T23:03:06Z

pushed the validator tests on top of your changes @SarahFrench just a heads-up!

modular-magician · 2024-11-08T21:37:29Z

🟢 Tests passed during RECORDING mode:
TestAccContainerNodePool_resourceManagerTags [Debug log]
TestAccDataSourceGoogleBackupDRBackupPlan_basic [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.

🔴 Tests failed during RECORDING mode:
TestAccContainerCluster_withSecretManagerConfig [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

SarahFrench

Some final review comments - after this I think this PR would be ready to merge

mmv1/third_party/terraform/fwvalidators/framework_validators.go

mmv1/third_party/terraform/fwvalidators/framework_validators_test.go

mmv1/third_party/terraform/fwvalidators/framework_validators.go

modular-magician · 2024-11-11T15:11:52Z

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 ( 5 files changed, 531 insertions(+), 197 deletions(-))
google-beta provider: Diff ( 5 files changed, 531 insertions(+), 197 deletions(-))

modular-magician · 2024-11-11T15:14:41Z

Tests analytics

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

Click here to see the affected service packages

🔴 Errors occurred during REPLAYING mode. Please fix them to complete your PR.

View the build log

SarahFrench · 2024-11-11T16:19:13Z

mmv1/third_party/terraform/fwvalidators/framework_validators.go

 // Non Negative Duration Validator
-type nonnegativedurationValidator struct {
+type nonnegativeBoundedDuration struct {
 }


Please revert the changes to this validator - it doesn't make assertions about the range/bounds a duration should be in so this name is misleading. My feedback previously was about your validator and showing how this validator's name indicated how it validated the duration.

modular-magician · 2024-11-11T17:55:15Z

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 ( 4 files changed, 413 insertions(+), 79 deletions(-))
google-beta provider: Diff ( 4 files changed, 413 insertions(+), 79 deletions(-))

modular-magician · 2024-11-11T19:09:09Z

Tests analytics

Total tests: 4267
Passed tests: 3846
Skipped tests: 417
Affected tests: 4

Click here to see the affected service packages

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

Get to know how VCR tests work

mmv1/third_party/terraform/fwvalidators/framework_validators_test.go

modular-magician · 2024-11-11T19:21:13Z

🟢 Tests passed during RECORDING mode:
TestAccBackupDRBackupVault_fullUpdate [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.

🔴 Tests failed during RECORDING mode:
TestAccCloudbuildv2Connection_GlePrivConnection [Error message] [Debug log]
TestAccCloudbuildv2Connection_GlePrivUpdateConnection [Error message] [Debug log]
TestAccContainerCluster_withSecretManagerConfig [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

…est.go Co-authored-by: Sarah French <[email protected]>

modular-magician · 2024-11-11T19:41:16Z

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 ( 4 files changed, 413 insertions(+), 79 deletions(-))
google-beta provider: Diff ( 4 files changed, 413 insertions(+), 79 deletions(-))

modular-magician · 2024-11-11T19:41:19Z

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 ( 4 files changed, 413 insertions(+), 79 deletions(-))
google-beta provider: Diff ( 4 files changed, 413 insertions(+), 79 deletions(-))

modular-magician · 2024-11-11T19:41:25Z

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 ( 4 files changed, 413 insertions(+), 79 deletions(-))
google-beta provider: Diff ( 4 files changed, 413 insertions(+), 79 deletions(-))

modular-magician · 2024-11-11T20:53:20Z

Tests analytics

Total tests: 4267
Passed tests: 3847
Skipped tests: 417
Affected tests: 3

Click here to see the affected service packages

Action taken

Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

Get to know how VCR tests work

modular-magician · 2024-11-11T20:54:17Z

Tests analytics

Total tests: 4267
Passed tests: 3847
Skipped tests: 417
Affected tests: 3

Click here to see the affected service packages

Action taken

Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

Get to know how VCR tests work

modular-magician · 2024-11-11T20:54:29Z

Tests analytics

Total tests: 4267
Passed tests: 3847
Skipped tests: 417
Affected tests: 3

Click here to see the affected service packages

Action taken

Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

Get to know how VCR tests work

modular-magician · 2024-11-11T21:04:42Z

🔴 Tests failed during RECORDING mode:
TestAccCloudbuildv2Connection_GlePrivConnection [Error message] [Debug log]
TestAccCloudbuildv2Connection_GlePrivUpdateConnection [Error message] [Debug log]
TestAccContainerCluster_withSecretManagerConfig [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

modular-magician · 2024-11-11T21:05:09Z

🔴 Tests failed during RECORDING mode:
TestAccCloudbuildv2Connection_GlePrivConnection [Error message] [Debug log]
TestAccCloudbuildv2Connection_GlePrivUpdateConnection [Error message] [Debug log]
TestAccContainerCluster_withSecretManagerConfig [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

modular-magician · 2024-11-11T21:06:13Z

🔴 Tests failed during RECORDING mode:
TestAccCloudbuildv2Connection_GlePrivConnection [Error message] [Debug log]
TestAccCloudbuildv2Connection_GlePrivUpdateConnection [Error message] [Debug log]
TestAccContainerCluster_withSecretManagerConfig [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

SarahFrench

LGTM

move framework_validators to fwvalidators package

7b01fdd

This was referenced Nov 7, 2024

use fwvalidators to fix import cycle error BBBmau/magic-modules#24

Closed

ephemeral: add ephemeral_google_service_account_token #12140

Open

SarahFrench requested changes Nov 7, 2024

View reviewed changes

reference fwvalidators in framework_provider

e7f003a

Fix reference to NonNegativeDurationValidator

03e6663

This comment was marked as outdated.

Sign in to view

Fix reference to CredentialsValidator in test

46af2ac

SarahFrench reviewed Nov 7, 2024

View reviewed changes

mmv1/third_party/terraform/fwvalidators/framework_validators.go Outdated Show resolved Hide resolved

Update mmv1/third_party/terraform/fwvalidators/framework_validators.go

04adbfe

This comment was marked as duplicate.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment has been minimized.

Sign in to view

This comment was marked as duplicate.

Sign in to view

This comment has been minimized.

Sign in to view

This comment was marked as duplicate.

Sign in to view

update validator with simpler regex and tests

8c0375c

BBBmau force-pushed the add-fwvalidators branch from 123448a to 8c0375c Compare November 7, 2024 22:43

This comment was marked as outdated.

Sign in to view

BBBmau requested a review from SarahFrench November 7, 2024 23:03

This comment was marked as outdated.

Sign in to view

SarahFrench reviewed Nov 11, 2024

View reviewed changes

address some refactors and extra tests

1479924

BBBmau requested a review from SarahFrench November 11, 2024 15:01

SarahFrench reviewed Nov 11, 2024

View reviewed changes

revert NonNegativeDurationValidator

e42d381

SarahFrench reviewed Nov 11, 2024

View reviewed changes

BBBmau and others added 4 commits November 11, 2024 11:30

Update mmv1/third_party/terraform/fwvalidators/framework_validators_t…

f072c5e

…est.go Co-authored-by: Sarah French <[email protected]>

Update mmv1/third_party/terraform/fwvalidators/framework_validators_t…

a042066

…est.go Co-authored-by: Sarah French <[email protected]>

Update mmv1/third_party/terraform/fwvalidators/framework_validators_t…

6bbcef9

…est.go Co-authored-by: Sarah French <[email protected]>

Update mmv1/third_party/terraform/fwvalidators/framework_validators_t…

9f526f5

…est.go Co-authored-by: Sarah French <[email protected]>

SarahFrench approved these changes Nov 11, 2024

View reviewed changes

BBBmau merged commit 70bef40 into GoogleCloudPlatform:FEATURE-BRANCH-ephemeral-resource Nov 12, 2024
9 of 10 checks passed

move framework_validators.go to new fwvalidators package #12268

move framework_validators.go to new fwvalidators package #12268

Conversation

BBBmau commented Nov 7, 2024

modular-magician commented Nov 7, 2024

Diff report

modular-magician commented Nov 7, 2024

Tests analytics

SarahFrench left a comment

Choose a reason for hiding this comment

BBBmau commented Nov 7, 2024

modular-magician commented Nov 7, 2024

Diff report

modular-magician commented Nov 7, 2024

Tests analytics

This comment was marked as outdated.

This comment was marked as duplicate.

This comment was marked as outdated.

This comment has been minimized.

This comment has been minimized.

This comment was marked as duplicate.

This comment has been minimized.

This comment was marked as duplicate.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

BBBmau commented Nov 7, 2024

This comment was marked as outdated.

modular-magician commented Nov 8, 2024

SarahFrench left a comment

Choose a reason for hiding this comment

modular-magician commented Nov 11, 2024

Diff report

modular-magician commented Nov 11, 2024

Tests analytics

SarahFrench Nov 11, 2024

Choose a reason for hiding this comment

BBBmau Nov 11, 2024

Choose a reason for hiding this comment

modular-magician commented Nov 11, 2024

Diff report

modular-magician commented Nov 11, 2024

Tests analytics

Action taken

modular-magician commented Nov 11, 2024

modular-magician commented Nov 11, 2024

Diff report

modular-magician commented Nov 11, 2024

Diff report

modular-magician commented Nov 11, 2024

Diff report

modular-magician commented Nov 11, 2024

Tests analytics

Action taken

modular-magician commented Nov 11, 2024

Tests analytics

Action taken

modular-magician commented Nov 11, 2024

Tests analytics

Action taken

modular-magician commented Nov 11, 2024

modular-magician commented Nov 11, 2024

modular-magician commented Nov 11, 2024

SarahFrench left a comment

Choose a reason for hiding this comment

move `framework_validators.go` to new `fwvalidators` package #12268

move `framework_validators.go` to new `fwvalidators` package #12268