Skip to content
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

Bugfix: Make security_and_analysis settings optional #1489

Merged
merged 4 commits into from
Jan 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions examples/repository_security_and_analysis/README.md
@@ -0,0 +1,18 @@
# Repository Visibility Example
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirming that executing the examples succeeds as planned.


This demos setting `security_and_analysis` for a repository. See https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-security-and-analysis-settings-for-your-repository for details on what these settings do.

This example will create a repositories in the specified `owner` organization. See https://www.terraform.io/docs/providers/github/index.html for details on configuring [`providers.tf`](./providers.tf) accordingly.

Alternatively, you may use variables passed via command line:

```console
export GITHUB_OWNER=
export GITHUB_TOKEN=
```

```console
terraform apply \
-var "owner=${GITHUB_OWNER}" \
-var "github_token=${GITHUB_TOKEN}"
```
19 changes: 19 additions & 0 deletions examples/repository_security_and_analysis/main.tf
@@ -0,0 +1,19 @@
resource "github_repository" "terraformed" {
name = "terraformed"
description = "A repository created by terraform"
visibility = "public"

security_and_analysis {
# Cannot set advanced_security for public repositories as it is always on by default.
# advanced_security {
# status = "enabled"
# }
secret_scanning {
status = "enabled"
}
secret_scanning_push_protection {
status = "enabled"
}
}
}

4 changes: 4 additions & 0 deletions examples/repository_security_and_analysis/outputs.tf
@@ -0,0 +1,4 @@
output "repository" {
description = "Example repository JSON blob"
value = github_repository.terraformed
}
12 changes: 12 additions & 0 deletions examples/repository_security_and_analysis/providers.tf
@@ -0,0 +1,12 @@
provider "github" {
owner = var.owner
token = var.github_token
}

terraform {
required_providers {
github = {
source = "integrations/github"
}
}
}
9 changes: 9 additions & 0 deletions examples/repository_security_and_analysis/variables.tf
@@ -0,0 +1,9 @@
variable "owner" {
description = "GitHub owner used to configure the provider"
type = string
}

variable "github_token" {
description = "GitHub access token used to configure the provider"
type = string
}
134 changes: 65 additions & 69 deletions github/resource_github_repository.go
Expand Up @@ -60,13 +60,14 @@ func resourceGithubRepository() *schema.Resource {
"security_and_analysis": {
Type: schema.TypeList,
Optional: true,
Computed: true,
MaxItems: 1,
Description: "Security and analysis settings for the repository. To use this parameter you must have admin permissions for the repository or be an owner or security manager for the organization that owns the repository.",
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"advanced_security": {
Type: schema.TypeList,
Required: true,
Optional: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
Expand All @@ -80,7 +81,7 @@ func resourceGithubRepository() *schema.Resource {
},
"secret_scanning": {
Type: schema.TypeList,
Required: true,
Optional: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
Expand All @@ -94,7 +95,7 @@ func resourceGithubRepository() *schema.Resource {
},
"secret_scanning_push_protection": {
Type: schema.TypeList,
Required: true,
Optional: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
Expand Down Expand Up @@ -352,6 +353,54 @@ func calculateVisibility(d *schema.ResourceData) string {
return "public"
}

func tryGetSecurityAndAnalysisSettingStatus(securityAndAnalysis map[string]interface{}, setting string) (bool, string) {
value, ok := securityAndAnalysis[setting]
if !ok {
return false, ""
}

asList := value.([]interface{})
if len(asList) == 0 || asList[0] == nil {
return false, ""
}

return true, asList[0].(map[string]interface{})["status"].(string)
}

func calculateSecurityAndAnalysis(d *schema.ResourceData) *github.SecurityAndAnalysis {
value, ok := d.GetOk("security_and_analysis")
if !ok {
return nil
}

asList := value.([]interface{})
if len(asList) == 0 || asList[0] == nil {
return nil
}

lookup := asList[0].(map[string]interface{})

var securityAndAnalysis github.SecurityAndAnalysis

if ok, status := tryGetSecurityAndAnalysisSettingStatus(lookup, "advanced_security"); ok {
securityAndAnalysis.AdvancedSecurity = &github.AdvancedSecurity{
Status: github.String(status),
}
}
if ok, status := tryGetSecurityAndAnalysisSettingStatus(lookup, "secret_scanning"); ok {
securityAndAnalysis.SecretScanning = &github.SecretScanning{
Status: github.String(status),
}
}
if ok, status := tryGetSecurityAndAnalysisSettingStatus(lookup, "secret_scanning_push_protection"); ok {
securityAndAnalysis.SecretScanningPushProtection = &github.SecretScanningPushProtection{
Status: github.String(status),
}
}

return &securityAndAnalysis
}

func resourceGithubRepositoryObject(d *schema.ResourceData) *github.Repository {
return &github.Repository{
Name: github.String(d.Get("name").(string)),
Expand Down Expand Up @@ -379,6 +428,7 @@ func resourceGithubRepositoryObject(d *schema.ResourceData) *github.Repository {
Archived: github.Bool(d.Get("archived").(bool)),
Topics: expandStringList(d.Get("topics").(*schema.Set).List()),
AllowUpdateBranch: github.Bool(d.Get("allow_update_branch").(bool)),
SecurityAndAnalysis: calculateSecurityAndAnalysis(d),
}
}

Expand Down Expand Up @@ -477,14 +527,6 @@ func resourceGithubRepositoryCreate(d *schema.ResourceData, meta interface{}) er
}
}

securityAndAnalysis := expandSecurityAndAnalysis(d.Get("security_and_analysis").([]interface{}))
if securityAndAnalysis != nil {
_, _, err := client.Repositories.Edit(ctx, owner, repoName, securityAndAnalysis)
if err != nil {
return err
}
}

return resourceGithubRepositoryUpdate(d, meta)
}

Expand Down Expand Up @@ -634,29 +676,6 @@ func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta interface{}) er
}
}

if d.HasChange("security_and_analysis") && !d.IsNewResource() {
opts := expandSecurityAndAnalysis(d.Get("security_and_analysis").([]interface{}))
if opts != nil {
_, _, err := client.Repositories.Edit(ctx, owner, repoName, opts)
if err != nil {
return err
}
} else { // disable security and analysis
_, _, err := client.Repositories.Edit(ctx, owner, repoName, &github.Repository{
SecurityAndAnalysis: &github.SecurityAndAnalysis{
AdvancedSecurity: &github.AdvancedSecurity{
Status: github.String("disabled")},
SecretScanning: &github.SecretScanning{
Status: github.String("disabled")},
SecretScanningPushProtection: &github.SecretScanningPushProtection{
Status: github.String("disabled")}},
})
if err != nil {
return err
}
}
}

if d.HasChange("topics") {
topics := repoReq.Topics
_, _, err = client.Repositories.ReplaceAllTopics(ctx, owner, *repo.Name, topics)
Expand Down Expand Up @@ -815,45 +834,22 @@ func flattenSecurityAndAnalysis(securityAndAnalysis *github.SecurityAndAnalysis)
return []interface{}{}
}

advancedSecurityMap := make(map[string]interface{})
advancedSecurityMap["status"] = securityAndAnalysis.GetAdvancedSecurity().GetStatus()

secretScanningMap := make(map[string]interface{})
secretScanningMap["status"] = securityAndAnalysis.GetSecretScanning().GetStatus()

secretScanningPushProtectionMap := make(map[string]interface{})
secretScanningPushProtectionMap["status"] = securityAndAnalysis.GetSecretScanningPushProtection().GetStatus()

securityAndAnalysisMap := make(map[string]interface{})
securityAndAnalysisMap["advanced_security"] = []interface{}{advancedSecurityMap}
securityAndAnalysisMap["secret_scanning"] = []interface{}{secretScanningMap}
securityAndAnalysisMap["secret_scanning_push_protection"] = []interface{}{secretScanningPushProtectionMap}

return []interface{}{securityAndAnalysisMap}
}

func expandSecurityAndAnalysis(input []interface{}) *github.Repository {
if len(input) == 0 || input[0] == nil {
return nil
}

securityAndAnalysis := input[0].(map[string]interface{})
update := &github.SecurityAndAnalysis{}

advancedSecurity := securityAndAnalysis["advanced_security"].([]interface{})[0].(map[string]interface{})
update.AdvancedSecurity = &github.AdvancedSecurity{
Status: github.String(advancedSecurity["status"].(string)),
advancedSecurity := securityAndAnalysis.GetAdvancedSecurity()
if advancedSecurity != nil {
securityAndAnalysisMap["advanced_security"] = []interface{}{map[string]interface{}{
"status": advancedSecurity.GetStatus(),
}}
}

secretScanning := securityAndAnalysis["secret_scanning"].([]interface{})[0].(map[string]interface{})
update.SecretScanning = &github.SecretScanning{
Status: github.String(secretScanning["status"].(string)),
}
securityAndAnalysisMap["secret_scanning"] = []interface{}{map[string]interface{}{
"status": securityAndAnalysis.GetSecretScanning().GetStatus(),
}}

secretScanningPushProtection := securityAndAnalysis["secret_scanning_push_protection"].([]interface{})[0].(map[string]interface{})
update.SecretScanningPushProtection = &github.SecretScanningPushProtection{
Status: github.String(secretScanningPushProtection["status"].(string)),
}
securityAndAnalysisMap["secret_scanning_push_protection"] = []interface{}{map[string]interface{}{
"status": securityAndAnalysis.GetSecretScanningPushProtection().GetStatus(),
}}

return &github.Repository{SecurityAndAnalysis: update}
return []interface{}{securityAndAnalysisMap}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason this can't be a concrete type here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A concrete type could be introduced so that there is one, but the flattenSecurityAndAnalysis function return type previously was []interface{}, so I kept it the same when I was refactoring it / making the properties optional.

I also removed the expandSecurityAndAnalysis function. The diff is lining the changes with flattenSecurityAndAnalysis up with the expandSecurityAndAnalysis removal.

}