fix(server): don't use cluster scope list + watch in namespaced mode. Fixes #13177 #13189
Conversation
Signed-off-by: Jiacheng Xu <[email protected]>
jiachengxu
force-pushed
the
13177-namespaced
branch
from
2c50f4f
to
a148a36
Compare
jiachengxu
changed the title
agilgur5
changed the title
Signed-off-by: Jiacheng Xu <[email protected]>
| @@ -638,7 +638,7 @@ func getWorkflowServer() (workflowpkg.WorkflowServiceServer, context.Context) { | |||
| if err = wfStore.Add(&wfObj5); err != nil { | |||
| panic(err) | |||
| } | |||
| server := NewWorkflowServer(instanceIdSvc, offloadNodeStatusRepo, archivedRepo, wfClientset, wfStore, wfStore) | |||
| server := NewWorkflowServer(instanceIdSvc, offloadNodeStatusRepo, archivedRepo, wfClientset, wfStore, wfStore, nil) | |||
There was a problem hiding this comment.
Wasn't this one previously correct with metav1.NamespaceAll?
With the new && namespace != nil check, this now takes a different path (but still passes apparently?)
There was a problem hiding this comment.
oh, I think I changed it by accident and I change it back to the metav1.NamespaceAll.
With the new && namespace != nil check, this now takes a different path (but still passes apparently?)
The value of the namespace doesn't matter regarding the test because we don't start the reflector to perform list and watch in the test and we just manually add objects to the store.
There was a problem hiding this comment.
But the reflector is set since the store is not nil in this case, so it would start wouldn't it? It just isn't used actively in this test
There was a problem hiding this comment.
the Run method is not called in the test, so the reflector is not started.
Signed-off-by: Jiacheng Xu <[email protected]>
…ixes argoproj#13177 (argoproj#13189) Signed-off-by: Jiacheng Xu <[email protected]> Signed-off-by: Yulin Li <[email protected]>
…ixes #13177 (#13189) Signed-off-by: Jiacheng Xu <[email protected]> (cherry picked from commit 8f3860d)
Fixes #13177
Verification
Try to apply the following manifests into a k8s cluster:
With the image built from this PR, don't see the following permission errors anymore: