-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add support for Database Migration Services - Connection Profile resource #7447
Conversation
Hello! I am a robot who works on Magic Modules PRs. I've detected that you're a community contributor. @c2thorn, a repository maintainer, has been assigned to assist you and help review your changes. ❓ First time contributing? Click here for more detailsYour assigned reviewer will help review your code by:
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. |
/gcbrun |
I think the failure is due to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got a quarter through the review when I realized cameron was on the PR. I'm going to leave me comments here, but Cameron I'll leave the rest to you !
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 6 files changed, 3241 insertions(+), 184 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 75 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccComputeSharedVpc_basic|TestAccIAM2AccessBoundaryPolicy|TestAccKmsKeyRing_basic|TestAccComputeResourceUsageExportBucket|TestAccLoggingBucketConfigProject_cmekSettings|TestAccIAM2DenyPolicy_iamDenyPolicyUpdate|TestAccIAM2DenyPolicy_iamDenyPolicyBasicExample|TestAccKmsKeyRingIamPolicy_withCondition|TestAccKmsKeyRingIamPolicy|TestAccKmsKeyRingIamMember_withCondition|TestAccKmsKeyRingIamMember|TestAccKmsKeyRingIamBinding_withCondition|TestAccKmsKeyRingIamBinding|TestAccKmsCryptoKeyVersion_patch|TestAccKmsCryptoKeyVersion_skipInitialVersion|TestAccKmsCryptoKeyVersion_basic|TestAccKmsCryptoKey_importOnly|TestAccKmsCryptoKey_destroyDuration|TestAccKmsCryptoKey_template|TestAccKmsCryptoKey_basic|TestAccKmsCryptoKeyIamPolicy_withCondition|TestAccKmsCryptoKeyIamPolicy|TestAccKmsCryptoKeyIamMember_withCondition|TestAccKmsCryptoKeyIamMember|TestAccKmsCryptoKeyIamBinding_withCondition|TestAccKmsCryptoKeyIamBinding|TestAccServiceNetworkingPeeredDNSDomain_basic|TestAccDialogflowEntityType_update|TestAccDialogflowAgent_update|TestAccProject_deleteDefaultNetwork|TestAccFirebaserulesRelease_BasicRelease|TestAccResourceGoogleProjectDefaultServiceAccountsDeprivilege|TestAccResourceGoogleProjectDefaultServiceAccountsDeleteRevertIgnoreFailure|TestAccResourceGoogleProjectDefaultServiceAccountsDelete|TestAccResourceGoogleProjectDefaultServiceAccountsDisable|TestAccDialogflowIntent_update|TestAccDialogflowIntent_basic|TestAccResourceGoogleProjectDefaultServiceAccountsBasic|TestAccDialogflowFulfillment_update|TestAccComputeProjectDefaultNetworkTier_basic|TestAccComputeProjectMetadata_modify_2|TestAccComputeProjectMetadata_basic|TestAccComputeProjectMetadata_modify_1|TestAccComputeProjectDefaultNetworkTier_modify|TestAccComputeInstanceFromMachineImage_diffProject|TestAccComputeForwardingRule_update|TestAccBinaryAuthorizationPolicy_update|TestAccBinaryAuthorizationPolicy_separateProject|TestAccBinaryAuthorizationPolicy_full|TestAccBinaryAuthorizationPolicy_basic|TestAccAssuredWorkloadsWorkload_FullHandWritten|TestAccAssuredWorkloadsWorkload_BasicHandWritten|TestAccAppEngineFlexibleAppVersion_appEngineFlexibleAppVersionExample|TestAccAppEngineStandardAppVersion_update|TestAccApigeeEnvKeystore_apigeeEnvironmentKeystoreTestExample|TestAccApigeeSyncAuthorization_update|TestAccApigeeAddonsConfig_apigeeAddonsTestExample|TestAccApigeeSyncAuthorization_apigeeSyncAuthorizationBasicTestExample|TestAccApigeeEnvReferences_apigeeEnvironmentReferenceTestExample|TestAccAppEngineFlexibleAppVersion_update|TestAccIapAppEngineServiceIamMemberGenerated_withCondition|TestAccIapAppEngineServiceIamBindingGenerated_withCondition|TestAccIapAppEngineServiceIamPolicyGenerated_withCondition|TestAccIapAppEngineServiceIamPolicyGenerated|TestAccIapAppEngineServiceIamBindingGenerated|TestAccIapAppEngineServiceIamMemberGenerated|TestAccLoggingProjectCmekSettings_basic|TestAccDataSourceDnsManagedZone_basic|TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic|TestAccApigeeEnvironmentIamPolicyGenerated|TestAccApigeeEnvironmentIamMemberGenerated|TestAccOsConfigOsPolicyAssignment_basicOsPolicyAssignment|TestAccApigeeEnvironmentIamBindingGenerated|TestAccLoggingBucketConfig_CreateBuckets_withCustomId|TestAccWorkstationsWorkstationConfig_workstationConfigEncryptionKeyExample |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
@ScottSuarez if you have time can you take over the review fully? I'll be OOO until Tuesday otherwise. |
I'm actually oof next week as well. Monday - Wednesday |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 7 files changed, 3796 insertions(+), 184 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 74 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccComputeSharedVpc_basic|TestAccIAM2AccessBoundaryPolicy|TestAccKmsKeyRing_basic|TestAccComputeResourceUsageExportBucket|TestAccKmsCryptoKeyIamBinding|TestAccKmsCryptoKeyIamBinding_withCondition|TestAccLoggingBucketConfig_CreateBuckets_withCustomId|TestAccLoggingBucketConfigProject_cmekSettings|TestAccOsConfigOsPolicyAssignment_basicOsPolicyAssignment|TestAccApigeeEnvironmentIamPolicyGenerated|TestAccApigeeEnvironmentIamMemberGenerated|TestAccApigeeEnvironmentIamBindingGenerated|TestAccIAM2DenyPolicy_iamDenyPolicyBasicExample|TestAccKmsKeyRingIamPolicy_withCondition|TestAccKmsKeyRingIamPolicy|TestAccKmsKeyRingIamMember_withCondition|TestAccKmsKeyRingIamMember|TestAccKmsKeyRingIamBinding_withCondition|TestAccKmsKeyRingIamBinding|TestAccKmsCryptoKeyVersion_patch|TestAccKmsCryptoKeyVersion_skipInitialVersion|TestAccKmsCryptoKeyVersion_basic|TestAccKmsCryptoKey_importOnly|TestAccKmsCryptoKey_destroyDuration|TestAccKmsCryptoKey_template|TestAccKmsCryptoKey_basic|TestAccKmsCryptoKeyIamPolicy_withCondition|TestAccKmsCryptoKeyIamPolicy|TestAccDialogflowIntent_update|TestAccDialogflowIntent_basic|TestAccDialogflowFulfillment_update|TestAccDialogflowEntityType_update|TestAccKmsCryptoKeyIamMember_withCondition|TestAccKmsCryptoKeyIamMember|TestAccDialogflowAgent_update|TestAccDatabaseMigrationServiceConnectionProfile_databaseMigrationServiceConnectionProfileAlloydbExample|TestAccDatabaseMigrationServiceConnectionProfile_databaseMigrationServiceConnectionProfilePostgreExample|TestAccDatabaseMigrationServiceConnectionProfile_databaseMigrationServiceConnectionProfileCloudsqlExample|TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic|TestAccLoggingProjectCmekSettings_basic|TestAccComputeProjectMetadata_modify_1|TestAccComputeProjectMetadata_basic|TestAccComputeProjectMetadata_modify_2|TestAccComputeProjectDefaultNetworkTier_modify|TestAccComputeProjectDefaultNetworkTier_basic|TestAccComputeInstanceFromMachineImage_diffProject|TestAccAppEngineFlexibleAppVersion_update|TestAccAppEngineFlexibleAppVersion_appEngineFlexibleAppVersionExample|TestAccAppEngineStandardAppVersion_update|TestAccAssuredWorkloadsWorkload_FullHandWritten|TestAccApigeeAddonsConfig_apigeeAddonsTestExample|TestAccApigeeSyncAuthorization_update|TestAccApigeeSyncAuthorization_apigeeSyncAuthorizationBasicTestExample|TestAccBinaryAuthorizationPolicy_update|TestAccApigeeEnvReferences_apigeeEnvironmentReferenceTestExample|TestAccApigeeEnvKeystore_apigeeEnvironmentKeystoreTestExample|TestAccAssuredWorkloadsWorkload_BasicHandWritten|TestAccBinaryAuthorizationPolicy_separateProject|TestAccBinaryAuthorizationPolicy_full|TestAccBinaryAuthorizationPolicy_basic|TestAccIapAppEngineServiceIamMemberGenerated_withCondition|TestAccIapAppEngineServiceIamBindingGenerated_withCondition|TestAccIapAppEngineServiceIamBindingGenerated|TestAccIapAppEngineServiceIamPolicyGenerated|TestAccIapAppEngineServiceIamMemberGenerated|TestAccResourceGoogleProjectDefaultServiceAccountsDeleteRevertIgnoreFailure|TestAccResourceGoogleProjectDefaultServiceAccountsBasic|TestAccResourceGoogleProjectDefaultServiceAccountsDeprivilege|TestAccIapAppEngineServiceIamPolicyGenerated_withCondition|TestAccResourceGoogleProjectDefaultServiceAccountsDisable|TestAccResourceGoogleProjectDefaultServiceAccountsDelete|TestAccDataSourceDnsManagedZone_basic|TestAccServiceNetworkingPeeredDNSDomain_basic|TestAccProject_deleteDefaultNetwork |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
removing myself to have one reviewer.. Let me know if you still need my feedback... Looks like the integration tests are failing |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 12 files changed, 4132 insertions(+), 358 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: Please add acceptance tests which include these fields. |
Tests analyticsTotal tests: Errors occurred during REPLAYING mode. Please fix them to complete your PR |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 12 files changed, 4132 insertions(+), 358 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: Please add acceptance tests which include these fields. |
Tests analyticsTotal tests: Errors occurred during REPLAYING mode. Please fix them to complete your PR |
did this build and work locally? |
Tests analyticsTotal tests: Action takenFound 7 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccDataSourceGoogleFirebaseAndroidAppConfig|TestAccFirebaserulesRelease_BasicRelease|TestAccDatabaseMigrationServiceConnectionProfile_databaseMigrationServiceConnectionProfileAlloydbExample|TestAccDatabaseMigrationServiceConnectionProfile_databaseMigrationServiceConnectionProfilePostgreExample|TestAccDatabaseMigrationServiceConnectionProfile_databaseMigrationServiceConnectionProfileCloudsqlExample|TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample|TestAccApigeeKeystoresAliasesPkcs12_ApigeeKeystoresAliasesPkcs12Example |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 12 files changed, 4134 insertions(+), 358 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: Please add acceptance tests which include these fields. |
Tests analyticsTotal tests: Action takenFound 4 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccDatabaseMigrationServiceConnectionProfile_databaseMigrationServiceConnectionProfileAlloydbExample|TestAccApigeeKeystoresAliasesPkcs12_ApigeeKeystoresAliasesPkcs12Example|TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample|TestAccDataSourceGoogleFirebaseAndroidAppConfig |
Tests passed during RECORDING mode: All tests passed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mostly formatting and test changes
mmv1/templates/terraform/examples/database_migration_service_connection_profile_alloydb.tf.erb
Outdated
Show resolved
Hide resolved
mmv1/templates/terraform/examples/database_migration_service_connection_profile_cloudsql.tf.erb
Outdated
Show resolved
Hide resolved
mmv1/templates/terraform/examples/database_migration_service_connection_profile_postgre.tf.erb
Outdated
Show resolved
Hide resolved
mmv1/third_party/terraform/tests/resource_database_migration_service_connection_profile_test.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the postgres example file needs the new "s", also the changelog entry needs to be just the resource name with backticks @NickElliot
Both changes made! |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 12 files changed, 4137 insertions(+), 358 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: Please add acceptance tests which include these fields. |
Tests analyticsTotal tests: Action takenFound 58 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccDatabaseMigrationServiceConnectionProfile_update|TestAccDatabaseMigrationServiceConnectionProfile_databaseMigrationServiceConnectionProfileAlloydbExample|TestAccDatabaseMigrationServiceConnectionProfile_databaseMigrationServiceConnectionProfilePostgresExample|TestAccDatabaseMigrationServiceConnectionProfile_databaseMigrationServiceConnectionProfileCloudsqlExample|TestAccFirebaserulesRelease_BasicRelease|TestAccComputeRouterInterface_basic|TestAccDNSRecordSet_routingPolicy|TestAccComputeManagedSslCertificate_managedSslCertificateBasicExample|TestAccComputeNetworkFirewallPolicyRule_GlobalHandWritten|TestAccComputeVpnTunnel_defaultTrafficSelectors|TestAccComputeGlobalForwardingRule_privateServiceConnectGoogleApisExample|TestAccComputeVpnTunnel_regionFromGateway|TestAccComputeVpnTunnel_vpnTunnelBetaExample|TestAccComputeVpnTunnel_vpnTunnelBasicExample|TestAccComputeVpnGateway_targetVpnGatewayBasicExample|TestAccComputeRegionNetworkEndpointGroup_regionNetworkEndpointGroupPscServiceAttachmentExample|TestAccComputeRoute_routeIlbVipExample|TestAccComputeRoute_routeIlbExample|TestAccComputeGlobalForwardingRule_globalForwardingRuleHttpExample|TestAccComputeGlobalForwardingRule_externalHttpLbMigBackendCustomHeaderExample|TestAccComputeGlobalForwardingRule_externalTcpProxyLbMigBackendExample|TestAccComputeServiceAttachment_serviceAttachmentBasicExampleUpdate|TestAccComputeServiceAttachment_serviceAttachmentExplicitProjectsExample|TestAccComputeServiceAttachment_serviceAttachmentBasicExample|TestAccComputeForwardingRule_serviceDirectoryRegistrations|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccComputeForwardingRule_networkTier|TestAccComputeForwardingRule_ip|TestAccComputeGlobalForwardingRule_labels|TestAccComputeForwardingRule_forwardingRuleVpcPscExample|TestAccComputeGlobalForwardingRule_ipv6|TestAccComputeForwardingRule_forwardingRuleRegionalHttpXlbExample|TestAccComputeForwardingRule_forwardingRuleHttpLbExample|TestAccComputeGlobalForwardingRule_updateTarget|TestAccComputeGlobalForwardingRule_globalForwardingRuleHybridExample|TestAccComputeGlobalForwardingRule_globalForwardingRuleExternalManagedExample|TestAccComputeBackendBucket_externalCdnLbWithBackendBucketExample|TestAccComputeForwardingRule_forwardingRuleInternallbExample|TestAccComputeGlobalForwardingRule_globalForwardingRuleInternalExample|TestAccComputeForwardingRule_forwardingRuleL3DefaultExample|TestAccComputeRegionNetworkFirewallPolicyRule_RegionalHandWritten|TestAccComputeFirewallPolicyRule_update|TestAccComputeForwardingRule_forwardingRuleBasicExample|TestAccComputeFirewallPolicyRule_multipleRules|TestAccComputeForwardingRule_forwardingRuleGlobalInternallbExample|TestAccCloudRunV2Service_cloudrunv2ServiceFullUpdate|TestAccComputeForwardingRule_forwardingRuleExternallbExample|TestAccComputeForwardingRule_internalTcpUdpLbWithMigBackendExample|TestAccAlloydbBackup_missingLocation|TestAccComputeForwardingRule_internalHttpLbWithMigBackendExample|TestAccComputePacketMirroring_computePacketMirroringFullExample|TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample|TestAccApigeeKeystoresAliasesPkcs12_ApigeeKeystoresAliasesPkcs12Example|TestAccAlloydbCluster_missingLocation|TestAccDataSourceGoogleFirebaseAndroidAppConfig|TestAccDataSourceGoogleGlobalForwardingRule|TestAccDataSourceAlloydbLocations_basic|TestAccDataSourceGoogleForwardingRule |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
Adds support for connection profiles within the Database Migration Service. Currently there would be a minimal usage case as the service relies on a subsequent transfer job being established which we are not supporting adding MMV1 support for more temporary job resources at this time, but lays the groundwork for future support as there is some demand.
If this PR is for Terraform, I acknowledge that I have:
make test
andmake lint
in the generated providers to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)