Open kevin-namba opened 1 year ago
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"
This might happen because the resource key in the live resource differs from the resource used when pruning.
rbac.authorization.k8s.io/v1:ClusterRole:test:pod-reader
rbac.authorization.k8s.io/v1:ClusterRole:default:pod-reader
(estimated by the code below)
As this PR https://github.com/pipe-cd/pipecd/pull/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.
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 resource key stored as livestate
MakeResourceKey
https://github.com/pipe-cd/pipecd/blob/7b471d7a9f2eb09ccbc0a9222b6e7adb08ea2edf/pkg/app/piped/platformprovider/kubernetes/resourcekey.go#L249-L260The resource key on the annotation of actual k8s resource
app.pipecd.yaml (spec.input.namespace)
Consider the case to apply ClusterRole
test
)
Solution candidates
The point is whether to fix or not as if 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.
load manifests in the same way with K8S_SYNC stage indirectly. (by using loadRunningManifests) https://github.com/pipe-cd/pipecd/blob/7b1103d027fcdbc2468af60deb5da758fcc8f6ae/pkg/app/piped/executor/kubernetes/baseline.go#L46
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
๐ the namespace of the resourceKey is spec.input.namespace
> original namespace
https://github.com/pipe-cd/pipecd/blob/7b1103d027fcdbc2468af60deb5da758fcc8f6ae/pkg/app/piped/platformprovider/kubernetes/loader.go#L90-L91
There are two problems
spec.input.namespace
), resourceKey is also affected by it.So I'll resolve 1 in this issue, and later will try 2.
๐ Prepared the issue for 2 https://github.com/pipe-cd/pipecd/issues/4861
Previous context: https://github.com/pipe-cd/pipecd/issues/4269#issuecomment-2031079364
After some investigations, I decided to fix the logic for the resource key as the ideal state.
I think this problem is the incorrect namespace in the resource key created by MakeResourceKey
.
livestate side
default
when the resource obj doesn't have the namespace.When reading the manifests from git
spec.input.namespace
in app.pipecd.yaml if it is set.default
when the namespace on the manifest is "".So for example, cluster-scoped resource don't have any namespace, but if we set the spec.input.namespace
in the app.pipecd.yaml, it sets the value as the namespace to the resource key.
Here is the ideal state of the resource key.
livestate side
When reading the manifests from git
spec.input.namespace
in app.pipecd.yaml if it is set.default
if both of spec.input.namespace
and the namespace are not setSo I investigated the scope of the usage of MakeResourceKey
.
At first, I drew the call graph.
This shows that the fix affects 4 components.
Next, I categorized each functions by the usecases. The resource key is usually created in the three cases below
I think the 1st and 3rd case don't need MakeResourceKey
below
So the problem is 2nd case. I think it can be solved by reading the resource from the git repo with the proper namespace.
So, it would be nice to unify the responsibility to the loader.LoadManifests. Especially, add function to the loader like this.
func (l *loader) refineNamespace(m Manifest) string {
clusterScoped := true // TODO: determine cluster scoped
if clusterScoped {
return ""
}
if l.input.Namespace != "" {
return l.input.Namespace
}
ns := m.GetNamespace()
if ns == "" {
return "default"
}
return ns
}
Finally, fixing the manifest's namespace only in the loader.Manifests
would be nice.
TODO
pipecd.dev/resource-key
What happened: prune cluster scoped resources (such as
clusterrole
) does not work when specify namespace in app configWhat you expected to happen:
How to reproduce it:
Environment:
piped
version:control-plane
version: