From 16ecc0461104fd23867275783ee2b82f739f8723 Mon Sep 17 00:00:00 2001 From: Blake Pettersson Date: Wed, 30 Apr 2025 09:54:43 +0200 Subject: [PATCH 1/3] feat: add finalizers field Finalizers are a valid field for projects, apps and appsets. Fixes #391 and #404. Signed-off-by: Blake Pettersson --- .../resource_argocd_application_set_test.go | 60 +++++++++++++++++++ argocd/resource_argocd_application_test.go | 52 ++++++++++++++++ argocd/resource_argocd_project_test.go | 47 +++++++++++++++ argocd/schema_metadata.go | 6 ++ argocd/structure_metadata.go | 5 ++ docs/resources/application.md | 1 + docs/resources/application_set.md | 1 + docs/resources/project.md | 1 + 8 files changed, 173 insertions(+) diff --git a/argocd/resource_argocd_application_set_test.go b/argocd/resource_argocd_application_set_test.go index 4971b1c7..4527bb11 100644 --- a/argocd/resource_argocd_application_set_test.go +++ b/argocd/resource_argocd_application_set_test.go @@ -950,6 +950,31 @@ func TestAccArgoCDApplicationSet_syncPolicy(t *testing.T) { }) } +func TestAccArgoCDApplicationSet_finalizers(t *testing.T) { + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccPreCheckFeatureSupported(t, features.ApplicationSet) }, + ProviderFactories: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccArgoCDApplicationSet_finalizers(), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "argocd_application_set.finalizers", + "metadata.0.finalizers.0", + "argocd.argoproj.io/applicationset", + ), + ), + }, + { + ResourceName: "argocd_application_set.finalizers", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"metadata.0.resource_version"}, + }, + }, + }) +} + func TestAccArgoCDApplicationSet_syncPolicyWithApplicationsSyncPolicy(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { @@ -3083,6 +3108,41 @@ resource "argocd_application_set" "sync_policy" { }` } +func testAccArgoCDApplicationSet_finalizers() string { + return ` +resource "argocd_application_set" "finalizers" { + metadata { + name = "finalizers" + finalizers = ["argocd.argoproj.io/applicationset"] + } + + spec { + generator { + clusters {} # Automatically use all clusters defined within Argo CD + } + + template { + metadata { + name = "appset-finalizers-{{name}}" + } + + spec { + source { + repo_url = "https://github.com/argoproj/argocd-example-apps/" + target_revision = "HEAD" + path = "guestbook" + } + + destination { + server = "{{server}}" + namespace = "default" + } + } + } + } +}` +} + func testAccArgoCDApplicationSet_syncPolicyWithApplicationsSync() string { return ` resource "argocd_application_set" "applications_sync_policy" { diff --git a/argocd/resource_argocd_application_test.go b/argocd/resource_argocd_application_test.go index 7c03fd63..e49eebdb 100644 --- a/argocd/resource_argocd_application_test.go +++ b/argocd/resource_argocd_application_test.go @@ -760,6 +760,29 @@ func TestAccArgoCDApplication_EmptySyncPolicyBlock(t *testing.T) { }) } +func TestAccArgoCDApplication_Finalizers(t *testing.T) { + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProviderFactories: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccArgoCDApplication_finalizers(), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet( + "argocd_application.finalizers", + "metadata.0.uid", + ), + resource.TestCheckResourceAttr( + "argocd_application.finalizers", + "metadata.0.finalizers.0", + "finalizer.argocd.argoproj.io", + ), + ), + }, + }, + }) +} + func TestAccArgoCDApplication_NoAutomatedBlock(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -2416,6 +2439,35 @@ resource "argocd_application" "multiple_sources" { } }` } +func testAccArgoCDApplication_finalizers() string { + return ` +resource "argocd_application" "finalizers" { + metadata { + name = "finalizers" + namespace = "argocd" + finalizers = ["finalizer.argocd.argoproj.io"] + } + + spec { + project = "default" + + source { + repo_url = "https://raw.githubusercontent.com/bitnami/charts/archive-full-index/bitnami" + chart = "apache" + target_revision = "9.4.1" + } + + destination { + server = "https://kubernetes.default.svc" + namespace = "managed-namespace" + } + + sync_policy { + sync_options = ["CreateNamespace=true"] + } + } +}` +} func testAccArgoCDApplication_ManagedNamespaceMetadata() string { return ` diff --git a/argocd/resource_argocd_project_test.go b/argocd/resource_argocd_project_test.go index 67ebb7fa..06f1fa98 100644 --- a/argocd/resource_argocd_project_test.go +++ b/argocd/resource_argocd_project_test.go @@ -96,6 +96,25 @@ func TestAccArgoCDProject(t *testing.T) { ), ), }, + { + Config: testAccArgoCDProjectSimpleWithFinalizers(name), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet( + "argocd_project.simple", + "metadata.0.uid", + ), + resource.TestCheckResourceAttr( + "argocd_project.simple", + "metadata.0.finalizers.0", + "finalizer1", + ), + resource.TestCheckResourceAttr( + "argocd_project.simple", + "metadata.0.finalizers.1", + "finalizer2", + ), + ), + }, }, }) } @@ -401,6 +420,34 @@ func testAccArgoCDProjectSimpleWithoutOrphaned(name string) string { `, name) } +func testAccArgoCDProjectSimpleWithFinalizers(name string) string { + return fmt.Sprintf(` + resource "argocd_project" "simple" { + metadata { + name = "%s" + namespace = "argocd" + labels = { + acceptance = "true" + } + annotations = { + "this.is.a.really.long.nested.key" = "yes, really!" + } + finalizers = ["finalizer1", "finalizer2"] + } + + spec { + description = "simple project" + source_repos = ["*"] + + destination { + name = "anothercluster" + namespace = "bar" + } + } + } + `, name) +} + func testAccArgoCDProjectSimpleWithEmptyOrphaned(name string) string { return fmt.Sprintf(` resource "argocd_project" "simple" { diff --git a/argocd/schema_metadata.go b/argocd/schema_metadata.go index 658cfd33..86f275e7 100644 --- a/argocd/schema_metadata.go +++ b/argocd/schema_metadata.go @@ -65,5 +65,11 @@ func metadataFields(objectName string) map[string]*schema.Schema { Description: fmt.Sprintf("The unique in time and space value for this %s. More info: http://kubernetes.io/docs/user-guide/identifiers#uids", objectName), Computed: true, }, + "finalizers": { + Type: schema.TypeList, + Description: "List of finalizers for the resource.", + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, } } diff --git a/argocd/structure_metadata.go b/argocd/structure_metadata.go index 01e1a45a..92758a14 100644 --- a/argocd/structure_metadata.go +++ b/argocd/structure_metadata.go @@ -20,6 +20,10 @@ func expandMetadata(d *schema.ResourceData) (meta meta.ObjectMeta) { meta.Labels = expandStringMap(m["labels"].(map[string]interface{})) } + if v, ok := m["finalizers"].([]interface{}); ok && len(v) > 0 { + meta.Finalizers = expandStringList(v) + } + if v, ok := m["name"]; ok { meta.Name = v.(string) } @@ -37,6 +41,7 @@ func flattenMetadata(meta meta.ObjectMeta, d *schema.ResourceData) []interface{} "name": meta.Name, "namespace": meta.Namespace, "resource_version": meta.ResourceVersion, + "finalizers": meta.Finalizers, "uid": fmt.Sprintf("%v", meta.UID), } diff --git a/docs/resources/application.md b/docs/resources/application.md index 81d7939b..ba2ea669 100644 --- a/docs/resources/application.md +++ b/docs/resources/application.md @@ -193,6 +193,7 @@ resource "argocd_application" "multiple_sources" { Optional: - `annotations` (Map of String) An unstructured key value map stored with the applications.argoproj.io that may be used to store arbitrary metadata. More info: http://kubernetes.io/docs/user-guide/annotations +- `finalizers` (List of String) List of finalizers for the resource. - `labels` (Map of String) Map of string keys and values that can be used to organize and categorize (scope and select) the applications.argoproj.io. May match selectors of replication controllers and services. More info: http://kubernetes.io/docs/user-guide/labels - `name` (String) Name of the applications.argoproj.io, must be unique. Cannot be updated. More info: http://kubernetes.io/docs/user-guide/identifiers#names - `namespace` (String) Namespace of the applications.argoproj.io, must be unique. Cannot be updated. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/ diff --git a/docs/resources/application_set.md b/docs/resources/application_set.md index 52ba3427..78575062 100644 --- a/docs/resources/application_set.md +++ b/docs/resources/application_set.md @@ -578,6 +578,7 @@ resource "argocd_application_set" "progressive_sync" { Optional: - `annotations` (Map of String) An unstructured key value map stored with the applicationsets.argoproj.io that may be used to store arbitrary metadata. More info: http://kubernetes.io/docs/user-guide/annotations +- `finalizers` (List of String) List of finalizers for the resource. - `labels` (Map of String) Map of string keys and values that can be used to organize and categorize (scope and select) the applicationsets.argoproj.io. May match selectors of replication controllers and services. More info: http://kubernetes.io/docs/user-guide/labels - `name` (String) Name of the applicationsets.argoproj.io, must be unique. Cannot be updated. More info: http://kubernetes.io/docs/user-guide/identifiers#names - `namespace` (String) Namespace of the applicationsets.argoproj.io, must be unique. Cannot be updated. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/ diff --git a/docs/resources/project.md b/docs/resources/project.md index 26207ac4..969ecde5 100644 --- a/docs/resources/project.md +++ b/docs/resources/project.md @@ -151,6 +151,7 @@ resource "argocd_project" "myproject" { Optional: - `annotations` (Map of String) An unstructured key value map stored with the appprojects.argoproj.io that may be used to store arbitrary metadata. More info: http://kubernetes.io/docs/user-guide/annotations +- `finalizers` (List of String) List of finalizers for the resource. - `labels` (Map of String) Map of string keys and values that can be used to organize and categorize (scope and select) the appprojects.argoproj.io. May match selectors of replication controllers and services. More info: http://kubernetes.io/docs/user-guide/labels - `name` (String) Name of the appprojects.argoproj.io, must be unique. Cannot be updated. More info: http://kubernetes.io/docs/user-guide/identifiers#names - `namespace` (String) Namespace of the appprojects.argoproj.io, must be unique. Cannot be updated. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/ From e739d9ac6d8d3a86f6c3b29f67ca356e9c53f717 Mon Sep 17 00:00:00 2001 From: Blake Pettersson Date: Sun, 29 Jun 2025 19:50:27 +0200 Subject: [PATCH 2/3] fix: finalizer additions * Add existing finalizers when updating an application * Filter out non-user configured finalizers from tf state Signed-off-by: Blake Pettersson --- argocd/resource_argocd_application.go | 14 +++++++ argocd/structure_metadata.go | 58 ++++++++++++++++++++++++++- argocd/structure_metadata_test.go | 58 +++++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 1 deletion(-) diff --git a/argocd/resource_argocd_application.go b/argocd/resource_argocd_application.go index 41cfd998..793edc27 100644 --- a/argocd/resource_argocd_application.go +++ b/argocd/resource_argocd_application.go @@ -305,6 +305,20 @@ func resourceArgoCDApplicationUpdate(ctx context.Context, d *schema.ResourceData } } + // Check if resource is being deleted to prevent updates during deletion + if apps.Items[0].DeletionTimestamp != nil { + return []diag.Diagnostic{ + { + Severity: diag.Error, + Summary: fmt.Sprintf("cannot update application %s: resource is being deleted", objectMeta.Name), + Detail: "The application has a deletion timestamp and is in the process of being deleted. Updates are not allowed during deletion.", + }, + } + } + + // Use safer metadata expansion that preserves system finalizers + objectMeta = expandMetadataForUpdate(d, apps.Items[0].ObjectMeta) + validate := d.Get("validate").(bool) if _, err = si.ApplicationClient.Update(ctx, &applicationClient.ApplicationUpdateRequest{ Application: &application.Application{ diff --git a/argocd/structure_metadata.go b/argocd/structure_metadata.go index 92758a14..995bf2c6 100644 --- a/argocd/structure_metadata.go +++ b/argocd/structure_metadata.go @@ -35,13 +35,48 @@ func expandMetadata(d *schema.ResourceData) (meta meta.ObjectMeta) { return meta } +// expandMetadataForUpdate safely expands metadata for updates, merging user-configured +// finalizers with existing system finalizers to prevent accidental removal +func expandMetadataForUpdate(d *schema.ResourceData, existingMeta meta.ObjectMeta) (meta meta.ObjectMeta) { + meta = expandMetadata(d) + + // Merge finalizers: keep existing system finalizers, add/update user finalizers + if len(existingMeta.Finalizers) > 0 { + userFinalizers := make(map[string]bool) + for _, f := range meta.Finalizers { + userFinalizers[f] = true + } + + // Start with existing finalizers + merged := make([]string, 0, len(existingMeta.Finalizers)+len(meta.Finalizers)) + for _, existing := range existingMeta.Finalizers { + if userFinalizers[existing] { + // User explicitly configured this finalizer, keep it + merged = append(merged, existing) + delete(userFinalizers, existing) + } else { + // System finalizer not configured by user, preserve it + merged = append(merged, existing) + } + } + + // Add any new user finalizers + for finalizer := range userFinalizers { + merged = append(merged, finalizer) + } + + meta.Finalizers = merged + } + + return meta +} + func flattenMetadata(meta meta.ObjectMeta, d *schema.ResourceData) []interface{} { m := map[string]interface{}{ "generation": meta.Generation, "name": meta.Name, "namespace": meta.Namespace, "resource_version": meta.ResourceVersion, - "finalizers": meta.Finalizers, "uid": fmt.Sprintf("%v", meta.UID), } @@ -51,6 +86,9 @@ func flattenMetadata(meta meta.ObjectMeta, d *schema.ResourceData) []interface{} labels := d.Get("metadata.0.labels").(map[string]interface{}) m["labels"] = metadataRemoveInternalKeys(meta.Labels, labels) + finalizers := d.Get("metadata.0.finalizers").([]interface{}) + m["finalizers"] = metadataFilterFinalizers(meta.Finalizers, finalizers) + return []interface{}{m} } @@ -72,3 +110,21 @@ func metadataIsInternalKey(annotationKey string) bool { return strings.HasSuffix(u.Hostname(), "kubernetes.io") || annotationKey == "notified.notifications.argoproj.io" } + +func metadataFilterFinalizers(apiFinalizers []string, configuredFinalizers []interface{}) []string { + configured := make(map[string]bool) + for _, v := range configuredFinalizers { + if s, ok := v.(string); ok { + configured[s] = true + } + } + + result := make([]string, 0) + for _, finalizer := range apiFinalizers { + // Only include finalizers that were explicitly configured by the user + if configured[finalizer] { + result = append(result, finalizer) + } + } + return result +} diff --git a/argocd/structure_metadata_test.go b/argocd/structure_metadata_test.go index 9552e329..f23e6690 100644 --- a/argocd/structure_metadata_test.go +++ b/argocd/structure_metadata_test.go @@ -3,6 +3,8 @@ package argocd import ( "fmt" "testing" + + "github.com/stretchr/testify/require" ) func TestMetadataIsInternalKey(t *testing.T) { @@ -35,3 +37,59 @@ func TestMetadataIsInternalKey(t *testing.T) { }) } } + +func TestMetadataFilterFinalizers(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + apiFinalizers []string + configuredFinalizers []interface{} + expected []string + }{ + { + name: "empty lists", + apiFinalizers: []string{}, + configuredFinalizers: []interface{}{}, + expected: []string{}, + }, + { + name: "no configured finalizers", + apiFinalizers: []string{"system.finalizer", "user.finalizer"}, + configuredFinalizers: []interface{}{}, + expected: []string{}, + }, + { + name: "only configured finalizers returned", + apiFinalizers: []string{"system.finalizer", "user.finalizer", "another.user.finalizer"}, + configuredFinalizers: []interface{}{"user.finalizer", "another.user.finalizer"}, + expected: []string{"user.finalizer", "another.user.finalizer"}, + }, + { + name: "configured finalizer not in API response", + apiFinalizers: []string{"system.finalizer"}, + configuredFinalizers: []interface{}{"user.finalizer"}, + expected: []string{}, + }, + { + name: "mixed scenario - system and user finalizers", + apiFinalizers: []string{"resources.argoproj.io/finalizer", "user.custom/finalizer", "kubernetes.io/finalizer"}, + configuredFinalizers: []interface{}{"user.custom/finalizer"}, + expected: []string{"user.custom/finalizer"}, + }, + { + name: "invalid type in configured finalizers", + apiFinalizers: []string{"system.finalizer", "user.finalizer"}, + configuredFinalizers: []interface{}{"user.finalizer", 123, nil}, + expected: []string{"user.finalizer"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + result := metadataFilterFinalizers(tc.apiFinalizers, tc.configuredFinalizers) + require.Equal(t, tc.expected, result) + }) + } +} From f174a651644935597bcbb9243ab433d213129771 Mon Sep 17 00:00:00 2001 From: Blake Pettersson Date: Sun, 29 Jun 2025 19:59:45 +0200 Subject: [PATCH 3/3] chore: lint Signed-off-by: Blake Pettersson --- argocd/structure_metadata.go | 4 ++++ argocd/structure_metadata_test.go | 1 + 2 files changed, 5 insertions(+) diff --git a/argocd/structure_metadata.go b/argocd/structure_metadata.go index 995bf2c6..8d4adc57 100644 --- a/argocd/structure_metadata.go +++ b/argocd/structure_metadata.go @@ -49,6 +49,7 @@ func expandMetadataForUpdate(d *schema.ResourceData, existingMeta meta.ObjectMet // Start with existing finalizers merged := make([]string, 0, len(existingMeta.Finalizers)+len(meta.Finalizers)) + for _, existing := range existingMeta.Finalizers { if userFinalizers[existing] { // User explicitly configured this finalizer, keep it @@ -113,6 +114,7 @@ func metadataIsInternalKey(annotationKey string) bool { func metadataFilterFinalizers(apiFinalizers []string, configuredFinalizers []interface{}) []string { configured := make(map[string]bool) + for _, v := range configuredFinalizers { if s, ok := v.(string); ok { configured[s] = true @@ -120,11 +122,13 @@ func metadataFilterFinalizers(apiFinalizers []string, configuredFinalizers []int } result := make([]string, 0) + for _, finalizer := range apiFinalizers { // Only include finalizers that were explicitly configured by the user if configured[finalizer] { result = append(result, finalizer) } } + return result } diff --git a/argocd/structure_metadata_test.go b/argocd/structure_metadata_test.go index f23e6690..25aacfca 100644 --- a/argocd/structure_metadata_test.go +++ b/argocd/structure_metadata_test.go @@ -88,6 +88,7 @@ func TestMetadataFilterFinalizers(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { t.Parallel() + result := metadataFilterFinalizers(tc.apiFinalizers, tc.configuredFinalizers) require.Equal(t, tc.expected, result) })