Deleting service default service accounts when creating a new project #4135

gabrielgbim · 2019-07-30T17:28:35Z

Community Note

Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
If you are interested in working on this issue or have submitted a pull request, please leave a comment. If the issue is assigned to the "modular-magician" user, it is either in the process of being autogenerated, or is planned to be autogenerated soon. If the issue is assigned to a user, that user is claiming responsibility for the issue. If the issue is assigned to "hashibot", a community member has claimed the issue already.

Description

When creating a new project, I would like to also delete the default service accounts that are created with it.
Since I will be using custom service accounts for the project, the default ones should be deleted.

It will be very helpful if the google_project resource offers this option. This way, I could avoid to trigger gcloud commands to delete service account after the project creation (and avoid gcloud depencency in the terraform run environment)

New or Affected Resource(s)

google_project

Potential Terraform Configuration

I would like to propose a new parameter in google_project resource. The new parameter could work as the existing auto_create_network (that delete auto created default network)

resource "google_project" "main" {
  name                    = var.name
  project_id              = var.project_id
  auto_create_network     = "false"
  delete_service_accounts = "true"
}

References

Pull request:
#4132

The text was updated successfully, but these errors were encountered:

rileykarson · 2020-02-07T23:22:53Z

#4132, GoogleCloudPlatform/magic-modules#2401 were both an implementation + upstreaming of this, but fell out of date. Once this is picked up, they'll be useful as references.

danawillow · 2020-03-20T20:29:44Z

I'm a bit confused about what the use case for this is. From what I can tell via https://cloud.google.com/iam/docs/service-accounts, there are a few service accounts that are created when specific APIs are enabled on the project (compute, etc.) that need to stick around, but also aren't created at the same time the project is created. Which service accounts are the ones you're hoping to delete with this issue?

thiagonache · 2020-10-08T21:39:03Z

Hi folks,
I would like to take this issue. I'll take @gabrielgbim as an example, but we need to support disable and deletion which is done currently via gcloud in the terraform project factory module. I think it's fairly simple, just need to "translate" the python code to go.

Please, let me know your considerations.

morgante · 2020-10-08T21:59:58Z

I'd actually like to propose we do this in a separate resource instead of coupling it to project creation.

thiagonache · 2020-10-08T22:06:31Z

Ok. Any considerations from the maintainers?

rileykarson · 2020-10-08T22:16:02Z

@morgante: That's so that it can run after service activations, right?

I think it's fine if we do that, since we'll catch a good number of default SAs that way. We should note in the resource that specifying the resource does not ensure that all default SAs have been removed- just that Terraform attempted to remove them at some point.

thiagonache · 2020-10-08T22:34:32Z

@morgante Also another question for you, we currently support disable, delete, and deprivilege. Should we support all these three states?

morgante · 2020-10-08T22:35:39Z

@morgante: That's so that it can run after service activations, right?

Right, I also want to avoid adding more logic to google_project as any failures in that resource lead to tainting the entire project (which is an expensive resource to delete or create).

@morgante Also another question for you, we currently support disable, delete, and deprivilege. Should we support all these three states?

Yes I think so (action = "delete") for example.

thiagonache · 2020-10-08T22:44:01Z

okay. I'll work on that. Once I have something I'll put in here for discussion.
Thank you, guys!

thiagonache · 2020-10-15T10:37:13Z

Hi folks,

I had time to give some thought to it today and this is what I came up with:

package main

import (
	"fmt"
	"log"

	"../pdsa" # stands for project default service account, just any name
)

func main() {
	var project string = "my-project" # tf input
	var action string = "delete" # tf input

	sas, err := pdsa.ListProjectSA(project)
	if err != nil {
		log.Fatalf("Cannot list service accounts on project %s", project)
	}

	for _, sa := range sas {
		fmt.Printf("%s Service Account %s\n", action, sa.Email)
		switch action {
		case "delete":
			err := pdsa.DeleteServiceAccount(fmt.Sprintf("projects/%s/serviceAccounts/%s", sa.ProjectId, sa.Email))
			if err != nil {
				log.Fatalf("Cannot delete service account %s", sa.Email)
			}
		case "disable":
			err := pdsa.DisableServiceAccount(fmt.Sprintf("projects/%s/serviceAccounts/%s", sa.ProjectId, sa.Email))
			if err != nil {
				log.Fatalf("Cannot delete service account %s", sa.Email)
			}
		case "deprivilege":
			err := pdsa.DeprivilegeServiceAccount(fmt.Sprintf("projects/%s/serviceAccounts/%s", sa.ProjectId, sa.Email))
			if err != nil {
				log.Fatalf("Cannot delete service account %s", sa.Email)
			}
		default:
			log.Fatalf("%s is not a valid action", action)
		}

	}
}

