Skip to content
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

Folder support: Assign/Reassign a google project to a folder. #438

Merged
merged 4 commits into from
Sep 22, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 74 additions & 16 deletions google/resource_google_project.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,17 @@ func resourceGoogleProject() *schema.Resource {
Required: true,
},
"org_id": &schema.Schema{
Type: schema.TypeString,
Required: true,
ForceNew: true,
Type: schema.TypeString,
Optional: true,
Computed: true,
ConflictsWith: []string{"folder_id"},
},
"folder_id": &schema.Schema{
Type: schema.TypeString,
Optional: true,
Computed: true,
ConflictsWith: []string{"org_id"},
StateFunc: parseFolderId,
},
"policy_data": &schema.Schema{
Type: schema.TypeString,
Expand Down Expand Up @@ -95,12 +103,10 @@ func resourceGoogleProjectCreate(d *schema.ResourceData, meta interface{}) error
project := &cloudresourcemanager.Project{
ProjectId: pid,
Name: d.Get("name").(string),
Parent: &cloudresourcemanager.ResourceId{
Id: d.Get("org_id").(string),
Type: "organization",
},
}

getParentResourceId(d, project)

if _, ok := d.GetOk("labels"); ok {
project.Labels = expandLabels(d)
}
Expand Down Expand Up @@ -155,7 +161,14 @@ func resourceGoogleProjectRead(d *schema.ResourceData, meta interface{}) error {
d.Set("labels", p.Labels)

if p.Parent != nil {
d.Set("org_id", p.Parent.Id)
switch p.Parent.Type {
case "organization":
d.Set("org_id", p.Parent.Id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add d.Set("folder_id", "") here. and the same for the `org_id field in the folder case below. The reason is that if you move a project from a folder to an org outside of Terraform, we want Terraform to clear the other field when reading the state.

Copy link
Contributor Author

@srivasta srivasta Sep 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

d.Set("folder_id", "")
case "folder":
d.Set("folder_id", p.Parent.Id)
d.Set("org_id", "")
}
}

// Read the billing account
Expand Down Expand Up @@ -183,9 +196,36 @@ func prefixedProject(pid string) string {
return "projects/" + pid
}

func getParentResourceId(d *schema.ResourceData, p *cloudresourcemanager.Project) error {
if v, ok := d.GetOk("org_id"); ok {
org_id := v.(string)
p.Parent = &cloudresourcemanager.ResourceId{
Id: org_id,
Type: "organization",
}
}

if v, ok := d.GetOk("folder_id"); ok {
p.Parent = &cloudresourcemanager.ResourceId{
Id: parseFolderId(v),
Type: "folder",
}
}
return nil
}

func parseFolderId(v interface{}) string {
folderId := v.(string)
if strings.HasPrefix(folderId, "folders/") {
return folderId[8:]
}
return folderId
}

func resourceGoogleProjectUpdate(d *schema.ResourceData, meta interface{}) error {
config := meta.(*Config)
pid := d.Id()
project_name := d.Get("name").(string)

// Read the project
// we need the project even though refresh has already been called
Expand All @@ -198,30 +238,46 @@ func resourceGoogleProjectUpdate(d *schema.ResourceData, meta interface{}) error
return fmt.Errorf("Error checking project %q: %s", pid, err)
}

// Project name has changed
d.Partial(true)

// Project display name has changed
if ok := d.HasChange("name"); ok {
p.Name = d.Get("name").(string)
p.Name = project_name
// Do update on project
p, err = config.clientResourceManager.Projects.Update(p.ProjectId, p).Do()
if err != nil {
return fmt.Errorf("Error updating project %q: %s", p.Name, err)
return fmt.Errorf("Error updating project %q: %s", project_name, err)
}
d.SetPartial("name")
}

// Project parent has changed
if d.HasChange("org_id") || d.HasChange("folder_id") {
getParentResourceId(d, p)

// Do update on project
p, err = config.clientResourceManager.Projects.Update(p.ProjectId, p).Do()
if err != nil {
return fmt.Errorf("Error updating project %q: %s", project_name, err)
}
d.SetPartial("org_id")
d.SetPartial("folder_id")
}

// Billing account has changed
if ok := d.HasChange("billing_account"); ok {
name := d.Get("billing_account").(string)
billing_name := d.Get("billing_account").(string)
ba := cloudbilling.ProjectBillingInfo{}
if name != "" {
ba.BillingAccountName = "billingAccounts/" + name
if billing_name != "" {
ba.BillingAccountName = "billingAccounts/" + billing_name
}
_, err = config.clientBilling.Projects.UpdateBillingInfo(prefixedProject(pid), &ba).Do()
if err != nil {
d.Set("billing_account", "")
if _err, ok := err.(*googleapi.Error); ok {
return fmt.Errorf("Error updating billing account %q for project %q: %v", name, prefixedProject(pid), _err)
return fmt.Errorf("Error updating billing account %q for project %q: %v", billing_name, prefixedProject(pid), _err)
}
return fmt.Errorf("Error updating billing account %q for project %q: %v", name, prefixedProject(pid), err)
return fmt.Errorf("Error updating billing account %q for project %q: %v", billing_name, prefixedProject(pid), err)
}
}

Expand All @@ -235,6 +291,8 @@ func resourceGoogleProjectUpdate(d *schema.ResourceData, meta interface{}) error
return fmt.Errorf("Error updating project %q: %s", p.Name, err)
}
}
d.Partial(false)

return nil
}

Expand Down
8 changes: 8 additions & 0 deletions google/resource_google_project_iam_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,14 @@ data "google_iam_policy" "admin" {
`, pid, name, org)
}

func testAccGoogleProject_createWithoutOrg(pid, name string) string {
return fmt.Sprintf(`
resource "google_project" "acceptance" {
project_id = "%s"
name = "%s"
}`, pid, name)
}

func testAccGoogleProject_create(pid, name, org string) string {
return fmt.Sprintf(`
resource "google_project" "acceptance" {
Expand Down
31 changes: 30 additions & 1 deletion google/resource_google_project_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,44 @@ var (
originalPolicy *cloudresourcemanager.Policy
)

// Test that a Project resource can be created without an organization
func TestAccGoogleProject_createWithoutOrg(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We run our acceptance tests using a service account (not user credentials). Unfortunately, a service account belongs to an org and therefor, cannot create a project without a parent organization.

One solution: we could add a skip if a service account is used. This means we could run it locally but they wouldn't be ran by our CI server.

@paddycarver Any other solutions for this? Running user credentials for the CI server? Not sure it is safe/worth it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added logic to skip the test if a service account is used. @paddycarver if you have a better suggestion, we can fix it in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm late to the party here--sorry, I was out of office on Thursday--but IIRC we were talking about the possibility of getting the service account our CI runs with whitelisted for creating a project without a parent organisation, though I've lost track of where that discussion landed or what its current status is. That's my preferred solution, honestly, but we can follow up on this with a more permanent fix in a separate PR.

creds := multiEnvSearch(credsEnvVars)
if strings.Contains(creds, "iam.gserviceaccount.com") {
t.Skip("Service accounts cannot create projects without a parent. Requires user credentials.")
}

pid := "terraform-" + acctest.RandString(10)
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
// This step creates a new project
resource.TestStep{
Config: testAccGoogleProject_createWithoutOrg(pid, pname),
Check: resource.ComposeTestCheckFunc(
testAccCheckGoogleProjectExists("google_project.acceptance", pid),
),
},
},
})
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that you can create folders, you can add acceptance tests for assigning a project to a folder and moving a project from a folder to an org and vice versa. Since you will be on vacation, I can also take care of writing the new tests. LMK what you prefer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds great. I won't have time for this till middle of next month, and so it would be great if you could add the tests with folder creation and movement between folders. I'll circle back in case you don't have time in the next couple of weeks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oki. I will write these tests today. Thanks!

// Test that a Project resource can be created and an IAM policy
// associated
func TestAccGoogleProject_create(t *testing.T) {
skipIfEnvNotSet(t,
[]string{
"GOOGLE_ORG",
}...,
)

pid := "terraform-" + acctest.RandString(10)
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
// This step imports an existing project
// This step creates a new project
resource.TestStep{
Config: testAccGoogleProject_create(pid, pname, org),
Check: resource.ComposeTestCheckFunc(
Expand Down
29 changes: 26 additions & 3 deletions website/docs/r/google_project.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,20 @@ resource "google_project" "my_project" {
}
```

To create a project under a specific folder

```hcl
resource "google_project" "my_project-in-a-folder" {
project_id = "your-project-id"
folder_id = "${google_folder.department1.name}"
}

resource "google_folder" "department1" {
display_name = "Department 1"
parent = "organizations/1234567"
}
```

## Argument Reference

The following arguments are supported:
Expand All @@ -53,7 +67,7 @@ The following arguments are supported:
Changing this forces a new project to be created. If this attribute is not
set, `id` must be set. As `id` is deprecated, consider this attribute
required. If you are using `project_id` and creating a new project, the
`org_id` and `name` attributes are also required.
`name` attribute is also required.

* `id` - (Deprecated) The project ID.
This attribute has unexpected behaviour and probably does not work
Expand All @@ -62,8 +76,17 @@ The following arguments are supported:
[below](#id-field) for more information about its behaviour.

* `org_id` - (Optional) The numeric ID of the organization this project belongs to.
This is required if you are creating a new project.
Changing this forces a new project to be created.
Changing this forces a new project to be created. Only one of
`org_id` or `folder_id` may be specified. If the `org_id` is
specified then the project is created at the top level. Changing
this forces the project to be migrated to the newly specified
organization.

* `folder_id` - (Optional) The numeric ID of the folder this project should be
created under. Only one of `org_id` or `folder_id` may be
specified. If the `folder_id` is specified, then the project is
created under the specified folder. Changing this forces the
project to be migrated to the newly specified folder.

* `billing_account` - (Optional) The alphanumeric ID of the billing account this project
belongs to. The user or service account performing this operation with Terraform
Expand Down