Add Customize Project Access Roles proposal

[jerolimov]

Issues:
Epic: https://issues.redhat.com/browse/ODC-4650
Story: https://issues.redhat.com/browse/ODC-5394

Rendered preview: https://github.com/jerolimov/openshift-enhancements/blob/customize-project-roles/enhancements/console/customize-project-roles.md

[jerolimov]

/cc @christianvogt I can create a meeting for a realtime sync on this. :)

[christianvogt]

I think we should support both Role and ClusterRole.
@spadgett what exactly did 3.11 support? I want to say that it was a single list that performed a lookup by name in both cluster roles and roles lists.

The namespace will be implied based on the project in which the rolebinding will be created.

[jhadvig]

@christianvogt check https://docs.openshift.com/container-platform/3.11/install_config/web_console_customization.html#changing-the-membership-whitelist which was done by loading extension scripts.
Based on the docs, we only supported ClusterRole.

[spadgett]

I would limit this to ClusterRole for now. I'm not sure it's a good idea to allow this to be configured for Role since Roles with the same names in different namespaces can have totally different permission. The cluster admin can't know what these will be when configuring the option. Also there is no way in the current UI to differentiate between Role and ClusterRole, so as a project admin you won't know exactly what permission you're granting.

[jerolimov]

We can show "(R)" or "(CR)" icon in the dropdown, right?

But I think your right, ClusterRole is fine because the admin could not predict what project admins allow with their namespaced Roles -- as you mentioned.

So I keep just a string of names at the moment. With the explicit name "...clusterRoles" we could also bring up another key if we want support multiple kinds in the future.

[jhadvig]

Also noticed that the dropdown has Role label and also the help-text above is linking to Role docs. If we want to just support ClusterRole for no we should make that clear, for both mentioned elements.

[jerolimov]

Good point, then we should also link directly to ClusterRoles and ClusterRoleBindings in the help text above the input. Sam, Christian, @serenamarie125 fine with that?

[christianvogt]

I'm not a fan of isolating the config under a developerPerspective stanza just because I don't want isolate a feature to one perspective.

[jerolimov]

@christianvogt @spadgett Updated the doc again. Going with ClusterRoles only for now. The list are only strings now, if we want support other roles later we can also keep the kind and validate that the user uses only ClusterRole in the first version.

/cc @jhadvig @rottencandy

And we have another issue with different ideas about the yaml property name. My initial approach was customization.projectAccess

Sam recommended a developerPerspective separation like customization.developerPerspective.clusterRoleOptions because this cluster role options will not be shown in the admin perspective. Btw: Why not?

Christian is against a separation for perspectives. Now we need a decision. I also can provide some other ideas:

My current suggestion:

spec:
  customization:
    brand: online
    projectAdmin:
      availableClusterRoles:
      - admin
      - edit
      - view
  ...

Wdyt?

[jerolimov]

/uncc adambkaplan cooktheryan
/assign spadgett
/retest

[jhadvig]

@jerolimov thanks for all this, I've added couple of comments, looks really good 👍
Regarding the structuring of the consoleserver config. I would not necessarily go with customization. developerPerspective since that field would also need to contain other fields that are DecConsole specific, like DeveloperCatalog. But since we are not separating the perspectives from the beginning, I don't think we should start now, but rather stick to the approach we have.

From my understanding, the customised ClusterRoles will be only visible in the DevPerspective -> Project Access page. For that reason I would suggest to use projectAccess, instead of projectAdmin field name:

apiVersion: operator.openshift.io/v1
kind: Console
metadata:
  name: cluster
  ...
spec:
  customization:
    brand: online
    projectAccess:
      availableClusterRoles:
      - admin
      - edit
      - view

We should also keep in mind that if in the future we decide to support also Roles, we can just add another field which would also contain a specific namespace:

apiVersion: operator.openshift.io/v1
kind: Console
metadata:
  name: cluster
  ...
spec:
  customization:
    brand: online
    projectAccess:
      availableClusterRoles:
      - admin
      - edit
      - view
      availableRoles:
        namespace: foo
        roles:
        - ...

[christianvogt]

^^ I'm good with the suggestion. Went full circle on projectAccess :)
I have a feeling that at some point the "perspectives" will be nothing more than an opinionated nav when we get to allow for custom navigation. In which case there won't be a hard line between what is "admin" vs "dev" perspective.

[jerolimov]

Thanks Jakub, I liked projectAccess also more then projectAdmin, also because this PR used "project access ..." always in its title. 😄

Doc updated. Anything else to-do here? 😏

[kotarusv]

This feature is very much required for Cisco. This is a kind of show stopper for our migration from 3.11

[jhadvig]

@jerolimov I think we should be good to merge this. @spadgett any additional comments?

[christianvogt]

/lgtm

[jerolimov]

As result of #668 (review) and our short discussed on slack, I just changed the annotation for the label to console.openshift.io/display-name. Nothing else has changed.

[spadgett]

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jhadvig, spadgett

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [spadgett]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment