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

Provider update changes security_and_analysis block #1466

Closed
jtgrohn opened this issue Jan 6, 2023 · 2 comments · Fixed by #1489
Closed

Provider update changes security_and_analysis block #1466

jtgrohn opened this issue Jan 6, 2023 · 2 comments · Fixed by #1489
Labels
Status: Up for grabs Issues that are ready to be worked on by anyone Type: Bug Something isn't working as documented

Comments

@jtgrohn
Copy link
Contributor

jtgrohn commented Jan 6, 2023

In addition to the public repo issues everyone has been commenting on in #1419 there is another issue for private/internal repos with updating the provider version. If a repo was created at/after the security_and_analysis block was introduced (e.g. at 5.9.0+), without the block explicitly set, and then is updated to a newer version (doesn't seem to matter which), the plan wants to remove the block. e.g.

Terraform will perform the following actions:

  # github_repository.repo will be updated in-place
  ~ resource "github_repository" "repo" {
        id                          = "security_and_analysis_test"
        name                        = "security_and_analysis_test"
      - vulnerability_alerts        = true -> null
        # (28 unchanged attributes hidden)

      - security_and_analysis {
          - advanced_security {
              - status = "enabled" -> null
            }

          - secret_scanning {
              - status = "enabled" -> null
            }

          - secret_scanning_push_protection {
              - status = "enabled" -> null
            }
        }
    }

Plan: 0 to add, 1 to change, 0 to destroy.
Steps to reproduce

create Terraform config:

provider "github" {
  token = "foo"
  owner = "bar"
}

terraform {

  required_providers {
    github = {
      source  = "integrations/github"
      version = "5.9.0"
    }
  }

  backend "local" {}
}

resource "github_repository" "repo" {
    name       = "security_and_analysis_test"
    visibility = "internal"
}}

apply the config
then update provider version in terraform config (no repository config changes), so the resulting config is:

provider "github" {
  token = "foo"
  owner = "bar"
}

terraform {
  required_providers {
    github = {
      source  = "integrations/github"
      version = "5.10.0"
    }
  }

  backend "local" {}
}

resource "github_repository" "repo" {
    name       = "security_and_analysis_test"
    visibility = "internal"
}

and plan again
the resulting plan tries to remove the security_and_analysis block (see above).

Originally posted by @jtgrohn in #1419 (comment)

@kfcampbell kfcampbell added Type: Bug Something isn't working as documented Status: Up for grabs Issues that are ready to be worked on by anyone Priority: Normal labels Jan 6, 2023
@kfcampbell
Copy link
Member

My guess is that we're missing a migration function here, similar to this code.

@F21
Copy link
Contributor

F21 commented Jan 9, 2023

Not sure if related, but I am currently working on #1458 and my tests are currently failing as I need to create a repo as part of the tests.

