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

Rename the grpc api for the RBAC #3573

Merged
merged 2 commits into from
Apr 25, 2022
Merged

Rename the grpc api for the RBAC #3573

merged 2 commits into from
Apr 25, 2022

Conversation

knanao
Copy link
Member

@knanao knanao commented Apr 22, 2022

What this PR does / why we need it:
Fix to rename the grpc API for the RBAC into XXXProjectUserGroup.

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

NONE

@pipecd-bot
Copy link
Collaborator

APPROVE

This pull request is NOT APPROVED. It will be approved when one of the following conditions is met:

  • Received a comment that contains /approve from an approver.
  • Received approval at review changes UI from at least 1 approvers.

@pipecd-bot
Copy link
Collaborator

GO_LINTER

The following files are not gofmt-ed. By commenting /golinter fmt, the formatted one will be appended to this pull request automatically.

pkg/app/server/service/webservice/service.pb.go
--- pkg/app/server/service/webservice/service.pb.go.orig
+++ pkg/app/server/service/webservice/service.pb.go
@@ -21,13 +21,15 @@
 package webservice
 
 import (
+	reflect "reflect"
+	sync "sync"
+
 	_ "github.com/envoyproxy/protoc-gen-validate/validate"
-	model "github.com/pipe-cd/pipecd/pkg/model"
 	protoreflect "google.golang.org/protobuf/reflect/protoreflect"
 	protoimpl "google.golang.org/protobuf/runtime/protoimpl"
 	wrapperspb "google.golang.org/protobuf/types/known/wrapperspb"
-	reflect "reflect"
-	sync "sync"
+
+	model "github.com/pipe-cd/pipecd/pkg/model"
 )
 
 const (
pkg/app/server/service/webservice/service_grpc.pb.go
--- pkg/app/server/service/webservice/service_grpc.pb.go.orig
+++ pkg/app/server/service/webservice/service_grpc.pb.go
@@ -8,6 +8,7 @@
 
 import (
 	context "context"
+
 	grpc "google.golang.org/grpc"
 	codes "google.golang.org/grpc/codes"
 	status "google.golang.org/grpc/status"

@pipecd-bot
Copy link
Collaborator

GO_LINTER

The following files are not gofmt-ed. By commenting /golinter fmt, the formatted one will be appended to this pull request automatically.

pkg/app/server/service/webservice/service.pb.go
--- pkg/app/server/service/webservice/service.pb.go.orig
+++ pkg/app/server/service/webservice/service.pb.go
@@ -21,13 +21,15 @@
 package webservice
 
 import (
+	reflect "reflect"
+	sync "sync"
+
 	_ "github.com/envoyproxy/protoc-gen-validate/validate"
-	model "github.com/pipe-cd/pipecd/pkg/model"
 	protoreflect "google.golang.org/protobuf/reflect/protoreflect"
 	protoimpl "google.golang.org/protobuf/runtime/protoimpl"
 	wrapperspb "google.golang.org/protobuf/types/known/wrapperspb"
-	reflect "reflect"
-	sync "sync"
+
+	model "github.com/pipe-cd/pipecd/pkg/model"
 )
 
 const (
pkg/app/server/service/webservice/service_grpc.pb.go
--- pkg/app/server/service/webservice/service_grpc.pb.go.orig
+++ pkg/app/server/service/webservice/service_grpc.pb.go
@@ -8,6 +8,7 @@
 
 import (
 	context "context"
+
 	grpc "google.golang.org/grpc"
 	codes "google.golang.org/grpc/codes"
 	status "google.golang.org/grpc/status"

@knanao knanao changed the title Rename the grpc for the RBAC Rename the grpc api for the RBAC Apr 22, 2022
@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for golang is 35.55%. This pull request increases coverage by 0.00%.

File Function Base Head Diff
pkg/app/server/grpcapi/web_api.go WebAPI.ListProjectUserGroups -- 0.00% +0.00%
pkg/app/server/grpcapi/web_api.go WebAPI.AddProjectUserGroup -- 0.00% +0.00%
pkg/app/server/grpcapi/web_api.go WebAPI.DeleteProjectUserGroup -- 0.00% +0.00%
pkg/app/server/grpcapi/web_api.go WebAPI.ListUserGroups 0.00% -- +-0.00%
pkg/app/server/grpcapi/web_api.go WebAPI.AddUserGroup 0.00% -- +-0.00%
pkg/app/server/grpcapi/web_api.go WebAPI.DeleteUserGroup 0.00% -- +-0.00%

@pipecd-bot
Copy link
Collaborator

COVERAGE

Code coverage for javascript is 77.53%. This pull request does not change code coverage.

@nghialv
Copy link
Member

nghialv commented Apr 22, 2022

@knanao Thank you.

Add the role field in DeleteProjectUserGroupRequest.

I still don't get why we need that field.
Firstly, please let me confirm the UI, will the UI ① be deleted as soon as the UI ② is added, right?
And the built-in role is not deletable but the old user group (configured via old RBAC) is deletable, right?

@knanao
Copy link
Member Author

knanao commented Apr 22, 2022

@nghialv

Firstly, please let me confirm the UI, will the UI ① be deleted as soon as the UI ② is added, right?

Nope.
I think we should provide some release stages not to confuse the user.
First of all, we're going to provide the view of the UserGroup tab and RBAC Role sections like this.
This means the user can just only see the group and role which will have already been added.
Screen Shot 2022-04-22 at 17 22 49

Second, we're gotta provide the Add function and Delete function to edit old user groups like this.
Besides, we're gotta remove the edit button for the old RBAC config.
Screen Shot 2022-04-22 at 17 23 34

Third, we're gotta provide the Add function to add a new RBAC role.
Screen Shot 2022-04-22 at 17 18 11

And Last one, we'll remove the view of the old RBAC config like this.
Screen Shot 2022-04-22 at 17 33 10

@knanao
Copy link
Member Author

knanao commented Apr 22, 2022

And the built-in role is not deletable but the old user group (configured via old RBAC) is deletable, right?

Exactly, but the user who is using the old RBAC won't be able to delete the old one via the old view from the second release.

@knanao
Copy link
Member Author

knanao commented Apr 25, 2022

We talked about this directly, this was decided like this consequently.

First, we're gonna provide the User Group tab, the new RBAC role section, and remove the old RBAC role section.
Screen Shot 2022-04-25 at 14 20 42

Finally, we're gotta provide the Add and Edit function to add a new RBAC role.
Screen Shot 2022-04-22 at 17 33 10

@pipecd-bot
Copy link
Collaborator

GO_LINTER

The following files are not gofmt-ed. By commenting /golinter fmt, the formatted one will be appended to this pull request automatically.

pkg/app/server/service/webservice/service.pb.go
--- pkg/app/server/service/webservice/service.pb.go.orig
+++ pkg/app/server/service/webservice/service.pb.go
@@ -21,13 +21,15 @@
 package webservice
 
 import (
+	reflect "reflect"
+	sync "sync"
+
 	_ "github.com/envoyproxy/protoc-gen-validate/validate"
-	model "github.com/pipe-cd/pipecd/pkg/model"
 	protoreflect "google.golang.org/protobuf/reflect/protoreflect"
 	protoimpl "google.golang.org/protobuf/runtime/protoimpl"
 	wrapperspb "google.golang.org/protobuf/types/known/wrapperspb"
-	reflect "reflect"
-	sync "sync"
+
+	model "github.com/pipe-cd/pipecd/pkg/model"
 )
 
 const (

@knanao
Copy link
Member Author

knanao commented Apr 25, 2022

@nghialv @khanhtc1202
I fixed to just only rename the grpc API and redefined the release stage.
That's why PTAL when you guys have enough time.

Copy link
Member

@nghialv nghialv left a comment

Choose a reason for hiding this comment

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

Thank you. Nice.

Copy link
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

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

👌🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants