fix: security_and_analysis.code_security bug#3527
Conversation
|
👋 Hi, and thank you for this contribution! This repo is maintained by GitHub and community members on a best-effort basis. We'll get to this as soon as we can. You can help us prioritize by joining the discussion on open issues and PRs, sharing details on the changes you need, and reviewing other contributions. 🤖 This is an automated message. |
deiga
left a comment
There was a problem hiding this comment.
Partial review.
Please add an acceptance testcase as well
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #3501, where the security_and_analysis.code_security block was declared in the github_repository schema but never wired into the read or write paths. As a result, code_security was never sent to the GitHub API on create/update and never read back into state, producing a permanent plan diff that never converged. The fix mirrors the existing handling for advanced_security.
Changes:
calculateSecurityAndAnalysisnow mapscode_securityinto theSecurityAndAnalysisrequest (write path).flattenSecurityAndAnalysisnow readsCodeSecurityfrom the API response into Terraform state when present (read path), consistent with the conditionaladvanced_securityhandling.- Adds three unit tests covering the write path, the read/flatten path, and a config round-trip.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
github/resource_github_repository.go |
Wires code_security into both the write (calculateSecurityAndAnalysis) and read (flattenSecurityAndAnalysis) paths, resolving the perpetual diff. |
github/resource_github_repository_test.go |
Adds unit tests for the calculate, flatten, and round-trip behavior of code_security. |
Verification notes: Confirmed against google/go-github v88.0.0 that SecurityAndAnalysis includes a CodeSecurity *CodeSecurity field and that CodeSecurity exposes Status *string, so GetCodeSecurity()/GetStatus() and the write assignment compile. Docs (docs/resources/repository.md) already document code_security.
Residual risk (non-blocking, pre-existing, outside this diff): Two sibling attributes declared in the same schema block — secret_scanning_ai_detection and secret_scanning_non_provider_patterns — remain unwired in both calculateSecurityAndAnalysis and flattenSecurityAndAnalysis, so they retain the same class of perpetual-diff bug this PR fixes for code_security. These are outside the scope of issue #3501 and the changed regions, so no inline comment is attached, but they may warrant a follow-up.
|
@deiga is this one of the parts of the repository resource that we planned to deprecate and replace with separate resources or data sources? |
|
@stevehipwell yep, though I haven't done a thorough analysis of feasibility yet |
Resolves #3501
Before the change?
security_and_analysis.code_securityattribute was defined in the schema but never wired into the calculateSecurityAndAnalysis or flattenSecurityAndAnalysis functions. This meant that setting code_security had no effect on the API call, and the API response for code_security was never read back into state. This caused perpetual diffs.After the change?
Pull request checklist
Does this introduce a breaking change?