This is the output from my failing test:

    testing.go:705: Step 0 error: After applying this step, the plan was not empty:

        DIFF:

        UPDATE: github_repository.test
          allow_auto_merge:                                                 "false" => "false"
          allow_merge_commit:                                               "true" => "true"
          allow_rebase_merge:                                               "true" => "true"
          allow_squash_merge:                                               "true" => "true"
          allow_update_branch:                                              "false" => "false"
          archived:                                                         "false" => "false"
          default_branch:                                                   "main" => "main"
          delete_branch_on_merge:                                           "false" => "false"
          description:                                                      "" => ""
          etag:                                                             "W/\"7909e6bcaf0c97d2e5edd4cc9b4f2003d37d6278c7f574824b1bb876176b4177\"" => "W/\"7909e6bcaf0c97d2e5edd4cc9b4f2003d37d6278c7f574824b1bb876176b4177\""
          full_name:                                                        "f21-gh-test/tf-acc-test-8wyck" => "f21-gh-test/tf-acc-test-8wyck"
          git_clone_url:                                                    "git://github.com/f21-gh-test/tf-acc-test-8wyck.git" => "git://github.com/f21-gh-test/tf-acc-test-8wyck.git"
          has_discussions:                                                  "false" => "false"
          has_downloads:                                                    "false" => "false"
          has_issues:                                                       "false" => "false"
          has_projects:                                                     "false" => "false"
          has_wiki:                                                         "false" => "false"
          homepage_url:                                                     "" => ""
          html_url:                                                         "https://github.com/f21-gh-test/tf-acc-test-8wyck" => "https://github.com/f21-gh-test/tf-acc-test-8wyck"
          http_clone_url:                                                   "https://github.com/f21-gh-test/tf-acc-test-8wyck.git" => "https://github.com/f21-gh-test/tf-acc-test-8wyck.git"
          id:                                                               "tf-acc-test-8wyck" => "tf-acc-test-8wyck"
          is_template:                                                      "false" => "false"
          merge_commit_message:                                             "PR_TITLE" => "PR_TITLE"
          merge_commit_title:                                               "MERGE_MESSAGE" => "MERGE_MESSAGE"
          name:                                                             "tf-acc-test-8wyck" => "tf-acc-test-8wyck"
          node_id:                                                          "R_kgDOIvk22A" => "R_kgDOIvk22A"
          pages.#:                                                          "0" => "0"
          private:                                                          "false" => "false"
          repo_id:                                                          "586757848" => "586757848"
          security_and_analysis.#:                                          "1" => "0"
          security_and_analysis.0.advanced_security.#:                      "1" => ""
          security_and_analysis.0.advanced_security.0.status:               "" => ""
          security_and_analysis.0.secret_scanning.#:                        "1" => ""
          security_and_analysis.0.secret_scanning.0.status:                 "disabled" => ""
          security_and_analysis.0.secret_scanning_push_protection.#:        "1" => ""
          security_and_analysis.0.secret_scanning_push_protection.0.status: "disabled" => ""
          squash_merge_commit_message:                                      "COMMIT_MESSAGES" => "COMMIT_MESSAGES"
          squash_merge_commit_title:                                        "COMMIT_OR_PR_TITLE" => "COMMIT_OR_PR_TITLE"
          ssh_clone_url:                                                    "git@github.com:f21-gh-test/tf-acc-test-8wyck.git" => "git@github.com:f21-gh-test/tf-acc-test-8wyck.git"
          svn_url:                                                          "https://github.com/f21-gh-test/tf-acc-test-8wyck" => "https://github.com/f21-gh-test/tf-acc-test-8wyck"
          template.#:                                                       "0" => "0"
          visibility:                                                       "public" => "public"
          vulnerability_alerts:                                             "false" => "false"



        STATE:

        github_actions_repository_oidc_subject_claim_customization_template.test:
          ID = tf-acc-test-8wyck
          provider = provider.github
          include_claim_keys.# = 3
          include_claim_keys.0 = repo
          include_claim_keys.1 = context
          include_claim_keys.2 = job_workflow_ref
          repository = tf-acc-test-8wyck
          use_default = false

          Dependencies:
            github_repository.test
        github_repository.test:
          ID = tf-acc-test-8wyck
          provider = provider.github
          allow_auto_merge = false
          allow_merge_commit = true
          allow_rebase_merge = true
          allow_squash_merge = true
          allow_update_branch = false
          archived = false
          default_branch = main
          delete_branch_on_merge = false
          description =
          etag = W/"7909e6bcaf0c97d2e5edd4cc9b4f2003d37d6278c7f574824b1bb876176b4177"
          full_name = f21-gh-test/tf-acc-test-8wyck
          git_clone_url = git://github.com/f21-gh-test/tf-acc-test-8wyck.git
          has_discussions = false
          has_downloads = false
          has_issues = false
          has_projects = false
          has_wiki = false
          homepage_url =
          html_url = https://github.com/f21-gh-test/tf-acc-test-8wyck
          http_clone_url = https://github.com/f21-gh-test/tf-acc-test-8wyck.git
          is_template = false
          merge_commit_message = PR_TITLE
          merge_commit_title = MERGE_MESSAGE
          name = tf-acc-test-8wyck
          node_id = R_kgDOIvk22A
          private = false
          repo_id = 586757848
          security_and_analysis.# = 1
          security_and_analysis.0.advanced_security.# = 1
          security_and_analysis.0.advanced_security.0.status =
          security_and_analysis.0.secret_scanning.# = 1
          security_and_analysis.0.secret_scanning.0.status = disabled
          security_and_analysis.0.secret_scanning_push_protection.# = 1
          security_and_analysis.0.secret_scanning_push_protection.0.status = disabled
          squash_merge_commit_message = COMMIT_MESSAGES
          squash_merge_commit_title = COMMIT_OR_PR_TITLE
          ssh_clone_url = git@github.com:f21-gh-test/tf-acc-test-8wyck.git
          svn_url = https://github.com/f21-gh-test/tf-acc-test-8wyck
          visibility = public
          vulnerability_alerts = false

It looks like there an issue with the security_and_analysis configuration for the repo that causes it to change during the test.

Edit: As a work around, I made the repos private during creation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Up for grabs Issues that are ready to be worked on by anyone Type: Bug Something isn't working as documented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants