fix: Fixed repository resource churn #2501
Conversation
stevehipwell
force-pushed
the
fix-repo-churn
branch
from
9d729e8 to
611ed13
Compare
stevehipwell
force-pushed
the
fix-repo-churn
branch
from
611ed13 to
1361601
Compare
| "vulnerability_alerts": { | ||
| Type: schema.TypeBool, | ||
| Optional: true, | ||
| Computed: true, |
There was a problem hiding this comment.
According to the documentation, "Computed is often used to represent values that are not user configurable or can not be known at time of terraform plan or apply, such as date of creation or a service specific UUID."
I suppose that Computed is appropriate here since it could be enabled by default at the org level.
There was a problem hiding this comment.
It is also required in combination with optional if an optional value will be persisted even if it's not been explicitly set.
| } | ||
| } | ||
|
|
||
| err := updateVulnerabilityAlerts(d, client, ctx, owner, repoName) |
There was a problem hiding this comment.
This seems wrong, the return resourceGithubRepositoryRead(d, meta) line below can end up calling updateVulnerabilityAlerts again. Is this change actually needed?
There was a problem hiding this comment.
This part of the TF provider is a bit of a mess, AFAIK 3 months later this is required.
|
|
||
| func updateVulnerabilityAlerts(d *schema.ResourceData, client *github.Client, ctx context.Context, owner, repoName string) error { | ||
| updateVulnerabilityAlerts := client.Repositories.DisableVulnerabilityAlerts | ||
| if vulnerabilityAlerts, ok := d.GetOk("vulnerability_alerts"); ok && vulnerabilityAlerts.(bool) { |
There was a problem hiding this comment.
Not new behavior, but this treats the absense of vulnerability_alerts = "true" as an alias for vulnerability_alerts = "false".
That is not true when a global org-level policy enables vulnerability alerts by default. Is this desirable behavior?
There was a problem hiding this comment.
@Lekensteyn this is a pattern used throughout the provider, the GetOk() function is intended to only respond if the value has been set.
Signed-off-by: Steve Hipwell <[email protected]>
stevehipwell
force-pushed
the
fix-repo-churn
branch
from
1361601 to
04417a7
Compare
|
@nickfloyd this would be a useful quality of life improvement. |
github-project-automation
bot
moved this from In Progress
to Done
in Terraform Provider
github-project-automation
bot
moved this from 👀 In review
to ✅ Done
in 🧰 Octokit Active
3 participants
Resolves #2489
Resolves #2495
Before the change?
github_repositoryresource churned onvulnerability_alertsgithub_repositoryresource churned on pages if build type was workflowAfter the change?
github_repositoryresourcePull request checklist
Does this introduce a breaking change?
Please see our docs on breaking changes to help!