magic-modules icon indicating copy to clipboard operation
magic-modules copied to clipboard

Refactor muxing 1 : Re-use same config to configure the SDK and PF providers, fix VCR testing

Open SarahFrench opened this issue 1 year ago • 17 comments

Summary

This PR:

  • Change how the plugin-framework implementation of the provider is configured
  • Changes code defining the VCR system to accommodate the change above
    • This also fixes https://github.com/hashicorp/terraform-provider-google/issues/14158
  • Makes changes to the data sources currently implemented with the plugin-framework, as the change in the core configuration struct leaks through to each data source currently (a future TODO I guess!)

Description

Previously when the provider was muxed (i.e. a SDKv2-implemented provider and plugin framework-implemented implemented provider in a trench coat) those two provider implementations were kept very parallel to each other. All the logic for interpreting user inputs to the provider block was duplicated, and that contained errors. Also this introduced the idea of two parallel, independent HTTP clients being used, which means all API interactions for SDKv2-implemented resources/data sources and plugin framework-implemented resources/data sources go via different clients internally. This had the consequence of breaking VCR tests, but only if HTTP traffic was split between the two clients.

This PR avoids the pitfalls of re-implementing core provider logic by re-using the old logic that's been in the provider for years. We use the 'SDKv2 flavoured' code to configure the plugin-framework provider.

This has the advantages of:

  • ensuring that the provider's behaviour is largely similar to the SDK provider*
  • resuming all HTTP traffic via a single client, enabling VCR testing to reuse working fully
  • matching the approach used in https://github.com/hashicorp/terraform-provider-aws, so we can navigate muxing together 🙏

This has the disadvantages of:

  • The 'SDKv2 flavour' type system enters the plugin-framework code. This is addressed by converting the types uses in the SDKv2 provider Config struct into the plugin-frameworks type system as needed.
  • In future when the provider becomes 100% plugin-framework implemented that core configuration logic will need to be refactored to use the plugin-frameworks type system (when that time comes, advantages of doing this include better Null/Unknown handling, ability to remove all resulting type conversions in the provider).

Release Note Template for Downstream PRs (will be copied)

provider: refactored how the provider configuration is handled internally

SarahFrench avatar Aug 29 '24 14:08 SarahFrench

There are lots of code deletions I can do as a result of this change, but I'll split that into a separate PR for clarity.

SarahFrench avatar Aug 29 '24 14:08 SarahFrench

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 ( 13 files changed, 127 insertions(+), 153 deletions(-)) google-beta provider: Diff ( 19 files changed, 156 insertions(+), 184 deletions(-))

Errors

google provider:

  • The diff processor failed to build. This is usually due to the downstream provider failing to compile.

modular-magician avatar Aug 29 '24 15:08 modular-magician

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 ( 13 files changed, 112 insertions(+), 153 deletions(-)) google-beta provider: Diff ( 19 files changed, 156 insertions(+), 184 deletions(-))

modular-magician avatar Aug 29 '24 15:08 modular-magician

Tests analytics

Total tests: 3930 Passed tests: 3519 Skipped tests: 407 Affected tests: 4

Click here to see the affected service packages

All service packages are affected

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
  • TestAccComputeInstanceTemplate_withNamePrefix
  • TestAccDataSourceGoogleFirebaseAndroidAppConfig
  • TestAccDataSourceGoogleFirebaseAppleAppConfig
  • TestAccFirebaseWebApp_firebaseWebAppFull

Get to know how VCR tests work

modular-magician avatar Aug 29 '24 16:08 modular-magician

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$ TestAccComputeInstanceTemplate_withNamePrefix[Debug log] TestAccDataSourceGoogleFirebaseAppleAppConfig[Debug log] TestAccFirebaseWebApp_firebaseWebAppFull[Debug log] $\textcolor{red}{\textsf{Tests failed when rerunning REPLAYING mode:}}$ TestAccComputeInstanceTemplate_withNamePrefix[Error message] [Debug log] TestAccDataSourceGoogleFirebaseAppleAppConfig[Error message] [Debug log]

Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made.

Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.


$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$ TestAccDataSourceGoogleFirebaseAndroidAppConfig[Error message] [Debug log]

