Add a guide on authoring tpgtools new resources #5452

slevenick · 2021-11-13T00:33:36Z

Let me know if you have feedback!

Fixes: hashicorp/terraform-provider-google#10544

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).
Generated Terraform, and ran make test and make lint to ensure it passes unit and linter tests.
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).
Ran relevant acceptance tests (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)

modular-magician · 2021-11-13T00:46:30Z

Hi! I'm the modular magician. Your PR hasn't generated any diffs, but I'll let you know if a future commit does.

rileykarson · 2021-11-13T00:48:45Z

tpgtools/README.md

+
+### Adding Documentation
+
+Documentation will be automatically generated based on the resource description in the DCL and examples will be generated from the samples for this resource.


Provided you've added samples ... otherwise, you'll need to write the documentation for the resource manually.

I've seen a few resources not get docs, I think samples are the thing that opts them in? Been a while since I wrote a resource w/ DCL+tpgtools.

rileykarson · 2021-11-13T00:49:34Z

tpgtools/README.md

@@ -176,3 +176,33 @@ doc_hide:
 test_hide:
  - basic_workload.yaml
 ```
+
+## New Resource Guide (tpgtools only)


Suggested change

## New Resource Guide (tpgtools only)

## New Resource Guide

This part feels implied!

rileykarson · 2021-11-13T00:51:02Z

tpgtools/README.md

@@ -176,3 +176,33 @@ doc_hide:
 test_hide:
  - basic_workload.yaml
 ```
+


I would consider putting this higher up, between "Usage" and "Development"

rileykarson · 2021-11-13T00:53:23Z

tpgtools/README.md

+
+For the first pass try nothing. If behavior is needed to change the resource's behavior in Terraform from the default DCL resource, then you can add these at a later date. Generally no overrides will be needed for a new resource, but they will be needed for resources that exist in mmv1 that are being converted to use tpgtools.
+
+### Adding Samples


I would consider putting the samples section here, instead of referencing it elsewhere in the README- that will avoid a context switch! Also, we may want to touch those up- I think we need to copy samples manually rn, and the go get call can be replaced by make upgrade-dcl, for two low-hanging fruit.

@ndmckinley also recently revamped how we pull in substitutions, I think less work is needed on those than is written here.

Yeah, this should Just Work for new versions of the DCL now. There's some no-op cleanup to do with deduplicating variables as it stands, but tpgtools handles it fine.

rileykarson · 2021-11-13T01:00:50Z

tpgtools/README.md

