Skip to content

Commit ed7cef2

Browse files
committed
feat: add support for Prune/Delete=confirm sync option
As deletion confirmation acts on the whole Application it could makes sense to add it as a sync option rather than a resource annotation. It also could be more convenient to integrate in certain workflows for users. Signed-off-by: Arthur Outhenin-Chalandre <[email protected]>
1 parent 74a3275 commit ed7cef2

File tree

7 files changed

+131
-3
lines changed

7 files changed

+131
-3
lines changed

controller/appcontroller.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1230,6 +1230,11 @@ func (ctrl *ApplicationController) finalizeApplicationDeletion(app *appv1.Applic
12301230
return err
12311231
}
12321232

1233+
if app.Spec.SyncPolicy != nil && app.Spec.SyncPolicy.SyncOptions.HasOption(synccommon.SyncOptionDeleteRequireConfirm) && !deletionApproved {
1234+
logCtx.Infof("Application requires manual confirmation to be deleted")
1235+
return nil
1236+
}
1237+
12331238
for k := range objsMap {
12341239
// Wait for objects pending deletion to complete before proceeding with next sync wave
12351240
if objsMap[k].GetDeletionTimestamp() != nil {

controller/sync.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,7 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, project *v1alp
347347
clientSideApplyManager,
348348
),
349349
sync.WithPruneConfirmed(app.IsDeletionConfirmed(state.StartedAt.Time)),
350+
sync.WithRequiresPruneConfirmation(syncOp.SyncOptions.HasOption(common.SyncOptionPruneRequireConfirm)),
350351
sync.WithSkipDryRunOnMissingResource(syncOp.SyncOptions.HasOption(common.SyncOptionSkipDryRunOnMissingResource)),
351352
}
352353

docs/user-guide/sync-options.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,20 @@ metadata:
3434
To confirm the pruning you can use Argo CD UI, CLI or manually apply the `argocd.argoproj.io/deletion-approved: <ISO formatted timestamp>`
3535
annotation to the application.
3636

37+
It also can be enabled at the application level like in the example below:
38+
39+
```yaml
40+
apiVersion: argoproj.io/v1alpha1
41+
kind: Application
42+
spec:
43+
syncPolicy:
44+
syncOptions:
45+
- Prune=confirm
46+
```
47+
48+
Note that setting a Prune sync option on the application level will not override
49+
the same sync option set on a specific resource, both will still be applied.
50+
3751
## Disable Kubectl Validation
3852

3953
For a certain class of objects, it is necessary to `kubectl apply` them using the `--validate=false` flag. Examples of this are Kubernetes types which uses `RawExtension`, such as [ServiceCatalog](https://github.com/kubernetes-incubator/service-catalog/blob/master/pkg/apis/servicecatalog/v1beta1/types.go#L497). You can do that using this annotation:
@@ -101,6 +115,20 @@ metadata:
101115
To confirm the deletion you can use Argo CD UI, CLI or manually apply the `argocd.argoproj.io/deletion-approved: <ISO formatted timestamp>`
102116
annotation to the application.
103117

118+
It also can be enabled at the application level like in the example below:
119+
120+
```yaml
121+
apiVersion: argoproj.io/v1alpha1
122+
kind: Application
123+
spec:
124+
syncPolicy:
125+
syncOptions:
126+
- Delete=confirm
127+
```
128+
129+
Note that setting a Delete sync option on the application level will not override
130+
the same sync option set on a specific resource, both will still be applied.
131+
104132
## Selective Sync
105133

106134
Currently, when syncing using auto sync Argo CD applies every object in the application.

gitops-engine/pkg/sync/sync_context.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,13 @@ func WithPrune(prune bool) SyncOpt {
115115
}
116116
}
117117

118+
// WithRequiresPruneConfirmation specifies if pruning resources requires a confirmation
119+
func WithRequiresPruneConfirmation(requiresConfirmation bool) SyncOpt {
120+
return func(ctx *syncContext) {
121+
ctx.requiresPruneConfirmation = requiresConfirmation
122+
}
123+
}
124+
118125
// WithPruneConfirmed specifies if prune is confirmed for resources that require confirmation
119126
func WithPruneConfirmed(confirmed bool) SyncOpt {
120127
return func(ctx *syncContext) {
@@ -367,6 +374,7 @@ type syncContext struct {
367374
pruneLast bool
368375
prunePropagationPolicy *metav1.DeletionPropagation
369376
pruneConfirmed bool
377+
requiresPruneConfirmation bool
370378
clientSideApplyMigrationManager string
371379
enableClientSideApplyMigration bool
372380

@@ -1406,7 +1414,7 @@ func (sc *syncContext) runTasks(tasks syncTasks, dryRun bool) runState {
14061414
if !sc.pruneConfirmed {
14071415
var resources []string
14081416
for _, task := range pruneTasks {
1409-
if resourceutil.HasAnnotationOption(task.liveObj, common.AnnotationSyncOptions, common.SyncOptionPruneRequireConfirm) {
1417+
if sc.requiresPruneConfirmation || resourceutil.HasAnnotationOption(task.liveObj, common.AnnotationSyncOptions, common.SyncOptionPruneRequireConfirm) {
14101418
resources = append(resources, fmt.Sprintf("%s/%s/%s", task.obj().GetAPIVersion(), task.obj().GetKind(), task.name()))
14111419
}
14121420
}

gitops-engine/pkg/sync/sync_context_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"net/http"
99
"net/http/httptest"
1010
"reflect"
11+
"strconv"
1112
"strings"
1213
"testing"
1314
"time"
@@ -836,6 +837,43 @@ func TestDoNotPrunePruneFalse(t *testing.T) {
836837
assert.Equal(t, synccommon.OperationSucceeded, phase)
837838
}
838839

840+
// make sure that we need confirmation to prune with Prune=confirm
841+
func TestPruneConfirm(t *testing.T) {
842+
for _, appLevelConfirmation := range []bool{true, false} {
843+
t.Run("appLevelConfirmation="+strconv.FormatBool(appLevelConfirmation), func(t *testing.T) {
844+
syncCtx := newTestSyncCtx(nil, WithOperationSettings(false, true, false, false))
845+
syncCtx.requiresPruneConfirmation = appLevelConfirmation
846+
pod := testingutils.NewPod()
847+
if appLevelConfirmation {
848+
pod.SetAnnotations(map[string]string{synccommon.AnnotationSyncOptions: "Prune=true"})
849+
} else {
850+
pod.SetAnnotations(map[string]string{synccommon.AnnotationSyncOptions: "Prune=confirm"})
851+
}
852+
pod.SetNamespace(testingutils.FakeArgoCDNamespace)
853+
syncCtx.resources = groupResources(ReconciliationResult{
854+
Live: []*unstructured.Unstructured{pod},
855+
Target: []*unstructured.Unstructured{nil},
856+
})
857+
858+
syncCtx.Sync()
859+
phase, msg, resources := syncCtx.GetState()
860+
861+
assert.Equal(t, synccommon.OperationRunning, phase)
862+
assert.Empty(t, resources)
863+
assert.Equal(t, "Waiting for pruning confirmation of v1/Pod/my-pod", msg)
864+
865+
syncCtx.pruneConfirmed = true
866+
syncCtx.Sync()
867+
868+
phase, _, resources = syncCtx.GetState()
869+
assert.Equal(t, synccommon.OperationSucceeded, phase)
870+
assert.Len(t, resources, 1)
871+
assert.Equal(t, synccommon.ResultCodePruned, resources[0].Status)
872+
assert.Equal(t, "pruned", resources[0].Message)
873+
})
874+
}
875+
}
876+
839877
// // make sure Validate=false means we don't validate
840878
func TestSyncOptionValidate(t *testing.T) {
841879
tests := []struct {

test/e2e/app_management_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3087,6 +3087,42 @@ func TestDeletionConfirmation(t *testing.T) {
30873087
Then().Expect(DoesNotExist())
30883088
}
30893089

3090+
func TestDeletionConfirmationSyncOption(t *testing.T) {
3091+
ctx := Given(t)
3092+
ctx.
3093+
And(func() {
3094+
_, err := fixture.KubeClientset.CoreV1().ConfigMaps(fixture.DeploymentNamespace()).Create(
3095+
t.Context(), &corev1.ConfigMap{
3096+
ObjectMeta: metav1.ObjectMeta{
3097+
Name: "test-configmap",
3098+
Labels: map[string]string{
3099+
common.LabelKeyAppInstance: ctx.AppName(),
3100+
},
3101+
},
3102+
}, metav1.CreateOptions{})
3103+
require.NoError(t, err)
3104+
}).
3105+
Path(guestbookPath).
3106+
Async(true).
3107+
When().
3108+
CreateApp().Then().When().
3109+
PatchApp(`[{
3110+
"op": "add",
3111+
"path": "/spec/syncPolicy",
3112+
"value": { "syncOptions": ["Delete=confirm", "Prune=confirm"] }
3113+
}]`).Sync().
3114+
Then().Expect(OperationPhaseIs(OperationRunning)).
3115+
When().ConfirmDeletion().
3116+
Then().Expect(OperationPhaseIs(OperationSucceeded)).
3117+
When().Delete(true).
3118+
Then().
3119+
And(func(app *Application) {
3120+
assert.NotNil(t, app.DeletionTimestamp)
3121+
}).
3122+
When().ConfirmDeletion().
3123+
Then().Expect(DoesNotExist())
3124+
}
3125+
30903126
func TestLastTransitionTimeUnchangedError(t *testing.T) {
30913127
// Ensure that, if the health status hasn't changed, the lastTransitionTime is not updated.
30923128

ui/src/app/applications/components/application-details/application-details.tsx

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1137,6 +1137,18 @@ Are you sure you want to disable auto-sync and rollback application '${props.mat
11371137
[setExtensionPanelVisible]
11381138
);
11391139

1140+
const requiresDeletionConfirmation = (app: appModels.Application): boolean => {
1141+
if (app.status.resources.find(r => r.requiresDeletionConfirmation)) {
1142+
return true;
1143+
}
1144+
1145+
if (app.spec.syncPolicy?.syncOptions?.includes('Delete=confirm') || app.spec.syncPolicy?.syncOptions?.includes('Prune=confirm')) {
1146+
return true;
1147+
}
1148+
1149+
return false;
1150+
}
1151+
11401152
const getApplicationActionMenu = useCallback(
11411153
(app: appModels.Application, needOverlapLabelOnNarrowScreen: boolean) => {
11421154
const refreshing = app.metadata.annotations && app.metadata.annotations[appModels.AnnotationRefreshKey];
@@ -1163,7 +1175,7 @@ Are you sure you want to disable auto-sync and rollback application '${props.mat
11631175
action: () => AppUtils.showDeploy('all', null, appContext),
11641176
disabled: !app.spec.source && (!app.spec.sources || app.spec.sources.length === 0) && !app.spec.sourceHydrator
11651177
},
1166-
...(app.status?.operationState?.phase === 'Running' && app.status.resources.find(r => r.requiresDeletionConfirmation)
1178+
...(app.status?.operationState?.phase === 'Running' && requiresDeletionConfirmation(app)
11671179
? [
11681180
{
11691181
iconClassName: 'fa fa-check',
@@ -1187,7 +1199,7 @@ Are you sure you want to disable auto-sync and rollback application '${props.mat
11871199
disabled: !app.status.operationState
11881200
},
11891201
app.metadata.deletionTimestamp &&
1190-
app.status.resources.find(r => r.requiresDeletionConfirmation) &&
1202+
requiresDeletionConfirmation(app) &&
11911203
!((app.metadata.annotations || {})[appModels.AppDeletionConfirmedAnnotation] == 'true')
11921204
? {
11931205
iconClassName: 'fa fa-check',

0 commit comments

Comments
 (0)