Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve set active or create new project/namespace workflow #4122

Conversation

vrubezhny
Copy link
Contributor

@vrubezhny vrubezhny commented May 13, 2024

  • Now, when switching projects/namespaces, one can manually type in a project name to be set as an active one
  • No more 'Missing project/namespace' item for a project/namespace that doesn't exist on a cluster. This allows
    working normally on clusters with restrictions on list for projects.

Fixes: #3999

  • The project listing is fixed, so annotated projects are shown now

Fixes: #4101

Issue: #4080 (?)

@vrubezhny vrubezhny marked this pull request as draft May 13, 2024 22:23
@vrubezhny vrubezhny force-pushed the fix-set-or-create-active-project-workflow branch from 7331fce to f817a28 Compare May 13, 2024 22:56
@codecov-commenter
Copy link

codecov-commenter commented May 13, 2024

Codecov Report

Attention: Patch coverage is 42.16867% with 48 lines in your changes are missing coverage. Please review.

Project coverage is 44.60%. Comparing base (da60441) to head (beaaeec).
Report is 235 commits behind head on main.

Current head beaaeec differs from pull request most recent head 0c81c4e

Please upload reports for the commit 0c81c4e to get more accurate results.

Files Patch % Lines
src/oc/ocWrapper.ts 53.48% 20 Missing ⚠️
src/explorer.ts 34.78% 15 Missing ⚠️
src/openshift/project.ts 20.00% 12 Missing ⚠️
src/util/loginUtil.ts 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #4122       +/-   ##
===========================================
+ Coverage   32.37%   44.60%   +12.23%     
===========================================
  Files          85       88        +3     
  Lines        6505     7113      +608     
  Branches     1349     1502      +153     
===========================================
+ Hits         2106     3173     +1067     
+ Misses       4399     3940      -459     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@datho7561
Copy link
Contributor

getActiveProject() fails with a null dereference if you can't list projects, since the return value of fixActiveProject is null. One consequence of this failure is that the Application Explorer says you are logged out, even if you are allowed to perform some actions on the cluster (such as listing pods on all namespaces).

oc projects fails if the user cannot list namespaces, so you might need to use another method to detect the current project. Probably the best (and maybe only) approach is to read the kubeconfig.

I haven't tried this on an OpenShift instance, I've been using kind. I can do a short write up on creating a service account that can't list projects on a kind cluster if you are interested.

@vrubezhny
Copy link
Contributor Author

I haven't tried this on an OpenShift instance, I've been using kind. I can do a short write up on creating a service account that can't list projects on a kind cluster if you are interested.

@datho7561 This would be really helpful.

@vrubezhny
Copy link
Contributor Author

oc projects fails if the user cannot list namespaces, so you might need to use another method to detect the current project. Probably the best (and maybe only) approach is to read the kubeconfig.

I use Kube config to detect an active project, because the way I run oc projects, even if it succeeds, doesn't allow to get the active project info from its output (I execute it with -q argument, which makes its output more short and not requiring any special parsing, but also make it to lack an info on which project is currently set as active. (However I expect oc also to use Kube config in order to get the active project)

The problem may happen when Kube config has active project set, but that project doesn't really exist on the cluster.
Previously a Missing project was shown on the tree with the selected name, while with this fix (if there are any projects exist on the cluster) the first available project will be actually selected, but not the chosen one (either thru Kube config or by entering its name manually). This probably is worth to be restored.

@vrubezhny vrubezhny marked this pull request as draft May 14, 2024 16:21
@vrubezhny vrubezhny force-pushed the fix-set-or-create-active-project-workflow branch from f817a28 to 4906029 Compare May 14, 2024 16:37
@vrubezhny
Copy link
Contributor Author

Changed fixActiveProject so:

  • it never returns null.
  • If Kube Config has a namespace set that doesn't exists in the cluster projects, it doesn't change the active property of any cluster project , but still sets such "manually" set project as a active in Kube config (So it won't be changed neither to default nor to the first listed project, thus making it possible to correctly select active project when a cluster doesn't allow to get the list of projects)

@adietish
Copy link
Contributor

adietish commented May 15, 2024

Things dont work for me properly when only listing pods is allowed (as instructed in #3999):
The current namespace ("default") is displayed but reported as missing.
image
This is erroneous since I can list pods that exist in this namespace:

$ kubectl get pod -n default
NAME                           READY   STATUS             RESTARTS           AGE
apple-app                      1/1     Running            0                  67m
banana-app                     1/1     Running            0                  67m
frontend-9cr6z                 0/1     ImagePullBackOff   0                  67m
frontend-kjrpx                 0/1     ErrImagePull       0                  67m
frontend-vbhb7                 0/1     ErrImagePull       0                  67m
mehdb-0                        0/1     CrashLoopBackOff   18 (15s ago)       68m
sise-deploy-69d88467b4-vhfsd   0/1     CrashLoopBackOff   3415 (2m10s ago)   58d

I think that you cannot know if a namespace exists or not if you're not allowed to get the namespace resource or all the namespaces.
If you're not allowed to list all the namespaces or a specific namespace, you always get 401 Forbidden, whether it exists or not.

  1. When trying to get a namespace that exists:
$ kubectl get ns default
Error from server (Forbidden): namespaces "default2" is forbidden: User "user1" cannot get resource "namespaces" in API group "" in the namespace "default"
  1. When trying to get a namespace that DOESNT exist:
$ kubectl get ns default2
Error from server (Forbidden): namespaces "default2" is forbidden: User "user1" cannot get resource "namespaces" in API group "" in the namespace "default2"

401 Forbidden in both cases.

@adietish
Copy link
Contributor

adietish commented May 15, 2024

Furthermore I cannot select my current namespace. I cannot confirm the upcoming prompt by hitting the return key, it stays open (namespace "default" exists, I simply cannot retrieve it. I can only retrieve pods within it):

Screen.Recording.2024-05-15.at.17.04.57.mov

@vrubezhny vrubezhny force-pushed the fix-set-or-create-active-project-workflow branch from 4906029 to 4d0a1bd Compare May 15, 2024 16:01
@vrubezhny
Copy link
Contributor Author

Furthermore I cannot select my current namespace. I cannot confirm the upcoming prompt by hitting the return key, it stays open (namespace "default" exists, I simply cannot retrieve it. I can only retrieve pods within it):

It's a filter, not an input field for a project/namespace name, you should click Create new... or Manually set... item first, then type in a project name:

Screencast.from.2024-05-15.18-17-48.webm

@vrubezhny
Copy link
Contributor Author

I've made no item like xxx - Missing project/namespace to appear anymore. For a project/namespace that doesn't exist on a cluster or exist but cannot be listed due to restrictions in cluster configuration a normal project/namespace item will appear with all its children. The content of the children items Deployments/Workloads/etc is to be defined depending on the existence or applied restrictions.

@vrubezhny vrubezhny force-pushed the fix-set-or-create-active-project-workflow branch 2 times, most recently from 27e29db to d087993 Compare May 15, 2024 17:53
adietish
adietish previously approved these changes May 16, 2024
Copy link
Contributor

@adietish adietish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vrubezhny Things work fine now 😄

I can use the namespace "default", there's no warning "Missing Namespace" any more and I can list the pods that are running and that I am allowed to list (according to my custom, restrictive RBAC).

The only thing that I am wondering about is that listing forbidden resources cause error notifications to appear while the cathegory stays empty.
image
I am wondering if a child node with the error would be preferrable (screenshot of intellij-kubernetes)?

@vrubezhny
Copy link
Contributor Author

I am wondering if a child node with the error would be preferrable (screenshot of intellij-kubernetes)?

@datho7561 What do you think about this proposal?
If we can detect (by oc auth can-i get ... or somehow else) that listing a node objects is restricted, we can create some meaningful labels like Deployments cannot be shown due to the cluster configuration restrictions or similar and not to try to get, f.i., Deployments children which definitely leads into the mentioned error message appearance.

@datho7561
Copy link
Contributor

@datho7561 What do you think about this proposal? If we can detect (by oc auth can-i get ... or somehow else) that listing a node objects is restricted, we can create some meaningful labels like Deployments cannot be shown due to the cluster configuration restrictions or similar and not to try to get, f.i., Deployments children which definitely leads into the mentioned error message appearance.

I like this idea.

@datho7561
Copy link
Contributor

I think we can use kubectl api-versions to detect if a user is authenticated with the server when they are using a non-OpenShift cluster. I'll double check, but I think this API is available when a user is authenticated with a server, regardless of the roles they have been assigned.

@vrubezhny vrubezhny force-pushed the fix-set-or-create-active-project-workflow branch 2 times, most recently from aa392f2 to 121fae5 Compare May 16, 2024 18:24
@vrubezhny
Copy link
Contributor Author

vrubezhny commented May 16, 2024

I think we can use kubectl api-versions to detect if a user is authenticated with the server when they are using a non-OpenShift cluster. I'll double check, but I think this API is available when a user is authenticated with a server, regardless of the roles they have been assigned.

The current PR version is updated to use oc whoami --show-server followed by (if successful) oc api-versions (if not empty response) to detect whether we're really logged in to a cluster or not.

Also, Delete project/namespace context menu action isn't shown when the project/namespace doesn't exist on the cluster or it's not allowed to list projects/namespaces on the cluster due to restricted configuration

@vrubezhny vrubezhny force-pushed the fix-set-or-create-active-project-workflow branch from 121fae5 to daf9a93 Compare May 16, 2024 19:30
@vrubezhny
Copy link
Contributor Author

vrubezhny commented May 16, 2024

The Change Active Project/Namespace inline and context menu actions are now enabled without any restrictions on OpenShift and Kubernates cluster projects. The contents of quick pick is limited according to the restrictions applied to the cluster: in the most restricted case (when it's not allowed to create or list projects/namespaces) it will contain the only one item Manually set active Namespace/Project:

Screencast.from.2024-05-16.22-39-15.webm

Tested using the "restricted" Kind cluster service account created according the redhat-developer/idetools.dev#49

src/openshift/project.ts Outdated Show resolved Hide resolved
@vrubezhny vrubezhny force-pushed the fix-set-or-create-active-project-workflow branch 3 times, most recently from 382d4f3 to beaaeec Compare May 17, 2024 15:56
@vrubezhny vrubezhny marked this pull request as ready for review May 17, 2024 15:57
@vrubezhny vrubezhny requested a review from datho7561 May 17, 2024 15:57
@vrubezhny
Copy link
Contributor Author

Now the App. Explorer Tree on a "restricted" cluster looks like the following:

Screenshot from 2024-05-17 16-54-57

cc @adietish

Copy link
Contributor

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me!

@datho7561
Copy link
Contributor

@vrubezhny feel free to merge, unless you want to wait on feedback from Andre on the appearance of the error tree nodes.

- Now, when switching projects/namespaces, one can manually type in a project name to be set as an active one.
- No more 'Missing project/namespace' item for a project/namespace that doesn't exist on a cluster. This allows
  working normally on clusters with restrictions on list for projects.

Fixes: redhat-developer#3999

- The project listing is fixed, so annotated projects are shown now

Fixes: redhat-developer#4101

- For a Sandbox cluster a project which name contains current user name is used as a default one when logging in
  (A follow up to redhat-developer#4109)

Issue: redhat-developer#4080 (?)

Improved the detection whether we're logged in to a cluster or not.

Signed-off-by: Victor Rubezhny <vrubezhny@redhat.com>
@vrubezhny vrubezhny force-pushed the fix-set-or-create-active-project-workflow branch from beaaeec to 0c81c4e Compare May 17, 2024 18:40
@vrubezhny vrubezhny merged commit 59117e6 into redhat-developer:main May 17, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants