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

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

Closed
damienwebdev opened this issue May 3, 2023 · 10 comments
Closed
Labels
bug Something isn't working

Comments

@damienwebdev
Copy link
Member

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

@damienwebdev damienwebdev added the bug Something isn't working label May 3, 2023
@sprankhub
Copy link
Contributor

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:

for_each = var.repositories

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.

integrations/terraform-provider-github#737 sounds like a related issue.

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

@Jakski
Copy link
Contributor

Jakski commented May 23, 2023

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 integrations/terraform-provider-github#737 to resolve, we have at least 2 options:

  • Ignore some changes for all CODEOWNERS resources. As far as I understand from failed pipeline, ignoring commit_author and commit_email might be sufficient. It means that it won't be updated automatically after initial creation, but on the other hand this resource doesn't overwrite existing commits. In case of changed commiter details it just created a new commit. CODEOWNERS content shouldn't change for archived repositories, so this should work.
  • Let Terraform fail resource deletion, but allow human to remove it from GitHub, e.g.: using a comment under issue. We use comments this way for plans already. It's not as great, since each time we archive repository someone would need to issue comment like /state rm 'github_repository_file.codeowners["mage-os-website"]'.

@sprankhub
Copy link
Contributor

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

@sprankhub
Copy link
Contributor

Any update on this, @Jakski? Thanks!

@DavidLambauer
Copy link
Contributor

@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
Copy link
Contributor

@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? 😂

@DavidLambauer
Copy link
Contributor

DavidLambauer commented Jun 28, 2023 via email

@sprankhub
Copy link
Contributor

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

@Jakski
Copy link
Contributor

Jakski commented Jun 30, 2023

Sorry for delay. #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
Copy link
Contributor

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

Thanks, @Jakski!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants