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

STOR-1065: Publish ClusterRoles for csi driver sidecars #379

Conversation

mpatlasov
Copy link
Contributor

@mpatlasov mpatlasov commented Jun 4, 2023

The PR adds a bunch of new ClusterRoles to manifests/ dir. This way those new ClusterRoles will be always created, regardless of cloud. They are building blocks to compose ClusterRoles for CSI dirver sidecars. For example, external-provisioner sidecar for aws-ebs csi driver can compose the same ClusterRole as in https://github.com/openshift/aws-ebs-csi-driver-operator/blob/master/assets/rbac/provisioner_role.yaml by adding ClusterRoleBindings for openshift-csi-main-provisioner-role and openshift-csi-provisioner-volumesnapshot-reader-role. The only exception is leases rules which need to be moved from ClusterRoles to per-namespace Roles anyway.

As soon as this change is merged into cluster-storage-operator, it will be possible to get rid of ClusterRole definitions in csi driver operators, they will only define ClusterRoleBindings referring these new ClusterRoles.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 4, 2023
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jun 4, 2023

@mpatlasov: This pull request references STOR-1065 which is a valid jira issue.

In response to this:

Each pkg/operator/csidriveroperator/csioperatorclient/<csi-driver-operator-name>.go publishes new ClusterRole[s]. The result of summing up these roles (per csi driver) is the set of rules identical to assets/rbac/provisioner_role.yaml in the corresponding csi driver operator repo (e.g. https://github.com/openshift/aws-ebs-csi-driver-operator/blob/master/assets/rbac/provisioner_role.yaml).

As soon as this change is merged into cluster-storage-operator, it will be possible to get rid of ClusterRole definitions in csi driver operators, they will only define ClusterRoleBindings referring these new ClusterRoles.

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.

@mpatlasov
Copy link
Contributor Author

/assign @gnufied
/label docs-approved
/label px-approved

@openshift-ci openshift-ci bot added docs-approved Signifies that Docs has signed off on this PR px-approved Signifies that Product Support has signed off on this PR labels Jun 4, 2023
@openshift-ci openshift-ci bot requested review from dobsonj and gnufied June 4, 2023 21:40
@mpatlasov
Copy link
Contributor Author

/retest

@mpatlasov mpatlasov force-pushed the publish-external-provisioner-clusterroles-for-sidecars branch from 204737a to d44576a Compare June 5, 2023 20:40
assets/sidecars/configmap_provisioner_role.yaml Outdated Show resolved Hide resolved
assets/sidecars/lease_provisioner_role.yaml Outdated Show resolved Hide resolved
assets/sidecars/volumeattachment_provisioner_role.yaml Outdated Show resolved Hide resolved
Comment on lines 3 to 11
metadata:
name: volumesnapshot-external-provisioner-role
rules:
- apiGroups: ["snapshot.storage.k8s.io"]
resources: ["volumesnapshots"]
verbs: ["get", "list"]
- apiGroups: ["snapshot.storage.k8s.io"]
resources: ["volumesnapshotcontents"]
verbs: ["get", "list"]
Copy link
Contributor

Choose a reason for hiding this comment

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

dtto, if a storage backed supports snapshots, this (and more) will be part of external-snapshotter role.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as in the comment above: provisioner needs only two verbs: ["get", "list"], while snapshotter wants more: ["get", "list", "watch", "update"].

assets/sidecars/secret_provisioner_role.yaml Outdated Show resolved Hide resolved
@mpatlasov mpatlasov force-pushed the publish-external-provisioner-clusterroles-for-sidecars branch from d44576a to 2d6174e Compare June 7, 2023 05:15
@mpatlasov mpatlasov changed the title STOR-1065: Publish ClusterRoles for external-provisioner sidecar STOR-1065: Publish ClusterRoles for csi driver sidecars Jun 7, 2023
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jun 7, 2023

@mpatlasov: This pull request references STOR-1065 which is a valid jira issue.

In response to this:

The PR adds a bunch of new ClusterRoles to manifests/ dir. This way those new ClusterRoles will be always created, regardless of cloud. They are building blocks to compose ClusterRoles for CSI dirver sidecars. For example, external-attacher sidecar for aws-ebs csi driver can compose the same ClusterRole as in https://github.com/openshift/aws-ebs-csi-driver-operator/blob/master/assets/rbac/attacher_role.yaml by adding ClusterRoleBindings for main-attacher-role, openshift-csi-csinode-reader-role, openshift-csi-volumeattachment-reader-role, and openshift-csi-volumeattachment-writer-role. The only exception is leases rules which need to be moved from ClusterRoles to per-namespace Roles anyway.

As soon as this change is merged into cluster-storage-operator, it will be possible to get rid of ClusterRole definitions in csi driver operators, they will only define ClusterRoleBindings referring these new ClusterRoles.

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.

@mpatlasov
Copy link
Contributor Author

The last force-push fully reworks PR putting all ClusterRole building blocks (for all sidecars) to manifests/ dir. See this comment.

@mpatlasov
Copy link
Contributor Author

/retest

1 similar comment
@mpatlasov
Copy link
Contributor Author

/retest

@mpatlasov mpatlasov force-pushed the publish-external-provisioner-clusterroles-for-sidecars branch from 2d6174e to 00e9da7 Compare June 9, 2023 00:09
@mpatlasov
Copy link
Contributor Author

New force-push updates PR:

  1. remove manifests/openshift-csi-csinodeinfo-reader-role.yaml altogether (per discussion with Jan)
  2. added annotations to make CVO happy

@mpatlasov
Copy link
Contributor Author

/retest ci/prow/hypershift-aws-e2e-external

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 9, 2023

@mpatlasov: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test e2e-aws-ovn
  • /test e2e-aws-ovn-upgrade
  • /test e2e-gcp-ovn
  • /test hypershift-aws-e2e-external
  • /test images
  • /test unit
  • /test verify
  • /test verify-deps

The following commands are available to trigger optional jobs:

  • /test e2e-aws-csi
  • /test e2e-aws-shared-resources
  • /test e2e-azure-csi
  • /test e2e-azure-file-csi
  • /test e2e-azure-ovn
  • /test e2e-gcp-csi
  • /test e2e-openstack
  • /test e2e-openstack-parallel
  • /test e2e-ovirt
  • /test e2e-ovn-vsphere
  • /test e2e-vsphere-csi

Use /test all to run the following jobs that were automatically triggered:

  • pull-ci-openshift-cluster-storage-operator-master-e2e-aws-ovn
  • pull-ci-openshift-cluster-storage-operator-master-e2e-aws-ovn-upgrade
  • pull-ci-openshift-cluster-storage-operator-master-e2e-azure-csi
  • pull-ci-openshift-cluster-storage-operator-master-e2e-azure-file-csi
  • pull-ci-openshift-cluster-storage-operator-master-e2e-azure-ovn
  • pull-ci-openshift-cluster-storage-operator-master-e2e-gcp-ovn
  • pull-ci-openshift-cluster-storage-operator-master-e2e-ovn-vsphere
  • pull-ci-openshift-cluster-storage-operator-master-e2e-vsphere-csi
  • pull-ci-openshift-cluster-storage-operator-master-hypershift-aws-e2e-external
  • pull-ci-openshift-cluster-storage-operator-master-images
  • pull-ci-openshift-cluster-storage-operator-master-unit
  • pull-ci-openshift-cluster-storage-operator-master-verify
  • pull-ci-openshift-cluster-storage-operator-master-verify-deps

In response to this:

/retest ci/prow/hypershift-aws-e2e-external

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.

@mpatlasov
Copy link
Contributor Author

/test ci/prow/hypershift-aws-e2e-external

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 9, 2023

@mpatlasov: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test e2e-aws-ovn
  • /test e2e-aws-ovn-upgrade
  • /test e2e-gcp-ovn
  • /test hypershift-aws-e2e-external
  • /test images
  • /test unit
  • /test verify
  • /test verify-deps

The following commands are available to trigger optional jobs:

  • /test e2e-aws-csi
  • /test e2e-aws-shared-resources
  • /test e2e-azure-csi
  • /test e2e-azure-file-csi
  • /test e2e-azure-ovn
  • /test e2e-gcp-csi
  • /test e2e-openstack
  • /test e2e-openstack-parallel
  • /test e2e-ovirt
  • /test e2e-ovn-vsphere
  • /test e2e-vsphere-csi

Use /test all to run the following jobs that were automatically triggered:

  • pull-ci-openshift-cluster-storage-operator-master-e2e-aws-ovn
  • pull-ci-openshift-cluster-storage-operator-master-e2e-aws-ovn-upgrade
  • pull-ci-openshift-cluster-storage-operator-master-e2e-azure-csi
  • pull-ci-openshift-cluster-storage-operator-master-e2e-azure-file-csi
  • pull-ci-openshift-cluster-storage-operator-master-e2e-azure-ovn
  • pull-ci-openshift-cluster-storage-operator-master-e2e-gcp-ovn
  • pull-ci-openshift-cluster-storage-operator-master-e2e-ovn-vsphere
  • pull-ci-openshift-cluster-storage-operator-master-e2e-vsphere-csi
  • pull-ci-openshift-cluster-storage-operator-master-hypershift-aws-e2e-external
  • pull-ci-openshift-cluster-storage-operator-master-images
  • pull-ci-openshift-cluster-storage-operator-master-unit
  • pull-ci-openshift-cluster-storage-operator-master-verify
  • pull-ci-openshift-cluster-storage-operator-master-verify-deps

In response to this:

/test ci/prow/hypershift-aws-e2e-external

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.

@mpatlasov
Copy link
Contributor Author

/test hypershift-aws-e2e-external

@mpatlasov
Copy link
Contributor Author

/retest

@mpatlasov
Copy link
Contributor Author

/retest

@mpatlasov mpatlasov force-pushed the publish-external-provisioner-clusterroles-for-sidecars branch from 00e9da7 to d114bbb Compare June 20, 2023 23:34
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jun 20, 2023

@mpatlasov: This pull request references STOR-1065 which is a valid jira issue.

In response to this:

The PR adds a bunch of new ClusterRoles to manifests/ dir. This way those new ClusterRoles will be always created, regardless of cloud. They are building blocks to compose ClusterRoles for CSI dirver sidecars. For example, external-attacher sidecar for aws-ebs csi driver can compose the same ClusterRole as in https://github.com/openshift/aws-ebs-csi-driver-operator/blob/master/assets/rbac/attacher_role.yaml by adding ClusterRoleBindings for main-attacher-role, openshift-csi-volumeattachment-reader-role, and openshift-csi-volumeattachment-writer-role. The only exception is leases rules which need to be moved from ClusterRoles to per-namespace Roles anyway.

As soon as this change is merged into cluster-storage-operator, it will be possible to get rid of ClusterRole definitions in csi driver operators, they will only define ClusterRoleBindings referring these new ClusterRoles.

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.

@mpatlasov
Copy link
Contributor Author

/test e2e-azure-csi
/test e2e-azure-ovn

@gnufied
Copy link
Member

gnufied commented Jun 23, 2023

/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 23, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 23, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gnufied, mpatlasov

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 23, 2023
@openshift-merge-robot openshift-merge-robot merged commit 0888a23 into openshift:master Jun 23, 2023
10 of 14 checks passed
mpatlasov added a commit to mpatlasov/aws-efs-csi-driver-operator that referenced this pull request Jun 25, 2023
PR openshift/cluster-storage-operator#379 published builiding blocks of sidecar ClusterRoles. Now, aws-efs csi driver operator may compose its sidecars ClusterRoles from those building blocks.

This PR also moves permissions for `leases` resource from ClusterRole to per-namespace Role (`assets/rbac/lease_leader_election_role.yaml`).
mpatlasov added a commit to mpatlasov/gcp-pd-csi-driver-operator that referenced this pull request Jun 26, 2023
PR openshift/cluster-storage-operator#379 published builiding blocks of sidecar ClusterRoles. Now, gcp-pd csi driver operator may compose its sidecars ClusterRoles from those building blocks.

This PR also moves permissions for `leases` resource from ClusterRole to per-namespace Role (`assets/rbac/lease_leader_election_role.yaml`).
mpatlasov added a commit to mpatlasov/gcp-filestore-csi-driver-operator that referenced this pull request Jun 26, 2023
PR openshift/cluster-storage-operator#379 published builiding blocks of sidecar ClusterRoles. Now, gcp-filestore csi driver operator may compose its sidecars ClusterRoles from those building blocks.

This PR also moves permissions for `leases` resource from ClusterRole to per-namespace Role (`assets/rbac/lease_leader_election_role.yaml`).
mpatlasov added a commit to mpatlasov/gcp-filestore-csi-driver-operator that referenced this pull request Jun 26, 2023
PR openshift/cluster-storage-operator#379 published builiding blocks of sidecar ClusterRoles. Now, gcp-filestore csi driver operator may compose its sidecars ClusterRoles from those building blocks.

This PR also moves permissions for `leases` resource from ClusterRole to per-namespace Role (`assets/rbac/lease_leader_election_role.yaml`).
mpatlasov added a commit to mpatlasov/gcp-filestore-csi-driver-operator that referenced this pull request Jun 26, 2023
PR openshift/cluster-storage-operator#379 published builiding blocks of sidecar ClusterRoles. Now, gcp-filestore csi driver operator may compose its sidecars ClusterRoles from those building blocks.

This PR also moves permissions for `leases` resource from ClusterRole to per-namespace Role (`assets/rbac/lease_leader_election_role.yaml`).
mpatlasov added a commit to mpatlasov/gcp-pd-csi-driver-operator that referenced this pull request Jun 27, 2023
PR openshift/cluster-storage-operator#379 published builiding blocks of sidecar ClusterRoles. Now, gcp-pd csi driver operator may compose its sidecars ClusterRoles from those building blocks.

This PR also moves permissions for `leases` resource from ClusterRole to per-namespace Role (`assets/rbac/lease_leader_election_role.yaml`).
mpatlasov added a commit to mpatlasov/azure-disk-csi-driver-operator that referenced this pull request Jun 28, 2023
PR openshift/cluster-storage-operator#379 published builiding blocks of sidecar ClusterRoles. Now, azure-disk csi driver operator may compose its sidecars ClusterRoles from those building blocks.

This PR also moves permissions for `leases` resource from ClusterRole to per-namespace Role (`assets/rbac/lease_leader_election_role.yaml`).
mpatlasov added a commit to mpatlasov/azure-file-csi-driver-operator that referenced this pull request Jun 28, 2023
PR openshift/cluster-storage-operator#379 published builiding blocks of sidecar ClusterRoles. Now, azure-file csi driver operator may compose its sidecars ClusterRoles from those building blocks.

This PR also moves permissions for `leases` resource from ClusterRole to per-namespace Role (`assets/rbac/lease_leader_election_role.yaml`).
mpatlasov added a commit to mpatlasov/ibm-powervs-block-csi-driver-operator that referenced this pull request Jul 3, 2023
PR openshift/cluster-storage-operator#379 published builiding blocks of sidecar ClusterRoles. Now, ibm-powervs-block csi driver operator may compose its sidecars ClusterRoles from those building blocks.

This PR also moves permissions for `leases` resource from ClusterRole to per-namespace Role (`assets/rbac/lease_leader_election_role.yaml`).
mpatlasov added a commit to mpatlasov/ibm-vpc-block-csi-driver-operator that referenced this pull request Jul 3, 2023
PR openshift/cluster-storage-operator#379 published builiding blocks of sidecar ClusterRoles. Now, ibm-vpc-block csi driver operator may compose its sidecars ClusterRoles from those building blocks.

This PR also moves permissions for `leases` resource from ClusterRole to per-namespace Role (`assets/rbac/lease_leader_election_role.yaml`).
mpatlasov added a commit to mpatlasov/openstack-cinder-csi-driver-operator that referenced this pull request Jul 7, 2023
PR openshift/cluster-storage-operator#379 published builiding blocks of sidecar ClusterRoles. Now, openstack-cinder csi driver operator may compose its sidecars ClusterRoles from those building blocks.

This PR also moves permissions for `leases` resource from ClusterRole to per-namespace Role (`assets/rbac/lease_leader_election_role.yaml`).
mpatlasov added a commit to mpatlasov/vmware-vsphere-csi-driver-operator that referenced this pull request Jul 7, 2023
PR openshift/cluster-storage-operator#379 publishes builiding blocks of sidecar ClusterRoles. Now, vmware-vsphere csi driver operator may compose its sidecars ClusterRoles from those building blocks.

This PR also moves permissions for `leases` resource from ClusterRole to per-namespace Role (`assets/rbac/lease_leader_election_role.yaml`).
mpatlasov added a commit to mpatlasov/csi-driver-manila-operator that referenced this pull request Jul 10, 2023
PR openshift/cluster-storage-operator#379 publishes builiding blocks of sidecar ClusterRoles. Now, manila csi driver operator may compose its sidecars ClusterRoles from those building blocks.

This PR also moves permissions for `leases` resource from ClusterRole to per-namespace Role (`assets/rbac/lease_leader_election_role.yaml`).
mpatlasov added a commit to mpatlasov/csi-driver-manila-operator that referenced this pull request Jul 10, 2023
PR openshift/cluster-storage-operator#379 publishes builiding blocks of sidecar ClusterRoles. Now, manila csi driver operator may compose its sidecars ClusterRoles from those building blocks.

This PR also moves permissions for `leases` resource from ClusterRole to per-namespace Role (`assets/rbac/lease_leader_election_role.yaml`).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. docs-approved Signifies that Docs has signed off on this PR jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants