From 9a3a5fb205427520a17f84faf1494b3c657e4681 Mon Sep 17 00:00:00 2001 From: Riley Karson Date: Wed, 3 Jul 2019 14:15:15 -0700 Subject: [PATCH] Upstream Bigtable Instance IAM PR --- .../resource_bigtable_instance_iam_test.go | 211 ++++++++++++++++++ third_party/terraform/utils/config.go.erb | 19 ++ third_party/terraform/utils/field_helpers.go | 50 +++++ .../terraform/utils/field_helpers_test.go | 79 +++++++ .../terraform/utils/iam_bigtable_instance.go | 119 ++++++++++ third_party/terraform/utils/provider.go.erb | 6 + .../provider_handwritten_endpoint.go.erb | 11 + .../terraform/website-compiled/google.erb | 12 + .../docs/provider_reference.html.markdown | 1 + .../r/bigtable_instance_iam.html.markdown | 108 +++++++++ 10 files changed, 616 insertions(+) create mode 100644 third_party/terraform/tests/resource_bigtable_instance_iam_test.go create mode 100644 third_party/terraform/utils/iam_bigtable_instance.go create mode 100644 third_party/terraform/website/docs/r/bigtable_instance_iam.html.markdown diff --git a/third_party/terraform/tests/resource_bigtable_instance_iam_test.go b/third_party/terraform/tests/resource_bigtable_instance_iam_test.go new file mode 100644 index 000000000000..aef7780d25e9 --- /dev/null +++ b/third_party/terraform/tests/resource_bigtable_instance_iam_test.go @@ -0,0 +1,211 @@ +package google + +import ( + "fmt" + "testing" + + "github.com/hashicorp/terraform/helper/acctest" + "github.com/hashicorp/terraform/helper/resource" +) + +func TestAccBigtableInstanceIamBinding(t *testing.T) { + t.Parallel() + + instance := "tf-bigtable-iam-" + acctest.RandString(10) + account := "tf-bigtable-iam-" + acctest.RandString(10) + role := "roles/bigtable.user" + + importId := fmt.Sprintf("projects/%s/instances/%s %s", + getTestProjectFromEnv(), instance, role) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + // Test IAM Binding creation + Config: testAccBigtableInstanceIamBinding_basic(instance, account, role), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_bigtable_instance_iam_binding.binding", "role", role), + ), + }, + { + ResourceName: "google_bigtable_instance_iam_binding.binding", + ImportStateId: importId, + ImportState: true, + ImportStateVerify: true, + }, + { + // Test IAM Binding update + Config: testAccBigtableInstanceIamBinding_update(instance, account, role), + }, + { + ResourceName: "google_bigtable_instance_iam_binding.binding", + ImportStateId: importId, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccBigtableInstanceIamMember(t *testing.T) { + t.Parallel() + + instance := "tf-bigtable-iam-" + acctest.RandString(10) + account := "tf-bigtable-iam-" + acctest.RandString(10) + role := "roles/bigtable.user" + + importId := fmt.Sprintf("projects/%s/instances/%s %s serviceAccount:%s", + getTestProjectFromEnv(), + instance, + role, + serviceAccountCanonicalEmail(account)) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + // Test IAM Binding creation + Config: testAccBigtableInstanceIamMember(instance, account, role), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_bigtable_instance_iam_member.member", "role", role), + resource.TestCheckResourceAttr( + "google_bigtable_instance_iam_member.member", "member", "serviceAccount:"+serviceAccountCanonicalEmail(account)), + ), + }, + { + ResourceName: "google_bigtable_instance_iam_member.member", + ImportStateId: importId, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccBigtableInstanceIamPolicy(t *testing.T) { + t.Parallel() + + instance := "tf-bigtable-iam-" + acctest.RandString(10) + account := "tf-bigtable-iam-" + acctest.RandString(10) + role := "roles/bigtable.user" + + importId := fmt.Sprintf("projects/%s/instances/%s", + getTestProjectFromEnv(), instance) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + // Test IAM Binding creation + Config: testAccBigtableInstanceIamPolicy(instance, account, role), + }, + { + ResourceName: "google_bigtable_instance_iam_policy.policy", + ImportStateId: importId, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func testAccBigtableInstanceIamBinding_basic(instance, account, role string) string { + return fmt.Sprintf(testBigtableInstanceIam+` + +resource "google_service_account" "test-account1" { + account_id = "%s-1" + display_name = "Dataproc IAM Testing Account" +} + +resource "google_service_account" "test-account2" { + account_id = "%s-2" + display_name = "Iam Testing Account" +} + +resource "google_bigtable_instance_iam_binding" "binding" { + instance = "${google_bigtable_instance.instance.name}" + role = "%s" + members = [ + "serviceAccount:${google_service_account.test-account1.email}", + ] +} +`, instance, acctest.RandString(10), account, account, role) +} + +func testAccBigtableInstanceIamBinding_update(instance, account, role string) string { + return fmt.Sprintf(testBigtableInstanceIam+` +resource "google_service_account" "test-account1" { + account_id = "%s-1" + display_name = "Dataproc IAM Testing Account" +} + +resource "google_service_account" "test-account2" { + account_id = "%s-2" + display_name = "Iam Testing Account" +} + +resource "google_bigtable_instance_iam_binding" "binding" { + instance = "${google_bigtable_instance.instance.name}" + role = "%s" + members = [ + "serviceAccount:${google_service_account.test-account1.email}", + "serviceAccount:${google_service_account.test-account2.email}", + ] +} +`, instance, acctest.RandString(10), account, account, role) +} + +func testAccBigtableInstanceIamMember(instance, account, role string) string { + return fmt.Sprintf(testBigtableInstanceIam+` +resource "google_service_account" "test-account" { + account_id = "%s" + display_name = "Dataproc IAM Testing Account" +} + +resource "google_bigtable_instance_iam_member" "member" { + instance = "${google_bigtable_instance.instance.name}" + role = "%s" + member = "serviceAccount:${google_service_account.test-account.email}" +} +`, instance, acctest.RandString(10), account, role) +} + +func testAccBigtableInstanceIamPolicy(instance, account, role string) string { + return fmt.Sprintf(testBigtableInstanceIam+` +resource "google_service_account" "test-account" { + account_id = "%s" + display_name = "Dataproc IAM Testing Account" +} + +data "google_iam_policy" "policy" { + binding { + role = "%s" + members = ["serviceAccount:${google_service_account.test-account.email}"] + } +} + +resource "google_bigtable_instance_iam_policy" "policy" { + instance = "${google_bigtable_instance.instance.name}" + policy_data = "${data.google_iam_policy.policy.policy_data}" +} +`, instance, acctest.RandString(10), account, role) +} + +// Smallest instance possible for testing +var testBigtableInstanceIam = ` +resource "google_bigtable_instance" "instance" { + name = "%s" + instance_type = "DEVELOPMENT" + + cluster { + cluster_id = "c-%s" + zone = "us-central1-b" + storage_type = "HDD" + } +}` diff --git a/third_party/terraform/utils/config.go.erb b/third_party/terraform/utils/config.go.erb index ce79b389c737..8cc90e015d0d 100644 --- a/third_party/terraform/utils/config.go.erb +++ b/third_party/terraform/utils/config.go.erb @@ -18,6 +18,7 @@ import ( "google.golang.org/api/accesscontextmanager/v1beta" <% end -%> "google.golang.org/api/bigquery/v2" + "google.golang.org/api/bigtableadmin/v2" "google.golang.org/api/cloudbilling/v1" "google.golang.org/api/cloudbuild/v1" "google.golang.org/api/cloudfunctions/v1" @@ -193,6 +194,13 @@ type Config struct { clientStorageTransfer *storagetransfer.Service bigtableClientFactory *BigtableClientFactory + BigtableAdminBasePath string + // 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 } var defaultClientScopes = []string{ @@ -463,6 +471,17 @@ func (c *Config) LoadAndValidate() error { TokenSource: tokenSource, } + bigtableAdminBasePath := removeBasePathVersion(c.BigtableAdminBasePath) + log.Printf("[INFO] Instantiating Google Cloud BigtableAdmin for path %s", bigtableAdminBasePath) + + clientBigtable, err := bigtableadmin.NewService(context, option.WithHTTPClient(client)) + if err != nil { + return err + } + clientBigtable.UserAgent = userAgent + clientBigtable.BasePath = bigtableAdminBasePath + c.clientBigtableProjectsInstances = bigtableadmin.NewProjectsInstancesService(clientBigtable) + sourceRepoClientBasePath := removeBasePathVersion(c.SourceRepoBasePath) log.Printf("[INFO] Instantiating Google Cloud Source Repo client for path %s", sourceRepoClientBasePath) c.clientSourceRepo, err = sourcerepo.NewService(context, option.WithHTTPClient(client)) diff --git a/third_party/terraform/utils/field_helpers.go b/third_party/terraform/utils/field_helpers.go index a822c4ea9bb8..981a170659a3 100644 --- a/third_party/terraform/utils/field_helpers.go +++ b/third_party/terraform/utils/field_helpers.go @@ -14,6 +14,8 @@ const ( regionalLinkTemplate = "projects/%s/regions/%s/%s/%s" regionalLinkBasePattern = "projects/(.+)/regions/(.+)/%s/(.+)" regionalPartialLinkBasePattern = "regions/(.+)/%s/(.+)" + projectLinkTemplate = "projects/%s/%s/%s" + projectBasePattern = "projects/(.+)/%s/(.+)" organizationLinkTemplate = "organizations/%s/%s/%s" organizationBasePattern = "organizations/(.+)/%s/(.+)" ) @@ -355,3 +357,51 @@ func getRegionFromSchema(regionSchemaField, zoneSchemaField string, d TerraformR return "", fmt.Errorf("Cannot determine region: set in this resource, or set provider-level 'region' or 'zone'.") } + +type ProjectFieldValue struct { + Project string + Name string + + resourceType string +} + +func (f ProjectFieldValue) RelativeLink() string { + if len(f.Name) == 0 { + return "" + } + + return fmt.Sprintf(projectLinkTemplate, f.Project, f.resourceType, f.Name) +} + +// Parses a project field with the following formats: +// - projects/{my_projects}/{resource_type}/{resource_name} +func parseProjectFieldValue(resourceType, fieldValue, projectSchemaField string, d TerraformResourceData, config *Config, isEmptyValid bool) (*ProjectFieldValue, error) { + if len(fieldValue) == 0 { + if isEmptyValid { + return &ProjectFieldValue{resourceType: resourceType}, nil + } + return nil, fmt.Errorf("The project field for resource %s cannot be empty", resourceType) + } + + r := regexp.MustCompile(fmt.Sprintf(projectBasePattern, resourceType)) + if parts := r.FindStringSubmatch(fieldValue); parts != nil { + return &ProjectFieldValue{ + Project: parts[1], + Name: parts[2], + + resourceType: resourceType, + }, nil + } + + project, err := getProjectFromSchema(projectSchemaField, d, config) + if err != nil { + return nil, err + } + + return &ProjectFieldValue{ + Project: project, + Name: GetResourceNameFromSelfLink(fieldValue), + + resourceType: resourceType, + }, nil +} diff --git a/third_party/terraform/utils/field_helpers_test.go b/third_party/terraform/utils/field_helpers_test.go index 49847f777607..08d5bbc0a051 100644 --- a/third_party/terraform/utils/field_helpers_test.go +++ b/third_party/terraform/utils/field_helpers_test.go @@ -353,3 +353,82 @@ func TestParseRegionalFieldValue(t *testing.T) { }) } } + +func TestParseProjectFieldValue(t *testing.T) { + const resourceType = "instances" + cases := map[string]struct { + FieldValue string + ExpectedRelativeLink string + ExpectedError bool + IsEmptyValid bool + ProjectSchemaField string + ProjectSchemaValue string + Config *Config + }{ + "instance is a full self link": { + FieldValue: "https://www.googleapis.com/compute/v1/projects/myproject/instances/my-instance", + ExpectedRelativeLink: "projects/myproject/instances/my-instance", + }, + "instance is a relative self link": { + FieldValue: "projects/myproject/instances/my-instance", + ExpectedRelativeLink: "projects/myproject/instances/my-instance", + }, + "instance is a partial relative self link": { + FieldValue: "projects/instances/my-instance", + Config: &Config{Project: "default-project"}, + ExpectedRelativeLink: "projects/default-project/instances/my-instance", + }, + "instance is the name only": { + FieldValue: "my-instance", + Config: &Config{Project: "default-project"}, + ExpectedRelativeLink: "projects/default-project/instances/my-instance", + }, + "instance is the name only and has a project set in schema": { + FieldValue: "my-instance", + ProjectSchemaField: "project", + ProjectSchemaValue: "schema-project", + Config: &Config{Project: "default-project"}, + ExpectedRelativeLink: "projects/schema-project/instances/my-instance", + }, + "instance is the name only and has a project set in schema but the field is not specified.": { + FieldValue: "my-instance", + ProjectSchemaValue: "schema-project", + Config: &Config{Project: "default-project"}, + ExpectedRelativeLink: "projects/default-project/instances/my-instance", + }, + "instance is empty and it is valid": { + FieldValue: "", + IsEmptyValid: true, + ExpectedRelativeLink: "", + }, + "instance is empty and it is not valid": { + FieldValue: "", + IsEmptyValid: false, + ExpectedError: true, + }, + } + + for tn, tc := range cases { + fieldsInSchema := make(map[string]interface{}) + + if len(tc.ProjectSchemaValue) > 0 && len(tc.ProjectSchemaField) > 0 { + fieldsInSchema[tc.ProjectSchemaField] = tc.ProjectSchemaValue + } + + d := &ResourceDataMock{ + FieldsInSchema: fieldsInSchema, + } + + v, err := parseProjectFieldValue(resourceType, tc.FieldValue, tc.ProjectSchemaField, d, tc.Config, tc.IsEmptyValid) + + if err != nil { + if !tc.ExpectedError { + t.Errorf("bad: %s, did not expect an error. Error: %s", tn, err) + } + } else { + if v.RelativeLink() != tc.ExpectedRelativeLink { + t.Errorf("bad: %s, expected relative link to be '%s' but got '%s'", tn, tc.ExpectedRelativeLink, v.RelativeLink()) + } + } + } +} diff --git a/third_party/terraform/utils/iam_bigtable_instance.go b/third_party/terraform/utils/iam_bigtable_instance.go new file mode 100644 index 000000000000..bab4f1057b28 --- /dev/null +++ b/third_party/terraform/utils/iam_bigtable_instance.go @@ -0,0 +1,119 @@ +package google + +import ( + "fmt" + "google.golang.org/api/bigtableadmin/v2" + + "github.com/hashicorp/errwrap" + "github.com/hashicorp/terraform/helper/schema" + "google.golang.org/api/cloudresourcemanager/v1" +) + +var IamBigtableInstanceSchema = map[string]*schema.Schema{ + "instance": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + "project": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, +} + +type BigtableInstanceIamUpdater struct { + project string + instance string + Config *Config +} + +func NewBigtableInstanceUpdater(d *schema.ResourceData, config *Config) (ResourceIamUpdater, error) { + project, err := getProject(d, config) + if err != nil { + return nil, err + } + + d.Set("project", project) + + return &BigtableInstanceIamUpdater{ + project: project, + instance: d.Get("instance").(string), + Config: config, + }, nil +} + +func BigtableInstanceIdParseFunc(d *schema.ResourceData, config *Config) error { + fv, err := parseProjectFieldValue("instances", d.Id(), "project", d, config, false) + if err != nil { + return err + } + + d.Set("project", fv.Project) + d.Set("instance", fv.Name) + + // Explicitly set the id so imported resources have the same ID format as non-imported ones. + d.SetId(fv.RelativeLink()) + return nil +} + +func (u *BigtableInstanceIamUpdater) GetResourceIamPolicy() (*cloudresourcemanager.Policy, error) { + req := &bigtableadmin.GetIamPolicyRequest{} + p, err := u.Config.clientBigtableProjectsInstances.GetIamPolicy(u.GetResourceId(), req).Do() + if err != nil { + return nil, errwrap.Wrapf(fmt.Sprintf("Error retrieving IAM policy for %s: {{err}}", u.DescribeResource()), err) + } + + cloudResourcePolicy, err := bigtableToResourceManagerPolicy(p) + if err != nil { + return nil, errwrap.Wrapf(fmt.Sprintf("Invalid IAM policy for %s: {{err}}", u.DescribeResource()), err) + } + + return cloudResourcePolicy, nil +} + +func (u *BigtableInstanceIamUpdater) SetResourceIamPolicy(policy *cloudresourcemanager.Policy) error { + bigtablePolicy, err := resourceManagerToBigtablePolicy(policy) + if err != nil { + return errwrap.Wrapf(fmt.Sprintf("Invalid IAM policy for %s: {{err}}", u.DescribeResource()), err) + } + + req := &bigtableadmin.SetIamPolicyRequest{Policy: bigtablePolicy} + _, err = u.Config.clientBigtableProjectsInstances.SetIamPolicy(u.GetResourceId(), req).Do() + if err != nil { + return errwrap.Wrapf(fmt.Sprintf("Error setting IAM policy for %s: {{err}}", u.DescribeResource()), err) + } + + return nil +} + +func (u *BigtableInstanceIamUpdater) GetResourceId() string { + return fmt.Sprintf("projects/%s/instances/%s", u.project, u.instance) +} + +func (u *BigtableInstanceIamUpdater) GetMutexKey() string { + return fmt.Sprintf("iam-bigtable-instance-%s-%s", u.project, u.instance) +} + +func (u *BigtableInstanceIamUpdater) DescribeResource() string { + return fmt.Sprintf("Bigtable Instance %s/%s", u.project, u.instance) +} + +func resourceManagerToBigtablePolicy(p *cloudresourcemanager.Policy) (*bigtableadmin.Policy, error) { + out := &bigtableadmin.Policy{} + err := Convert(p, out) + if err != nil { + return nil, errwrap.Wrapf("Cannot convert a bigtable policy to a cloudresourcemanager policy: {{err}}", err) + } + return out, nil +} + +func bigtableToResourceManagerPolicy(p *bigtableadmin.Policy) (*cloudresourcemanager.Policy, error) { + out := &cloudresourcemanager.Policy{} + err := Convert(p, out) + if err != nil { + return nil, errwrap.Wrapf("Cannot convert a cloudresourcemanager policy to a bigtable policy: {{err}}", err) + } + return out, nil +} diff --git a/third_party/terraform/utils/provider.go.erb b/third_party/terraform/utils/provider.go.erb index 7d90fcaf7153..c23f4f1133fb 100644 --- a/third_party/terraform/utils/provider.go.erb +++ b/third_party/terraform/utils/provider.go.erb @@ -132,6 +132,7 @@ func Provider() terraform.ResourceProvider { CloudFunctionsCustomEndpointEntryKey: CloudFunctionsCustomEndpointEntry, CloudIoTCustomEndpointEntryKey: CloudIoTCustomEndpointEntry, StorageTransferCustomEndpointEntryKey: StorageTransferCustomEndpointEntry, + BigtableAdminCustomEndpointEntryKey: BigtableAdminCustomEndpointEntry, }, DataSourcesMap: map[string]*schema.Resource{ @@ -231,6 +232,9 @@ func ResourceMapWithErrors() (map[string]*schema.Resource, error) { "google_bigquery_dataset": resourceBigQueryDataset(), "google_bigquery_table": resourceBigQueryTable(), "google_bigtable_instance": resourceBigtableInstance(), + "google_bigtable_instance_iam_binding": ResourceIamBindingWithImport(IamBigtableInstanceSchema, NewBigtableInstanceUpdater, BigtableInstanceIdParseFunc), + "google_bigtable_instance_iam_member": ResourceIamMemberWithImport(IamBigtableInstanceSchema, NewBigtableInstanceUpdater, BigtableInstanceIdParseFunc), + "google_bigtable_instance_iam_policy": ResourceIamPolicyWithImport(IamBigtableInstanceSchema, NewBigtableInstanceUpdater, BigtableInstanceIdParseFunc), "google_bigtable_table": resourceBigtableTable(), "google_billing_account_iam_binding": ResourceIamBindingWithImport(IamBillingAccountSchema, NewBillingAccountIamUpdater, BillingAccountIdParseFunc), "google_billing_account_iam_member": ResourceIamMemberWithImport(IamBillingAccountSchema, NewBillingAccountIamUpdater, BillingAccountIdParseFunc), @@ -439,6 +443,7 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) { config.CloudFunctionsBasePath = d.Get(CloudFunctionsCustomEndpointEntryKey).(string) config.CloudIoTBasePath = d.Get(CloudIoTCustomEndpointEntryKey).(string) config.StorageTransferBasePath = d.Get(StorageTransferCustomEndpointEntryKey).(string) + config.BigtableAdminBasePath = d.Get(BigtableAdminCustomEndpointEntryKey).(string) if err := config.LoadAndValidate(); err != nil { return nil, err @@ -504,6 +509,7 @@ func ConfigureBasePaths(c *Config) { c.CloudFunctionsBasePath = CloudFunctionsDefaultBasePath c.CloudIoTBasePath = CloudIoTDefaultBasePath c.StorageTransferBasePath = StorageTransferDefaultBasePath + c.BigtableAdminBasePath = BigtableAdminDefaultBasePath } func validateCredentials(v interface{}, k string) (warnings []string, errors []error) { diff --git a/third_party/terraform/utils/provider_handwritten_endpoint.go.erb b/third_party/terraform/utils/provider_handwritten_endpoint.go.erb index 26e07536a563..a7b802dbab7b 100644 --- a/third_party/terraform/utils/provider_handwritten_endpoint.go.erb +++ b/third_party/terraform/utils/provider_handwritten_endpoint.go.erb @@ -252,6 +252,17 @@ var StorageTransferCustomEndpointEntry = &schema.Schema{ }, StorageTransferDefaultBasePath), } +var BigtableAdminDefaultBasePath = "https://bigtableadmin.googleapis.com/v2/" +var BigtableAdminCustomEndpointEntryKey = "bigtable_custom_endpoint" +var BigtableAdminCustomEndpointEntry = &schema.Schema{ + Type: schema.TypeString, + Optional: true, + ValidateFunc: validateCustomEndpoint, + DefaultFunc: schema.MultiEnvDefaultFunc([]string{ + "GOOGLE_BIGTABLE_CUSTOM_ENDPOINT", + }, BigtableAdminDefaultBasePath), +} + func validateCustomEndpoint(v interface{}, k string) (ws []string, errors []error) { re := `.*/[^/]+/$` return validateRegexp(re)(v, k) diff --git a/third_party/terraform/website-compiled/google.erb b/third_party/terraform/website-compiled/google.erb index cc3d1d0d4675..a65a1a84130d 100644 --- a/third_party/terraform/website-compiled/google.erb +++ b/third_party/terraform/website-compiled/google.erb @@ -228,6 +228,18 @@