Add support for ResourceRefs attaching to arbitrary types and referencing multiple targets#8127
Conversation
|
The generate diffs appears to be failing because there is no diffs. |
|
/gcbrun |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR hasn't generated any diffs, but I'll let you know if a future commit does. |
Tests analyticsTotal tests: Action takenFound 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccComputeNetworkEndpoints_networkEndpointsBasic|TestAccComputeFirewallPolicyRule_multipleRules|TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample |
|
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 hasn't generated any diffs, but I'll let you know if a future commit does. |
Tests analyticsTotal tests: Action takenFound 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccComputeNetworkEndpoints_networkEndpointsBasic|TestAccComputeFirewallPolicyRule_multipleRules |
|
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 Beta: Diff ( 1 file changed, 1 insertion(+), 5 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 47 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccContainerAwsCluster_BasicHandWritten|TestAccComputeRegionInstanceTemplate_with18TbScratchDisk|TestAccComputeRegionInstanceTemplate_withScratchDisk|TestAccComputeNetworkEdgeSecurityService_update|TestAccComputeRegionInstanceTemplate_guestAccelerator|TestAccComputeRegionInstanceTemplate_primaryAliasIpRange|TestAccComputeInstanceTemplate_guestAccelerator|TestAccComputeInstanceTemplate_secondaryAliasIpRange|TestAccComputeRegionAutoscaler_regionAutoscalerBasicExample|TestAccComputeInstanceTemplate_primaryAliasIpRange|TestAccComputeInstanceFromTemplate_self_link_unique|TestAccComputeInstanceFromTemplate_basic|TestAccComputeInstanceTemplate_metadata_startup_script|TestAccComputeInstanceTemplate_subnet_auto|TestAccComputeInstanceTemplate_withScratchDisk|TestAccComputeInstanceTemplate_regionDisks|TestAccComputeRegionInstanceTemplate_metadata_startup_script|TestAccComputeInstanceTemplate_disks|TestAccComputeInstanceTemplate_NetworkAttachment|TestAccComputeRegionInstanceTemplate_subnet_custom|TestAccComputeRegionInstanceTemplate_subnet_auto|TestAccComputeInstanceTemplate_guestAcceleratorSkip|TestAccComputeInstanceFromTemplate_012_removableFields|TestAccComputeRegionInstanceTemplate_minCpuPlatform|TestAccComputeInstanceFromRegionTemplate_basic|TestAccComputeInstanceTemplate_minCpuPlatform|TestAccComputeInstanceFromTemplate_overrideScratchDisk|TestAccClouddeployDeliveryPipeline_DeliveryPipeline|TestAccClouddeployDeliveryPipeline_CanaryrunDeliveryPipeline|TestAccClouddeployDeliveryPipeline_CanaryServiceNetworkingDeliveryPipeline|TestAccComputeInstanceTemplate_subnet_custom|TestAccComputeRegionInstanceTemplate_guestAcceleratorSkip|TestAccCloudbuildv2Connection_GlePrivUpdateConnection|TestAccComputeInstanceFromTemplate_overrideAttachedDisk|TestAccCloudbuildv2Connection_GleOldConnection|TestAccCloudbuildv2Connection_GleConnection|TestAccComputeInstanceFromTemplate_overrideBootDisk|TestAccContainerAwsNodePool_BetaBasicHandWritten|TestAccContainerAwsCluster_BetaBasicHandWritten|TestAccComputeFirewallPolicyRule_multipleRules|TestAccClouddeployDeliveryPipeline_CanaryDeliveryPipeline|TestAccCloudbuildv2Connection_GlePrivConnection|TestAccCloudbuildv2Connection_GitlabConnection|TestAccComputeRegionInstanceTemplate_secondaryAliasIpRange|TestAccContainerAwsNodePool_BasicHandWritten|TestAccComputeInstanceTemplate_with18TbScratchDisk|TestAccComputeRegionInstanceTemplate_regionDisks |
rileykarson
left a comment
rileykarson
left a comment
There was a problem hiding this comment.
Rake and yaml lint look unhappy!
… for a resourceproduct field that is accessible through a new supplied values property. moved existing resourceref parsing/validation to a custom_expand
…rce refs without existing custom expands or url_param_only
…compute that uses the baseline resourceref logic
|
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 Beta: Diff ( 1 file changed, 1 insertion(+), 5 deletions(-)) |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR hasn't generated any diffs, but I'll let you know if a future commit does. |
Tests analyticsTotal tests: Action takenFound 47 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccContainerAwsNodePool_BetaBasicHandWritten|TestAccContainerAwsNodePool_BasicHandWritten|TestAccContainerAwsCluster_BetaBasicHandWritten|TestAccContainerAwsCluster_BasicHandWritten|TestAccComputeRegionInstanceTemplate_subnet_auto|TestAccComputeRegionInstanceTemplate_regionDisks|TestAccComputeRegionAutoscaler_regionAutoscalerBasicExample|TestAccComputeRegionInstanceTemplate_with18TbScratchDisk|TestAccComputeInstanceTemplate_withScratchDisk|TestAccComputeInstanceTemplate_with18TbScratchDisk|TestAccComputeRegionInstanceTemplate_withScratchDisk|TestAccComputeInstanceTemplate_NetworkAttachment|TestAccComputeInstanceTemplate_minCpuPlatform|TestAccComputeInstanceFromTemplate_012_removableFields|TestAccComputeRegionInstanceTemplate_minCpuPlatform|TestAccComputeInstanceFromTemplate_overrideScratchDisk|TestAccComputeRegionInstanceTemplate_guestAcceleratorSkip|TestAccComputeInstanceTemplate_guestAccelerator|TestAccComputeInstanceFromTemplate_overrideAttachedDisk|TestAccComputeInstanceTemplate_primaryAliasIpRange|TestAccComputeInstanceTemplate_secondaryAliasIpRange|TestAccComputeInstanceTemplate_metadata_startup_script|TestAccComputeRegionInstanceTemplate_guestAccelerator|TestAccComputeRegionInstanceTemplate_secondaryAliasIpRange|TestAccComputeInstanceTemplate_subnet_custom|TestAccComputeInstanceTemplate_subnet_auto|TestAccComputeRegionInstanceTemplate_primaryAliasIpRange|TestAccComputeInstanceTemplate_regionDisks|TestAccComputeRegionInstanceTemplate_metadata_startup_script|TestAccComputeInstanceTemplate_guestAcceleratorSkip|TestAccComputeInstanceTemplate_disks|TestAccComputeRegionInstanceTemplate_subnet_custom|TestAccComputeInstanceFromTemplate_overrideBootDisk|TestAccComputeFirewallPolicyRule_multipleRules|TestAccCloudbuildv2Connection_GlePrivUpdateConnection|TestAccComputeInstanceFromRegionTemplate_basic|TestAccCloudbuildv2Connection_GlePrivConnection|TestAccCloudbuildv2Connection_GleOldConnection|TestAccCloudbuildv2Connection_GleConnection|TestAccComputeInstanceFromTemplate_self_link_unique|TestAccCloudbuildv2Connection_GitlabConnection|TestAccComputeInstanceFromTemplate_basic|TestAccBigtableAppProfile_bigtableAppProfileAnyclusterExample|TestAccClouddeployDeliveryPipeline_DeliveryPipeline|TestAccClouddeployDeliveryPipeline_CanaryrunDeliveryPipeline|TestAccClouddeployDeliveryPipeline_CanaryServiceNetworkingDeliveryPipeline|TestAccClouddeployDeliveryPipeline_CanaryDeliveryPipeline |
|
/gcbrun |
|
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. TF Conversion: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccContainerAwsNodePool_BetaBasicHandWritten|TestAccComputeFirewallPolicyRule_multipleRules |
|
|
…cing multiple targets (GoogleCloudPlatform#8127) * updated type.rb to remove validation on resource refs. moved existing resourceref parsing/validation to a custom_expand * added resource_ref_with_validation custom expand to all compute resource refs without existing custom expands or url_param_only * adding expand to the single field in the entirety of MMV1 outside of compute that uses the baseline resourceref logic * removing resourceref as string
…cing multiple targets (GoogleCloudPlatform#8127) * updated type.rb to remove validation on resource refs. moved existing resourceref parsing/validation to a custom_expand * added resource_ref_with_validation custom expand to all compute resource refs without existing custom expands or url_param_only * adding expand to the single field in the entirety of MMV1 outside of compute that uses the baseline resourceref logic * removing resourceref as string
…cing multiple targets (GoogleCloudPlatform#8127) * updated type.rb to remove validation on resource refs. moved existing resourceref parsing/validation to a custom_expand * added resource_ref_with_validation custom expand to all compute resource refs without existing custom expands or url_param_only * adding expand to the single field in the entirety of MMV1 outside of compute that uses the baseline resourceref logic * removing resourceref as string
…cing multiple targets (GoogleCloudPlatform#8127) * updated type.rb to remove validation on resource refs. moved existing resourceref parsing/validation to a custom_expand * added resource_ref_with_validation custom expand to all compute resource refs without existing custom expands or url_param_only * adding expand to the single field in the entirety of MMV1 outside of compute that uses the baseline resourceref logic * removing resourceref as string
3 participants
Fundamentally this PR is removing validation -- the Terraform schema is unable to access the data of resources other than itself for runtime validation, and generator does not access files outside of the folder from which it is currently generating. In current Terraform-only support for Magic Modules, the resource ref ultimately does nothing other than apply some logic on self-links to create a bit of validation for compute resources. Every resource-ref outside of compute engine is a url_parameter, or uses separate custom expands to process it as a self link generated off of a name/process the input value as a string directly.
This update moves the current compute-first implementation to a separate custom_expand that needs to be specifically declared, and updates resource_ref to use the "process as a string" logic. If the resource_ref is to a resource within the same product (and no resource_product is declared in case of a shared resource name), building MMV1 will still attempt to validate the import field exists in the target resource.
Additionally, the type.rb file has been updated to have an output property that directly contains the suppled "resource", "resourceproduct" and "imports" strings from the yaml attributes. Nothing uses them at this time, but it makes those pieces more available to potential custom_* functions in the future.
If all goes well this PR should generate almost no diffs.
If this PR is for Terraform, I acknowledge that I have:
make testandmake lintin the generated providers to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)