Add bigtable instance IAM resources #3939

sandeepsukhani · 2019-06-27T13:40:59Z

Currently, there is no way in Terraform to manage IAM roles for Bigtable Instances.
This PR adds support for that.

Added Tests
Added Docs

Added IAM resources for bigtable instances (`google_bigtable_instance_iam_policy`,`google_bigtable_instance_iam_binding`, `google_bigtable_instance_iam_member`)

rileykarson

Great work! Just a few small comments so far.

Do you mind splitting the vendoring updates to a separate PR (you can follow this guide if you can't get go mod vendor to work), and updating the bottom of https://github.com/terraform-providers/terraform-provider-google/blob/master/website/docs/provider_reference.html.markdown with the custom endpoint reference as well?

rileykarson · 2019-06-27T19:55:00Z

google/config.go

-	bigtableClientFactory *BigtableClientFactory
+	bigtableClientFactory   *BigtableClientFactory
+	BigtableAdminBasePath   string
+	clientBigtableInstances *bigtableadmin.ProjectsInstancesService


Suggested change

clientBigtableInstances *bigtableadmin.ProjectsInstancesService

// Unlike other clients, the Bigtable Admin client doesn't use a single

// service. Instead, there are several distinct services created off

// the base service object. To imitate most other handwritten clients,

// we expose those directly instead of providing the `Service` object

// as a factory.

clientBigtableProjectsInstances *bigtableadmin.ProjectsInstancesService

Let's call this clientBigtableProjectsInstances to match the service name. Also adding a comment to make it clear why bigtable is different than other clients.

rileykarson · 2019-06-27T19:55:45Z

google/config.go

@@ -427,6 +430,16 @@ func (c *Config) LoadAndValidate() error {
 		TokenSource: tokenSource,
 	}

+	bigtableAdminBasePath := removeBasePathVersion(c.BigtableAdminBasePath)


Do you mind adding a logging message similar to other clients? This is mostly for consistency, it can also help with debugging if the value isn't being set.

rileykarson · 2019-06-27T19:57:48Z

google/iam_bigtable_instance.go

+	out := &bigtableadmin.Policy{}
+	err := Convert(p, out)
+	if err != nil {
+		return nil, errwrap.Wrapf("Cannot convert a dataproc policy to a cloudresourcemanager policy: {{err}}", err)


Suggested change

return nil, errwrap.Wrapf("Cannot convert a dataproc policy to a cloudresourcemanager policy: {{err}}", err)

return nil, errwrap.Wrapf("Cannot convert a bigtable policy to a cloudresourcemanager policy: {{err}}", err)

rileykarson · 2019-06-27T19:58:10Z

google/iam_bigtable_instance.go

+	out := &cloudresourcemanager.Policy{}
+	err := Convert(p, out)
+	if err != nil {
+		return nil, errwrap.Wrapf("Cannot convert a cloudresourcemanager policy to a dataproc policy: {{err}}", err)


Suggested change

return nil, errwrap.Wrapf("Cannot convert a cloudresourcemanager policy to a dataproc policy: {{err}}", err)

return nil, errwrap.Wrapf("Cannot convert a cloudresourcemanager policy to a bigtable policy: {{err}}", err)

rileykarson · 2019-06-27T19:59:16Z

google/provider_handwritten_endpoint.go

@@ -227,6 +227,17 @@ var StorageTransferCustomEndpointEntry = &schema.Schema{
 	}, StorageTransferDefaultBasePath),
 }

+var BigtableAdminDefaultBasePath = "https://bigtableadmin.googleapis.com/v2/"
+var BigtableAdminCustomEndpointEntryKey = "bigtableadmin_custom_endpoint"


Suggested change

var BigtableAdminCustomEndpointEntryKey = "bigtableadmin_custom_endpoint"

var BigtableAdminCustomEndpointEntryKey = "bigtable_custom_endpoint"

Internally, let's keep "Bigtable Admin", but externally we publish resources as "Bigtable" so for user-facing values let's use that naming.

rileykarson · 2019-06-27T19:59:24Z

google/provider_handwritten_endpoint.go

+	Optional:     true,
+	ValidateFunc: validateCustomEndpoint,
+	DefaultFunc: schema.MultiEnvDefaultFunc([]string{
+		"GOOGLE_BIGTABLE_ADMIN_CUSTOM_ENDPOINT",


Suggested change

"GOOGLE_BIGTABLE_ADMIN_CUSTOM_ENDPOINT",

"GOOGLE_BIGTABLE_CUSTOM_ENDPOINT",

sandeepsukhani · 2019-06-28T07:18:18Z

@rileykarson Thanks for reviewing this!
I have done all the suggested changes.
You can find PR for adding dependencies here #3945

rileykarson

Merged the vendoring PR, do you mind rebasing / merging master?

rileykarson · 2019-07-01T19:51:59Z

website/docs/provider_reference.html.markdown

@@ -217,6 +217,7 @@ be used for configuration are below:

 * `access_context_manager_custom_endpoint` (`GOOGLE_ACCESS_CONTEXT_MANAGER_CUSTOM_ENDPOINT`) - `https://accesscontextmanager.googleapis.com/v1/`
 * `app_engine_custom_endpoint` (`GOOGLE_APP_ENGINE_CUSTOM_ENDPOINT`) - `https://appengine.googleapis.com/v1/`
+* `bigtable_custom_endpoint` (`GOOGLE_BIGTABLE_CUSTOM_ENDPOINT`) - `https://bigtableadmin.googleapis.com/v2/`


nit: I think this goes after bigquery alphabetically?

Added relevant docs and tests

sandeepsukhani · 2019-07-02T06:43:48Z

@rileykarson Rebased and done with suggested changes. Thanks for the review!

rileykarson

Thanks for your great work @sandlis! I'll upstream this to Magic Modules so the changes are applied to google-beta too, and then merge this PR.

ghost · 2019-08-08T13:50:31Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

ghost added size/xxl dependencies documentation labels Jun 27, 2019

rileykarson self-requested a review June 27, 2019 17:51

rileykarson reviewed Jun 27, 2019

View reviewed changes

sandeepsukhani mentioned this pull request Jun 28, 2019

Add bigtableadmin api client package for adding bigtable instance iam support #3945

Merged

sandeepsukhani force-pushed the bigtable-instance-iam-support branch from 1b9523c to f954028 Compare June 28, 2019 07:15

ghost added size/xl and removed size/xxl labels Jun 28, 2019

rileykarson reviewed Jul 1, 2019

View reviewed changes

Add bigtable instance IAM resources

470472a

Added relevant docs and tests

sandeepsukhani force-pushed the bigtable-instance-iam-support branch from f954028 to 470472a Compare July 2, 2019 06:22

rileykarson approved these changes Jul 3, 2019

View reviewed changes

rileykarson mentioned this pull request Jul 3, 2019

Upstream Bigtable Instance IAM PR GoogleCloudPlatform/magic-modules#2015

Merged

rileykarson merged commit 7439085 into hashicorp:master Jul 8, 2019

emilymye added the changelog: new-resource label Jul 12, 2019

ghost locked and limited conversation to collaborators Aug 8, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add bigtable instance IAM resources #3939

Add bigtable instance IAM resources #3939

sandeepsukhani commented Jun 27, 2019 •

edited by emilymye

Loading

rileykarson left a comment

rileykarson Jun 27, 2019

rileykarson Jun 27, 2019

rileykarson Jun 27, 2019

rileykarson Jun 27, 2019

rileykarson Jun 27, 2019

rileykarson Jun 27, 2019

rileykarson Jun 27, 2019

sandeepsukhani commented Jun 28, 2019

rileykarson left a comment

rileykarson Jul 1, 2019

sandeepsukhani commented Jul 2, 2019

rileykarson left a comment •

edited

Loading

ghost commented Aug 8, 2019

-	clientBigtableInstances *bigtableadmin.ProjectsInstancesService
+	// Unlike other clients, the Bigtable Admin client doesn't use a single
+	// service. Instead, there are several distinct services created off
+	// the base service object. To imitate most other handwritten clients,
+	// we expose those directly instead of providing the `Service` object
+	// as a factory.
+	clientBigtableProjectsInstances *bigtableadmin.ProjectsInstancesService

	return nil, errwrap.Wrapf("Cannot convert a dataproc policy to a cloudresourcemanager policy: {{err}}", err)
	return nil, errwrap.Wrapf("Cannot convert a bigtable policy to a cloudresourcemanager policy: {{err}}", err)

	var BigtableAdminCustomEndpointEntryKey = "bigtableadmin_custom_endpoint"
	var BigtableAdminCustomEndpointEntryKey = "bigtable_custom_endpoint"

	"GOOGLE_BIGTABLE_ADMIN_CUSTOM_ENDPOINT",
	"GOOGLE_BIGTABLE_CUSTOM_ENDPOINT",

Add bigtable instance IAM resources #3939

Add bigtable instance IAM resources #3939

Conversation

sandeepsukhani commented Jun 27, 2019 • edited by emilymye Loading

rileykarson left a 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

sandeepsukhani commented Jun 28, 2019

rileykarson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandeepsukhani commented Jul 2, 2019

rileykarson left a comment • edited Loading

Choose a reason for hiding this comment

ghost commented Aug 8, 2019

sandeepsukhani commented Jun 27, 2019 •

edited by emilymye

Loading

rileykarson left a comment •

edited

Loading