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

unique_writer_identity default value change on google_logging_project_sink does not trigger a change on dependant usages of writer_identity attribute #16622

Closed
ferrastas opened this issue Nov 29, 2023 · 9 comments · Fixed by GoogleCloudPlatform/magic-modules#9561, hashicorp/terraform-provider-google-beta#6742 or #16776

Comments

@ferrastas
Copy link

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

1.6.3

Affected Resource(s)

  • google_logging_project_sink

Terraform Configuration Files

resource "google_pubsub_topic" "logging" {
  project = var.project_id
  name    = "logging-topic"
}

resource "google_logging_project_sink" "logging" {
  project     = var.project_id
  name        = "project-logging-sink"
  destination = "pubsub.googleapis.com/${google_pubsub_topic.logging.id}"
}

resource "google_project_iam_member" "logging" {
  project = var.project_id
  role    = "roles/pubsub.publisher"
  member  = google_logging_project_sink.logging.writer_identity
}

Debug Output

Panic Output

Expected Behavior

After upgrading the provider to 5.X and according to the documentation, the default value for unique_writer_identity changes from false to true, this implies a change on the attribute writer_identity from cloud-logs@system.gserviceaccount.com to service-<PROJECT_NUMBER>@gcp-sa-logging.iam.gserviceaccount.com.

And I expect that all usages of that attribute will be marked for recreation/modification.

Actual Behavior

The value for writer_identity is silently changed, dependant usages are not triggered and we got a permission issue on our end.

Kicking a new terraform plan detects the changes on that attribute and dependent resources are correctly marked for modification

Steps to Reproduce

  1. Apply the resources on the latest 4.x provider version available
  2. Upgrade the provider to 5.x
  3. Run terraform plan
    (IAM resources will not be present on the plan)

Important Factoids

References

@ferrastas ferrastas added the bug label Nov 29, 2023
@github-actions github-actions bot added forward/review In review; remove label to forward service/logging labels Nov 29, 2023
@ferrastas
Copy link
Author

For the record, I tried to manually taint the google_project_iam_member.logging resource and I got this error during apply:

Error: Provider produced inconsistent final plan
When expanding the plan for google_project_iam_member.logging to include new values learned so far during apply, provider "registry.terraform.io/hashicorp/google" produced an invalid new value for .member: was cty.StringVal("serviceAccount:cloud-logs@system.gserviceaccount.com"), but now cty.StringVal("serviceAccount:service-[project-id]@gcp-sa-logging.iam.gserviceaccount.com").

This is a bug in the provider, which should be reported in the provider's own issue tracker.

@ferrastas
Copy link
Author

I also tested to explicitly define unique_writer_identity = true, but same result, seems that writer_identity change never triggers a change when it is used.

@ferrastas
Copy link
Author

Screenshot 2023-11-29 at 13 25 47 To add more fun 😅 we flipped `unique_writer_identity` to use the old SA, but TF detected a change on the `writer_identity` with the wrong content

@edwardmedia edwardmedia self-assigned this Nov 29, 2023
@edwardmedia
Copy link
Contributor

@ferrastas your case is related to the known breaking change in v5.0.0.

logging: changed the default value of unique_writer_identity from false to true in google_logging_project_sink.

I guess there are not many options we have about this. Do you agree?

@ferrastas
Copy link
Author

@edwardmedia I'm aware of this BC and I already explained about it in the "expected behaviour" section. What I'm reporting is that writer_identity (https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/logging_project_sink#writer_identity) does not reflect the correct service account value until the next terraform plan.

I expect that if I set unique_writer_identity to true (or I rely on the new default) I will get service-<PROJECT_NUMBER>@gcp-sa-logging.iam.gserviceaccount.com when using google_logging_project_sink.foo.writer_identity but that's not the case.

@edwardmedia
Copy link
Contributor

edwardmedia commented Nov 29, 2023

@ferrastas I do see what you meant now. This issue is tricky as the issue is between google_logging_project_sink and google_project_iam_member. During the 1st apply, google_project_iam_member has no knowledge about the value of google_logging_project_sink.logging.writer_identity being updated.

There is little options do we have to fix the issue. Likely it requires a solution from Terraform core. Adding the label of upstream-terraform

@rileykarson
Copy link
Collaborator

@edwardmedia we can probably resolve this with a CustomizeDiff, actually. diff.SetNewComputed should let us taint the value of writer_identity when unique_writer_identity is updating from false -> true.

Is that something you'd be able to test out yourself? I'm looking into a different issue atm.


While we're there we could also call diff.ForceNew on false -> true, which is not possible:

Error: googleapi: Error 400: The sink is using a unique writerIdentity, which cannot be changed. Use uniqueWriterIdentity=true when updating this sink., badRequest

@ferrastas
Copy link
Author

@rileykarson +1, I just faced this other issue you mentioned when playing around with the flag 😭 .

Copy link

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.