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

security_and_analysis segment not applying properly #1487

Open
halversonea opened this issue Jan 15, 2023 · 9 comments
Open

security_and_analysis segment not applying properly #1487

halversonea opened this issue Jan 15, 2023 · 9 comments
Labels
Status: Up for grabs Issues that are ready to be worked on by anyone Type: Bug Something isn't working as documented Type: Support Any questions, information, or general needs around the SDK or GitHub APIs

Comments

@halversonea
Copy link

I believe I've found a bug in the provider registry.terraform.io/integrations/github.
I'm able to create repos successfully with the following code:

resource "github_repository" "code_store" {
  name        = var.repository_name
  description = var.repository_description
  auto_init   = true
  allow_squash_merge = true
  allow_merge_commit = false
  allow_rebase_merge = false
  delete_branch_on_merge = true
}

But then when I try to update them in any way I get the following error:

Error: PATCH https://github.office.COMPANY_NAME.com/api/v3/repos/OWNER_NAME/repo-test-eric1a: 422 Secret Scanning is always enabled for public repos []

So I tried including the following security_and_analysis segment:

resource "github_repository" "code_store" {
  name        = var.repository_name
  description = var.repository_description
  auto_init   = true
  allow_squash_merge = true
  allow_merge_commit = false
  allow_rebase_merge = false
  delete_branch_on_merge = true

  security_and_analysis {
    advanced_security {
      status = "enabled"
    }

    secret_scanning {
      status = "enabled"
    }

    secret_scanning_push_protection {
      status = "enabled"
    }
  }
}

But this results in the following error:

Error: PATCH https://github.office.COMPANY_NAME.com/api/v3/repos/OWNER_NAME/repo-test-eric3a: 422 Enabling advanced security is restricted by a policy []

So I attempted to force-disable the extra security settings:

resource "github_repository" "code_store" {
  name        = var.repository_name
  description = var.repository_description
  auto_init   = true
  allow_squash_merge = true
  allow_merge_commit = false
  allow_rebase_merge = false
  delete_branch_on_merge = true

  security_and_analysis {
    advanced_security {
      status = "disabled"
    }

    secret_scanning {
      status = "disabled"
    }

    secret_scanning_push_protection {
      status = "disabled"
    }
  }
}

But this again gives the following error:

Error: PATCH https://github.office.COMPANY_NAME.com/api/v3/repos/OWNER_NAME/repo-test-eric1a: 422 Secret Scanning is always enabled for public repos []

All of these attempts were done fresh with a destroy cleaning up everything before attempting the create again and this is when creating a public repo.

I'm using Terraform v1.3.7 and provider registry.terraform.io/integrations/github v5.14.0 though I was getting the same errors on earlier versions of both terraform and the github provider.

Any suggestions would be greatly appreciated.

@nickfloyd nickfloyd added Type: Bug Something isn't working as documented Type: Support Any questions, information, or general needs around the SDK or GitHub APIs labels Jan 17, 2023
@halversonea
Copy link
Author

For anyone else facing this issue - I suspect that some part of my company's GitHub Enterprise configuration conflicts with some changes GitHub made to security scanning behavior last year. I believe this is still an issue that needs to be addressed but I was able to move forward by ignoring changes to security_and_analysis.

resource "github_repository" "code_store" {
  name        = var.repository_name
  description = var.repository_description
  auto_init   = true
  allow_squash_merge = true
  allow_merge_commit = false
  allow_rebase_merge = false
  delete_branch_on_merge = true

  lifecycle {
    ignore_changes = [
      security_and_analysis,
    ]
  } 
}

@kfcampbell
Copy link
Member

@halversonea this should be resolved by #1489, which I'll release shortly. Please reopen this issue if it persists after the latest release!

@halversonea
Copy link
Author

halversonea commented Jan 18, 2023

I'm still getting the same error when trying to make a change in 5.15.0:

422 Secret Scanning is always enabled for public repos []

Strangely my workaround with the ignore lifecycle that worked once has also stopped working in 5.14.0 and 5.15.0, not sure why it would be inconsistent.

Editing to add:
I don't have permissions to re-open this issue. If I see it's still closed after a while I'll re-submit.

@kfcampbell kfcampbell reopened this Jan 18, 2023
@kfcampbell kfcampbell added Status: Up for grabs Issues that are ready to be worked on by anyone Priority: Normal labels Jan 18, 2023
@fgeronimo-panther
Copy link

fgeronimo-panther commented Jan 20, 2023

We also ran into issues on 5.15.0 and our workaround was to declare a dynamic block that states when repo visibility is private, set status for advanced security and if public repo, send nothing. Variables advanced security/secret_scanning/secret_scanning_push_protection are set to enabled by default.

security_and_analysis {
    dynamic "advanced_security" {
      for_each = (var.visibility == "private") ? [1] : []
      content {
        status = var.advanced_security
      }
    }
    secret_scanning {
      status = var.secret_scanning
    }
    secret_scanning_push_protection {
      status = var.secret_scanning_push_protection
    }
  }

@halversonea
Copy link
Author

@fgeronimo-panther Can you share what settings you're using for public repos? Every combination we've tried has an issue. I thought we had it fixed with that lifecycle rule but that has also failed except for that one initial success.

@dion-gionet
Copy link
Contributor

@halversonea Have you tried removing the advanced_security block from the security_and_analysis block since the latest update? This works on our end for public repos.

security_and_analysis {
    secret_scanning {
      status = "enabled"
    }

    secret_scanning_push_protection {
      status = "enabled"
    }
  }

@brettcurtis
Copy link

I still have this issue: https://github.com/osinfra-io/github-organization-management/actions/runs/4710976433/jobs/8355054613#step:7:131

If I keep running my workflow (plan/apply) a few times it eventually goes through.

Copy link

👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!

@github-actions github-actions bot added the Status: Stale Used by stalebot to clean house label Apr 20, 2024
@SafeEval
Copy link

I'm still interested in seeing a resolution to this issue. It's a sore spot in an otherwise great TF provider.

I think the root cause is that the github_repository resource is just too big and trying to do too much. The provider would be more flexible with less unexpected behavior if the security_and_analysis attribute were factored out and split into a few composable resources.

As an example, the aws_security_group resource used to be monolithic. Then the aws_vpc_security_group_egress_rule and aws_vpc_security_group_ingress_rule resources were added to make it composable. Now rules can be modified independently of the security group resource itself. A similar design approach would benefit the github_repository resource.

@github-actions github-actions bot removed the Status: Stale Used by stalebot to clean house label Apr 26, 2024
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 Type: Support Any questions, information, or general needs around the SDK or GitHub APIs
Projects
None yet
Development

No branches or pull requests

7 participants