+
+This guide is written to document the process for adding a resource to the Google Terraform Provider (TPG) after it has been added to the [DCL](https://github.com/GoogleCloudPlatform/declarative-resource-client-library).
+
+### Adding Resource Overrides


For this + the header below: I'd recommend assuming that the override file will always be blank, explain briefly what an override is, and recommend that folks reach out to an experienced team member if they believe an override will be needed for their resource(s). To cover brownfield resources, I'd leave them out of this flow entirely and add them as an appendix afterwards- "those were the guidelines for new resources, here's how a brownfield resource is different"

Motivation(s):

This keeps things simple for folks that are making their first contribution. It isn't true that you never need overrides yet, but we need to make it true.

Deferring to an experienced engineer will mean they can help solve the root cause that needed an override in the first place.

This is in contrast to the MMv1 field guide, where I explain some MMv1 overrides- using them is pretty normal, even for simple contributions.

We expect most brownfield conversions to be done for our team (or another experienced member of a sister team) so having the information out of a new dev's critical path means there's less information overload

rileykarson · 2021-11-13T01:05:14Z

tpgtools/README.md

+
+See [guide above](#samples) for adding samples.
+
+### Adding Additional Information to Handwritten/MMv1 Generated Files


nit: "Additional Information..." feels optional. "Registering the Resource", maybe?

rileykarson · 2021-11-13T01:09:11Z

tpgtools/README.md

+
+Additionally you may need to add a few line to provider.go for the endpoint if this resource is in a new product that doesn't have any other resources currently in the provider. Find the correct lines in the diff on `provider_dcl_endpoints.go` within the provider once it has been generated with the new resource.
+
+### Adding Documentation


We may want a section about writing handwritten tests as well! Copying the DCL samples may not result in the level of testing we want, or we may need customised tests for some resources.

modular-magician · 2021-11-13T02:38:57Z

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic|TestAccAssuredWorkloadsWorkload_BasicHandWritten|TestAccAssuredWorkloadsWorkload_FullHandWritten|TestAccCloudFunctionsFunction_vpcConnector|TestAccComputeGlobalForwardingRule_privateServiceConnectGoogleApisExample|TestAccComputeForwardingRule_update|TestAccComputeInstanceFromMachineImage_basic|TestAccComputeInstanceFromMachineImage_overrideMetadataDotStartupScript|TestAccComputeInstanceFromMachineImage_diffProject|TestAccComputeRegionHealthCheck_tcp_update|TestAccComputeRegionHealthCheck_typeTransition|TestAccComputeRegionHealthCheck_logConfigDisabled|TestAccComputeRegionNetworkEndpointGroup_regionNetworkEndpointGroupAppengineExample|TestAccComputeServiceAttachment_serviceAttachmentBasicExampleUpdate|TestAccContainerCluster_withILBSubsetting|TestAccContainerCluster_regionalWithNodePool|TestAccContainerCluster_withNodePoolBasic|TestAccContainerCluster_withNodePoolResize|TestAccContainerCluster_withNodePoolUpdateVersion|TestAccContainerCluster_withNodePoolAutoscaling|TestAccContainerCluster_withNodePoolMultiple|TestAccContainerCluster_withNodePoolNodeConfig|TestAccContainerCluster_withEnableKubernetesAlpha|TestAccContainerNodePool_basic|TestAccContainerNodePool_basicWithClusterId|TestAccContainerNodePool_nodeLocations|TestAccContainerNodePool_maxPodsPerNode|TestAccContainerNodePool_withNodeConfig|TestAccContainerNodePool_withWorkloadIdentityConfig|TestAccContainerNodePool_withSandboxConfig|TestAccContainerNodePool_withKubeletConfig|TestAccContainerNodePool_withLinuxNodeConfig|TestAccContainerNodePool_withUpgradeSettings|TestAccContainerNodePool_withInvalidUpgradeSettings|TestAccContainerNodePool_withManagement|TestAccContainerNodePool_withGPU|TestAccContainerNodePool_withNodeConfigScopeAlias|TestAccContainerNodePool_regionalAutoscaling|TestAccContainerNodePool_resize|TestAccContainerNodePool_autoscaling|TestAccContainerNodePool_version|TestAccContainerNodePool_regionalClusters|TestAccContainerNodePool_EmptyGuestAccelerator|TestAccContainerNodePool_012_ConfigModeAttr|TestAccContainerNodePool_ephemeralStorageConfig|TestAccDataprocWorkflowTemplate_basic|TestAccEventarcTrigger_BasicHandWritten|TestAccServiceNetworkingPeeredDNSDomain_basic|TestAccMonitoringMonitoredProject_BasicMonitoredProject|TestAccNetworkServicesEdgeCacheOrigin_networkServicesEdgeCacheOriginBasicExample|TestAccNetworkServicesEdgeCacheOrigin_networkServicesEdgeCacheOriginAdvancedExample|TestAccNetworkServicesEdgeCacheService_networkServicesEdgeCacheServiceAdvancedExample|TestAccOrgPolicyPolicy_EnforcePolicy|TestAccOrgPolicyPolicy_FolderPolicy|TestAccOrgPolicyPolicy_ProjectPolicy|TestAccPrivatecaCertificateTemplate_BasicCertificateTemplate|TestAccSqlDatabaseInstance_withPrivateNetwork|TestAccSqlUser_postgresIAM You can view the result here: https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=218107

modular-magician · 2021-11-13T03:40:50Z

Tests failed during RECORDING mode: TestAccComputeInstanceFromMachineImage_overrideMetadataDotStartupScript|TestAccComputeInstanceFromMachineImage_basic|TestAccComputeInstanceFromMachineImage_diffProject|TestAccCloudFunctionsFunction_vpcConnector|TestAccComputeRegionNetworkEndpointGroup_regionNetworkEndpointGroupAppengineExample|TestAccContainerNodePool_nodeLocations|TestAccEventarcTrigger_BasicHandWritten|TestAccContainerNodePool_withInvalidUpgradeSettings|TestAccSqlUser_postgresIAM Please fix these to complete your PR

modular-magician · 2021-11-19T20:00:32Z

Hi! I'm the modular magician. Your PR hasn't generated any diffs, but I'll let you know if a future commit does.

rileykarson · 2021-11-19T20:15:40Z

tpgtools/README.md

+Start by copying the relevant samples from the DCL for your new resource. These will be added to
+the [tpgtools/api folder](https://github.com/GoogleCloudPlatform/magic-modules/tree/master/tpgtools/api) under the relevant product.
+These samples can be found under the samples/ folder within the DCL for the resource being added. For example, assured
+workloads can be found [here](https://github.com/GoogleCloudPlatform/declarative-resource-client-library/tree/main/services/google/assuredworkloads/samples).

 You may need to re-serialize `serialization.go` if you are adding newer resources.
 To do this


Nit: we can use make upgrade-dcl below rather than go get directly.

rileykarson · 2021-11-19T20:16:02Z

tpgtools/README.md

+Start by copying the relevant samples from the DCL for your new resource. These will be added to
+the [tpgtools/api folder](https://github.com/GoogleCloudPlatform/magic-modules/tree/master/tpgtools/api) under the relevant product.
+These samples can be found under the samples/ folder within the DCL for the resource being added. For example, assured
+workloads can be found [here](https://github.com/GoogleCloudPlatform/declarative-resource-client-library/tree/main/services/google/assuredworkloads/samples).

 You may need to re-serialize `serialization.go` if you are adding newer resources.


nit: Given this is a new resource guide, we can omit this condition.

rileykarson · 2021-11-19T20:18:34Z

tpgtools/README.md

+
+#### Adding to provider.go
+
+Add the resource definition within the `ResourceMapWithErrors` map. There is currently a block of tpgtools resources, so adding to that block is preferred. If this resource is available only at a specific version (beta, private) then add version tags around the resource definition.


nit: Consider using a code block to show a sample diff!

modular-magician · 2021-11-19T21:03:48Z

Hi! I'm the modular magician. Your PR hasn't generated any diffs, but I'll let you know if a future commit does.

…5452) * Add a guide on authoring tpgtools new resources * Wrong headers * Simplify samples, reorder per PR feedback * PR feedback

Add a guide on authoring tpgtools new resources

d89e50d

google-cla bot added the cla: yes label Nov 13, 2021

Wrong headers

49e98f9

slevenick requested a review from rileykarson November 13, 2021 00:34

rileykarson reviewed Nov 13, 2021

View reviewed changes

Simplify samples, reorder per PR feedback

c15f18a

rileykarson approved these changes Nov 19, 2021

View reviewed changes

PR feedback

9095a61

slevenick merged commit c6fc3a4 into GoogleCloudPlatform:master Nov 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a guide on authoring tpgtools new resources #5452

Add a guide on authoring tpgtools new resources #5452

slevenick commented Nov 13, 2021 •

edited

Loading

modular-magician commented Nov 13, 2021

rileykarson Nov 13, 2021

rileykarson Nov 13, 2021

rileykarson Nov 13, 2021

rileykarson Nov 13, 2021

rileykarson Nov 13, 2021

nat-henderson Nov 16, 2021

rileykarson Nov 13, 2021

rileykarson Nov 13, 2021

rileykarson Nov 13, 2021

modular-magician commented Nov 13, 2021

modular-magician commented Nov 13, 2021

modular-magician commented Nov 19, 2021

rileykarson Nov 19, 2021

rileykarson Nov 19, 2021

rileykarson Nov 19, 2021

modular-magician commented Nov 19, 2021


		### Adding Documentation

		Documentation will be automatically generated based on the resource description in the DCL and examples will be generated from the samples for this resource.


		For the first pass try nothing. If behavior is needed to change the resource's behavior in Terraform from the default DCL resource, then you can add these at a later date. Generally no overrides will be needed for a new resource, but they will be needed for resources that exist in mmv1 that are being converted to use tpgtools.

		### Adding Samples


		This guide is written to document the process for adding a resource to the Google Terraform Provider (TPG) after it has been added to the [DCL](https://github.com/GoogleCloudPlatform/declarative-resource-client-library).

		### Adding Resource Overrides


		See [guide above](#samples) for adding samples.

		### Adding Additional Information to Handwritten/MMv1 Generated Files


		Additionally you may need to add a few line to provider.go for the endpoint if this resource is in a new product that doesn't have any other resources currently in the provider. Find the correct lines in the diff on `provider_dcl_endpoints.go` within the provider once it has been generated with the new resource.

		### Adding Documentation


		#### Adding to provider.go

		Add the resource definition within the `ResourceMapWithErrors` map. There is currently a block of tpgtools resources, so adding to that block is preferred. If this resource is available only at a specific version (beta, private) then add version tags around the resource definition.

Add a guide on authoring tpgtools new resources #5452

Add a guide on authoring tpgtools new resources #5452

Conversation

slevenick commented Nov 13, 2021 • edited Loading

modular-magician commented Nov 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

modular-magician commented Nov 13, 2021

modular-magician commented Nov 13, 2021

modular-magician commented Nov 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

modular-magician commented Nov 19, 2021

slevenick commented Nov 13, 2021 •

edited

Loading