This would be the main code for the new resource (of course it is not in terraform format yet).

Please, let me know if I can keep following this path.

thiagonache · 2020-10-15T11:08:23Z

package pdsa

import (
	"context"
	"fmt"

	iam "google.golang.org/api/iam/v1"
)

// ListProjectSA returns slice of iam.ServiceAccount struct
func ListProjectSA(project string) ([]*iam.ServiceAccount, error) {
	ctx := context.Background()
	service, err := iam.NewService(ctx)
	if err != nil {
		return nil, fmt.Errorf("iam.NewService: %v", err)
	}
	response, err := service.Projects.ServiceAccounts.List("projects/" + project).Do()
	if err != nil {
		return nil, fmt.Errorf("Projects.ServiceAccounts.List: %v", err)
	}
	return response.Accounts, nil
}

// DeleteServiceAccount deletes
func DeleteServiceAccount(name string) error {
	ctx := context.Background()
	service, err := iam.NewService(ctx)
	if err != nil {
		return fmt.Errorf("iam.NewService: %v", err)
	}
	_, err = service.Projects.ServiceAccounts.Delete(name).Do()
	if err != nil {
		return fmt.Errorf("Cannot delete service account: %v", err)
	}
	return nil
}

This is the pdsa package for your consideration too

thiagonache · 2020-10-15T12:49:23Z

terraforming it:

// resourceGoogleProjectDefaultServiceAccounts returns a *schema.Resource that allows a customer
// to manage all the default serviceAccounts.
// It does mean that terraform tried to perform the action in the SA at some point but does not ensure that
// all defaults serviceAccounts where managed. Eg.: API was activated after project creation.
func resourceGoogleProjectDefaultServiceAccounts() *schema.Resource {
	return &schema.Resource{
		SchemaVersion: 0,

		Create: resourceGoogleProjectDefaultServiceAccountsCreate,
		Read:   resourceGoogleProjectDefaultServiceAccountsRead,
		Update: resourceGoogleProjectDefaultServiceAccountsUpdate,
		Delete: resourceGoogleProjectDefaultServiceAccountsDelete,

		// This resource should not have import, right?
		// Importer: &schema.ResourceImporter{
		// 	State: resourceGoogleProjectDefaultServiceAccountsImportState,
		// },

		Timeouts: &schema.ResourceTimeout{
			Create: schema.DefaultTimeout(10 * time.Minute),
			Update: schema.DefaultTimeout(10 * time.Minute),
			Read:   schema.DefaultTimeout(10 * time.Minute),
			Delete: schema.DefaultTimeout(10 * time.Minute),
		},

		MigrateState: resourceGoogleProjectDefaultServiceAccountsMigrateState,

		Schema: map[string]*schema.Schema{
			"project_id": {
				Type:         schema.TypeString,
				Required:     true,
				ForceNew:     true,
				ValidateFunc: validateProjectID(),
				Description:  `The project ID.`,
			},
			"action": {
				Type:         schema.TypeString,
				Required:     true,
				ValidateFunc: validateServiceAccountAction(),
				Description:  `The action to be performed in the default service accounts.`,
			},
		},
	}
}

