prune cluster scoped resources does not work when specify namespace in app config

[kevin-namba]

What happened:
prune cluster scoped resources (such as clusterrole) does not work when specify namespace in app config

What you expected to happen:

How to reproduce it:

Environment:

• piped version:
• control-plane version:
• Others:

[ffjlabo]

Reproduced on QuickSync like this↓ (name: pod-reader)

[result]

before

% k get clusterrole                                                                          (git)-[delete-cluster-role]
NAME                                                                   CREATED AT
admin                                                                  2024-03-25T07:29:57Z
cluster-admin                                                          2024-03-25T07:29:56Z
edit                                                                   2024-03-25T07:29:57Z
kindnet                                                                2024-03-25T07:30:01Z
kubeadm:get-nodes                                                      2024-03-25T07:29:58Z
local-path-provisioner-role                                            2024-03-25T07:30:02Z
pod-reader                                                             2024-03-28T09:37:08Z
system:aggregate-to-admin                                              2024-03-25T07:29:57Z

after

% k get clusterrole                                                                                 (git)-[delete-cluster-role]
NAME                                                                   CREATED AT
admin                                                                  2024-03-25T07:29:57Z
cluster-admin                                                          2024-03-25T07:29:56Z
edit                                                                   2024-03-25T07:29:57Z
kindnet                                                                2024-03-25T07:30:01Z
kubeadm:get-nodes                                                      2024-03-25T07:29:58Z
local-path-provisioner-role                                            2024-03-25T07:30:02Z
pod-reader                                                             2024-03-28T09:37:08Z
system:aggregate-to-admin                                              2024-03-25T07:29:57Z

[sample]

apiVersion: pipecd.dev/v1beta1
kind: KubernetesApp
spec:
  name: clusterrole-test
  input:
    namespace: test
  labels:
    env: clusterrole-test
    team: product
  quickSync:
    prune: true
  description: |
    This is a test app for clusterrole. Especially for testing the prune on QuickSync.

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: pod-reader
  annotations:
    pipecd.dev/order: "1"
rules:
- apiGroups: [""] # "" indicates the core API group
  resources: ["pods"]
  verbs: ["get", "watch", "list"]

apiVersion: v1
kind: Namespace
metadata:
  name: test
  labels:
    name: test
  annotations:
    pipecd.dev/order: "0"

[ffjlabo]

This might happen because the resource key in the live resource differs from the resource used when pruning.

• The actual resource key: rbac.authorization.k8s.io/v1:ClusterRole:test:pod-reader
• The one used when pruning: rbac.authorization.k8s.io/v1:ClusterRole:default:pod-reader (estimated by the code below)

  • pipecd/pkg/app/piped/executor/kubernetes/kubernetes.go

    Lines 293 to 297 in ad7d699

    	if errors.Is(err, provider.ErrNotFound) {
    	lp.Infof("- no resource %s to delete", k.ReadableString())
    	deletedCount++
    	continue
    	}

  • pipecd/pkg/app/piped/platformprovider/kubernetes/resourcekey.go

    Lines 103 to 105 in ad7d699

    	func (k ResourceKey) ReadableString() string {
    	return fmt.Sprintf("name=%q, kind=%q, namespace=%q, apiVersion=%q", k.Name, k.Kind, k.Namespace, k.APIVersion)
    	}

[ffjlabo]

As this PR #3991, all of the resources for the application belong to the namespace set on the spec.input.namespace in app.pipecd.yaml.
So in PipeCD, the ClusterRole resource is also treated as the one which belong the namespace.

[ffjlabo]

Root cause:

the resource key stored as livestate and the one on the annotation of actual k8s resource are different values, but actually, they are compared as if they are equal.

The Code:

pipecd/pkg/app/piped/platformprovider/kubernetes/applier.go

Lines 161 to 163 in 7b471d7

	if k.String() != m.GetAnnotations()[LabelResourceKey] {
	return ErrNotFound
	}

Details

The resource key stored as livestate

• created by the livestatestre.reflector after applying
• based on the actual k8s resource on the cluster with MakeResourceKey

  pipecd/pkg/app/piped/platformprovider/kubernetes/resourcekey.go

  Lines 249 to 260 in 7b471d7

  	func MakeResourceKey(obj *unstructured.Unstructured) ResourceKey {
  	k := ResourceKey{
  	APIVersion: obj.GetAPIVersion(),
  	Kind: obj.GetKind(),
  	Namespace: obj.GetNamespace(),
  	Name: obj.GetName(),
  	}
  	if k.Namespace == "" {
  	k.Namespace = DefaultNamespace
  	}
  	return k
  	}

More details

• applier.Delete(ctx, k)

  pipecd/pkg/app/piped/executor/kubernetes/kubernetes.go

  Line 287 in 7b471d7

  err = applier.Delete(ctx, k)

• removeKeys are determined by findRemoveKeys using manifests (get from git repo) and liveResources (get from AppLiveResourceLister.ListKubernetesResources)

  pipecd/pkg/app/piped/executor/kubernetes/sync.go

  Line 119 in 7b471d7

  if err := deleteResources(ctx, e.applierGetter, removeKeys, e.LogPersister); err != nil {

  • first, determine values from git except for the namespace
  • then, determine the namespace from the livestate's one
  • => The namespace of the resourceKey depends on livestate's one
  • ListKubernetesResources

    • pipecd/pkg/app/piped/controller/controller.go

      Lines 725 to 727 in 7b471d7

      	func (l appLiveResourceLister) ListKubernetesResources() ([]provider.Manifest, bool) {
      	return l.lister.ListKubernetesAppLiveResources(l.platformProvider, l.appID)
      	}

    • originally, get from the livestate store
      • -> ListKubernetesAppLiveResources

        pipecd/pkg/app/piped/livestatestore/livestatestore.go

        Lines 219 to 225 in 7b471d7

        	func (g LiveResourceLister) ListKubernetesAppLiveResources(platformProvider, appID string) ([]provider.Manifest, bool) {
        	kg, ok := g.KubernetesGetter(platformProvider)
        	if !ok {
        	return nil, false
        	}
        	return kg.GetAppLiveManifests(appID), true
        	}

      • -> s.store.GetAppLiveManifests

        pipecd/pkg/app/piped/livestatestore/kubernetes/store.go

        Lines 302 to 316 in 7b471d7

        	func (s *store) GetAppLiveManifests(appID string) []provider.Manifest {
        	s.mu.RLock()
        	app, ok := s.apps[appID]
        	s.mu.RUnlock()
        	if !ok {
        	return nil
        	}
        	nodes := app.getManagingNodes()
        	manifests := make([]provider.Manifest, 0, len(nodes))
        	for i := range nodes {
        	manifests = append(manifests, nodes[i].Manifest())
        	}
        	return manifests
        	}

      • -> getManagingNodes

        pipecd/pkg/app/piped/livestatestore/kubernetes/appnodes.go

        Lines 164 to 169 in 7b471d7

        	func (a *appNodes) getManagingNodes() map[string]node {
        	a.mu.RLock()
        	defer a.mu.RUnlock()
        	return a.managingNodes
        	}

• the resourceKey is stored by reflector based on the resource applied in the cluster.
  • using MakeResourceKey

    pipecd/pkg/app/piped/platformprovider/kubernetes/resourcekey.go

    Lines 249 to 260 in 7b471d7

    	func MakeResourceKey(obj *unstructured.Unstructured) ResourceKey {
    	k := ResourceKey{
    	APIVersion: obj.GetAPIVersion(),
    	Kind: obj.GetKind(),
    	Namespace: obj.GetNamespace(),
    	Name: obj.GetName(),
    	}
    	if k.Namespace == "" {
    	k.Namespace = DefaultNamespace
    	}
    	return k
    	}

  • treated the namespace as default if the obj.namespace is empty,
    ---code reading memo---
  • set the handlers (onAdd, onUpdate)

    pipecd/pkg/app/piped/livestatestore/kubernetes/kubernetes.go

    Lines 102 to 103 in 7b471d7

    	onAdd: s.store.onAddResource,
    	onUpdate: s.store.onUpdateResource,

  • start informers with them to detect the changes

    pipecd/pkg/app/piped/livestatestore/kubernetes/reflector.go

    Lines 219 to 224 in 7b471d7

    	di.AddEventHandler(cache.ResourceEventHandlerFuncs{
    	AddFunc: r.onObjectAdd,
    	UpdateFunc: r.onObjectUpdate,
    	DeleteFunc: r.onObjectDelete,
    	})
    	go di.Run(r.stopCh)

  • -> onObjectAdd, onObjectUpdate

    • pipecd/pkg/app/piped/livestatestore/kubernetes/reflector.go

      Lines 251 to 252 in 7b471d7

      	u := obj.(*unstructured.Unstructured)
      	key := provider.MakeResourceKey(u)

    • pipecd/pkg/app/piped/livestatestore/kubernetes/reflector.go

      Lines 282 to 286 in 7b471d7

      	u := obj.(*unstructured.Unstructured)
      	oldU := oldObj.(*unstructured.Unstructured)
      	// Ignore all predefined ones.
      	key := provider.MakeResourceKey(u)

  • -> onAddResource, onUpdateResource

    • pipecd/pkg/app/piped/livestatestore/kubernetes/store.go

      Line 170 in 7b471d7

      s.addResource(obj, appID)

    • pipecd/pkg/app/piped/livestatestore/kubernetes/store.go

      Lines 185 to 186 in 7b471d7

      	s.addResource(obj, appID)
      	}

  • -> addResource

    pipecd/pkg/app/piped/livestatestore/kubernetes/store.go

    Lines 156 to 157 in 7b471d7

    	if event, ok := app.addDependedResource(uid, key, obj, now); ok {
    	s.addEvent(event)

  • -> addManagingResource

    pipecd/pkg/app/piped/livestatestore/kubernetes/appnodes.go

    Lines 66 to 67 in 7b471d7

    	oriNode, hasOriNode := a.managingNodes[uid]
    	version := a.version

The resource key on the annotation of actual k8s resource

• created by before applying
• based on the k8s resource manifest, then fixed by app.pipecd.yaml (spec.input.namespace)

More details

• the annotation pipecd.dev/resource-key is set based on the resource loaded from the git repo before applying
  • loadManifests

    pipecd/pkg/app/piped/executor/kubernetes/sync.go

    Lines 28 to 35 in 7b471d7

    	manifests, err := loadManifests(
    	ctx,
    	e.Deployment.ApplicationId,
    	e.commit,
    	e.AppManifestsCache,
    	e.loader,
    	e.Logger,
    	)

  • addBuiltinAnnotations

    pipecd/pkg/app/piped/executor/kubernetes/kubernetes.go

    Line 223 in 7b471d7

    provider.LabelResourceKey: manifests[i].Key.String(),

• When loading manifests, piped executor determines the resourceKey based on the manifest and app.pipecd.yaml
  • first, determine the resourceKey using MakeResourceKey
    • -> ParseManifests (all of helm, kustomize, default) https://github.com/pipe-cd/pipecd/blob/7b471d7a9f2eb09ccbc0a9222b6e7adb08ea2edf/pkg/app/piped/platformprovider/kubernetes/manifest.- go#L243-L246
  • finally, fix it based on spec.input.namespace on app.pipecd.yaml

    • pipecd/pkg/app/piped/platformprovider/kubernetes/loader.go

      Line 90 in 7b471d7

      setNamespace(manifests, l.input.Namespace)

Example:

Consider the case to apply ClusterRole

• spec.input.namespace exists (namespace: test)
  • livestate: rbac.authorization.k8s.io/v1:ClusterRole:default:pod-reader
  • annotation: rbac.authorization.k8s.io/v1:ClusterRole:test:pod-reader
• doesn't exist
  • livestate: rbac.authorization.k8s.io/v1:ClusterRole:default:pod-reader
  • annotation: rbac.authorization.k8s.io/v1:ClusterRole:default:pod-reader

[ffjlabo]

Solution candidates

The point is whether to fix or not as if they are equal.

1. fix not to compare them. (they are not equal)
2. fix to use of annotation when creating the resourceKey on the livestate. (they are equal)

In both cases, there is no user effect.
Considering the effective scope for the fix, 1 is safer. But I think it is confusing that they are not equal.

[ffjlabo]

Other stages

Baseline

load manifests in the same way with K8S_SYNC stage indirectly.
(by using loadRunningManifests)

pipecd/pkg/app/piped/executor/kubernetes/baseline.go

Line 46 in 7b1103d

manifests, err := e.loadRunningManifests(ctx)

So the resourceKey is based on the k8s resource manifest, then fixed by app.pipecd.yaml (spec.input.namespace).

then, store it and use it on the BASELINE_CLEAN

[ffjlabo]

📝 the namespace of the resourceKey is spec.input.namespace > original namespace

pipecd/pkg/app/piped/platformprovider/kubernetes/loader.go

Lines 90 to 91 in 7b1103d

	setNamespace(manifests, l.input.Namespace)
	sortManifests(manifests)

[ffjlabo]

There are two problems

1. We can't delete the cluster scoped resources if we set the namespace to the app.pipecd.yaml.

• This is because the resource key stored as livestate and the one on the annotation of actual k8s resource are different. values.

2. The resourceKey rule of cluster scoped resources is undefined.

• Currently, if we set the namespace with app.pipecd.yaml (spec.input.namespace), resourceKey is also affected by it.

So I'll resolve 1 in this issue, and later will try 2.

[ffjlabo]

📝 Prepared the issue for 2
#4861