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

Add mechanism for users to specify accessible namespaces #702

Merged
merged 7 commits into from
Nov 9, 2020

Conversation

Nokel81
Copy link
Collaborator

@Nokel81 Nokel81 commented Aug 17, 2020

The namespaces which are accessible to them. This is generally useful for when the user doesn't have permission to list the namespaces.

  • Add new component "EditableList" which provides a simple way to
    display a list of items that can be added too.

  • Add the ClusterAccessibleNamespaces to the GeneralClusterSettings

Signed-off-by: Sebastian Malton smalton@mirantis.com

closes #486

This is a draft PR because it should be targeting master but vue_react_migration hasn't been merged to master yet.

@Nokel81 Nokel81 added the enhancement New feature or request label Aug 17, 2020
@Nokel81 Nokel81 added this to the 3.6.0 milestone Aug 17, 2020
@Nokel81 Nokel81 requested review from jakolehm and ixrock August 17, 2020 19:38
@Nokel81 Nokel81 self-assigned this Aug 17, 2020
@Nokel81 Nokel81 force-pushed the accessible-namespaces branch from 348a23a to 962797b Compare August 19, 2020 14:32
@nevalla nevalla closed this Aug 20, 2020
@nevalla nevalla reopened this Aug 20, 2020
@Nokel81 Nokel81 marked this pull request as ready for review August 20, 2020 13:01
@Nokel81 Nokel81 changed the base branch from vue_react_migration to master August 20, 2020 13:01
@Nokel81 Nokel81 force-pushed the accessible-namespaces branch from 962797b to 7f1208b Compare August 20, 2020 13:02
@Nokel81
Copy link
Collaborator Author

Nokel81 commented Aug 20, 2020

@nevalla changed base to master

@Nokel81 Nokel81 requested review from ixrock and jakolehm August 21, 2020 14:39
@Nokel81 Nokel81 force-pushed the accessible-namespaces branch 2 times, most recently from a1f73b3 to 604db32 Compare September 1, 2020 18:00
@aleksfront
Copy link
Contributor

@Nokel81 Do you have real-world example of a test case for a user without namespace listing rights? Currently I am unable to create such a user. Tested with

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  # "namespace" omitted since ClusterRoles are not namespaced
  name: ns-getter
rules:
- apiGroups: [""]
  resources: ["namespaces"]
  verbs: ["create"]

and

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: read-namespaces-global
subjects:
- kind: User
  name: user1 # Name is case sensitive
  apiGroup: rbac.authorization.k8s.io
roleRef:
  kind: ClusterRole
  name: ns-getter
  apiGroup: rbac.authorization.k8s.io

With this settings the results are actually really strange: namespaces wouldn't listed no matter what's included in the Accessible Namespaces.

@Nokel81
Copy link
Collaborator Author

Nokel81 commented Sep 2, 2020

@aleksfront Sorry I do not have a testing case, I am going off of what what the initial issue said

@nevalla nevalla modified the milestones: 3.6.0, 3.7.0 Sep 4, 2020
@Nokel81 Nokel81 force-pushed the accessible-namespaces branch from d8eda18 to a27b50e Compare September 28, 2020 16:24
@Nokel81 Nokel81 requested review from aleksfront and ixrock October 1, 2020 15:44
@nevalla nevalla modified the milestones: 3.7.0, 4.0.0 Oct 26, 2020
@Nokel81 Nokel81 force-pushed the accessible-namespaces branch from a27b50e to 5d4b767 Compare November 2, 2020 17:02
@Nokel81 Nokel81 requested a review from nevalla November 3, 2020 14:18
@nevalla
Copy link
Contributor

nevalla commented Nov 5, 2020

Generally looking good and I was able to see those namespaces that I was set in cluster settings. Related to configuring namespaces, can we have some hint that editable list that it requires pressing enter to add an item?

Also, probably out of scope of this PR, but after configuring allowed namespaces we should somehow send a signal to the dashboard to update namespaces list.

@Nokel81
Copy link
Collaborator Author

Nokel81 commented Nov 5, 2020

@nevalla Good idea, when I close this I will make a follow up ticket.

@aleksfront Given Lauri's review, can you accept this PR?

Copy link
Contributor

@aleksfront aleksfront left a comment

Choose a reason for hiding this comment

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

LGTM with minor i18n comments!

Sebastian Malton and others added 7 commits November 9, 2020 10:05
…them. This is generally useful for when the user doesn't have permission to list the namespaces.

- Add new component "EditableList" which provides a simple way to
  display a list of items that can be added too.

- Add the ClusterAccessibleNamespaces to the GeneralClusterSettings

Signed-off-by: Sebastian Malton <smalton@mirantis.com>
Signed-off-by: Sebastian Malton <smalton@mirantis.com>
Signed-off-by: Sebastian Malton <smalton@mirantis.com>
Signed-off-by: Sebastian Malton <smalton@mirantis.com>
Signed-off-by: Sebastian Malton <smalton@mirantis.com>
Signed-off-by: Sebastian Malton <smalton@mirantis.com>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possibility to define available namespaces
5 participants