Perma-diff with google_cloudbuildv2_repository github_config app_installation_id

[joe-a-t]

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 an 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 an issue is assigned to a user, that user is claiming responsibility for the issue. If an issue is assigned to hashibot, a community member has claimed the issue already.

Terraform Version

Terraform v1.3.9
on darwin_amd64
+ provider registry.terraform.io/hashicorp/google v4.59.0
+ provider registry.terraform.io/hashicorp/google-beta v4.59.0

Affected Resource(s)

• google_cloudbuildv2_repository

Terraform Configuration Files

resource "google_cloudbuildv2_connection" "my-connection" {
  provider = google-beta
  location = "us-west1"
  name = "my-connection"

  github_config {
    authorizer_credential {
      oauth_token_secret_version = google_secret_manager_secret_version.github-token-secret-version.id
    }
  }
}

Debug Output

Panic Output

Expected Behavior

Once connection is fully established, Terraform is happy and shows no changes.

Actual Behavior

Terraform shows this perma-diff

        ~ github_config {
          - app_installation_id = 123456789 -> null

            # (1 unchanged block hidden)
        }

which upon applying, fails with Error: Error updating Connection: operation received error: error code "3", message: this update would change the connection's installation state from COMPLETE to PENDING_INSTALL_APP, which is not allowed, details: []

This situation continues even if you terraform state rm and terraform import the resource. Per https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/cloudbuildv2_connection#app_installation_id, the app_installation_id should be optional so we should not need to include it (and it would be impossible to include it before creation since this value is a random ID that is created on the Google side).

The current work-around is to add

  lifecycle {
    ignore_changes = [
      github_config[0].app_installation_id,
    ]
  }

Steps to Reproduce

1. terraform apply to create the connection
2. terraform plan after the connection has been fully created

Important Factoids

References

• b/276794160

[edwardmedia]

looks like Computed needs to be added on app_installation_id

DCL resource

[vicpadilla]

Field app_installation_id needs to be specified in your terraform config. It's shown as optional just because at the API level (for other clients like gcloud) the field is optional and is then populated by manual installation steps.

When creating connections via terraform, you need to know both, the user's token and the installation ID. If you don't specify both, terraform will still work, but will create a connection in an incomplete state that you then need to finish configuring in the google cloud console (which defeats terraform's purpose).

The error you are getting comes from the API, as terraform is trying to update the connection removing the installation_id, and we don't allow that, as that would take the connection from a complete to an incomplete state (where again, you'd need to perform manual installation steps in the cloud console).

Your installation ID can be found in the URL of your Cloud Build GitHub App. Example, in the following URL, https://github.com/settings/installations/1234567, the installation ID is the numerical value 1234567. See more details at https://cloud.google.com/build/docs/automating-builds/github/connect-repo-github?generation=2nd-gen#connecting_a_github_host_programmatically

Also, note that terraform import will copy the state from the resource into your terraform state (tfstate file), but then you need to manually copy that into your terraform source code (usually main.tf). You can see the current state with the terraform show command.

[melinath]

@vicpadilla if this field has a complex API-side default it should probably use Optional + Computed rather than just optional. I'm not sure offhand how to do that in the DCL but it corresponds to default_from_api in MMv1, which is described in detail at https://googlecloudplatform.github.io/magic-modules/develop/resource/#add-fields.

If Optional + Computed doesn't make sense (for example if a user has to run a separate command to configure the field and then paste it into Terraform)... can this field actually meaningfully be configured with Terraform? If not, could make sense to consider converting it to an output-only field (as part of a major release).

At a minimum, it would be great to update the documentation for this field to better clarify how users are expected to use it.

[vicpadilla]

No, the field is not autopopulated by the service. It's more like the second case you mention: the user has to do something on the side to get that field populated. But that something-additional is not meant for terraform flows, it's meant for installations done via gcloud or the cloud console UI.

There is a way to meaningfully populate this field, and we explain that here https://cloud.google.com/build/docs/automating-builds/github/connect-repo-github?generation=2nd-gen#connecting_a_github_host_programmatically

I think an alternative is to make those fields required in DCL, even if they are not required at the API level. Not sure if that would be a breaking change.

[joe-a-t]

