- 
                Notifications
    You must be signed in to change notification settings 
- Fork 878
Update security_and_analysis settings only when there are changes #2397
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
          
     Merged
      
      
            kfcampbell
  merged 2 commits into
  integrations:main
from
jamestran201:update-repo-security-analysis-separately
  
      
      
   
  Oct 17, 2024 
      
    
                
     Merged
            
            Update security_and_analysis settings only when there are changes #2397
                    kfcampbell
  merged 2 commits into
  integrations:main
from
jamestran201:update-repo-security-analysis-separately
  
      
      
   
  Oct 17, 2024 
              
            Conversation
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
    The `resourceGithubRepositoryUpdate` function currently passes the current state of security_and_analysis to the PATCH /repo request. If the repo has an enforced security configuration, this can result in the API returning a 422 response because changing security settings is not allowed in this case. Even though the security_and_analysis settings remain the same, and only some other settings are changed, the request will still fail. To avoid the request failing when security_and_analysis remains the same, we will only pass it in the request payload when changes have been made.
      
        
      
      
  
    1 task
  
| Would love to see this shipped, it is a very annoying issue for our org | 
              
                    kfcampbell
  
              
              approved these changes
              
                  
                    Oct 17, 2024 
                  
              
              
            
            
    
  ihor-hrytskiv 
      pushed a commit
        to ihor-hrytskiv/terraform-provider-github
      that referenced
      this pull request
    
      Oct 28, 2024 
    
    
      
  
    
      
    
  
…tegrations#2397) The `resourceGithubRepositoryUpdate` function currently passes the current state of security_and_analysis to the PATCH /repo request. If the repo has an enforced security configuration, this can result in the API returning a 422 response because changing security settings is not allowed in this case. Even though the security_and_analysis settings remain the same, and only some other settings are changed, the request will still fail. To avoid the request failing when security_and_analysis remains the same, we will only pass it in the request payload when changes have been made. Co-authored-by: Keegan Campbell <[email protected]>
      
        
      
      
  
    1 task
  
  This was referenced Jan 28, 2025 
      
      
     Closed
  
    
  niklasfaschingmoia 
      added a commit
        to moia-oss/terraform-provider-github
      that referenced
      this pull request
    
      Jul 7, 2025 
    
    
      
  
    
      
    
  
Same issue as integrations#2397. Alternatively we could also just send only archived=true and not all fields (since we just ended management of the repo via terraform and thus care whether it matches the settings).
    
  niklasfaschingmoia 
      added a commit
        to moia-oss/terraform-provider-github
      that referenced
      this pull request
    
      Jul 8, 2025 
    
    
      
  
    
      
    
  
See integrations#2644 archive_on_destroy currently fails when the security_and_analysis repository configuration is controlled by an enterprise advanced security configuration [1]. │ Error: PATCH https://api.github.com/repos/foo/bar: 422 An enforced security configuration prevented modifying secret scanning enablement. Contact your organization owner for details. [] Since archive_on_destroy is only meant to archive the repository on destroy we don't actually need to send a patch request to sync the repository configuration to match terraform and we could just send a request that does that repoReq := &github.Repository{Archived: github.Bool(true)} but I guess that's a personal preference and for consistency let's just do what we already do in `resourceGithubRepositoryUpdate` [2] and filter out the SecurityAndAnalysis patches from the request when they didn't change. [1] https://docs.github.com/en/enterprise-cloud@latest/admin/managing-code-security/securing-your-enterprise/creating-a-custom-security-configuration-for-your-enterprise [2] integrations#2252 integrations#2397
  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment
  
      
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
The
resourceGithubRepositoryUpdatefunction currently passes the current state of security_and_analysis to the PATCH /repo request. If the repo has an enforced security configuration, this can result in the API returning a 422 response because changing security settings is not allowed in this case. Even though the security_and_analysis settings remain the same, and only some other settings are changed, the request will still fail.To avoid the request failing when security_and_analysis remains the same, we will only pass it in the request payload when changes have been made.
Resolves #2383
Before the change?
Currently, if a repository has Secret Scanning enabled. The payload in the PATCH /repository request will include this data, thus attempts to enable Secret Scanning on the repository again. This can happen even with this template that doesn't modify Secret Scanning
After the change?
The request payload for PATCH /repository will only contain the
security_and_analysisobject when security features are being modified. This means that the value for at least one of the features is being updated:advanced_security,secret_scanning,secret_scanning_push_protection,secret_scanning_non_provider_patterns,secret_scanning_validity_checks. By doing this, we will only attempt to modify these settings when a change is specified by the template.Pull request checklist
I didn't write an acceptance test for this change because the setup requires applying a security configuration to a repo. I tried the following manual test:
main.tfthat looks liketerraform applyallow_rebase_mergefromfalsetotruein the templateterraform applyagainignore_vulnerability_alerts_during_readis set for this test because the terraform state stored locally can be different from the repo's state after a security configuration is applied. This causes the update command to attempt changing Dependabot alerts enablement state, which will fail if the state is enforced by a security configuration.Does this introduce a breaking change?
Please see our docs on breaking changes to help!