Skip to content

Commit 337c85e

Browse files
fix: finalizer additions
* Add existing finalizers when updating an application * Filter out non-user configured finalizers from tf state Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
1 parent ec4ab16 commit 337c85e

File tree

3 files changed

+132
-3
lines changed

3 files changed

+132
-3
lines changed

argocd/resource_argocd_application.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,13 @@ import (
1010

1111
"github.com/argoproj-labs/terraform-provider-argocd/internal/features"
1212
"github.com/argoproj-labs/terraform-provider-argocd/internal/provider"
13-
applicationClient "github.com/argoproj/argo-cd/v2/pkg/apiclient/application"
14-
application "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
1513
"github.com/argoproj/gitops-engine/pkg/health"
1614
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
1715
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
1816
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
17+
18+
applicationClient "github.com/argoproj/argo-cd/v2/pkg/apiclient/application"
19+
application "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
1920
)
2021

2122
func resourceArgoCDApplication() *schema.Resource {
@@ -305,6 +306,20 @@ func resourceArgoCDApplicationUpdate(ctx context.Context, d *schema.ResourceData
305306
}
306307
}
307308

309+
// Check if resource is being deleted to prevent updates during deletion
310+
if apps.Items[0].DeletionTimestamp != nil {
311+
return []diag.Diagnostic{
312+
{
313+
Severity: diag.Error,
314+
Summary: fmt.Sprintf("cannot update application %s: resource is being deleted", objectMeta.Name),
315+
Detail: "The application has a deletion timestamp and is in the process of being deleted. Updates are not allowed during deletion.",
316+
},
317+
}
318+
}
319+
320+
// Use safer metadata expansion that preserves system finalizers
321+
objectMeta = expandMetadataForUpdate(d, apps.Items[0].ObjectMeta)
322+
308323
validate := d.Get("validate").(bool)
309324
if _, err = si.ApplicationClient.Update(ctx, &applicationClient.ApplicationUpdateRequest{
310325
Application: &application.Application{

argocd/structure_metadata.go

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,48 @@ func expandMetadata(d *schema.ResourceData) (meta meta.ObjectMeta) {
3535
return meta
3636
}
3737

38+
// expandMetadataForUpdate safely expands metadata for updates, merging user-configured
39+
// finalizers with existing system finalizers to prevent accidental removal
40+
func expandMetadataForUpdate(d *schema.ResourceData, existingMeta meta.ObjectMeta) (meta meta.ObjectMeta) {
41+
meta = expandMetadata(d)
42+
43+
// Merge finalizers: keep existing system finalizers, add/update user finalizers
44+
if len(existingMeta.Finalizers) > 0 {
45+
userFinalizers := make(map[string]bool)
46+
for _, f := range meta.Finalizers {
47+
userFinalizers[f] = true
48+
}
49+
50+
// Start with existing finalizers
51+
merged := make([]string, 0, len(existingMeta.Finalizers)+len(meta.Finalizers))
52+
for _, existing := range existingMeta.Finalizers {
53+
if userFinalizers[existing] {
54+
// User explicitly configured this finalizer, keep it
55+
merged = append(merged, existing)
56+
delete(userFinalizers, existing)
57+
} else {
58+
// System finalizer not configured by user, preserve it
59+
merged = append(merged, existing)
60+
}
61+
}
62+
63+
// Add any new user finalizers
64+
for finalizer := range userFinalizers {
65+
merged = append(merged, finalizer)
66+
}
67+
68+
meta.Finalizers = merged
69+
}
70+
71+
return meta
72+
}
73+
3874
func flattenMetadata(meta meta.ObjectMeta, d *schema.ResourceData) []interface{} {
3975
m := map[string]interface{}{
4076
"generation": meta.Generation,
4177
"name": meta.Name,
4278
"namespace": meta.Namespace,
4379
"resource_version": meta.ResourceVersion,
44-
"finalizers": meta.Finalizers,
4580
"uid": fmt.Sprintf("%v", meta.UID),
4681
}
4782

@@ -51,6 +86,9 @@ func flattenMetadata(meta meta.ObjectMeta, d *schema.ResourceData) []interface{}
5186
labels := d.Get("metadata.0.labels").(map[string]interface{})
5287
m["labels"] = metadataRemoveInternalKeys(meta.Labels, labels)
5388

89+
finalizers := d.Get("metadata.0.finalizers").([]interface{})
90+
m["finalizers"] = metadataFilterFinalizers(meta.Finalizers, finalizers)
91+
5492
return []interface{}{m}
5593
}
5694

@@ -72,3 +110,21 @@ func metadataIsInternalKey(annotationKey string) bool {
72110

73111
return strings.HasSuffix(u.Hostname(), "kubernetes.io") || annotationKey == "notified.notifications.argoproj.io"
74112
}
113+
114+
func metadataFilterFinalizers(apiFinalizers []string, configuredFinalizers []interface{}) []string {
115+
configured := make(map[string]bool)
116+
for _, v := range configuredFinalizers {
117+
if s, ok := v.(string); ok {
118+
configured[s] = true
119+
}
120+
}
121+
122+
result := make([]string, 0)
123+
for _, finalizer := range apiFinalizers {
124+
// Only include finalizers that were explicitly configured by the user
125+
if configured[finalizer] {
126+
result = append(result, finalizer)
127+
}
128+
}
129+
return result
130+
}

argocd/structure_metadata_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package argocd
33
import (
44
"fmt"
55
"testing"
6+
7+
"github.com/stretchr/testify/require"
68
)
79

810
func TestMetadataIsInternalKey(t *testing.T) {
@@ -35,3 +37,59 @@ func TestMetadataIsInternalKey(t *testing.T) {
3537
})
3638
}
3739
}
40+
41+
func TestMetadataFilterFinalizers(t *testing.T) {
42+
t.Parallel()
43+
44+
testCases := []struct {
45+
name string
46+
apiFinalizers []string
47+
configuredFinalizers []interface{}
48+
expected []string
49+
}{
50+
{
51+
name: "empty lists",
52+
apiFinalizers: []string{},
53+
configuredFinalizers: []interface{}{},
54+
expected: []string{},
55+
},
56+
{
57+
name: "no configured finalizers",
58+
apiFinalizers: []string{"system.finalizer", "user.finalizer"},
59+
configuredFinalizers: []interface{}{},
60+
expected: []string{},
61+
},
62+
{
63+
name: "only configured finalizers returned",
64+
apiFinalizers: []string{"system.finalizer", "user.finalizer", "another.user.finalizer"},
65+
configuredFinalizers: []interface{}{"user.finalizer", "another.user.finalizer"},
66+
expected: []string{"user.finalizer", "another.user.finalizer"},
67+
},
68+
{
69+
name: "configured finalizer not in API response",
70+
apiFinalizers: []string{"system.finalizer"},
71+
configuredFinalizers: []interface{}{"user.finalizer"},
72+
expected: []string{},
73+
},
74+
{
75+
name: "mixed scenario - system and user finalizers",
76+
apiFinalizers: []string{"resources.argoproj.io/finalizer", "user.custom/finalizer", "kubernetes.io/finalizer"},
77+
configuredFinalizers: []interface{}{"user.custom/finalizer"},
78+
expected: []string{"user.custom/finalizer"},
79+
},
80+
{
81+
name: "invalid type in configured finalizers",
82+
apiFinalizers: []string{"system.finalizer", "user.finalizer"},
83+
configuredFinalizers: []interface{}{"user.finalizer", 123, nil},
84+
expected: []string{"user.finalizer"},
85+
},
86+
}
87+
88+
for _, tc := range testCases {
89+
t.Run(tc.name, func(t *testing.T) {
90+
t.Parallel()
91+
result := metadataFilterFinalizers(tc.apiFinalizers, tc.configuredFinalizers)
92+
require.Equal(t, tc.expected, result)
93+
})
94+
}
95+
}

0 commit comments

Comments
 (0)