mage-os / terraform

Terraform files for managing the organization repository permissions.
4 stars 8 forks source link

fix: terraform attempts to change the CODEOWNERS on archived repos #54

Closed damienwebdev closed 1 year ago

damienwebdev commented 1 year ago

See: https://github.com/mage-os/terraform/actions/runs/4861725626/jobs/8667116974#step:6:440

sprankhub commented 1 year ago

That's a tricky one. So we archived one repository mage-os-website and Terraform still tries to update the CODEOWNERS file, which GitHub prevents, because the repository is archived. I thought we could just filter out archived repositories here:

https://github.com/mage-os/terraform/blob/379dff8d645a69ac3b4fd57afcacf67f1d62c7e5/main.tf#L161

That's possible by replacing it with the following:

  for_each = {
    for key, value in var.repositories : key => value if !try(value.archived, false)
  }

However, Terraform then tries to remove the CODEOWNERS file, which obviously fails as well for the same reason.

https://github.com/integrations/terraform-provider-github/issues/737 sounds like a related issue.

Does anyone with more Terraform experience has a good idea for this? @DavidLambauer @Jakski @mautz-et-tong

Jakski commented 1 year ago

I tried to achieve it with ignore_changes, but I've got the same error as described on forum - it seems that we can't set this attribute from variables. Without waiting for https://github.com/integrations/terraform-provider-github/issues/737 to resolve, we have at least 2 options:

sprankhub commented 1 year ago

I would vote for the first approach. Would you be willing to create a PR for this, @Jakski?

sprankhub commented 1 year ago

Any update on this, @Jakski? Thanks!

DavidLambauer commented 1 year ago

@sprankhub do we actually need the archived repos in the tf state? What about just removing them from the state and that's it?

sprankhub commented 1 year ago

@DavidLambauer, are you sure this actually works? Wouldn't Terraform then complain that there is a repository, which is not managed by Terraform?

Or what about just deleting that repository altogether? :joy:

DavidLambauer commented 1 year ago

If you delete the state, tf will most likely try to add it again. We’d need to drop it from the variables.tf as well.

sprankhub commented 1 year ago

Let's discuss how to handle this in the next tech meeting.

Jakski commented 1 year ago

Sorry for delay. https://github.com/mage-os/terraform/pull/64 should solve the issue.

If you delete the state, tf will most likely try to add it again. We’d need to drop it from the variables.tf as well.

Correct.

sprankhub commented 1 year ago

IMHO, the solution implemented by @Jakski is good for now.

Thanks, @Jakski!