Bug 1872375: Add RedHat Helm chart repository as default repo in OCP payload #458
Conversation
| - helmchartrepositories | ||
| verbs: | ||
| - get | ||
| - list |
There was a problem hiding this comment.
Looks good to me, all authenticated users should be allowed to 'read' helm chart repositories. That'll naturally drive the content in the developer catalog.
Any advanced configuration needs to be done by the admin outside the scope of console.
There was a problem hiding this comment.
no, because the operator does not watch for changes. The permissions are needed for console /api/helm/charts/index.yaml endpoiint - it retrieves the list of all HelmChartRepository CRs from the cluster, and use them to retrieve individual index.yaml files that get combined at the end.
manifests/05-helm.yaml
Outdated
| spec: | ||
| name: Red Hat Helm Charts | ||
| connectionConfig: | ||
| url: https://redhat-developer.github.io/redhat-helm-charts |
There was a problem hiding this comment.
So,
- All authenticated users get to see helm chart repos
- Ootb, they see the default one. https://redhat-developer.github.io/redhat-helm-charts was previously hard-coded in the console config - instead of giving it any special treatment, it is being made just another CR. 👍
There was a problem hiding this comment.
Ootb, they see the default one. https://redhat-developer.github.io/redhat-helm-charts was previously hard-coded in the console config
you mean in the console-config CM in openshift-console namespace ?
There was a problem hiding this comment.
HelmChartRepository is cluster scoped.
|
@spadgett @christianvogt @rohitkrai03 |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
manifests/05-helm.yaml
Outdated
| spec: | ||
| name: Red Hat Helm Charts | ||
| connectionConfig: | ||
| url: https://redhat-developer.github.io/redhat-helm-charts |
There was a problem hiding this comment.
Ootb, they see the default one. https://redhat-developer.github.io/redhat-helm-charts was previously hard-coded in the console config
you mean in the console-config CM in openshift-console namespace ?
| - helmchartrepositories | ||
| verbs: | ||
| - get | ||
| - list |
@jhadvig - yes - it is already deployed as the part console-operator payload |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
| subjects: | ||
| - kind: Group | ||
| apiGroup: rbac.authorization.k8s.io | ||
| name: 'system:authenticated' |
There was a problem hiding this comment.
I believe you're need a PR like openshift/origin#23975 to get this to pass CI.
There was a problem hiding this comment.
Pushed in openshift/origin#25485
pedjak
changed the title
openshift-ci-robot
added
bugzilla/severity-urgent
|
@pedjak: This pull request references Bugzilla bug 1872375, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/bugzilla refresh |
openshift-ci-robot
added
bugzilla/severity-high
|
@pedjak: This pull request references Bugzilla bug 1872375, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci-robot
removed
the
bugzilla/invalid-bug
Although console endpoint `/api/helm/charts/index.yaml` handles the situation when there is no HelmChartRepository CR present in the cluster, we should align us to other default cluster settings and provide the default HelmChartRepository CR in the payload. Default configuration can be removed/edited by cluster admin. Prior introducing openshift/console#5933 all authenticated users could browse the charts from the chart repo. This PR restores that functionality by introducing additional `helm-chartrepos-viewer` ClusterRole, binding it to all authenticated users.
pedjak
force-pushed
the
helm-defaul-ocp-payload-and-perms
branch
from
3dab1c5
to
afb9dd3
Compare
|
/retest |
|
#458 (comment) will need to be delivered before merging. |
|
/retest |
1 similar comment
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jhadvig, pedjak 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:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@pedjak: All pull requests linked via external trackers have merged: Bugzilla bug 1872375 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Although console endpoint
/api/helm/charts/index.yamlhandles the situationwhen there is no HelmChartRepository CR present in the cluster, we should align
us to other default cluster settings and provide the default HelmChartRepository CR in the payload
Prior introducing openshift/console#5933 all authenticated users could browse the charts from the chart repo.
This PR restores that functionality by introducing additional
helm-chartrepos-viewerClusterRole,binding it to all authenticated users.