Add console extension manifests

[benjaminapetersen]

Adds 3 new CRDs for console extensions:

• ConsoleLink
• ConsoleCLIDownload
• ConsoleNotification (Banners)

Related to Console PRs:

• ConsoleLink Add ConsoleLink CRD to enable custom links in help and user dropdowns console#1360
• ConsoleCLIDownload Add ConsoleCLIDownload CRD extension console#1441
• ConsoleNotification (Banners) Add ConsoleNotification CRD that can appear above, below, or above and below the page console#1542

[benjaminapetersen]

/hold

WIP. Need to make sure we know where these will live in the API, if the group is correct, and if any will be converted into top-level config/operator config items instead of custom resources.

[benjaminapetersen]

fyi @rhamilto @spadgett

[spadgett]

@jwforres fyi

[benjaminapetersen]

Possibly needs to be links [ ]

[benjaminapetersen]

see https://github.com/openshift/console/pull/1441/files#r276345530

[benjaminapetersen]

Any reason to further consider this? Would there be a link for the binary + a link for docs, for example? @rhamilto @spadgett

[benjaminapetersen]

level=fatal msg="failed to initialize the cluster: Could not update rolebinding \"console-extensions\" (243 of 378): timed out waiting for the condition"

[benjaminapetersen]

/retest

flake details:

Jun 04 21:24:00.789 E clusteroperator/monitoring changed Degraded to True: Failed to rollout the stack. Error: running task Updating prometheus-adapter failed: reconciling PrometheusAdapter Deployment failed: updating deployment object failed: waiting for DeploymentRollout of prometheus-adapter: deployment prometheus-adapter is not ready. status: (replicas: 3, updated: 2, ready: 2, unavailable: 1)
Jun 04 21:25:58.536 E ns/openshift-monitoring pod/prometheus-adapter-7b8ccf5fd4-tdzv6 node/ip-10-0-139-114.ec2.internal container=prometheus-adapter container exited with code 2 (Error): 

[bparees]

I spoke with @enj about the permissions aspect of this. He indicated it is not a blocker, but I can understand his concerns about doing this which largely fall in two areas:

1. there's risk that someone extends this api resource in the future and inadvertently exposes something to all users that should not be exposed. I definitely share this concern. Even if it's not inadvertent you might find yourself wanting to extend this api in the future and being unable to because of the way it is shared.

2. there have been previous exploits that resulted from the platform allowing all users to do something we thought was safe that turned out not to be which has resulted in a general movement away from the use of "all users" type permissions. I think there's less risk of that here (it is a read only resource with very limited surface area).

My view on the way forward is:

Proceed with this as is (Granting permission to all authenticated users) for now. I don't want to see the UI team have to go implement a more complicated config controller flow right this minute, and @spadgett expressed some legitimate concerns about the quality of user experience we could provide if we did so (though i think it might be worth probing what options exist there a bit more, in terms of forcing browser refreshes or something, as suggested by @enj). But file away a tech debt card to refactor this in the future(to use a config controller or other operator logic pattern) so we can eventually remove this permission. Ideally if the additional 4.2 runway allows it, fix it in 4.2, but worst case, keep it on the roadmap, don't view this as the long term solution.

@enj @spadgett Does that seem reasonable?

[enj]

I spoke with @enj about the permissions aspect of this. He indicated it is not a blocker, but I can understand his concerns about doing this which largely fall in two areas:

1. there's risk that someone extends this api resource in the future and inadvertently exposes something to all users that should not be exposed.  I definitely share this concern.  Even if it's not inadvertent you might find yourself wanting to extend this api in the future and being unable to because of the way it is shared.

2. there have been previous exploits that resulted from the platform allowing all users to do something we thought was safe that turned out not to be which has resulted in a general movement away from the use of "all users" type permissions.

I think there's less risk of that here (it is a read only resource with very limited surface area).

For context, the exploit resulted in full cluster compromise via discovery, i.e. an API that is by definition read only and "safe" (because it is just metadata about what APIs are available on the cluster). It had nothing to do with the data itself and was a by product of the complexity of the underling wiring (and CRDs are very complex under the hood). That complexity combined with a missing error check is what led to the CVE.

Do I think another similar CVE is likely? No.

Would I like to assume it will happen and take precautions? Yes.

My view on the way forward is:

Proceed with this as is (Granting permission to all authenticated users) for now. I don't want to see the UI team have to go implement a more complicated config controller flow right this minute, and @spadgett expressed some legitimate concerns about the quality of user experience we could provide if we did so (though i think it might be worth probing what options exist there a bit more, in terms of forcing browser refreshes or something, as suggested by @enj). But file away a tech debt card to refactor this in the future(to use a config controller or other operator logic pattern) so we can eventually remove this permission. Ideally if the additional 4.2 runway allows it, fix it in 4.2, but worst case, keep it on the roadmap, don't view this as the long term solution.

@enj @spadgett Does that seem reasonable?

Sounds reasonable. @spadgett @benjaminapetersen I am trusting you guys to make an honest attempt at an architecture that does not involve assigning RBAC to all authenticated users.

[benjaminapetersen]

/retest

[spadgett]

/retest

[spadgett]

/hold cancel

[spadgett]

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [benjaminapetersen,spadgett]

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