Skip to content

Commit

Permalink
fix: Cleanup tags when component is removed (#28)
Browse files Browse the repository at this point in the history
When we receive a deletion request for the resource, we will also try to
clean the TagBindings associated, which helps clean up unused
tagValues/tagkeys residing on the GCP project

Tested locally the following scenarios:
----------------------------------------
Multiple Resources created with multiple tags and deleting the resources
only deletes the unused tags leaving behind the used tags(Since it is
binding to other resources)

Tested change in tag key-values which creates the new ones and deletes
the old ones if unused.

Creation and deletion of tags for single resources.

---------

Co-authored-by: jijinp <[email protected]>
  • Loading branch information
jithumkunjuraman and jijinp authored Nov 11, 2024
1 parent ba2b2ca commit 623751f
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 2 deletions.
4 changes: 2 additions & 2 deletions hack/verify-git-clean.sh
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/usr/bin/env bash
set -euo pipefail

go mod tidy
if [ -n "$(git status --porcelain)" ]; then
echo "Working directory is not clean. Please run make generate helm, stash your changes or remove untracked files."
echo "Working directory is not clean. Please run make generate helm or run go mod tidy, stash your changes or remove untracked files."
exit 1
fi
36 changes: 36 additions & 0 deletions internal/controller/taggable_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,23 @@ func (r *TaggableResourceReconciler[T, P, PT]) Reconcile(ctx context.Context, re
if err := r.Update(ctx, resource); err != nil {
return ctrl.Result{}, err
}
log.Info("resource deletion request received trying to delete associated tagValue/tagKey if unused")
projectID := r.determineProjectID(ctx, resource)
labels := resource.GetLabels()
for k, v := range r.LabelMatcher(labels) {
// return tagValue.Name, tagKey.Name, nil
valueID, keyID, err := r.getValueAndKeyID(ctx, projectID, k, v)
if err != nil {
return ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second}, err
}
if err := r.TagsManager.DeleteValueIfUnused(ctx, projectID, keyID, valueID); err != nil {
return ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second}, err
}
if err := r.TagsManager.DeleteKeyIfUnused(ctx, projectID, keyID); err != nil {
return ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second}, err
}
}

}
// Stop reconciliation as the resource is being deleted
return ctrl.Result{}, nil
Expand Down Expand Up @@ -320,11 +337,30 @@ func (r *TaggableResourceReconciler[T, P, PT]) handleTagBindingsDeletion(ctx con
return fmt.Errorf("error deleting tag binding %s: %w", tagBinding.Name, err)
}
}
// Cleanup finalizer
controllerutil.RemoveFinalizer(&tagBinding, "cnrm.cloud.google.com/finalizer")
if err := r.Update(ctx, &tagBinding); err != nil {
return fmt.Errorf("error removing finalizer from resource %s: %w", tagBinding.Name, err)
}
}

return nil
}

func (r *TaggableResourceReconciler[T, P, PT]) getValueAndKeyID(ctx context.Context, projectID, key, value string) (string, string, error) {
tagValue, err := r.TagsManager.LookupValue(ctx, projectID, key, value)
if err != nil {
return "", "", fmt.Errorf("failed to lookup tag value: %w", err)
}

tagKey, err := r.TagsManager.LookupKey(ctx, projectID, key)
if err != nil {
return "", "", fmt.Errorf("failed to lookup tag key: %w", err)
}

return tagValue.Name, tagKey.Name, nil
}

func CreateTaggableResourceController[T any, P ResourceMetadataProvider[T], PT ResourcePointer[T]](mgr ctrl.Manager, tagsManager gcp.TagsManager, provider P, labelMatcher func(map[string]string) map[string]string) {
if err := (&TaggableResourceReconciler[T, P, PT]{
Client: mgr.GetClient(),
Expand Down
50 changes: 50 additions & 0 deletions internal/gcp/tags.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ type TagsManager interface {
LookupValue(ctx context.Context, projectID string, key string, value string) (*resourcemanagerpb.TagValue, error)
CreateValue(ctx context.Context, projectID string, key string, value string) (*resourcemanagerpb.TagValue, error)
GetProjectInfo(ctx context.Context, projectID string) (*resourcemanagerpb.Project, error)
DeleteValueIfUnused(ctx context.Context, projectID string, key string, value string) error
DeleteKeyIfUnused(ctx context.Context, projectID string, key string) error
}

type tagsManager struct {
Expand Down Expand Up @@ -178,3 +180,51 @@ func (m *tagsManager) GetProjectInfo(ctx context.Context, projectID string) (*re
m.cache.Set(cacheKey, project, tagCacheDuration)
return project, nil
}

func (m *tagsManager) DeleteValueIfUnused(ctx context.Context, projectID string, key string, value string) error {

req := &resourcemanagerpb.DeleteTagValueRequest{
Name: value,
}

op, err := m.valuesClient.DeleteTagValue(ctx, req)
if err != nil {
var ae *apierror.APIError
if errors.As(err, &ae) && (ae.GRPCStatus().Code() == codes.FailedPrecondition || ae.GRPCStatus().Code() == codes.NotFound) {
// tag value is in use
return nil
}
return fmt.Errorf("failed to call tagValue deletion request: %w", err)
}

_, err = op.Wait(ctx)
if err != nil {
return fmt.Errorf("failed to delete the tagValue %w", err)
}

m.cache.Delete(cacheKeyTagValue(key, value))
return nil
}

func (m *tagsManager) DeleteKeyIfUnused(ctx context.Context, projectID string, key string) error {

// Attempt to delete the tag key
req := &resourcemanagerpb.DeleteTagKeyRequest{
Name: key,
}
op, err := m.keysClient.DeleteTagKey(ctx, req)
if err != nil {
var ae *apierror.APIError
if errors.As(err, &ae) && (ae.GRPCStatus().Code() == codes.FailedPrecondition || ae.GRPCStatus().Code() == codes.NotFound) {
return nil
}
return fmt.Errorf("failed to call tagKey deletion request: %w", err)
}

_, err = op.Wait(ctx)
if err != nil {
return fmt.Errorf("failed to delete the tagKey %w", err)
}
m.cache.Delete(cacheKeyTagKey(key))
return nil
}

0 comments on commit 623751f

Please sign in to comment.