-
Notifications
You must be signed in to change notification settings - Fork 741
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
[MAINT]: Public Repositories do not play well with Advanced Security settings #1419
Comments
We are having a similar issue when enabling this for a public repo, with the following configuration block -
We get the error:
I've tried setting status to null, and also tried removing the status ( |
Ideally, this sort of thing wouldn't throw a 422 at the API level. In the provider, perhaps an inelegant solution would be ignoring 422s for those specific requests. |
Hey @nickfloyd - could we rollback #1304 and release a new version? This unintentionally introduced a breaking change. By sending these values in API calls to public repos, GitHub rejects them. I think we should only add these fields to the API if they are explicitly supplied, or rollback and explore a different engineering option. For now, pinning to |
@sethvargo I'll take a look, I have some ideas of how to handle. |
Thank you @kuhlman-labs! I appreciate it. |
@sethvargo This PR should fix this issue: |
Just a small correction: unless I'm mistaken, the change was introduced in Edit: Actually, pinning |
Exactly @usmonster - we're facing the same issue as of right now. We're seeing two distinct errors for private and public repos accordingly (settings have only been enabled for public repos since this is a free feature. We're explicitly disabling them for private repos):
|
We went ahead and pinned |
@usmonster we don't pin to any specific API version in this provider, which as far as I'm aware with the recently-introduced API versioning means we automatically use the latest. I don't think the verison of the provider correlates with API version at all. |
I understand that everyone is probably just getting back from the holiday/end of year break but I'm wondering if there's an update for this issue/a fix being released soon. Like other have mentioned, we get the We are currently on v5.11.0 and are unable to revert back to v5.8.0 (which I understand works around this issue) due to some additional features we needed in v5.10.0+. To elaborate on those features, we needed to give a custom role to a team which, if I'm reading the release notes correctly, became available in v5.10.0 Because we're in this bind we are unfortunately stuck when it comes to creating new public repos. If there's something I'm missing which could help us out or there are more details I'm happy to hear suggestions or feedback. |
@miotke would you prefer to ignore 422 errors in the public repo case? What do you see as a path forward? |
Hi @kfcampbell, I saw that you mentioned this as a possible path forward above, it would probably resolve the immediate need. My only concern with ignoring an error is if there's something we actually need to be alerted of. Although if we can depend on GitHub (we probably can) to ensure that advanced security is always enabled for public repos it's probably not a huge concern. All that said, I'm not sure if this is an error worth ignoring or if we need to actually take action on it. |
@kuhlman-labs, I took a look at the PR but I don't think I can contribute any useful feedback since I don't know go or how this implementation works. If I am understanding that PR correctly the public repos will not include any advanced security options but GitHub will enforce it by default because it's a public repo? Sorry, I wish I was able to supply more useful feedback 😞. |
@miotke The logic of the PR basically changes the request that is sent to the API based on the repos visibility. Advanced Security is enabled on public repos by default so if you send an API request with that field you will get an error, but you can still control the other aspects of advanced security like push protection and secret scanning on public repos. |
In addition to the public repo issues everyone has been commenting on, there is another issue for private/internal repos with updating the provider version. If a repo was created at/after the 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 reproducecreate 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 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 |
@jtgrohn do you mind creating a new issue with those details? My guess is that we're missing a migration function, but I'd like to keep that separate from this issue. |
@usmonster did you have any problems with downgrading to v5.8.0? I am unable to downgrade because of this:
Strangely, the last time when I needed to downgrade the GH provider I didn't have this problem. |
Version |
No it doesn't. provider "github" {
token = "foo"
owner = "bar"
}
terraform {
required_providers {
github = {
source = "integrations/github"
version = "5.13.0"
}
}
}
resource "github_repository" "repo" {
name = "public_security_and_analysis"
visibility = "public"
security_and_analysis {
advanced_security {
status = "enabled"
}
secret_scanning {
status = "enabled"
}
secret_scanning_push_protection {
status = "disabled"
}
}
} still results in a github_repository.repo: Creating...
╷
│ Error: PATCH https://api.github.com/repos/bar/public_security_and_analysis: 422 Advanced security is always available for public repos []
│
│ with github_repository.repo,
│ on main.tf line 18, in resource "github_repository" "repo":
│ 18: resource "github_repository" "repo" {
│
╵ |
Describe the need
First of all, thank you very much for adding the GitHub Advanced Security settings in terraform, see #1104 . This has been a boon to our organization, and we've used it to track and add settings to every repository using a common setup module.
Most of our repositories are private, but one of them is public. I've tried a dynamic setting to remove the
advanced_security
block from the public repository, but now, trying to remove it, I get the message422 Advanced security is always available for public repos []
.I do want to keep other security settings, like "Secret scanning", without having a required block specifying advanced security for the public repo. Please advise if there's a better way to go about this, otherwise, let me know if this is something that can be adjusted in the
security_and_analysis
block.SDK Version
No response
API Version
No response
Relevant log output
No response
Code of Conduct
The text was updated successfully, but these errors were encountered: