Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: application resource deletion protection #20497

Merged
merged 1 commit into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions assets/swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

49 changes: 49 additions & 0 deletions cmd/argocd/commands/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
k8swatch "k8s.io/apimachinery/pkg/watch"
Expand Down Expand Up @@ -98,6 +99,7 @@ func NewApplicationCommand(clientOpts *argocdclient.ClientOptions) *cobra.Comman
command.AddCommand(NewApplicationLogsCommand(clientOpts))
command.AddCommand(NewApplicationAddSourceCommand(clientOpts))
command.AddCommand(NewApplicationRemoveSourceCommand(clientOpts))
command.AddCommand(NewApplicationConfirmDeletionCommand(clientOpts))
return command
}

Expand Down Expand Up @@ -3202,3 +3204,50 @@ func NewApplicationRemoveSourceCommand(clientOpts *argocdclient.ClientOptions) *
command.Flags().IntVar(&sourcePosition, "source-position", -1, "Position of the source from the list of sources of the app. Counting starts at 1.")
return command
}

func NewApplicationConfirmDeletionCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command {
var appNamespace string
command := &cobra.Command{
Use: "confirm-deletion APPNAME",
Short: "Confirms deletion/pruning of an application resources",
Run: func(c *cobra.Command, args []string) {
ctx := c.Context()

if len(args) != 1 {
c.HelpFunc()(c, args)
os.Exit(1)
}

argocdClient := headless.NewClientOrDie(clientOpts, c)
conn, appIf := argocdClient.NewApplicationClientOrDie()
defer argoio.Close(conn)

appName, appNs := argo.ParseFromQualifiedName(args[0], appNamespace)

app, err := appIf.Get(ctx, &application.ApplicationQuery{
Name: &appName,
Refresh: getRefreshType(false, false),
AppNamespace: &appNs,
})
errors.CheckError(err)

annotations := app.Annotations
if annotations == nil {
annotations = map[string]string{}
app.Annotations = annotations
}
annotations[common.AnnotationDeletionApproved] = metav1.Now().Format(time.RFC3339)

_, err = appIf.Update(ctx, &application.ApplicationUpdateRequest{
Application: app,
Validate: ptr.To(false),
Project: &app.Spec.Project,
})
errors.CheckError(err)

fmt.Printf("Application '%s' updated successfully\n", app.ObjectMeta.Name)
},
}
command.Flags().StringVarP(&appNamespace, "app-namespace", "N", "", "Namespace of the target application where the source will be appended")
return command
}
6 changes: 6 additions & 0 deletions controller/appcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1171,6 +1171,8 @@ func (ctrl *ApplicationController) finalizeApplicationDeletion(app *appv1.Applic
config := metrics.AddMetricsTransportWrapper(ctrl.metricsServer, app, clusterRESTConfig)

if app.CascadedDeletion() {
deletionApproved := app.IsDeletionConfirmed(app.DeletionTimestamp.Time)

logCtx.Infof("Deleting resources")
// ApplicationDestination points to a valid cluster, so we may clean up the live objects
objs := make([]*unstructured.Unstructured, 0)
Expand All @@ -1188,6 +1190,10 @@ func (ctrl *ApplicationController) finalizeApplicationDeletion(app *appv1.Applic

if ctrl.shouldBeDeleted(app, objsMap[k]) {
objs = append(objs, objsMap[k])
if res, ok := app.Status.FindResource(k); ok && res.RequiresDeletionConfirmation && !deletionApproved {
logCtx.Infof("Resource %v requires manual confirmation to delete", k)
return nil
}
}
}

Expand Down
14 changes: 6 additions & 8 deletions controller/appcontroller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,7 @@ func TestAutoSyncParameterOverrides(t *testing.T) {

// TestFinalizeAppDeletion verifies application deletion
func TestFinalizeAppDeletion(t *testing.T) {
now := metav1.Now()
defaultProj := v1alpha1.AppProject{
ObjectMeta: metav1.ObjectMeta{
Name: "default",
Expand All @@ -843,11 +844,9 @@ func TestFinalizeAppDeletion(t *testing.T) {
t.Run("CascadingDelete", func(t *testing.T) {
app := newFakeApp()
app.SetCascadedDeletion(v1alpha1.ResourcesFinalizerName)
app.DeletionTimestamp = &now
app.Spec.Destination.Namespace = test.FakeArgoCDNamespace
appObj := kube.MustToUnstructured(&app)
ctrl := newFakeController(&fakeData{apps: []runtime.Object{app, &defaultProj}, managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{
kube.GetResourceKey(appObj): appObj,
}}, nil)
ctrl := newFakeController(&fakeData{apps: []runtime.Object{app, &defaultProj}, managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{}}, nil)
patched := false
fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset)
defaultReactor := fakeAppCs.ReactionChain[0]
Expand Down Expand Up @@ -886,6 +885,7 @@ func TestFinalizeAppDeletion(t *testing.T) {
}
app := newFakeApp()
app.SetCascadedDeletion(v1alpha1.ResourcesFinalizerName)
app.DeletionTimestamp = &now
app.Spec.Destination.Namespace = test.FakeArgoCDNamespace
app.Spec.Project = "restricted"
appObj := kube.MustToUnstructured(&app)
Expand Down Expand Up @@ -931,10 +931,8 @@ func TestFinalizeAppDeletion(t *testing.T) {
t.Run("DeleteWithDestinationClusterName", func(t *testing.T) {
app := newFakeAppWithDestName()
app.SetCascadedDeletion(v1alpha1.ResourcesFinalizerName)
appObj := kube.MustToUnstructured(&app)
ctrl := newFakeController(&fakeData{apps: []runtime.Object{app, &defaultProj}, managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{
kube.GetResourceKey(appObj): appObj,
}}, nil)
app.DeletionTimestamp = &now
ctrl := newFakeController(&fakeData{apps: []runtime.Object{app, &defaultProj}, managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{}}, nil)
patched := false
fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset)
defaultReactor := fakeAppCs.ReactionChain[0]
Expand Down
3 changes: 3 additions & 0 deletions controller/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
goSync "sync"
"time"

synccommon "github.com/argoproj/gitops-engine/pkg/sync/common"
v1 "k8s.io/api/core/v1"

"github.com/argoproj/gitops-engine/pkg/diff"
Expand Down Expand Up @@ -746,6 +747,8 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
Group: gvk.Group,
Hook: isHook(obj),
RequiresPruning: targetObj == nil && liveObj != nil && isSelfReferencedObj,
RequiresDeletionConfirmation: targetObj != nil && resourceutil.HasAnnotationOption(targetObj, synccommon.AnnotationSyncOptions, synccommon.SyncOptionDeleteRequireConfirm) ||
liveObj != nil && resourceutil.HasAnnotationOption(liveObj, synccommon.AnnotationSyncOptions, synccommon.SyncOptionDeleteRequireConfirm),
Comment on lines +750 to +751
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also need to deal with users deleting the resource directly like using actions to deleted the resource from the cluster? If so did we handle it already (couldn't find it so wanted to verify)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The RequiresDeletionConfirmation is used during app deletion finalizing. So if user delete resource manually then controller won't attempt to delete it and confirmation won't be necessary. I think this is expected behavior.

}
if targetObj != nil {
resState.SyncWave = int64(syncwaves.Wave(targetObj))
Expand Down
1 change: 1 addition & 0 deletions controller/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha
sync.WithReplace(syncOp.SyncOptions.HasOption(common.SyncOptionReplace)),
sync.WithServerSideApply(syncOp.SyncOptions.HasOption(common.SyncOptionServerSideApply)),
sync.WithServerSideApplyManager(cdcommon.ArgoCDSSAManager),
sync.WithPruneConfirmed(app.IsDeletionConfirmed(state.StartedAt.Time)),
}

if syncOp.SyncOptions.HasOption("CreateNamespace=true") {
Expand Down
1 change: 1 addition & 0 deletions docs/user-guide/commands/argocd_app.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

50 changes: 50 additions & 0 deletions docs/user-guide/commands/argocd_app_confirm-deletion.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 28 additions & 0 deletions docs/user-guide/sync-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,20 @@ The sync-status panel shows that pruning was skipped, and why:

The app will be out of sync if Argo CD expects a resource to be pruned. You may wish to use this along with [compare options](compare-options.md).

## Resource Pruning With Confirmation

Resources such as Namespaces are critical and should not be pruned without confirmation. You can set the `Prune=confirm`
sync option to require manual confirmation before pruning.

```yaml
metadata:
annotations:
argocd.argoproj.io/sync-options: Prune=confirm
```

To confirm the pruning you can use Argo CD UI, CLI or manually apply the `argocd.argoproj.io/deletion-approved: <ISO formatted timestamp>`
annotation to the application.

## Disable Kubectl Validation

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 using this annotations:
Expand Down Expand Up @@ -70,6 +84,20 @@ metadata:
argocd.argoproj.io/sync-options: Delete=false
```

## Resource Deletion With Confirmation

Resources such as Namespaces are critical and should not be deleted without confirmation. You can set the `Delete=confirm`
sync option to require manual confirmation before deletion.

```yaml
metadata:
annotations:
argocd.argoproj.io/sync-options: Delete=confirm
```

To confirm the deletion you can use Argo CD UI, CLI or manually apply the `argocd.argoproj.io/deletion-approved: <ISO formatted timestamp>`
annotation to the application.

## Selective Sync

Currently when syncing using auto sync Argo CD applies every object in the application.
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ require (
github.com/TomOnTime/utfutil v0.0.0-20180511104225-09c41003ee1d
github.com/alicebob/miniredis/v2 v2.33.0
github.com/antonmedv/expr v1.15.1
github.com/argoproj/gitops-engine v0.7.1-0.20240917171920-72bcdda3f0a5
github.com/argoproj/gitops-engine v0.7.1-0.20241023134423-09e5225f8472
github.com/argoproj/notifications-engine v0.4.1-0.20241007194503-2fef5c9049fd
github.com/argoproj/pkg v0.13.7-0.20230626144333-d56162821bd1
github.com/aws/aws-sdk-go v1.55.5
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ github.com/antonmedv/expr v1.15.1/go.mod h1:0E/6TxnOlRNp81GMzX9QfDPAmHo2Phg00y4J
github.com/apache/thrift v0.12.0/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ=
github.com/apache/thrift v0.13.0/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ=
github.com/appscode/go v0.0.0-20191119085241-0887d8ec2ecc/go.mod h1:OawnOmAL4ZX3YaPdN+8HTNwBveT1jMsqP74moa9XUbE=
github.com/argoproj/gitops-engine v0.7.1-0.20240917171920-72bcdda3f0a5 h1:K/e+NsNmE4BccRu21QpqUxkTHxU9YWjU3M775Ck+V/E=
github.com/argoproj/gitops-engine v0.7.1-0.20240917171920-72bcdda3f0a5/go.mod h1:b1vuwkyMUszyUK+USUJqC8vJijnQsEPNDpC+sDdDLtM=
github.com/argoproj/gitops-engine v0.7.1-0.20241023134423-09e5225f8472 h1:NSUzj5CWkOR6xrbGBT4dhZ7WsHhT/pbud+fsvQuUe7k=
github.com/argoproj/gitops-engine v0.7.1-0.20241023134423-09e5225f8472/go.mod h1:b1vuwkyMUszyUK+USUJqC8vJijnQsEPNDpC+sDdDLtM=
github.com/argoproj/notifications-engine v0.4.1-0.20241007194503-2fef5c9049fd h1:lOVVoK89j9Nd4+JYJiKAaMNYC1402C0jICROOfUPWn0=
github.com/argoproj/notifications-engine v0.4.1-0.20241007194503-2fef5c9049fd/go.mod h1:N0A4sEws2soZjEpY4hgZpQS8mRIEw6otzwfkgc3g9uQ=
github.com/argoproj/pkg v0.13.7-0.20230626144333-d56162821bd1 h1:qsHwwOJ21K2Ao0xPju1sNuqphyMnMYkyB3ZLoLtxWpo=
Expand Down
4 changes: 4 additions & 0 deletions manifests/core-install.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions manifests/crds/application-crd.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions manifests/crds/applicationset-crd.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions manifests/ha/install.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions manifests/install.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading