Skip to content
This repository was archived by the owner on Dec 3, 2024. It is now read-only.

Create proto definitions for cosi #8

Merged

Conversation

brahmaroutu
Copy link
Contributor

Create the initial photo definitions for COSI Provisioner.

/assign @wlan0

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 25, 2020
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 25, 2020
@wlan0
Copy link
Contributor

wlan0 commented Oct 26, 2020

thanks @brahmaroutu. LGTM

@wlan0
Copy link
Contributor

wlan0 commented Oct 26, 2020

/assign wlan0

@wlan0
Copy link
Contributor

wlan0 commented Oct 26, 2020

/approve

@wlan0
Copy link
Contributor

wlan0 commented Oct 26, 2020

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 26, 2020
@brahmaroutu
Copy link
Contributor Author

@xing-yang Can you please review.

@wlan0
Copy link
Contributor

wlan0 commented Nov 2, 2020

@brahmaroutu I have the same questions as @xing-yang

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 4, 2020
go.mod Outdated
@@ -0,0 +1,9 @@
module github.com/container-object-storage-interface/spec

go 1.14

Choose a reason for hiding this comment

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

Can this be updated to 1.15. 1.15 is required for k8s 1.19.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@brahmaroutu
Copy link
Contributor Author

brahmaroutu commented Nov 7, 2020

@xing-yang @wlan0 I have updated the proto file as per the comments adding detailed documentation of the calls and parameters. Please review.

cosi.proto Outdated
rpc ProvisionerCreateBucket (ProvisionerCreateBucketRequest) returns (ProvisionerCreateBucketResponse) {}
// This call is made to delete the bucket in the backend.
// If the bucket has already been deleted, then no error should be returned.
// If the bucket has already been deleted, then no error should be returned.

Choose a reason for hiding this comment

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

Line 65 and 66 are the same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed, sorry

Choose a reason for hiding this comment

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

Looks like you have not pushed your changes? I still see two duplicate lines:).

cosi.proto Outdated
// This call grants access to a particular principal. The principal is the account for which this access should be granted.
// If the principal is set, then it should be used as the username of the created credentials.
// If the principal is empty, then a new service account should be created in the backend that satisfies the requested access_policy.
// The principal returned in the repsonce will be used as the unique identifier for deleting this access by calling ProvisionerRevokeBucketAccess.

Choose a reason for hiding this comment

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

typo: repsonce -> response

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

rpc ProvisionerGrantBucketAccess (ProvisionerGrantBucketAccessRequest) returns (ProvisionerGrantBucketAccessResponse);
rpc ProvisionerRevokeBucketAccess (ProvisionerRevokeBucketAccessRequest) returns (ProvisionerRevokeBucketAccessResponse);
}

Choose a reason for hiding this comment

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

Sounds good. In CSI spec repo, we only need to update spec.md and csi.proto will be generated as well.

Copy link

@xing-yang xing-yang left a comment

Choose a reason for hiding this comment

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

Looks like you forgot to push your changes?

cosi.proto Outdated
rpc ProvisionerCreateBucket (ProvisionerCreateBucketRequest) returns (ProvisionerCreateBucketResponse) {}
// This call is made to delete the bucket in the backend.
// If the bucket has already been deleted, then no error should be returned.
// If the bucket has already been deleted, then no error should be returned.

Choose a reason for hiding this comment

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

Looks like you have not pushed your changes? I still see two duplicate lines:).

@brahmaroutu
Copy link
Contributor Author

@xing-yang I am sorry I did not push the code. I now have updated the code. Please review.

@xing-yang
Copy link

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 19, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brahmaroutu, wlan0, xing-yang

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 19, 2020
@k8s-ci-robot k8s-ci-robot merged commit 709c64d into kubernetes-retired:master Nov 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants