Bugfix: Make security_and_analysis settings optional (#1489)

* make advanced security property optional, defaulting to disabled

for public repositories
- default to
    - not setting advanced_security - as it is enabled by default and
      uneditable
    - disabling
        - secret scanning and
        - secret scanning push protection.

* add example repository security and analysis

* make security_and_analysis secret_scanning and secret_scanning_and_push_protection properties optional

Co-authored-by: Keegan Campbell <me@kfcampbell.com>

examples/repository_security_and_analysis/README.md

@@ -0,0 +1,18 @@
+# Repository Visibility Example
+
+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}"
+```

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"
+    }
+  }
+}
+

examples/repository_security_and_analysis/outputs.tf

@@ -0,0 +1,4 @@
+output "repository" {
+  description = "Example repository JSON blob"
+  value       = github_repository.terraformed
+}

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"
+    }
+  }
+}

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
+}

github/resource_github_repository.go

@@ -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{
@@ -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{
@@ -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{
@@ -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)),
@@ -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),
 	}
 }
 
@@ -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)
 }
 
@@ -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)
@@ -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}
 }