Bug fixes to argocd_project_token + addition of renew_after (#245)

Author: onematchfox

argocd/provider.go

@@ -23,8 +23,6 @@ import (
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 )
 
-var apiClientConnOpts apiclient.ClientOptions
-
 // Used to handle concurrent access to ArgoCD common configuration
 var tokenMutexConfiguration = &sync.RWMutex{}
 
@@ -336,9 +334,6 @@ func initApiClient(ctx context.Context, d *schema.ResourceData) (apiClient apicl
 		}
 	}
 
-	// Export provider API client connections options for use in other spawned api clients
-	apiClientConnOpts = opts
-
 	authToken, authTokenOk := d.GetOk("auth_token")
 	switch authTokenOk {
 	case true:

argocd/resource_argocd_project_token.go

@@ -22,6 +22,77 @@ func resourceArgoCDProjectToken() *schema.Resource {
 		ReadContext:   resourceArgoCDProjectTokenRead,
 		UpdateContext: resourceArgoCDProjectTokenUpdate,
 		DeleteContext: resourceArgoCDProjectTokenDelete,
+		CustomizeDiff: func(ctx context.Context, d *schema.ResourceDiff, m interface{}) error {
+			ia := d.Get("issued_at").(string)
+			if ia == "" {
+				// Blank issued_at indicates a new token - nothing to do here
+				return nil
+			}
+
+			issuedAt, err := convertStringToInt64(ia)
+			if err != nil {
+				return fmt.Errorf("invalid issued_at: %w", err)
+			}
+
+			if ra, ok := d.GetOk("renew_after"); ok {
+				renewAfterDuration, err := time.ParseDuration(ra.(string))
+				if err != nil {
+					return fmt.Errorf("invalid renew_after: %w", err)
+				}
+
+				if time.Now().Unix()-issuedAt > int64(renewAfterDuration.Seconds()) {
+					// Token is older than renewAfterDuration - force recreation
+					if err := d.SetNewComputed("issued_at"); err != nil {
+						return fmt.Errorf("failed to force new resource on field %q: %w", "issued_at", err)
+					}
+
+					return nil
+				}
+			}
+
+			ea, ok := d.GetOk("expires_at")
+			if !ok {
+				return nil
+			}
+
+			expiresAt, err := convertStringToInt64(ea.(string))
+			if err != nil {
+				return fmt.Errorf("invalid expires_at: %w", err)
+			}
+
+			if expiresAt == 0 {
+				// Token not set to expire - no need to check anything else
+				return nil
+			}
+
+			if expiresAt < time.Now().Unix() {
+				// Token has expired - force recreation
+				if err := d.SetNewComputed("expires_at"); err != nil {
+					return fmt.Errorf("failed to force new resource on field %q: %w", "expires_at", err)
+				}
+
+				return nil
+			}
+
+			rb, ok := d.GetOk("renew_before")
+			if !ok {
+				return nil
+			}
+
+			renewBeforeDuration, err := time.ParseDuration(rb.(string))
+			if err != nil {
+				return fmt.Errorf("invalid renew_before: %w", err)
+			}
+
+			if expiresAt-time.Now().Unix() < int64(renewBeforeDuration.Seconds()) {
+				// Token will expire within renewBeforeDuration - force recreation
+				if err := d.SetNewComputed("issued_at"); err != nil {
+					return fmt.Errorf("failed to force new resource on field %q: %w", "issued_at", err)
+				}
+			}
+
+			return nil
+		},
 
 		Schema: map[string]*schema.Schema{
 			"project": {
@@ -43,9 +114,15 @@ func resourceArgoCDProjectToken() *schema.Resource {
 				ForceNew:     true,
 				ValidateFunc: validateDuration,
 			},
+			"renew_after": {
+				Type:         schema.TypeString,
+				Description:  "Duration to control token silent regeneration based on token age. Valid time units are `ns`, `us` (or `µs`), `ms`, `s`, `m`, `h`. If set, then the token will be regenerated if it is older than `renew_after`. I.e. if `currentDate - issued_at > renew_after`.",
+				Optional:     true,
+				ValidateFunc: validateDuration,
+			},
 			"renew_before": {
 				Type:         schema.TypeString,
-				Description:  "Duration to control token silent regeneration, valid time units are `ns`, `us` (or `µs`), `ms`, `s`, `m`, `h`. If `expires_in` is set, Terraform will regenerate the token if `expires_in - renew_before < currentDate`.",
+				Description:  "Duration to control token silent regeneration based on remaining token lifetime. If `expires_in` is set, Terraform will regenerate the token if `expires_at - currentDate < renew_before`. Valid time units are `ns`, `us` (or `µs`), `ms`, `s`, `m`, `h`.",
 				Optional:     true,
 				ValidateFunc: validateDuration,
 				RequiredWith: []string{"expires_in"},
@@ -138,6 +215,7 @@ func resourceArgoCDProjectTokenCreate(ctx context.Context, d *schema.ResourceDat
 		}
 
 		renewBefore := int64(renewBeforeDuration.Seconds())
+
 		if renewBefore > expiresIn {
 			return []diag.Diagnostic{
 				{
@@ -146,16 +224,6 @@ func resourceArgoCDProjectTokenCreate(ctx context.Context, d *schema.ResourceDat
 				},
 			}
 		}
-
-		// Arbitrary protection against misconfiguration
-		if 300 > expiresIn-renewBefore {
-			return []diag.Diagnostic{
-				{
-					Severity: diag.Error,
-					Summary:  "token will expire within 5 minutes, check your settings",
-				},
-			}
-		}
 	}
 
 	featureTokenIDSupported, err := server.isFeatureSupported(featureTokenIDs)
@@ -376,18 +444,6 @@ func resourceArgoCDProjectTokenRead(ctx context.Context, d *schema.ResourceData,
 		return nil
 	}
 
-	var expiresIn int64
-
-	var renewBefore int64
-
-	// TODO: Bug identified during refactoring/linting - is this really correct?
-	// Change introduced in https://github.com/oboukili/terraform-provider-argocd/pull/12/files
-	computedExpiresIn := expiresIn - renewBefore
-	if err = isValidToken(token, computedExpiresIn); err != nil {
-		d.SetId("")
-		return nil
-	}
-
 	if err = d.Set("issued_at", convertInt64ToString(token.IssuedAt)); err != nil {
 		return []diag.Diagnostic{
 			{
@@ -457,7 +513,7 @@ func resourceArgoCDProjectTokenUpdate(ctx context.Context, d *schema.ResourceDat
 		}
 	}
 
-	return resourceArgoCDProjectRead(ctx, d, meta)
+	return resourceArgoCDProjectTokenRead(ctx, d, meta)
 }
 
 func resourceArgoCDProjectTokenDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {

argocd/resource_argocd_project_token_test.go

@@ -2,7 +2,6 @@ package argocd
 
 import (
 	"fmt"
-	"math"
 	"math/rand"
 	"regexp"
 	"testing"
@@ -23,9 +22,6 @@ func TestAccArgoCDProjectToken(t *testing.T) {
 
 	count := 3 + rand.Intn(7)
 	expIn1 := expiresInDurationFunc(rand.Intn(100000))
-	expIn2 := expiresInDurationFunc(rand.Intn(100000))
-	expIn3 := expiresInDurationFunc(rand.Intn(100000))
-	expIn4 := expiresInDurationFunc(rand.Intn(100000))
 
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:          func() { testAccPreCheck(t) },
@@ -50,27 +46,6 @@ func TestAccArgoCDProjectToken(t *testing.T) {
 					int64(expIn1.Seconds()),
 				),
 			},
-			{
-				Config:      testAccArgoCDProjectTokenMisconfiguration(expIn2),
-				ExpectError: regexp.MustCompile("token will expire within 5 minutes, check your settings"),
-			},
-			{
-				Config: testAccArgoCDProjectTokenRenewBeforeSuccess(expIn3),
-				Check: resource.ComposeTestCheckFunc(
-					testCheckTokenExpiresAt(
-						"argocd_project_token.renew",
-						int64(expIn3.Seconds()),
-					),
-					resource.TestCheckResourceAttrSet(
-						"argocd_project_token.renew",
-						"renew_before",
-					),
-				),
-			},
-			{
-				Config:      testAccArgoCDProjectTokenRenewBeforeFailure(expIn4),
-				ExpectError: regexp.MustCompile("renew_before .* cannot be greater than expires_in .*"),
-			},
 			{
 				Config: testAccArgoCDProjectTokenMultiple(count),
 				Check: resource.ComposeTestCheckFunc(
@@ -96,6 +71,70 @@ func TestAccArgoCDProjectToken(t *testing.T) {
 	})
 }
 
+func TestAccArgoCDProjectToken_RenewBefore(t *testing.T) {
+	resourceName := "argocd_project_token.renew_before"
+
+	expiresInSeconds := 30
+	expiresIn := fmt.Sprintf("%ds", expiresInSeconds)
+	expiresInDuration, _ := time.ParseDuration(expiresIn)
+
+	renewBeforeSeconds := expiresInSeconds - 1
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:          func() { testAccPreCheck(t) },
+		ProviderFactories: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccArgoCDProjectTokenRenewBeforeSuccess(expiresIn, "20s"),
+				Check: resource.ComposeTestCheckFunc(
+					testCheckTokenExpiresAt(resourceName, int64(expiresInDuration.Seconds())),
+					resource.TestCheckResourceAttr(resourceName, "renew_before", "20s"),
+				),
+			},
+			{
+				Config: testAccArgoCDProjectTokenRenewBeforeSuccess(expiresIn, fmt.Sprintf("%ds", renewBeforeSeconds)),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "renew_before", fmt.Sprintf("%ds", renewBeforeSeconds)),
+					testDelay(renewBeforeSeconds+1),
+				),
+				ExpectNonEmptyPlan: true, // token should be recreated when refreshed at end of step due to delay above
+			},
+			{
+				Config:      testAccArgoCDProjectTokenRenewBeforeFailure(expiresInDuration),
+				ExpectError: regexp.MustCompile("renew_before .* cannot be greater than expires_in .*"),
+			},
+		},
+	})
+}
+
+func TestAccArgoCDProjectToken_RenewAfter(t *testing.T) {
+	resourceName := "argocd_project_token.renew_after"
+
+	renewAfterSeconds := 1
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:          func() { testAccPreCheck(t) },
+		ProviderFactories: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccArgoCDProjectTokenRenewAfter(renewAfterSeconds),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "renew_after", fmt.Sprintf("%ds", renewAfterSeconds)),
+					testDelay(renewAfterSeconds+1),
+				),
+				ExpectNonEmptyPlan: true,
+			},
+			{
+				Config: testAccArgoCDProjectTokenRenewAfter(renewAfterSeconds),
+				Check: resource.ComposeTestCheckFunc(
+					testDelay(renewAfterSeconds),
+				),
+				ExpectNonEmptyPlan: true, // token should be recreated when refreshed at end of step due to delay above
+			},
+		},
+	})
+}
+
 func testAccArgoCDProjectTokenSimple() string {
 	return `
 resource "argocd_project_token" "simple" {
@@ -144,29 +183,23 @@ resource "argocd_project_token" "multiple2b" {
 `, count, count, count, count)
 }
 
-func testAccArgoCDProjectTokenMisconfiguration(expiresInDuration time.Duration) string {
-	expiresIn := int64(expiresInDuration.Seconds())
-	renewBefore := expiresIn
-
+func testAccArgoCDProjectTokenRenewBeforeSuccess(expiresIn, renewBefore string) string {
 	return fmt.Sprintf(`
-resource "argocd_project_token" "renew" {
+resource "argocd_project_token" "renew_before" {
   project = "myproject1"
   role    = "test-role1234"
-  expires_in = "%ds"
-  renew_before = "%ds"
+  expires_in = "%s"
+  renew_before = "%s"
 }
 `, expiresIn, renewBefore)
 }
 
-func testAccArgoCDProjectTokenRenewBeforeSuccess(expiresInDuration time.Duration) string {
+func testAccArgoCDProjectTokenRenewBeforeFailure(expiresInDuration time.Duration) string {
 	expiresIn := int64(expiresInDuration.Seconds())
-	renewBefore := int64(math.Min(
-		expiresInDuration.Seconds()-1,
-		expiresInDuration.Seconds()-(rand.Float64()*expiresInDuration.Seconds()),
-	)) % int64(expiresInDuration.Seconds())
+	renewBefore := int64(expiresInDuration.Seconds() + 1.0)
 
 	return fmt.Sprintf(`
-resource "argocd_project_token" "renew" {
+resource "argocd_project_token" "renew_before" {
   project = "myproject1"
   role    = "test-role1234"
   expires_in = "%ds"
@@ -175,21 +208,15 @@ resource "argocd_project_token" "renew" {
 `, expiresIn, renewBefore)
 }
 
-func testAccArgoCDProjectTokenRenewBeforeFailure(expiresInDuration time.Duration) string {
-	expiresIn := int64(expiresInDuration.Seconds())
-	renewBefore := int64(math.Max(
-		expiresInDuration.Seconds()+1.0,
-		expiresInDuration.Seconds()+(rand.Float64()*expiresInDuration.Seconds()),
-	))
-
+func testAccArgoCDProjectTokenRenewAfter(renewAfter int) string {
 	return fmt.Sprintf(`
-resource "argocd_project_token" "renew" {
-  project = "myproject1"
-  role    = "test-role1234"
-  expires_in = "%ds"
-  renew_before = "%ds"
+resource "argocd_project_token" "renew_after" {
+  project     = "myproject1"
+  description = "auto-renewing long-lived token"
+  role        = "test-role1234"
+  renew_after = "%ds"
 }
-`, expiresIn, renewBefore)
+`, renewAfter)
 }
 
 func testCheckTokenIssuedAt(resourceName string) resource.TestCheckFunc {
@@ -205,12 +232,12 @@ func testCheckTokenIssuedAt(resourceName string) resource.TestCheckFunc {
 
 		_issuedAt, ok := rs.Primary.Attributes["issued_at"]
 		if !ok {
-			return fmt.Errorf("testCheckTokenExpiresAt: issued_at is not set")
+			return fmt.Errorf("testCheckTokenIssuedAt: issued_at is not set")
 		}
 
 		_, err := convertStringToInt64(_issuedAt)
 		if err != nil {
-			return fmt.Errorf("testCheckTokenExpiresAt: string attribute 'issued_at' stored in state cannot be converted to int64: %s", err)
+			return fmt.Errorf("testCheckTokenIssuedAt: string attribute 'issued_at' stored in state cannot be converted to int64: %s", err)
 		}
 
 		return nil
@@ -282,3 +309,10 @@ func testTokenIssuedAtSet(name string, count int) resource.TestCheckFunc {
 		return nil
 	}
 }
+
+func testDelay(seconds int) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		time.Sleep(time.Duration(seconds) * time.Second)
+		return nil
+	}
+}