fix: namespaced resources owned by cluster scoped resources don't show up in the UI #8222
Conversation
chetan-rns
force-pushed
the
fix-cluster-owned
branch
from
bf7823b
to
981bf96
Compare
chetan-rns
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8222 +/- ##
==========================================
- Coverage 49.73% 49.56% -0.17%
==========================================
Files 274 273 -1
Lines 48948 48329 -619
==========================================
- Hits 24343 23955 -388
+ Misses 22230 22011 -219
+ Partials 2375 2363 -12 ☔ View full report in Codecov by Sentry. |
| if isNamespaced, _ := c.IsNamespaced(ownerGvk.GroupKind()); isNamespaced { | ||
| namespace = r.Ref.Namespace | ||
| } |
There was a problem hiding this comment.
I was just wondering if we need to check the namespace of the resource being allowed by the AppProject of the owning application? While a namespace-scoped object only refers to an owner in the same namespace, this check was not required. But a cluster-scoped owner can be referred to from any namespace, and the application managing the owner might not have access to those namespaces.
Would it be wise to display the dependents in non-allowed namespaces then?
| @@ -596,7 +596,7 @@ export const ApplicationResourceTree = (props: ApplicationResourceTreeProps) => | |||
| if (treeNodeKey(child) === treeNodeKey(root)) { | |||
| return; | |||
| } | |||
| if (node.namespace === child.namespace) { | |||
| if (node.namespace === child.namespace || node.namespace === undefined) { | |||
There was a problem hiding this comment.
Hi @chetan-rns, help me to understand why/when the node namespace will be undefined as opposed to default? This part sets the edges (lines) between 'parent' nodes and the child nodes. So if the child node's namespace is defined OR undefined, then there will be lines connecting to them all. Is this intentional?
|
Is the only thing blocking this a rebase? This would be super useful. |
|
@blakebarnett We need to get argoproj/gitops-engine#366 PR merged first which is under review |
todaywasawesome
added
the
lifecycle/rotten
chetan-rns
force-pushed
the
fix-cluster-owned
branch
from
981bf96
to
c80e818
Compare
chetan-rns
force-pushed
the
fix-cluster-owned
branch
from
c80e818
to
d57cb2e
Compare
|
Any chance to bring this into 2.11? @chetan-rns @crenshaw-dev |
|
We just built our own version to test that fix, we saw that the namespaced children are now visible in the UI, great 🎉 ! What we still don't see is the children of the children. e.g. Pods are not attached to the deployment, if the deployment is created by a cluster scoped resource. We can see the sub children for cluster scoped resources. Do you experience the same, or do you know what to enable to make that work, too? |
|
@chetan-rns Just pushed a PR to your fork, to also display grand children correctly: chetan-rns/gitops-engine#1 |
|
@woehrl01 Thanks for your help! The logic looks good. I will test it out once. |
…w up in the UI Signed-off-by: Chetan Banavikalmutt <chetanrns1997@gmail.com>
…ster resources Signed-off-by: Chetan Banavikalmutt <chetanrns1997@gmail.com>
Signed-off-by: Chetan Banavikalmutt <chetanrns1997@gmail.com>
chetan-rns
force-pushed
the
fix-cluster-owned
branch
2 times, most recently
from
7413ed1
to
6d9ed6e
Compare
Signed-off-by: Chetan Banavikalmutt <chetanrns1997@gmail.com>
chetan-rns
force-pushed
the
fix-cluster-owned
branch
from
6d9ed6e
to
539a7c6
Compare
Signed-off-by: Chetan Banavikalmutt chetanrns1997@gmail.com
Note on DCO:
If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.
Checklist:
Currently, the namespaced resources owned by cluster scoped resources are not shown on the UI. This PR uses the right namespace for parent resources instead of copying the same namespace as the resource
GitOps Engine: argoproj/gitops-engine#366
Fixes: #7733