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

Do not destroy the repository when you change the repository name #699

Merged
merged 5 commits into from
Apr 17, 2021

Conversation

k24dizzle
Copy link
Contributor

Inspired by the discussion in here: #616, the change for this was easier than I expected. I just made it so that changing the name field wouldn't force a new resource.

I tested this change locally using the following terraform config:

resource "github_repository" "test" {
  name        = "renamerepo"
  description = "testing renaming a repo"
  auto_init   = true
}

After planning and applying the config above, I went ahead and changed the config to:

resource "github_repository" "test" {
  name        = "renamerepo2"
  description = "testing renaming a repo"
  auto_init   = true
}

The following gist contains the resulting plan and apply of the provider before and after my changes.

I also went ahead and added a unit test for this:

    --- PASS: TestAccGithubRepositories/updates_a_repositories_name_without_error (10.03s)
        --- SKIP: TestAccGithubRepositories/updates_a_repositories_name_without_error/with_an_anonymous_account (0.00s)
        --- SKIP: TestAccGithubRepositories/updates_a_repositories_name_without_error/with_an_individual_account (0.00s)
        --- PASS: TestAccGithubRepositories/updates_a_repositories_name_without_error/with_an_organization_account (10.03s)

}

testCase := func(t *testing.T, mode string) {
resource.Test(t, resource.TestCase{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to add a check in here that the existing resource wasn't destroyed/a new resource wasn't created?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We manually verified that the rename operation did not cause a repository deletion by viewing audit logs for our test org:

CleanShot 2021-04-09 at 10 32 45@2x

Copy link
Contributor

@jcudit jcudit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this added test and some manual verification, this safe to be released for further testing in the community.

}

testCase := func(t *testing.T, mode string) {
resource.Test(t, resource.TestCase{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We manually verified that the rename operation did not cause a repository deletion by viewing audit logs for our test org:

CleanShot 2021-04-09 at 10 32 45@2x

@jcudit jcudit added this to the v4.9.0 milestone Apr 9, 2021
@jcudit jcudit merged commit fc0363a into integrations:master Apr 17, 2021
kfcampbell pushed a commit to kfcampbell/terraform-provider-github that referenced this pull request Jul 26, 2022
…tegrations#699)

* Don't make name a force new field for the github_repository resource

* Add a unit test

* fix up lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants