BUILD-284: Relocate shared resources api into openshift/api repository by coreydaley · Pull Request #979 · openshift/api

coreydaley · 2021-07-28T16:21:38Z

https://issues.redhat.com/browse/BUILD-284

openshift-ci · 2021-07-28T16:23:45Z

@coreydaley: This pull request references Bugzilla bug 1986557, 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

bug is open, matching expected state (open)
bug target release (4.9.0) matches configured target release for branch (4.9.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @xiuwang

Details

In response to this:

BUG 1986557: Integrate Shared Resources Operator with Cluster Storage Operator

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.

coreydaley · 2021-07-28T16:41:27Z

/assign @adambkaplan @bparees
ptal

bparees · 2021-07-28T16:46:08Z

why is this a bug? looks like part of a feature.

bparees · 2021-07-28T16:47:57Z

/unassign
/assign @mfojtik

@mfojtik please pick one of your trusted api reviewers

bparees · 2021-07-28T16:50:50Z

/hold

since this is going in as a no-FF feature, it will need qe/docs/px acks before it can merge.

but that should not hold up getting an api-review approval.

adambkaplan · 2021-07-29T17:24:44Z

/hold

Maintaining the hold due to openshift/csi-driver-shared-resource#39. I've put a few renames in flight to give a coherent branding experience:

API group - sharedresource.openshift.io. New custom resource is share.sharedresource.openshift.io.
CSI Driver name (to be registered by the cluster storage operator) - csi.sharedresource.openshift.io
Repos will be renamed to reflect the unified branding.

The API group and directory structure should be updated here (no longer use projectedresource).

adambkaplan · 2021-07-29T17:27:32Z

/cc @jitendar-singh @rolfedh @RickJWagner

For qe, docs, and PX acks on this.

coreydaley · 2021-07-30T19:35:12Z

@adambkaplan I have updated this pull request to correlate with the renaming of the projected resource driver. Can you ptal and see if this falls in line with your updates?

adambkaplan

I don't think we need the display name. Only other item is to add explicit owners, then it looks good to me.

adambkaplan

/lgtm

coreydaley · 2021-10-01T15:40:53Z

@deads2k @gabemontero @adambkaplan @jsafrane @soltysh @mfojtik
ptal, pull request has been updated with the new api from https://github.com/openshift/csi-driver-shared-resource/
Note: I am unable to remove the protobuf tags, they get automagically re-added when I run make update. Is that expected behavior?

gabemontero · 2021-10-01T15:44:40Z

/hold cancel

gabemontero

LGTM but I'll defer to others to apply the appropriate labels

adambkaplan

Looks like there is the one CRD yaml file that was missed, otherwise I think we're in good shape!

deads2k · 2021-10-04T13:01:29Z

+//  `oc create role shared-resource-my-share --verb=use --resource=sharedconfigmaps.sharedresource.openshift.io --resource-name=my-share`
+//  `oc create rolebinding shared-resource-my-share --role=shared-resource-my-share --serviceaccount=my-namespace:default`
+//
+// Administrators can create separate Roles and RoleBindings for their users to be able the list and/or view the


What do you see the default as? Autthenticated list/watch of SharedConfigMaps and SharedSecrets?

Are you asking if any authenticated user of the cluster will be able to list/watch the shared resources? If not, could you please elaborate on your question?

Are you asking if any authenticated user of the cluster will be able to list/watch the shared resources?

yes. What is the default policy?

I'll step in for this one (after discussing this with @coreydaley on slack today, as well has prior discussion I've had with @adambkaplan )

So yeah @coreydaley can augment the godoc here @deads2k, providing a compact summary of the following:

we envisioned some aggregation to the default roles .... for this one, I believe we are talking the view role

with that, users can get (that is missing in this current version), list, and watch SharedConfigMap and SharedSecret

so they can know which actual ConfigMap or Secret is being shared via a get on the SharedConfigMap or SharedSecret

but they cannot of course see inside the ConfigMap or Secret unless they have sufficient permissions to those objects in the namespaces they reside

minor sidebar - oc describe ... does a get under the covers

Any issues or things we missed there @deads2k ?

thanks

In putting ^^ together @deads2k I am working off of https://kubernetes.io/docs/reference/access-authn-authz/rbac/#aggregated-clusterroles and hence believe we could aggregate get/list/watch perms for our 2 new cluster scoped CRDs here, assuming we all agree that is "safe", and I am in fact interpreting that doc correctly.

@deads2k Updated with default permissions information. ptal

deads2k · 2021-10-04T14:22:55Z

after the reference and default policy, I think it is ready to merge.

deads2k · 2021-10-05T15:19:23Z

/lgtm
/approve

deads2k · 2021-10-05T15:20:47Z

/lgtm cancel

Operator Co-authored-by: Adam Kaplan <adam.kaplan@redhat.com> Co-authored-by: Gabe Montero <gmontero@redhat.com>

coreydaley · 2021-10-05T16:03:26Z

@deads2k Updated per your request, ptal.

deads2k · 2021-10-05T16:08:46Z

/lgtm

openshift-ci · 2021-10-05T16:11:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: coreydaley, deads2k, gabemontero, rolfedh, soltysh

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~~ [deads2k,soltysh]

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

openshift-ci Bot requested review from abhinavdahiya and derekwaynecarr July 28, 2021 16:23

openshift-ci Bot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Jul 28, 2021

openshift-ci Bot requested a review from xiuwang July 28, 2021 16:25

openshift-ci Bot assigned adambkaplan and bparees Jul 28, 2021

openshift-ci Bot assigned mfojtik and unassigned bparees Jul 28, 2021

openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 28, 2021

coreydaley changed the title ~~BUG 1986557: Integrate Shared Resources Operator with Cluster Storage Operator~~ [WIP] BUG 1986557: Integrate Shared Resources Operator with Cluster Storage Operator Jul 29, 2021

openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 29, 2021

openshift-ci Bot requested review from RickJWagner, jitendar-singh and rolfedh July 29, 2021 17:27

coreydaley commented Jul 30, 2021

View reviewed changes

Comment thread sharedresource/v1alpha1/0000_10_sharedresource.crd.yaml Outdated

coreydaley commented Jul 30, 2021

View reviewed changes

Comment thread sharedresource/v1alpha1/0000_10_sharedresource.crd.yaml Outdated

coreydaley changed the title ~~[WIP] BUG 1986557: Integrate Shared Resources Operator with Cluster Storage Operator~~ BUG 1986557: Integrate Shared Resources Operator with Cluster Storage Operator Jul 30, 2021

openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 30, 2021

adambkaplan suggested changes Aug 2, 2021

View reviewed changes

Comment thread sharedresource/OWNERS

Comment thread sharedresource/v1alpha1/0000_10_sharedresource.crd.yaml Outdated

Comment thread sharedresource/v1alpha1/0000_10_sharedresource.crd.yaml Outdated

adambkaplan approved these changes Aug 2, 2021

View reviewed changes

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 2, 2021

deads2k reviewed Sep 9, 2021

View reviewed changes

Comment thread storage/v1alpha1/register.go Outdated

deads2k reviewed Sep 9, 2021

View reviewed changes

Comment thread storage/v1alpha1/types_share.go Outdated

deads2k reviewed Sep 9, 2021

View reviewed changes

Comment thread storage/v1alpha1/types_share.go Outdated

deads2k reviewed Sep 9, 2021

View reviewed changes

Comment thread storage/v1alpha1/types_share.go Outdated

This was referenced Sep 16, 2021

BUILD-348: share discoverability ('use' verb, namespace scope roles); inject build-284 in repo openshift/csi-driver-shared-resource#51

Merged

BUILD-348: update shared resource via CSI Driver proposal openshift/enhancements#901

Merged

This was referenced Sep 29, 2021

BUILD-353: name changes, 2 CRDs instead of one for secrets and configmaps openshift/csi-driver-shared-resource#55

Merged

BUILD-353: csi driver, crd name changes openshift/csi-driver-shared-resource-operator#9

Merged

openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 1, 2021

gabemontero approved these changes Oct 1, 2021

View reviewed changes

adambkaplan suggested changes Oct 1, 2021

View reviewed changes

Comment thread sharedresource/v1alpha1/0000_10_sharedresource.crd.yaml Outdated

deads2k reviewed Oct 4, 2021

View reviewed changes

Comment thread sharedresource/OWNERS Outdated

deads2k reviewed Oct 4, 2021

View reviewed changes

Comment thread sharedresource/v1alpha1/types_share.go Outdated

deads2k reviewed Oct 4, 2021

View reviewed changes

Comment thread sharedresource/v1alpha1/types_share.go Outdated

openshift-ci Bot assigned deads2k Oct 5, 2021

openshift-ci Bot added lgtm Indicates that a PR is ready to be merged. and removed lgtm Indicates that a PR is ready to be merged. labels Oct 5, 2021

deads2k reviewed Oct 5, 2021

View reviewed changes

Comment thread sharedresource/v1alpha1/types_shared_secret.go

BUILD-284: Integrate Shared Resources Operator with Cluster Storage

9d91c1f

Operator Co-authored-by: Adam Kaplan <adam.kaplan@redhat.com> Co-authored-by: Gabe Montero <gmontero@redhat.com>

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Oct 5, 2021

openshift-merge-robot merged commit 4ec2bbe into openshift:master Oct 5, 2021

Conversation

coreydaley commented Jul 28, 2021

Uh oh!

openshift-ci Bot commented Jul 28, 2021

Uh oh!

coreydaley commented Jul 28, 2021

Uh oh!

bparees commented Jul 28, 2021

Uh oh!

bparees commented Jul 28, 2021

Uh oh!

bparees commented Jul 28, 2021

Uh oh!

adambkaplan commented Jul 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adambkaplan commented Jul 29, 2021

Uh oh!

Uh oh!

Uh oh!

coreydaley commented Jul 30, 2021

Uh oh!

adambkaplan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adambkaplan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coreydaley commented Oct 1, 2021

Uh oh!

gabemontero commented Oct 1, 2021

Uh oh!

gabemontero left a comment

Choose a reason for hiding this comment

Uh oh!

adambkaplan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

deads2k Oct 4, 2021

Choose a reason for hiding this comment

Uh oh!

coreydaley Oct 4, 2021

Choose a reason for hiding this comment

Uh oh!

deads2k Oct 4, 2021

Choose a reason for hiding this comment

Uh oh!

gabemontero Oct 4, 2021

Choose a reason for hiding this comment

Uh oh!

gabemontero Oct 4, 2021

Choose a reason for hiding this comment

Uh oh!

coreydaley Oct 5, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

deads2k commented Oct 4, 2021

Uh oh!

deads2k commented Oct 5, 2021

Uh oh!

deads2k commented Oct 5, 2021

Uh oh!

Uh oh!

coreydaley commented Oct 5, 2021

Uh oh!

deads2k commented Oct 5, 2021

Uh oh!

openshift-ci Bot commented Oct 5, 2021

Uh oh!

adambkaplan commented Jul 29, 2021 •

edited

Loading