func resourceGoogleProjectDefaultServiceAccountsCreate(d *schema.ResourceData, meta interface{}) error {
	config := meta.(*Config)
	userAgent, err := generateUserAgentString(d, config.userAgent)
	if err != nil {
		return err
	}
	pid, ok := d.Get("project_id").(string)
	if !ok {
		return fmt.Errorf("Cannot get project_id variable")
	}
	action, ok := d.Get("action").(string)
	if !ok {
		return fmt.Errorf("Cannot get action variable")
	}

	serviceAccounts, err := resourceGoogleProjectDefaultServiceAccountsList(config, d, userAgent)
	if err != nil {
		return fmt.Errorf("Error listing service accounts on project %s: %v", pid, err)
	}
	for _, sa := range serviceAccounts {
		switch action {
		case "delete":
			return fmt.Errorf("Deleting service account %s on project %s", sa.Email, pid)
		}
	}

	return nil
}

func resourceGoogleProjectDefaultServiceAccountsList(config *Config, d *schema.ResourceData, userAgent string) ([]*iam.ServiceAccount, error) {
	var pid string
	pid, ok := d.Get("project_id").(string)
	if !ok {
		return nil, fmt.Errorf("Cannot get project_id variable")
	}
	response, err := config.NewIamClient(userAgent).Projects.ServiceAccounts.List(prefixedProject(pid)).Do()
	if err != nil {
		return nil, fmt.Errorf("failed to list service accounts on project %q: %v", pid, err)
	}
	return response.Accounts, nil
}

thiagonache · 2020-10-15T13:40:50Z

@morgante what's the behaviour we want when the project id or the action is updated? I think we should ignore it since it could perform actions on non-default service accounts, does it make sense? Do you know if is there any way to filter only "defaults" SAs?

morgante · 2020-10-15T16:39:04Z

@thiagonache I think we should actually restrict this to only the default service accounts (based on filtering the name): https://cloud.google.com/iam/docs/service-accounts#default

If you change the project ID that should be treated as a delete & recreate IMO ("delete" the resource on the old project, "create" it on the new one). Changing the action should be an update.

Here's my proposed state changes:

Original action	New action: delete	New action: deprivilege	New action: disable	Delete resource
Delete	—	Raise error	Raise error	Do nothing
Disable	Delete SA	Reactivate and remove roles	—	Reactivate
Deprivilege	Delete SA	—	Disable SA	Do nothing

thiagonache · 2020-10-15T16:56:09Z

@thiagonache I think we should actually restrict this to only the default service accounts (based on filtering the name): https://cloud.google.com/iam/docs/service-accounts#default

I didn't know we could have just two. Honestly, I've never checked that before.

If you change the project ID that should be treated as a delete & recreate IMO ("delete" the resource on the old project, "create" it on the new one). Changing the action should be an update.

Agreed

Here's my proposed state changes:

Original action New action: delete New action: deprivilege New action: disable Delete resource
Delete — Raise error Raise error Raise error
Disable Delete SA Reactivate and remove roles — Reactivate
Deprivilege Delete SA — Disable SA Do nothing

Actually, if it is deleted we should do nothing on the destroy. Otherwise, we'll not be able to destroy the resource when the original action is delete. Does it make sense?

Thank you!!!

morgante · 2020-10-15T17:05:11Z

Actually, if it is deleted we should do nothing on the destroy. Otherwise, we'll not be able to destroy the resource when the original action is delete. Does it make sense?

Technically you could remove it from your state file, but that seems fine too.

rileykarson · 2020-10-15T17:08:22Z

Reactivation would add significant complexity to the resource, right? We'd need to know the list of default SAs and their associated IAM roles, and recreate them + assign those roles (maintenance would be pretty high, too- we'd need to update the resource any time a new default SA is added). In contrast, the initial delete/disable/deprivilege action we can infer based on the project number.

I would propose that both the action and project are ForceNew, making updates impossible, and that deletion does nothing in all cases. Changing from a lower severity action (eg disable -> delete) would work, since the resource would be dropped and then recreated taking the more severe action.

This leaves one big gap- reactivation. If that would have a significant benefit, we could consider adding that on delete. Do we expect users would want to walk back the disablement of all the accounts often?

@morgante: I'd like to avoid cases where users need to edit their state file to remove a resource.

morgante · 2020-10-15T17:22:08Z

Always using ForceNew is also reasonable.

I do think reactivation is important to include. We've seen a number of support cases where customers were unsure how to reactivate and needed to in order to use certain products.

thiagonache · 2020-10-15T17:28:27Z

Actually, if it is deleted we should do nothing on the destroy. Otherwise, we'll not be able to destroy the resource when the original action is delete. Does it make sense?

Technically you could remove it from your state file, but that seems fine too.

yeah... I meant just regarding the service account. on destroy we should always remove from the state.

thiagonache · 2020-10-15T17:29:25Z

Reactivation would add significant complexity to the resource, right? We'd need to know the list of default SAs and their associated IAM roles, and recreate them + assign those roles (maintenance would be pretty high, too- we'd need to update the resource any time a new default SA is added). In contrast, the initial delete/disable/deprivilege action we can infer based on the project number.

I would propose that both the action and project are ForceNew, making updates impossible, and that deletion does nothing in all cases. Changing from a lower severity action (eg disable -> delete) would work, since the resource would be dropped and then recreated taking the more severe action.

I was following this path ^^

This leaves one big gap- reactivation. If that would have a significant benefit, we could consider adding that on delete. Do we expect users would want to walk back the disablement of all the accounts often?

@morgante: I'd like to avoid cases where users need to edit their state file to remove a resource.

thiagonache · 2020-10-15T17:31:44Z

Always using ForceNew is also reasonable.

I do think reactivation is important to include. We've seen a number of support cases where customers were unsure how to reactivate and needed to in order to use certain products.

Yeah... I could figure out by myself but I did this issue when using GKE for the first time. But I agree it would be hard to maintain over time. The roles associated and the number of default services accounts may change.

morgante · 2020-10-15T17:34:25Z

We don't necessarily need to restore the roles, but I think reactivating any deactivate accounts is doable.

thiagonache · 2020-10-15T17:35:31Z

sounds good to me. @rileykarson ? (sorry for tagging out, since you already was messaged I'll not remove it)

rileykarson · 2020-10-15T17:37:22Z

I do think reactivation is important to include. We've seen a number of support cases where customers were unsure how to reactivate and needed to in order to use certain products.

Yeah... I could figure out by myself but I did this issue when using GKE for the first time. But I agree it would be hard to maintain over time. The roles associated and the number of default services accounts may change.

Reactivation seems fine to do, then! Specifically enabling a disabled service account, since we can list the service accounts and filter for disabled ones.

Restoring roles and undelete would require more information than we'll have available. We don't know the role:sa mapping to reprivelege, and won't know the complete list of SA emails to attempt to undelete.

@thiagonache: Something we could consider is a restore_policy enum which can be NONE or REACTIVATE- if set to REACTIVATE, the resource would attempt to restore all default SAs on delete.

thiagonache · 2020-10-15T17:39:58Z

I'll submit a draft PR soon.

thiagonache · 2020-10-15T20:06:38Z

hey, Riley.

Please, see the PR mentioned.

hey, Dana.

Looks like you were selected to review the PR. Looking forward to hearing from you.

thiagonache · 2020-10-19T17:31:18Z

Hey guys

I do think reactivation is important to include. We've seen a number of support cases where customers were unsure how to reactivate and needed to in order to use certain products.

Yeah... I could figure out by myself but I did this issue when using GKE for the first time. But I agree it would be hard to maintain over time. The roles associated and the number of default services accounts may change.

Reactivation seems fine to do, then! Specifically enabling a disabled service account, since we can list the service accounts and filter for disabled ones.

Restoring roles and undelete would require more information than we'll have available. We don't know the role:sa mapping to reprivelege, and won't know the complete list of SA emails to attempt to undelete.

@thiagonache: Something we could consider is a restore_policy enum which can be NONE or REACTIVATE- if set to REACTIVATE, the resource would attempt to restore all default SAs on delete.

Morgante or Riley, I've added support to undelete the service account when it was deleted too. If any of you guys want to take a look and see if I did not forget anything would be helpful.

ghost · 2020-11-30T13:50:07Z

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 👉 hashibot-feedback@hashicorp.com. Thanks!

ghost added the enhancement label Jul 30, 2019

paddycarver added the size/s label Dec 6, 2019

paddycarver added this to the Goals milestone Dec 6, 2019

megan07 modified the milestones: Goals, Backlog Mar 20, 2020

thiagonache mentioned this issue Oct 8, 2020

Remove gcloud dependency terraform-google-modules/terraform-google-project-factory#408

Closed

2 tasks

thiagonache mentioned this issue Oct 15, 2020

Adds new resource to manage default service accounts #7541

Merged

rileykarson mentioned this issue Oct 29, 2020

Add support for google_project_default_service_accounts resource GoogleCloudPlatform/magic-modules#4167

Merged

5 tasks

rileykarson closed this as completed in GoogleCloudPlatform/magic-modules#4167 Oct 30, 2020

This was referenced Nov 5, 2020

Add support for google_project_default_service_accounts resource hashicorp/terraform-provider-google-beta#2668

Merged

Add support for google_project_default_service_accounts resource #7709

Merged

ghost locked as resolved and limited conversation to collaborators Nov 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deleting service default service accounts when creating a new project #4135

Deleting service default service accounts when creating a new project #4135

gabrielgbim commented Jul 30, 2019

rileykarson commented Feb 7, 2020

danawillow commented Mar 20, 2020

thiagonache commented Oct 8, 2020

morgante commented Oct 8, 2020

thiagonache commented Oct 8, 2020

rileykarson commented Oct 8, 2020

thiagonache commented Oct 8, 2020

morgante commented Oct 8, 2020

thiagonache commented Oct 8, 2020

thiagonache commented Oct 15, 2020 •

edited

Loading

thiagonache commented Oct 15, 2020

thiagonache commented Oct 15, 2020

thiagonache commented Oct 15, 2020

morgante commented Oct 15, 2020 •

edited

Loading

thiagonache commented Oct 15, 2020 •

edited

Loading

morgante commented Oct 15, 2020

rileykarson commented Oct 15, 2020 •

edited

Loading

morgante commented Oct 15, 2020

thiagonache commented Oct 15, 2020

thiagonache commented Oct 15, 2020

thiagonache commented Oct 15, 2020

morgante commented Oct 15, 2020

thiagonache commented Oct 15, 2020 •

edited

Loading

rileykarson commented Oct 15, 2020 •

edited

Loading

thiagonache commented Oct 15, 2020

thiagonache commented Oct 15, 2020

thiagonache commented Oct 19, 2020 •

edited

Loading

ghost commented Nov 30, 2020

Deleting service default service accounts when creating a new project #4135

Deleting service default service accounts when creating a new project #4135

Comments

gabrielgbim commented Jul 30, 2019

Community Note

Description

New or Affected Resource(s)

Potential Terraform Configuration

References

rileykarson commented Feb 7, 2020

danawillow commented Mar 20, 2020

thiagonache commented Oct 8, 2020

morgante commented Oct 8, 2020

thiagonache commented Oct 8, 2020

rileykarson commented Oct 8, 2020

thiagonache commented Oct 8, 2020

morgante commented Oct 8, 2020

thiagonache commented Oct 8, 2020

thiagonache commented Oct 15, 2020 • edited Loading

thiagonache commented Oct 15, 2020

thiagonache commented Oct 15, 2020

thiagonache commented Oct 15, 2020

morgante commented Oct 15, 2020 • edited Loading

thiagonache commented Oct 15, 2020 • edited Loading

morgante commented Oct 15, 2020

rileykarson commented Oct 15, 2020 • edited Loading

morgante commented Oct 15, 2020

thiagonache commented Oct 15, 2020

thiagonache commented Oct 15, 2020

thiagonache commented Oct 15, 2020

morgante commented Oct 15, 2020

thiagonache commented Oct 15, 2020 • edited Loading

rileykarson commented Oct 15, 2020 • edited Loading

thiagonache commented Oct 15, 2020

thiagonache commented Oct 15, 2020

thiagonache commented Oct 19, 2020 • edited Loading

ghost commented Nov 30, 2020

thiagonache commented Oct 15, 2020 •

edited

Loading

morgante commented Oct 15, 2020 •

edited

Loading

thiagonache commented Oct 15, 2020 •

edited

Loading

rileykarson commented Oct 15, 2020 •

edited

Loading

thiagonache commented Oct 15, 2020 •

edited

Loading

rileykarson commented Oct 15, 2020 •

edited

Loading

thiagonache commented Oct 19, 2020 •

edited

Loading