maint: Integrate v6.8.3 into main (#2902)

* fix: Add state migration for github_actions_organization_secret destroy_on_drift field

This addresses the regression introduced in v6.7.0 where existing
github_actions_organization_secret resources would show invalid state
for the new destroy_on_drift field.

Adds schema migration from v0 to v1 that sets destroy_on_drift=true
for existing resources that don't have this field.

Fixes #2804

* Add destroy-on-drift property to the GitHub Action Secret resource schema

* Add schema migration

* Fixes merge drift issues with both org secrets and actions secrets

* Fix InternalValidate error: Add Update function to github_actions_secret - Fixes the upgrade issue from v6.7.1 to v6.7.2.

* Add validation test to prevent regression of Update function requirement

* fix: Add ForceNew to destroy_on_drift and selected_repository_ids fields

* Reconcile the encrypted_value, plaintext_value in the read to unknown went dates don't match

* Removes the superfluous update func now that we are calcualting stored date diff in read

* test clean up

* fix: Correctly boxes max_file_size as an int and addresses the schema.Set type mismatch with restricted_file_extensions

* Linting fixes

* Updates how we handle secrets to address provider config issues

* Updates deprecated func in test

* Corrects docs, tests, and adds validation to max_file_size

* Update github/respository_rules_utils_test.go

Co-authored-by: Steve Hipwell <steve.hipwell@gmail.com>

* Update github/respository_rules_utils_test.go

Co-authored-by: Steve Hipwell <steve.hipwell@gmail.com>

* fix tests

---------

Co-authored-by: Steve Hipwell <steve.hipwell@gmail.com>

Author: nickfloyd

github/resource_github_actions_organization_secret.go

@@ -17,7 +17,6 @@ func resourceGithubActionsOrganizationSecret() *schema.Resource {
 	return &schema.Resource{
 		Create: resourceGithubActionsOrganizationSecretCreateOrUpdate,
 		Read:   resourceGithubActionsOrganizationSecretRead,
-		Update: resourceGithubActionsOrganizationSecretCreateOrUpdate,
 		Delete: resourceGithubActionsOrganizationSecretDelete,
 		Importer: &schema.ResourceImporter{
 			State: func(d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) {
@@ -61,8 +60,8 @@ func resourceGithubActionsOrganizationSecret() *schema.Resource {
 			"visibility": {
 				Type:             schema.TypeString,
 				Required:         true,
-				ValidateDiagFunc: validateValueFunc([]string{"all", "private", "selected"}),
 				ForceNew:         true,
+				ValidateDiagFunc: validateValueFunc([]string{"all", "private", "selected"}),
 				Description:      "Configures the access that repositories have to the organization secret. Must be one of 'all', 'private', or 'selected'. 'selected_repository_ids' is required if set to 'selected'.",
 			},
 			"selected_repository_ids": {
@@ -72,6 +71,7 @@ func resourceGithubActionsOrganizationSecret() *schema.Resource {
 				},
 				Set:         schema.HashInt,
 				Optional:    true,
+				ForceNew:    true,
 				Description: "An array of repository ids that can access the organization secret.",
 			},
 			"created_at": {
@@ -85,9 +85,11 @@ func resourceGithubActionsOrganizationSecret() *schema.Resource {
 				Description: "Date of 'actions_secret' update.",
 			},
 			"destroy_on_drift": {
-				Type:     schema.TypeBool,
-				Default:  true,
-				Optional: true,
+				Type:        schema.TypeBool,
+				Default:     true,
+				Optional:    true,
+				ForceNew:    true,
+				Description: "Boolean indicating whether to recreate the secret if it's modified outside of Terraform. When `true` (default), Terraform will delete and recreate the secret if it detects external changes. When `false`, Terraform will acknowledge external changes but not recreate the secret.",
 			},
 		},
 	}
@@ -171,12 +173,6 @@ func resourceGithubActionsOrganizationSecretRead(d *schema.ResourceData, meta an
 		return err
 	}
 
-	if err = d.Set("encrypted_value", d.Get("encrypted_value")); err != nil {
-		return err
-	}
-	if err = d.Set("plaintext_value", d.Get("plaintext_value")); err != nil {
-		return err
-	}
 	if err = d.Set("created_at", secret.CreatedAt.String()); err != nil {
 		return err
 	}
@@ -222,14 +218,35 @@ func resourceGithubActionsOrganizationSecretRead(d *schema.ResourceData, meta an
 	// timestamp we've persisted in the state. In that case, we can no longer
 	// trust that the value (which we don't see) is equal to what we've declared
 	// previously.
-	//
-	// The only solution to enforce consistency between is to mark the resource
-	// as deleted (unset the ID) in order to fix potential drift by recreating
-	// the resource.
 	destroyOnDrift := d.Get("destroy_on_drift").(bool)
-	if updatedAt, ok := d.GetOk("updated_at"); ok && destroyOnDrift && updatedAt != secret.UpdatedAt.String() {
+	storedUpdatedAt, hasStoredUpdatedAt := d.GetOk("updated_at")
+
+	if hasStoredUpdatedAt && storedUpdatedAt != secret.UpdatedAt.String() {
 		log.Printf("[INFO] The secret %s has been externally updated in GitHub", d.Id())
-		d.SetId("")
+
+		if destroyOnDrift {
+			// Original behavior: mark for recreation
+			d.SetId("")
+			return nil
+		} else {
+			// Alternative approach: set sensitive values to empty to trigger update plan
+			// This tells Terraform that the current state is unknown and needs reconciliation
+			if err = d.Set("encrypted_value", ""); err != nil {
+				return err
+			}
+			if err = d.Set("plaintext_value", ""); err != nil {
+				return err
+			}
+			log.Printf("[INFO] Detected drift but destroy_on_drift=false, clearing sensitive values to trigger update")
+		}
+	} else {
+		// No drift detected, preserve the configured values in state
+		if err = d.Set("encrypted_value", d.Get("encrypted_value")); err != nil {
+			return err
+		}
+		if err = d.Set("plaintext_value", d.Get("plaintext_value")); err != nil {
+			return err
+		}
 	}
 
 	// Always update the timestamp to prevent repeated drift detection

github/resource_github_actions_organization_secret_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
 func TestAccGithubActionsOrganizationSecret(t *testing.T) {
@@ -184,4 +185,108 @@ func TestAccGithubActionsOrganizationSecret(t *testing.T) {
 			testCase(t, organization)
 		})
 	})
+
+	// Unit tests for drift detection behavior
+	t.Run("destroyOnDrift false clears sensitive values instead of recreating", func(t *testing.T) {
+		originalTimestamp := "2023-01-01T00:00:00Z"
+		newTimestamp := "2023-01-02T00:00:00Z"
+
+		d := schema.TestResourceDataRaw(t, resourceGithubActionsOrganizationSecret().Schema, map[string]any{
+			"secret_name":      "test-secret",
+			"plaintext_value":  "original-value",
+			"encrypted_value":  "original-encrypted",
+			"visibility":       "private",
+			"destroy_on_drift": false,
+			"updated_at":       originalTimestamp,
+		})
+		d.SetId("test-secret")
+
+		// Simulate drift detection logic when destroy_on_drift is false
+		destroyOnDrift := d.Get("destroy_on_drift").(bool)
+		storedUpdatedAt, hasStoredUpdatedAt := d.GetOk("updated_at")
+
+		if hasStoredUpdatedAt && storedUpdatedAt != newTimestamp {
+			if destroyOnDrift {
+				// Would clear ID for recreation
+				d.SetId("")
+			} else {
+				// Should clear sensitive values to trigger update
+				_ = d.Set("encrypted_value", "")
+				_ = d.Set("plaintext_value", "")
+			}
+			_ = d.Set("updated_at", newTimestamp)
+		}
+
+		// Should NOT have cleared the ID when destroy_on_drift=false
+		if d.Id() == "" {
+			t.Error("Expected ID to be preserved when destroy_on_drift=false, but it was cleared")
+		}
+
+		// Should have cleared sensitive values to trigger update plan
+		if plaintextValue := d.Get("plaintext_value").(string); plaintextValue != "" {
+			t.Errorf("Expected plaintext_value to be cleared for update plan, got %s", plaintextValue)
+		}
+
+		if encryptedValue := d.Get("encrypted_value").(string); encryptedValue != "" {
+			t.Errorf("Expected encrypted_value to be cleared for update plan, got %s", encryptedValue)
+		}
+
+		// Should have updated the timestamp
+		if updatedAt := d.Get("updated_at").(string); updatedAt != newTimestamp {
+			t.Errorf("Expected timestamp to be updated to %s, got %s", newTimestamp, updatedAt)
+		}
+	})
+
+	t.Run("destroyOnDrift true still recreates resource on drift", func(t *testing.T) {
+		originalTimestamp := "2023-01-01T00:00:00Z"
+		newTimestamp := "2023-01-02T00:00:00Z"
+
+		d := schema.TestResourceDataRaw(t, resourceGithubActionsOrganizationSecret().Schema, map[string]any{
+			"secret_name":      "test-secret",
+			"plaintext_value":  "original-value",
+			"visibility":       "private",
+			"destroy_on_drift": true, // Explicitly set to true
+			"updated_at":       originalTimestamp,
+		})
+		d.SetId("test-secret")
+
+		// Simulate drift detection logic when destroy_on_drift is true
+		destroyOnDrift := d.Get("destroy_on_drift").(bool)
+		storedUpdatedAt, hasStoredUpdatedAt := d.GetOk("updated_at")
+
+		if hasStoredUpdatedAt && storedUpdatedAt != newTimestamp {
+			if destroyOnDrift {
+				// Should clear ID for recreation (original behavior)
+				d.SetId("")
+				return // Exit early like the real function would
+			}
+		}
+
+		// Should have cleared the ID for recreation when destroy_on_drift=true
+		if d.Id() != "" {
+			t.Error("Expected ID to be cleared for recreation when destroy_on_drift=true, but it was preserved")
+		}
+	})
+
+	t.Run("destroy_on_drift field defaults", func(t *testing.T) {
+		// Test that destroy_on_drift defaults to true for backward compatibility
+		schema := resourceGithubActionsOrganizationSecret().Schema["destroy_on_drift"]
+		if schema.Default != true {
+			t.Error("destroy_on_drift should default to true for backward compatibility")
+		}
+	})
+
+	t.Run("default destroy_on_drift is true", func(t *testing.T) {
+		d := schema.TestResourceDataRaw(t, resourceGithubActionsOrganizationSecret().Schema, map[string]any{
+			"secret_name":     "test-secret",
+			"plaintext_value": "test-value",
+			"visibility":      "private",
+			// destroy_on_drift not set, should default to true
+		})
+
+		destroyOnDrift := d.Get("destroy_on_drift").(bool)
+		if !destroyOnDrift {
+			t.Error("Expected destroy_on_drift to default to true")
+		}
+	})
 }

github/resource_github_actions_secret.go

@@ -18,7 +18,6 @@ func resourceGithubActionsSecret() *schema.Resource {
 	return &schema.Resource{
 		Create: resourceGithubActionsSecretCreateOrUpdate,
 		Read:   resourceGithubActionsSecretRead,
-		Update: resourceGithubActionsSecretCreateOrUpdate,
 		Delete: resourceGithubActionsSecretDelete,
 		Importer: &schema.ResourceImporter{
 			State: resourceGithubActionsSecretImport,
@@ -73,6 +72,7 @@ func resourceGithubActionsSecret() *schema.Resource {
 				Type:        schema.TypeBool,
 				Default:     true,
 				Optional:    true,
+				ForceNew:    true,
 				Description: "Boolean indicating whether to recreate the secret if it's modified outside of Terraform. When `true` (default), Terraform will delete and recreate the secret if it detects external changes. When `false`, Terraform will acknowledge external changes but not recreate the secret.",
 			},
 		},
@@ -144,12 +144,6 @@ func resourceGithubActionsSecretRead(d *schema.ResourceData, meta any) error {
 		return err
 	}
 
-	if err = d.Set("encrypted_value", d.Get("encrypted_value")); err != nil {
-		return err
-	}
-	if err = d.Set("plaintext_value", d.Get("plaintext_value")); err != nil {
-		return err
-	}
 	if err = d.Set("created_at", secret.CreatedAt.String()); err != nil {
 		return err
 	}
@@ -165,17 +159,36 @@ func resourceGithubActionsSecretRead(d *schema.ResourceData, meta any) error {
 	// timestamp we've persisted in the state. In that case, we can no longer
 	// trust that the value (which we don't see) is equal to what we've declared
 	// previously.
-	//
-	// The only solution to enforce consistency between is to mark the resource
-	// as deleted (unset the ID) in order to fix potential drift by recreating
-	// the resource.
 	destroyOnDrift := d.Get("destroy_on_drift").(bool)
-	if updatedAt, ok := d.GetOk("updated_at"); ok && destroyOnDrift && updatedAt != secret.UpdatedAt.String() {
+	storedUpdatedAt, hasStoredUpdatedAt := d.GetOk("updated_at")
+
+	if hasStoredUpdatedAt && storedUpdatedAt != secret.UpdatedAt.String() {
 		log.Printf("[INFO] The secret %s has been externally updated in GitHub", d.Id())
-		d.SetId("")
-	}
 
-	// Always update the timestamp to prevent repeated drift detection
+		if destroyOnDrift {
+			// Original behavior: mark for recreation
+			d.SetId("")
+			return nil
+		} else {
+			// Alternative approach: set sensitive values to empty to trigger update plan
+			// This tells Terraform that the current state is unknown and needs reconciliation
+			if err = d.Set("encrypted_value", ""); err != nil {
+				return err
+			}
+			if err = d.Set("plaintext_value", ""); err != nil {
+				return err
+			}
+			log.Printf("[INFO] Detected drift but destroy_on_drift=false, clearing sensitive values to trigger update")
+		}
+	} else {
+		// No drift detected, preserve the configured values in state
+		if err = d.Set("encrypted_value", d.Get("encrypted_value")); err != nil {
+			return err
+		}
+		if err = d.Set("plaintext_value", d.Get("plaintext_value")); err != nil {
+			return err
+		}
+	} // Always update the timestamp to prevent repeated drift detection
 	if err = d.Set("updated_at", secret.UpdatedAt.String()); err != nil {
 		return err
 	}