$\textcolor{red}{\textsf{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 avatar Aug 29 '24 16:08 modular-magician

Tests analytics

Total tests: 3930 Passed tests: 3519 Skipped tests: 407 Affected tests: 4

Click here to see the affected service packages

All service packages are affected

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
  • TestAccComputeInstanceTemplate_withNamePrefix
  • TestAccDataSourceGoogleFirebaseAndroidAppConfig
  • TestAccDataSourceGoogleFirebaseAppleAppConfig
  • TestAccFirebaseWebApp_firebaseWebAppFull

Get to know how VCR tests work

modular-magician avatar Aug 29 '24 16:08 modular-magician

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$ TestAccComputeInstanceTemplate_withNamePrefix[Debug log] TestAccDataSourceGoogleFirebaseAppleAppConfig[Debug log] TestAccFirebaseWebApp_firebaseWebAppFull[Debug log] $\textcolor{red}{\textsf{Tests failed when rerunning REPLAYING mode:}}$ TestAccComputeInstanceTemplate_withNamePrefix[Error message] [Debug log] TestAccDataSourceGoogleFirebaseAppleAppConfig[Error message] [Debug log]

Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made.

Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.


$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$ TestAccDataSourceGoogleFirebaseAndroidAppConfig[Error message] [Debug log]

$\textcolor{red}{\textsf{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 avatar Aug 29 '24 16:08 modular-magician

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 ( 14 files changed, 115 insertions(+), 154 deletions(-)) google-beta provider: Diff ( 20 files changed, 161 insertions(+), 198 deletions(-))

modular-magician avatar Aug 29 '24 17:08 modular-magician

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 ( 14 files changed, 115 insertions(+), 154 deletions(-)) google-beta provider: Diff ( 20 files changed, 160 insertions(+), 197 deletions(-))

modular-magician avatar Aug 29 '24 17:08 modular-magician

I'll fix the unused import after the test run finishes- I want to see the outcome and each push delays it!

SarahFrench avatar Aug 29 '24 18:08 SarahFrench

Tests analytics

Total tests: 3930 Passed tests: 3521 Skipped tests: 406 Affected tests: 3

Click here to see the affected service packages

All service packages are affected

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
  • TestAccComputeInstanceTemplate_withNamePrefix
  • TestAccDataSourceGoogleFirebaseAndroidAppConfig
  • TestAccDataSourceGoogleFirebaseAppleAppConfig

Get to know how VCR tests work

modular-magician avatar Aug 29 '24 18:08 modular-magician

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$ TestAccComputeInstanceTemplate_withNamePrefix[Debug log] TestAccDataSourceGoogleFirebaseAppleAppConfig[Debug log] $\textcolor{red}{\textsf{Tests failed when rerunning REPLAYING mode:}}$ TestAccComputeInstanceTemplate_withNamePrefix[Error message] [Debug log]

Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made.

Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.


$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$ TestAccDataSourceGoogleFirebaseAndroidAppConfig[Error message] [Debug log]

$\textcolor{red}{\textsf{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 avatar Aug 29 '24 18:08 modular-magician

Tests analytics

Total tests: 3930 Passed tests: 3521 Skipped tests: 406 Affected tests: 3

Click here to see the affected service packages

All service packages are affected

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
  • TestAccComputeInstanceTemplate_withNamePrefix
  • TestAccDataSourceGoogleFirebaseAndroidAppConfig
  • TestAccDataSourceGoogleFirebaseAppleAppConfig

Get to know how VCR tests work

modular-magician avatar Aug 29 '24 18:08 modular-magician

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$ TestAccDataSourceGoogleFirebaseAndroidAppConfig[Debug log] TestAccDataSourceGoogleFirebaseAppleAppConfig[Debug log]

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$


$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$ TestAccComputeInstanceTemplate_withNamePrefix[Error message] [Debug log]

$\textcolor{red}{\textsf{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 avatar Aug 29 '24 18:08 modular-magician

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 ( 14 files changed, 115 insertions(+), 155 deletions(-)) google-beta provider: Diff ( 20 files changed, 160 insertions(+), 198 deletions(-))

modular-magician avatar Aug 29 '24 21:08 modular-magician

Tests analytics

Total tests: 3933 Passed tests: 3524 Skipped tests: 406 Affected tests: 3

Click here to see the affected service packages

All service packages are affected

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
  • TestAccComputeInstanceTemplate_withNamePrefix
  • TestAccFrameworkProviderBasePath_setBasePath
  • TestAccFrameworkProviderMeta_setModuleName

Get to know how VCR tests work

modular-magician avatar Aug 29 '24 22:08 modular-magician

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$ TestAccComputeInstanceTemplate_withNamePrefix[Debug log] TestAccFrameworkProviderBasePath_setBasePath[Debug log] TestAccFrameworkProviderMeta_setModuleName[Debug log] $\textcolor{red}{\textsf{Tests failed when rerunning REPLAYING mode:}}$ TestAccComputeInstanceTemplate_withNamePrefix[Error message] [Debug log]

Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made.

Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.


$\textcolor{green}{\textsf{All tests passed!}}$

View the build log or the debug log for each test

modular-magician avatar Aug 29 '24 22:08 modular-magician

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 ( 14 files changed, 104 insertions(+), 155 deletions(-)) google-beta provider: Diff ( 20 files changed, 149 insertions(+), 198 deletions(-))

modular-magician avatar Aug 30 '24 12:08 modular-magician

Tests analytics

Total tests: 3683 Passed tests: 3275 Skipped tests: 406 Affected tests: 2

Click here to see the affected service packages

All service packages are affected

Action taken

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

Get to know how VCR tests work

modular-magician avatar Aug 30 '24 13:08 modular-magician

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$ TestAccComputeInstanceTemplate_withNamePrefix[Debug log] TestAccFrameworkProviderBasePath_setBasePath[Debug log] $\textcolor{red}{\textsf{Tests failed when rerunning REPLAYING mode:}}$ TestAccComputeInstanceTemplate_withNamePrefix[Error message] [Debug log]

Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made.

Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.


$\textcolor{green}{\textsf{All tests passed!}}$

View the build log or the debug log for each test

modular-magician avatar Aug 30 '24 13:08 modular-magician

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 ( 14 files changed, 105 insertions(+), 157 deletions(-)) google-beta provider: Diff ( 20 files changed, 150 insertions(+), 200 deletions(-))

modular-magician avatar Sep 23 '24 16:09 modular-magician

Tests analytics

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

Click here to see the affected service packages

All service packages are affected

$\textcolor{red}{\textsf{Errors occurred during REPLAYING mode. Please fix them to complete your PR.}}$

View the build log

modular-magician avatar Sep 23 '24 16:09 modular-magician

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 ( 15 files changed, 146 insertions(+), 174 deletions(-)) google-beta provider: Diff ( 21 files changed, 191 insertions(+), 217 deletions(-))

modular-magician avatar Sep 23 '24 18:09 modular-magician

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 ( 15 files changed, 146 insertions(+), 173 deletions(-)) google-beta provider: Diff ( 21 files changed, 191 insertions(+), 216 deletions(-))

modular-magician avatar Sep 23 '24 18:09 modular-magician

Tests analytics

Total tests: 4079 Passed tests: 3672 Skipped tests: 406 Affected tests: 1

Click here to see the affected service packages

All service packages are affected

Action taken

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

Get to know how VCR tests work

modular-magician avatar Sep 23 '24 19:09 modular-magician

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$ TestAccNetworkConnectivitySpoke_networkConnectivitySpokeVpnTunnelBasicExample[Debug log] $\textcolor{red}{\textsf{Tests failed when rerunning REPLAYING mode:}}$ TestAccNetworkConnectivitySpoke_networkConnectivitySpokeVpnTunnelBasicExample[Error message] [Debug log]

Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made.

Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.


$\textcolor{red}{\textsf{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 avatar Sep 23 '24 19:09 modular-magician

Tests analytics

Total tests: 4087 Passed tests: 3679 Skipped tests: 406 Affected tests: 2

Click here to see the affected service packages

All service packages are affected

Action taken

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

Get to know how VCR tests work

modular-magician avatar Sep 23 '24 19:09 modular-magician

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$ TestAccFrameworkProviderBasePath_setBasePath[Debug log] TestAccNetworkConnectivitySpoke_networkConnectivitySpokeVpnTunnelBasicExample[Debug log] $\textcolor{red}{\textsf{Tests failed when rerunning REPLAYING mode:}}$ TestAccNetworkConnectivitySpoke_networkConnectivitySpokeVpnTunnelBasicExample[Error message] [Debug log]

Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made.

Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.


$\textcolor{green}{\textsf{All tests passed!}}$

View the build log or the debug log for each test

modular-magician avatar Sep 23 '24 19:09 modular-magician

Rebased to include Go rewrtie

SarahFrench avatar Sep 26 '24 10:09 SarahFrench

Just rebased to pull in the latest changes from main, including new acc tests for how the provider is configured

SarahFrench avatar Oct 01 '24 18:10 SarahFrench