But that GitHub app installation 1) does not appear to be be possible to create in Terraform since I don't see a way in https://registry.terraform.io/providers/integrations/github/latest/docs and 2) if there is an appropriate resource, in many cases would likely not be done in Terraform because it would be a significant amount of extra work getting and injecting a different set of credentials.

As a result, I would lean towards making it as easy as possible for people to ignore diffs here, even if that means there Cloud Build integration could be left in a state by Terraform where it is not fully set up and there is some manual Cloud Console UI intervention required to finish the connection with GitHub. I definitely agree with @melinath above that the bare minimum is updating documentation, but it would be highly preferable to have Terraform ignore changes to that value automatically if it is not set in the configuration block.

[melinath]

@vicpadilla thanks for explaining & re-linking those docs! Based on the issue body I thought that GCP was generating the app_installation_id, but it sounds like actually it's always manually configured on GitHub, and then the value is copied from GitHub to the Terraform configuration.

It also sounds like the github_config block doesn't really do anything unless app_installation_id is set. If that's correct, making the field required seems reasonable. That would be a breaking change - see https://googlecloudplatform.github.io/magic-modules/develop/make-a-breaking-change/ for details on how to make that change as part of the 5.0.0 release. (Though DCL-based resources are a little different.)

@joe-a-t I'm not sure I understand what you are asking for - do you want to have a Terraform resource to handle installing the Cloud Build Github App? If so that would be a request for GitHub's Terraform provider - we only handle interactions with GCP. Please let me know if I'm misunderstanding / missing anything. :-)

[vicpadilla]

But that GitHub app installation 1) does not appear to be be possible to create in Terraform since I don't see a way in https://registry.terraform.io/providers/integrations/github/latest/docs

That's correct, github does not offer a way to create installations via an API, neither via terraform. The installation process has to be done manually in a web browser. Then, having the installation ID, it's possible to use the github API. That is also how the github terraform provider works, see https://registry.terraform.io/providers/integrations/github/latest/docs#github-app-installation

[joe-a-t]

So my request is NOT to have a Terraform resource handling installing the Cloud Build GitHub App, I understand that request would have to go to GitHub and require changes in both their API and Terraform Provider.

I was more mentioning it to demonstrate that this process CANNOT be entirely set up in Terraform given the current capabilities. Right now, people can create both of those resources (the google_cloudbuildv2_connection and the GitHub App Installation) in either order. You can go do the manual stuff first in GitHub then create the GCP Cloud Build stuff or you can create the GCP Cloud Build stuff and then click buttons in the GCP UI to have it take you to GitHub and install the GitHub App.

I would even argue that it is easiest to do the GCP Cloud Build side first since that takes you right where you need to go for the GitHub side so it's easier to find the GitHub side needful. Additionally, if you do not specify the app_installation_id in Terraform, if the GitHub App Installation ever needs to be re-created (because the user who set it up left the organization or some kind of disaster recovery scenario), users who have left that app_installation_id out will not need to figure out how to make the change to the Terraform configuration and run it through all of their CICD processes to deploy said Terraform change with the new app_installation_id. Instead, users will be able to recover from that situation without Terraform configuration changes and by only doing the manual re-connection steps that the GCP UI will prompt them to do and is the minimal amount of work that would need to be done due to technical constraints on the GitHub side.

If you make the field required as @vicpadilla is suggesting, that would force all users to do the GitHub side manual intervention BEFORE they can apply their Terraform. And in that reinstallation scenario I mentioned above, that could cause substantial additional friction when recovering from some kind of disaster. For that reason, I am strongly against changing app_installation_id to a required field since that is the complete opposite of the change I was trying to see by opening this issue.

My request is for either

1. Change the provider to automatically ignore values that are set for app_installation_id if it is null in the current Terraform configuration
2. Improve the documentation to make the behavior of app_installation_id clearer.

Does that clarify things?

[vicpadilla]

If we allow the app_installation_id to not be specified in terraform, one could not create complete connections from terraform. One could only import existing connections.

Creating connections with all the info via terraform (even if gathering the info is non-trivial) is useful in various cases:

• For replicating the same configuration in multiple GCP projects
• Recovery: you accidentally deleted your GCP resources and need to recreate them. Hopefully the github App installation still exists, so you'll be able to reuse the same installation ID.
• Updating connections: suppose you uninstalled the github App and then installed it again (which gives you a different installation_id). Then you can go to your terraform file and update the app_installation_id and apply the new config.

