upgrade project module service agent issue

[Glasering]

Hey guys,
I'm trying to upgrade project module from version 29.0.0 up to 37.0.0 and i realized that it going to create some "Service Agents" that i didn't have so i added "service_agent_config" with "create_primary_agents = false" and "grant_default_roles = false" and i want to list the ones i want to create in "services_enabled" list, but it looks like it doesn't do nothing.
Is what i'm trying to do is possible that way?

service_agents_config = {
    create_primary_agents = false
    grant_default_roles   = false
    services_enabled = [
      "sqladmin.googleapis.com",
      "servicenetworking.googleapis.com",
      "pubsub.googleapis.com",
      "cloudbuild.googleapis.com",
      "artifactregistry.googleapis.com",
      "auditmanager.googleapis.com"
    ]
  }

[juliocc]

what do you mean it doesn't do anything? Can you share the plan and what you're expecting to see?

[Glasering]

this is the plan output:

# module.XXX.google_project_iam_member.service_agents["service-networking"] will be destroyed
  # (because key ["service-networking"] is not in for_each map)
  # (moved from module.XXXX.google_project_iam_member.servicenetworking[0])
  - resource "google_project_iam_member" "service_agents" {
      - etag    = "BwYgvqJW54s=" -> null
      - id      = "XXXX/roles/servicenetworking.serviceAgent/serviceAccount:[email protected]" -> null
      - member  = "serviceAccount:[email protected]" -> null
      - project = "XXXX" -> null
      - role    = "roles/servicenetworking.serviceAgent" -> null
    }

  # module.XXXX.google_project_service_identity.default["artifactregistry.googleapis.com"] will be destroyed
  # (because key ["artifactregistry.googleapis.com"] is not in for_each map)
  # (moved from module.XXXX.google_project_service_identity.jit_si["artifactregistry.googleapis.com"])
  - resource "google_project_service_identity" "default" {
      - email   = "[email protected]" -> null
      - id      = "projects/XXXX/services/artifactregistry.googleapis.com" -> null
      - project = "XXXX" -> null
      - service = "artifactregistry.googleapis.com" -> null
    }

  # module.XXXX.google_project_service_identity.default["cloudbuild.googleapis.com"] will be destroyed
  # (because key ["cloudbuild.googleapis.com"] is not in for_each map)
  # (moved from module.XXXX.google_project_service_identity.jit_si["cloudbuild.googleapis.com"])
  - resource "google_project_service_identity" "default" {
      - email   = "[email protected]" -> null
      - id      = "projects/XXXX/services/cloudbuild.googleapis.com" -> null
      - project = "XXXX" -> null
      - service = "cloudbuild.googleapis.com" -> null
    }

  # module.XXXX.google_project_service_identity.default["pubsub.googleapis.com"] will be destroyed
  # (because key ["pubsub.googleapis.com"] is not in for_each map)
  # (moved from module.XXXX.google_project_service_identity.jit_si["pubsub.googleapis.com"])
  - resource "google_project_service_identity" "default" {
      - email   = "[email protected]" -> null
      - id      = "projects/XXXX/services/pubsub.googleapis.com" -> null
      - project = "XXXX" -> null
      - service = "pubsub.googleapis.com" -> null
    }

  # module.XXXX.google_project_service_identity.default["servicenetworking.googleapis.com"] will be destroyed
  # (because key ["servicenetworking.googleapis.com"] is not in for_each map)
  # (moved from module.XXXX.google_project_service_identity.servicenetworking[0])
  - resource "google_project_service_identity" "default" {
      - email   = "[email protected]" -> null
      - id      = "projects/XXXX/services/servicenetworking.googleapis.com" -> null
      - project = "XXXX" -> null
      - service = "servicenetworking.googleapis.com" -> null
    }

  # module.XXXX.google_project_service_identity.default["sqladmin.googleapis.com"] will be destroyed
  # (because key ["sqladmin.googleapis.com"] is not in for_each map)
  # (moved from module.XXXX.google_project_service_identity.jit_si["sqladmin.googleapis.com"])
  - resource "google_project_service_identity" "default" {
      - email   = "[email protected]" -> null
      - id      = "projects/XXXX/services/sqladmin.googleapis.com" -> null
      - project = "XXXX" -> null
      - service = "sqladmin.googleapis.com" -> null
    }

my expectation is not to see any "DESTROYED"

[juliocc]

Looks like you already have some service agents. You probably want to keep create_primary_agents = true.

[Glasering]

