[MAINT]: Public Repositories do not play well with Advanced Security settings

[lbordowitz]

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 message 422 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

• I agree to follow this project's Code of Conduct

[davidkelliott]

We are having a similar issue when enabling this for a public repo, with the following configuration block -

  security_and_analysis {
    advanced_security {
      status = "enabled"
    }
    secret_scanning {
      status = "enabled"
    }
    secret_scanning_push_protection {
      status = "enabled"
    }
  }

We get the error:

Error: PATCH https://api.github.com/repos/ministryofjustice/modernisation-platform: 422 Advanced security is always available for public repos []

I've tried setting status to null, and also tried removing the status (advanced_security {})but neither are allowed.

[kfcampbell]

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.

[sethvargo]

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 5.10 is a workaround, but this is a fairly serious bug as it makes github_repository completely unusable in 5.11 and 5.12 and introduced a breaking change.

/cc @marzvrover @kfcampbell

[kuhlman-labs]

@sethvargo I'll take a look, I have some ideas of how to handle.

[kfcampbell]

Thank you @kuhlman-labs! I appreciate it.

[kuhlman-labs]

@sethvargo This PR should fix this issue:
#1431

[usmonster]

For now, pinning to 5.10 is a workaround, but this is a fairly serious bug as it makes github_repository completely unusable in 5.11 and 5.12 and introduced a breaking change.

Just a small correction: unless I'm mistaken, the change was introduced in 5.9.0, so the workaround should be to pin to 5.8.0. Hope this helps someone, and looking forward to the fix! 🙏

Edit: Actually, pinning 5.8.0 means we lose the fix provided in #1368 (released in 5.9.1 in response to GitHub API breaking changes), which means currently no version of the provider is usable by my team. 😓

[Luis-3M]

Exactly @usmonster - we're facing the same issue as of right now.
Currently on 5.10.0 of the provider with no way to look back or forward bc of what you've mentioned.

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):

• Private repos: 422 Advanced security has not been purchased []
• Public repos: 422 Advanced security is always available for public repos []

[usmonster]

We went ahead and pinned 5.8.0 since it seems the issue fixed by #1368 is apparently no longer present (maybe a fix on the API?). Hope this is a suitable workaround for some!

[kfcampbell]

@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.

[miotke]

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 422 Advanced security is always available for public repos [] error when trying to create a public repo.

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.

[kfcampbell]

@miotke would you prefer to ignore 422 errors in the public repo case? What do you see as a path forward?

[miotke]

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]

@miotke This PR fixes this issue, do you mind taking a look and giving feedback?
#1431

[miotke]

@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 😞.

[kuhlman-labs]

@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.

[jtgrohn]

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 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, 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).

[kfcampbell]

@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.

[bombelme]

@usmonster did you have any problems with downgrading to v5.8.0? I am unable to downgrade because of this:

╷
│ Error: Resource instance managed by newer provider version
│
│ The current state of module.repos["repo"].github_repository.this was created by a newer provider version than is currently selected. Upgrade the github provider to work with this state.
╵

Strangely, the last time when I needed to downgrade the GH provider I didn't have this problem.

[usmonster]

@bombelme, yes, we saw the same and "just" had to delete & re-import the state for github_repository resources. 🙃 Hope it helps a little while waiting for #1431 to land.

[gszakonyNitro]

Version 5.13.0 works without any issue.

[jtgrohn]

Version 5.13.0 works without any issue.

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 terraform apply failure of

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" {
│ 
╵