I believe none of those cases could work if we ignore the diff on app_installation_id.

As for improving the docs for app_installation_id, we will do that.

[joe-a-t]

The current state today is that we do not have to specify the app_installation_id in Terraform. Today I can create a connection from Terraform without the app_installation_id. It will not be a completed connection in that it will show that it is in a pending state, but from the Terraform side it will apply successfully and there will be resources that then exist in the GCP UI that will help walk me through the GitHub steps to finish the connection creation.

And I completely agree that if someone already has and specifies the app_installation_id in Terraform, that can be very useful for them and the provider should not ignore that diff. I have never tried to propose or advocated for a situation where if the Terraform configuration says app_installation_id = 12345 but the API returns app_installation_id = 67890 then it would be ignored. I am only talking about the situation where the Terraform configuration is app_installation_id = null (or is not specified) and then the API returns app_installation_id = 12345. In this one scenario, I think the provider should handle it gracefully by being silent.

[rileykarson]

default_from_api (server_default in DCL internals, I think) seems like a solid here. That way, users can specify the field- in which case Terraform will manage the value- but if they don't it's assumed to be managed by the server / another tool.

An alternative would be to change our examples / docs to push users towards using lifecycle.ignore_changes. We'd need to do that if 0 was a valid value for the field, which it probably isn't (or an empty string if it's a string). default_from_api overloads what the zero value for the type does due to a limitation in the Terraform SDK (and in Terraform Core, in older versions).

Making the field required here doesn't seem appropriate here because a manual flow post-creation seems to be a pretty reasonable way to use the resource. Not only are there local flows where that may be desirable, but a company may build a marketplace-style interface to set up an initial configuration by a module & then have a human finalise that part of the installation.

[joe-a-t]

Yeah @rileykarson's idea of default_from_api sounds like exactly what I'm asking for. It also sounds like an easy change that could be rolled out in the next minor release without any real disruptions?

[vicpadilla]

The main purpose of the Cloud Build repos integration with terraform (and of our Repos API itself) is allow users to programmatically connect their repositories and then use them for creating triggers, also programmatically.

There are two important cases that users should be able to perform in a reliable and reproducible way:

• Create connections and repositories using our guidance.
• Reuse those configs to recreate connections/repos in other projects

It's like "do it manually only the first time, and reproduce it programmatically (without manual intervention in a web browser) from that point on".

The fact that users can currently create an incomplete connection in terraform and then complete it in the Cloud Console is just a secondary effect of us leaving those fields as optional. However this is not the intended use for our terraform integration.

I think we should optimize for the main case, not for the secondary effect where users do some step in terraform and then go and click manually in the cloud console. There are other tools for that case: gcloud and the cloud console itself.

I think setting this value as server_default makes the primary usage error-prone: users could create incomplete configs that get completed via UI, and then their terraform plan would show that everything is okay. But then, when they try to reproduce that config in another project, or to re-create the same resources after some accidental deletion, turns out their terraform config would be useless because it wouldn't have all the necessary data (the installation_id was never added to the main.tf).

Another example where this can be more visible: for the github_enterprise_config case, only the host_uri field is required, all the other fields (private_key_secret_version, webhook_secret_secret_version, app_id, app_installation_id) are populated via installation steps followed in the cloud console. However, having a terraform config where a github_enterprise connection specifies only host_uri is far from useful, in terms of reusing that config to create more connections.

I think there is a gap in our terraform docs, where many details on how to get the fields like app_installation_id are missing. This takes users to confusing states where they don't realize that they need to replicate the installation_id in their main.tf file to get their resource definition match the current state. We should look at improving those docs either by adding descriptions to the examples if that is possible, or as code comments in the examples otherwise.

I don't feel strongly about making the fields required or not, as long as we can communicate somehow that the fields are needed for a fully functional connection.

[joe-a-t]

@rileykarson just wanted to bump this since it's been a month. Would it be possible to go ahead and implement that default_from_api? It sounds pretty simple and backwards compatible so I think it could be part of a minor release, but wanted to flag now in case if you'd prefer to have it be part of 5.0.0 since it is a bit of a behavior change.