When i'm doing create_primary_agents = true it still destroying "service-networking" and "cloudbuild.googleapis.com" and creates some other Service Agents from the project enabled services list.
here is the output:

  # module.XXXX.google_project_iam_member.service_agents["service-networking"] will be destroyed
  # (because key ["service-networking"] is not in for_each map)
  # (moved from module.XXXX.google_project_iam_member.servicenetworking[0])
  - resource "google_project_iam_member" "service_agents" {
      - etag    = "BwYDqoYg6hU=" -> null
      - id      = "XXXX/roles/servicenetworking.serviceAgent/serviceAccount:[email protected]" -> null
      - member  = "serviceAccount:[email protected]" -> null
      - project = "XXXX" -> null
      - role    = "roles/servicenetworking.serviceAgent" -> null
    }

  # module.XXXX.google_project_service_identity.jit_si["artifactregistry.googleapis.com"] has moved to module.XXXX.google_project_service_identity.default["artifactregistry.googleapis.com"]
    resource "google_project_service_identity" "default" {
        id      = "projects/XXXX/services/artifactregistry.googleapis.com"
        # (3 unchanged attributes hidden)
    }

  # module.XXXX.google_project_service_identity.default["cloudbuild.googleapis.com"] will be destroyed
  # (because key ["cloudbuild.googleapis.com"] is not in for_each map)
  # (moved from module.XXXX.google_project_service_identity.jit_si["cloudbuild.googleapis.com"])
  - resource "google_project_service_identity" "default" {
      - email   = "[email protected]" -> null
      - id      = "projects/XXXX/services/cloudbuild.googleapis.com" -> null
      - project = "XXXX" -> null
      - service = "cloudbuild.googleapis.com" -> null
    }

  # module.XXXX.google_project_service_identity.default["cloudkms.googleapis.com"] will be created
  + resource "google_project_service_identity" "default" {
      + email   = (known after apply)
      + id      = (known after apply)
      + member  = (known after apply)
      + project = "XXXX"
      + service = "cloudkms.googleapis.com"
    }

  # module.XXXX.google_project_service_identity.default["container.googleapis.com"] will be created
  + resource "google_project_service_identity" "default" {
      + email   = (known after apply)
      + id      = (known after apply)
      + member  = (known after apply)
      + project = "XXXX"
      + service = "container.googleapis.com"
    }

  # module.XXXX.google_project_service_identity.default["dns.googleapis.com"] will be created
  + resource "google_project_service_identity" "default" {
      + email   = (known after apply)
      + id      = (known after apply)
      + member  = (known after apply)
      + project = "XXXX"
      + service = "dns.googleapis.com"
    }

  # module.XXXX.google_project_service_identity.jit_si["pubsub.googleapis.com"] has moved to module.XXXX.google_project_service_identity.default["pubsub.googleapis.com"]
    resource "google_project_service_identity" "default" {
        id      = "projects/XXXX/services/pubsub.googleapis.com"
        # (3 unchanged attributes hidden)
    }

  # module.XXXX.google_project_service_identity.servicenetworking[0] has moved to module.XXXX.google_project_service_identity.default["servicenetworking.googleapis.com"]
    resource "google_project_service_identity" "default" {
        id      = "projects/XXXX/services/servicenetworking.googleapis.com"
        # (3 unchanged attributes hidden)
    }

  # module.XXXX.google_project_service_identity.jit_si["sqladmin.googleapis.com"] has moved to module.XXXX.google_project_service_identity.default["sqladmin.googleapis.com"]
    resource "google_project_service_identity" "default" {
        id      = "projects/XXXX/services/sqladmin.googleapis.com"
        # (3 unchanged attributes hidden)
    }

[juliocc]

The new service agents are WAI. Everything shown as moved is OK too.

The ones being destroyed are most likely because because of serice_agents_config.services_enabled. Try removing that from your configuration.

[Glasering]

Even when i removed serice_agents_config.services_enabled i got exactly the same plan.
The services list in serice_agents_config.services_enabled are part of the list in the services variable
I tried to use serice_agents_config.services_enabled because i thought that if I disabled the service_agents_config.create_primary_agents the ones in the serice_agents_config.services_enabled will be create

[Glasering]

I manged to gain what i want by editing the locals at module service-agents.tf
don't know if it suit you but this is what i did:

primary_service_agents = [
    for agent in local._project_service_agents : coalesce(agent.api, "cloudservices") if(
      var.service_agents_config.create_primary_agents
      ? (lookup(agent, "is_primary", true) && lookup(agent, "create_jit", true))
      : contains(var.service_agents_config.services_enabled, coalesce(agent.api, "cloudservices"))
    )
  ]

[juliocc]

There's nothing to fix. Creating all the service agents is default and intended behavior. If you don't want that, set create_primary_agents to false

[wiktorn]

Taking a second look at the plan above i see only one thing that would worry me:

  # module.XXXX.google_project_iam_member.service_agents["service-networking"] will be destroyed
  # (because key ["service-networking"] is not in for_each map)
  # (moved from module.XXXX.google_project_iam_member.servicenetworking[0])
  - resource "google_project_iam_member" "service_agents" {
      - etag    = "BwYDqoYg6hU=" -> null
      - id      = "XXXX/roles/servicenetworking.serviceAgent/serviceAccount:[email protected]" -> null
      - member  = "serviceAccount:[email protected]" -> null
      - project = "XXXX" -> null
      - role    = "roles/servicenetworking.serviceAgent" -> null
    }

As this removes the agent role from service agent, and if you leave it his way, things will get broken. You can remove this entry from state using terraform state rm , and your environment will be safe, but if this module is the only one that manages this project, I don't expect it to deploy cleanly, if you try to use this code to deploy a new project. Assuming that you have service-networking.googleapis.com in var.services, I'd assume that removing service-networking.googleapis.com from var.service_agents_config.services_enabled should actually fix this.

The other removal:

  # module.XXXX.google_project_service_identity.default["cloudbuild.googleapis.com"] will be destroyed
  # (because key ["cloudbuild.googleapis.com"] is not in for_each map)
  # (moved from module.XXXX.google_project_service_identity.jit_si["cloudbuild.googleapis.com"])
  - resource "google_project_service_identity" "default" {
      - email   = "[email protected]" -> null
      - id      = "projects/XXXX/services/cloudbuild.googleapis.com" -> null
      - project = "XXXX" -> null
      - service = "cloudbuild.googleapis.com" -> null
    }

will not affect running project, but may affect the correctness of provisioning of new project using this code. Does moving cloudbuild.googleapis.com from 'var.service_agents_config.services_enabledtovar.services` help here?

[Glasering]

My module runs over more then 20 projects and all the purpose of what i try to do here is to avoid touching the state.
I probably miss understood the meaning of the service_agents_config.services_enabled, i thought it for creating custom service agents that you want to create instead of creating for all the services.
networking.googleapis.com and cloudbuild.googleapis.com are already in the var.services.
Now i realized that even if i leave it to to create all the service agents, it still destroy the cloudbuild.googleapis.com

my services list:

    "accesscontextmanager.googleapis.com",
    "artifactregistry.googleapis.com",
    "bigquery.googleapis.com",
    "bigqueryreservation.googleapis.com",
    "bigquerystorage.googleapis.com",
    "billingbudgets.googleapis.com",
    "cloudbilling.googleapis.com",
    "cloudbuild.googleapis.com",
    "cloudkms.googleapis.com",
    "cloudresourcemanager.googleapis.com",
    "compute.googleapis.com",
    "container.googleapis.com",
    "dns.googleapis.com",
    "essentialcontacts.googleapis.com",
    "iam.googleapis.com",
    "iamcredentials.googleapis.com",
    "pubsub.googleapis.com",
    "servicenetworking.googleapis.com",
    "serviceusage.googleapis.com",
    "sourcerepo.googleapis.com",
    "sqladmin.googleapis.com",
    "stackdriver.googleapis.com",
    "storage-component.googleapis.com",
    "storage.googleapis.com",
    "sts.googleapis.com"

[wiktorn]

For destroy of module.XXXX.google_project_service_identity.default["cloudbuild.googleapis.com"], do not worry too much about this, as this is a no-op.

service_agents_config.services_enabled is a variable to tell the module, that those services are already enabled in the project, but you may want to use the module outputs to get those agent identities. In previous versions, there was no option to control this behavior separately, so module was creating outputs for all identities (and created identities for those, which were marked as such).

Currently, module is creating identities for all, and also grants default roles to all created agents.

Currently, you have the control, and you can use create_primary_agents = false to skip creation of identities, and grant_default_roles to skip default grants.

But as you are migrating from older codebase, I don't think that you should use those. And for Cloud Build, it looks like creation of the identity is just not required, so your plan that is removing this resource is expected, and this will not affect your environment (since this is a no-op).

[Glasering]

got it now, thanks