fix(jobs): support secret regional and uuid, fix diff detection and update

Author: Gnoale

internal/services/jobs/helpers.go

@@ -1,6 +1,8 @@
 package jobs
 
 import (
+	"bytes"
+	"context"
 	"fmt"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -93,7 +95,7 @@ func flattenJobDefinitionCron(cron *jobs.CronSchedule) []any {
 
 type JobDefinitionSecret struct {
 	SecretReferenceID string
-	SecretID          string
+	SecretID          regional.ID
 	SecretVersion     string
 	File              string
 	Environment       string
@@ -119,7 +121,7 @@ func expandJobDefinitionSecret(i any) []JobDefinitionSecret {
 		}
 
 		secret := JobDefinitionSecret{
-			SecretID:      secretMap["secret_id"].(string),
+			SecretID:      regional.ExpandID(secretMap["secret_id"].(string)),
 			SecretVersion: secretMap["secret_version"].(string),
 			File:          file,
 			Environment:   env,
@@ -134,45 +136,132 @@ func expandJobDefinitionSecret(i any) []JobDefinitionSecret {
 	return parsedSecrets
 }
 
-func CreateJobDefinitionSecret(rawSecretReference any, api *jobs.API, region scw.Region, jobID string) error {
-	parsedSecretReferences := expandJobDefinitionSecret(rawSecretReference)
-	secrets := []*jobs.CreateJobDefinitionSecretsRequestSecretConfig{}
+func flattenJobDefinitionSecret(jobSecrets []*jobs.Secret) []any {
+	secretRefs := make([]interface{}, len(jobSecrets))
 
-	for _, parsedSecretRef := range parsedSecretReferences {
-		var secretConfig *jobs.CreateJobDefinitionSecretsRequestSecretConfig
+	for i, secret := range jobSecrets {
+		secretRef := make(map[string]interface{})
+		secretRef["secret_id"] = secret.SecretManagerID
+		secretRef["secret_reference_id"] = secret.SecretID
+		secretRef["secret_version"] = secret.SecretManagerVersion
 
-		if parsedSecretRef.Environment != "" {
-			secretConfig = &jobs.CreateJobDefinitionSecretsRequestSecretConfig{
-				SecretManagerID:      parsedSecretRef.SecretID,
-				SecretManagerVersion: parsedSecretRef.SecretVersion,
-				EnvVarName:           &parsedSecretRef.Environment,
-			}
+		if secret.File != nil {
+			secretRef["file"] = secret.File.Path
 		}
 
-		if parsedSecretRef.File != "" {
-			secretConfig = &jobs.CreateJobDefinitionSecretsRequestSecretConfig{
-				SecretManagerID:      parsedSecretRef.SecretID,
-				SecretManagerVersion: parsedSecretRef.SecretVersion,
-				Path:                 &parsedSecretRef.File,
+		if secret.EnvVar != nil {
+			secretRef["environment"] = secret.EnvVar.Name
+		}
+
+		secretRefs[i] = secretRef
+	}
+
+	return secretRefs
+}
+
+func CreateJobDefinitionSecret(ctx context.Context, api *jobs.API, jobSecrets []JobDefinitionSecret, region scw.Region, jobID string) error {
+	secretConfigs := []*jobs.CreateJobDefinitionSecretsRequestSecretConfig{}
+
+	for _, parsedSecretRef := range jobSecrets {
+		secretConfig := &jobs.CreateJobDefinitionSecretsRequestSecretConfig{}
+
+		secretConfigs = append(secretConfigs, secretConfig)
+
+		if parsedSecretRef.SecretID.Region.String() != "" {
+			if parsedSecretRef.SecretID.Region.String() != region.String() {
+				return fmt.Errorf("the secret id %s is in the region %s, expected %s", parsedSecretRef.SecretID, parsedSecretRef.SecretID.Region, region)
 			}
 		}
 
-		if parsedSecretRef.Environment != "" && parsedSecretRef.File != "" {
-			return fmt.Errorf("the secret id %s must have exactly one mount point: file or environment", parsedSecretRef.SecretID)
+		secretConfig.SecretManagerID = parsedSecretRef.SecretID.ID
+		secretConfig.SecretManagerVersion = parsedSecretRef.SecretVersion
+
+		if err := validateJobDefinitionSecret(&parsedSecretRef); err != nil {
+			return err
 		}
 
-		if parsedSecretRef.Environment == "" && parsedSecretRef.File == "" {
-			return fmt.Errorf("the secret id %s is missing a mount point: file or environment", parsedSecretRef.SecretID)
+		if parsedSecretRef.Environment != "" {
+			secretConfig.EnvVarName = &parsedSecretRef.Environment
 		}
 
-		secrets = append(secrets, secretConfig)
+		if parsedSecretRef.File != "" {
+			secretConfig.Path = &parsedSecretRef.File
+		}
 	}
 
 	_, err := api.CreateJobDefinitionSecrets(&jobs.CreateJobDefinitionSecretsRequest{
 		Region:          region,
 		JobDefinitionID: jobID,
-		Secrets:         secrets,
-	})
+		Secrets:         secretConfigs,
+	}, scw.WithContext(ctx))
 
 	return err
 }
+
+func hashJobDefinitionSecret(secret *JobDefinitionSecret) int {
+	buf := bytes.NewBufferString(secret.SecretID.String())
+	buf.WriteString(secret.SecretVersion)
+
+	if secret.Environment != "" {
+		buf.WriteString(secret.Environment)
+	}
+
+	if secret.File != "" {
+		buf.WriteString(secret.File)
+	}
+
+	return schema.HashString(buf.String())
+}
+
+func DiffJobDefinitionSecrets(oldSecretRefs, newSecretRefs []JobDefinitionSecret) (toCreate []JobDefinitionSecret, toDelete []JobDefinitionSecret, err error) {
+	toCreate = make([]JobDefinitionSecret, 0)
+	toDelete = make([]JobDefinitionSecret, 0)
+
+	// hash the new and old secret sets
+	oldSecretRefsMap := make(map[int]JobDefinitionSecret, len(oldSecretRefs))
+	for _, secret := range oldSecretRefs {
+		oldSecretRefsMap[hashJobDefinitionSecret(&secret)] = secret
+	}
+
+	newSecretRefsMap := make(map[int]JobDefinitionSecret, len(newSecretRefs))
+
+	for _, secret := range newSecretRefs {
+		if err := validateJobDefinitionSecret(&secret); err != nil {
+			return toCreate, toDelete, err
+		}
+
+		newSecretRefsMap[hashJobDefinitionSecret(&secret)] = secret
+	}
+
+	// filter secrets to delete
+	for hash, secret := range oldSecretRefsMap {
+		if _, ok := newSecretRefsMap[hash]; !ok {
+			toDelete = append(toDelete, secret)
+		}
+	}
+
+	// filter secrets to create
+	for hash, secret := range newSecretRefsMap {
+		if _, ok := oldSecretRefsMap[hash]; !ok {
+			toCreate = append(toCreate, secret)
+		}
+	}
+
+	return toCreate, toDelete, nil
+}
+
+func validateJobDefinitionSecret(secret *JobDefinitionSecret) error {
+	if secret == nil {
+		return nil
+	}
+
+	if secret.Environment != "" && secret.File != "" {
+		return fmt.Errorf("the secret id %s must have exactly one mount point: file or environment", secret.SecretID)
+	}
+
+	if secret.Environment == "" && secret.File == "" {
+		return fmt.Errorf("the secret id %s is missing a mount point: file or environment", secret.SecretID)
+	}
+
+	return nil
+}

internal/services/jobs/definition.go renamed to internal/services/jobs/jobs.go

@@ -13,6 +13,7 @@ import (
 	"github.com/scaleway/scaleway-sdk-go/scw"
 	"github.com/scaleway/terraform-provider-scaleway/v2/internal/dsf"
 	"github.com/scaleway/terraform-provider-scaleway/v2/internal/httperrors"
+	"github.com/scaleway/terraform-provider-scaleway/v2/internal/locality"
 	"github.com/scaleway/terraform-provider-scaleway/v2/internal/locality/regional"
 	"github.com/scaleway/terraform-provider-scaleway/v2/internal/services/account"
 	"github.com/scaleway/terraform-provider-scaleway/v2/internal/types"
@@ -95,13 +96,22 @@ func ResourceDefinition() *schema.Resource {
 				Type:        schema.TypeSet,
 				Optional:    true,
 				Description: "A reference to a Secret Manager secret.",
+				Set: func(v interface{}) int {
+					secret := v.(map[string]interface{})
+					if secret["environment"] != "" {
+						return schema.HashString(locality.ExpandID(secret["secret_id"].(string)) + secret["secret_version"].(string) + secret["environment"].(string))
+					}
+
+					return schema.HashString(locality.ExpandID(secret["secret_id"].(string)) + secret["secret_version"].(string) + secret["file"].(string))
+				},
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"secret_id": {
-							Type:         schema.TypeString,
-							Description:  "The secret UUID.",
-							Required:     true,
-							ValidateFunc: validation.IsUUID,
+							Type:                  schema.TypeString,
+							Description:           "The secret unique identifier, it could be formatted as region/UUID or UUID. In case the region is passed, it must be the same as the job definition.",
+							Required:              true,
+							DiffSuppressOnRefresh: true,
+							DiffSuppressFunc:      dsf.Locality,
 						},
 						"secret_reference_id": {
 							Type:        schema.TypeString,
@@ -110,8 +120,9 @@ func ResourceDefinition() *schema.Resource {
 						},
 						"secret_version": {
 							Type:        schema.TypeString,
-							Description: "The secret version, default to Latest.",
-							Required:    true,
+							Description: "The secret version.",
+							Default:     "latest",
+							Optional:    true,
 						},
 						"file": {
 							Type:         schema.TypeString,
@@ -123,7 +134,7 @@ func ResourceDefinition() *schema.Resource {
 							Type:         schema.TypeString,
 							Optional:     true,
 							Description:  "An environment variable containing the secret value.",
-							ValidateFunc: validation.StringMatch(regexp.MustCompile(`^[A-Z]+(_[A-Z]+)*$`), "environment variable must be composed of uppercase letters separated by an underscore"),
+							ValidateFunc: validation.StringMatch(regexp.MustCompile(`^[A-Z|0-9]+(_[A-Z|0-9]+)*$`), "environment variable must be composed of uppercase letters separated by an underscore"),
 						},
 					},
 				},
@@ -171,7 +182,7 @@ func ResourceJobDefinitionCreate(ctx context.Context, d *schema.ResourceData, m
 	}
 
 	if rawSecretReference, ok := d.GetOk("secret_reference"); ok {
-		if err := CreateJobDefinitionSecret(rawSecretReference, api, region, definition.ID); err != nil {
+		if err := CreateJobDefinitionSecret(ctx, api, expandJobDefinitionSecret(rawSecretReference), region, definition.ID); err != nil {
 			return diag.FromErr(err)
 		}
 	}
@@ -209,25 +220,6 @@ func ResourceJobDefinitionRead(ctx context.Context, d *schema.ResourceData, m in
 		return diag.FromErr(err)
 	}
 
-	secretRefs := make([]interface{}, len(rawSecretRefs.Secrets))
-
-	for i, secret := range rawSecretRefs.Secrets {
-		secretRef := make(map[string]interface{})
-		secretRef["secret_id"] = secret.SecretManagerID
-		secretRef["secret_reference_id"] = secret.SecretID
-		secretRef["secret_version"] = secret.SecretManagerVersion
-
-		if secret.File != nil {
-			secretRef["file"] = secret.File.Path
-		}
-
-		if secret.EnvVar != nil {
-			secretRef["environment"] = secret.EnvVar.Name
-		}
-
-		secretRefs[i] = secretRef
-	}
-
 	_ = d.Set("name", definition.Name)
 	_ = d.Set("cpu_limit", int(definition.CPULimit))
 	_ = d.Set("memory_limit", int(definition.MemoryLimit))
@@ -239,7 +231,7 @@ func ResourceJobDefinitionRead(ctx context.Context, d *schema.ResourceData, m in
 	_ = d.Set("cron", flattenJobDefinitionCron(definition.CronSchedule))
 	_ = d.Set("region", definition.Region)
 	_ = d.Set("project_id", definition.ProjectID)
-	_ = d.Set("secret_reference", secretRefs)
+	_ = d.Set("secret_reference", flattenJobDefinitionSecret(rawSecretRefs.Secrets))
 
 	return nil
 }
@@ -304,21 +296,29 @@ func ResourceJobDefinitionUpdate(ctx context.Context, d *schema.ResourceData, m
 	}
 
 	if d.HasChange("secret_reference") {
-		oldRawSecretRefs, _ := d.GetChange("secret_reference")
+		oldRawSecretRefs, newRawSecretRefs := d.GetChange("secret_reference")
+
 		oldSecretRefs := expandJobDefinitionSecret(oldRawSecretRefs)
+		newSecretRefs := expandJobDefinitionSecret(newRawSecretRefs)
 
-		for _, oldSecretRef := range oldSecretRefs {
-			if err := api.DeleteJobDefinitionSecret(&jobs.DeleteJobDefinitionSecretRequest{
+		toCreate, toDelete, err := DiffJobDefinitionSecrets(oldSecretRefs, newSecretRefs)
+		if err != nil {
+			return diag.FromErr(err)
+		}
+
+		for _, secret := range toDelete {
+			deleteReq := &jobs.DeleteJobDefinitionSecretRequest{
 				Region:          region,
 				JobDefinitionID: id,
-				SecretID:        oldSecretRef.SecretReferenceID,
-			}); err != nil {
+				SecretID:        secret.SecretReferenceID,
+			}
+			if err := api.DeleteJobDefinitionSecret(deleteReq, scw.WithContext(ctx)); err != nil {
 				return diag.FromErr(err)
 			}
 		}
 
-		if rawSecretReference, ok := d.GetOk("secret_reference"); ok {
-			if err := CreateJobDefinitionSecret(rawSecretReference, api, region, id); err != nil {
+		if len(toCreate) > 0 {
+			if err := CreateJobDefinitionSecret(ctx, api, toCreate, region, id); err != nil {
 				return diag.FromErr(err)
 			}
 		}