Support multiple manual approvals #2802

ono-max · 2021-11-19T01:12:28Z

What this PR does / why we need it:

Which issue(s) this PR fixes:

Does this PR introduce a user-facing change?:

Support multiple manual approvals

pipecd-bot · 2021-11-19T01:13:31Z

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

pkg/app/piped/executor/waitapproval/waitapproval.go

--- pkg/app/piped/executor/waitapproval/waitapproval.go.orig
+++ pkg/app/piped/executor/waitapproval/waitapproval.go
@@ -30,9 +30,9 @@
 )
 
 const (
-	approvedByKey    = "ApprovedBy"
-	minApproverNum   = "MinApproverNum"
-	approversKey     = "CurrentApprovers"
+	approvedByKey  = "ApprovedBy"
+	minApproverNum = "MinApproverNum"
+	approversKey   = "CurrentApprovers"
 )
 
 type Executor struct {

pkg/app/piped/executor/waitapproval/waitapproval_test.go

--- pkg/app/piped/executor/waitapproval/waitapproval_test.go.orig
+++ pkg/app/piped/executor/waitapproval/waitapproval_test.go
@@ -18,12 +18,13 @@
 	"context"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"google.golang.org/grpc"
+
 	"github.com/pipe-cd/pipe/pkg/app/api/service/pipedservice"
 	"github.com/pipe-cd/pipe/pkg/app/piped/executor"
 	"github.com/pipe-cd/pipe/pkg/app/piped/metadatastore"
 	"github.com/pipe-cd/pipe/pkg/model"
-	"github.com/stretchr/testify/assert"
-	"google.golang.org/grpc"
 )
 
 func (c *fakeAPIClient) SaveDeploymentMetadata(ctx context.Context, req *pipedservice.SaveDeploymentMetadataRequest, opts ...grpc.CallOption) (*pipedservice.SaveDeploymentMetadataResponse, error) {

pkg/config/deployment.go

--- pkg/config/deployment.go.orig
+++ pkg/config/deployment.go
@@ -365,9 +365,9 @@
 type WaitApprovalStageOptions struct {
 	// The maximum length of time to wait before giving up.
 	// Defaults to 6h.
-	Timeout   Duration `json:"timeout"`
-	Approvers []string `json:"approvers"`
-	MinApproverNum int `json:"minApproverNum"`
+	Timeout        Duration `json:"timeout"`
+	Approvers      []string `json:"approvers"`
+	MinApproverNum int      `json:"minApproverNum"`
 }
 
 // AnalysisStageOptions contains all configurable values for a K8S_ANALYSIS stage.

pipecd-bot

Some issues were detected while linting go source files in your changes.

pkg/app/piped/executor/waitapproval/waitapproval.go

pkg/app/piped/executor/waitapproval/waitapproval_test.go

pipecd-bot · 2021-11-19T01:17:46Z

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

pkg/app/piped/executor/waitapproval/waitapproval.go

--- pkg/app/piped/executor/waitapproval/waitapproval.go.orig
+++ pkg/app/piped/executor/waitapproval/waitapproval.go
@@ -30,9 +30,9 @@
 )
 
 const (
-	approvedByKey    = "ApprovedBy"
-	minApproverNum   = "MinApproverNum"
-	approversKey     = "CurrentApprovers"
+	approvedByKey  = "ApprovedBy"
+	minApproverNum = "MinApproverNum"
+	approversKey   = "CurrentApprovers"
 )
 
 type Executor struct {

pkg/app/piped/executor/waitapproval/waitapproval_test.go

--- pkg/app/piped/executor/waitapproval/waitapproval_test.go.orig
+++ pkg/app/piped/executor/waitapproval/waitapproval_test.go
@@ -18,12 +18,13 @@
 	"context"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"google.golang.org/grpc"
+
 	"github.com/pipe-cd/pipe/pkg/app/api/service/pipedservice"
 	"github.com/pipe-cd/pipe/pkg/app/piped/executor"
 	"github.com/pipe-cd/pipe/pkg/app/piped/metadatastore"
 	"github.com/pipe-cd/pipe/pkg/model"
-	"github.com/stretchr/testify/assert"
-	"google.golang.org/grpc"
 )
 
 func (c *fakeAPIClient) SaveDeploymentMetadata(_ context.Context, req *pipedservice.SaveDeploymentMetadataRequest, _ ...grpc.CallOption) (*pipedservice.SaveDeploymentMetadataResponse, error) {

pipecd-bot · 2021-11-19T01:40:31Z

Code coverage for golang is 31.88%. This pull request decreases coverage by -0.03%.

File	Function	Base	Head	Diff
pkg/app/piped/executor/waitapproval/waitapproval.go	Register	`--`	`66.67%`	`+66.67%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.Execute	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.checkApproval	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.reportApproved	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.reportRequiringApproval	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.getMentionedAccounts	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.validateApproverNum	`--`	`86.36%`	`+86.36%`

pipecd-bot · 2021-11-19T03:53:46Z

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

pkg/app/piped/executor/waitapproval/waitapproval_test.go

--- pkg/app/piped/executor/waitapproval/waitapproval_test.go.orig
+++ pkg/app/piped/executor/waitapproval/waitapproval_test.go
@@ -18,12 +18,13 @@
 	"context"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"google.golang.org/grpc"
+
 	"github.com/pipe-cd/pipe/pkg/app/api/service/pipedservice"
 	"github.com/pipe-cd/pipe/pkg/app/piped/executor"
 	"github.com/pipe-cd/pipe/pkg/app/piped/metadatastore"
 	"github.com/pipe-cd/pipe/pkg/model"
-	"github.com/stretchr/testify/assert"
-	"google.golang.org/grpc"
 )
 
 type fakeLogPersister struct{}
@@ -67,7 +68,6 @@
 	c.stages[req.StageId] = md
 	return &pipedservice.SaveStageMetadataResponse{}, nil
 }
-
 
 func TestValidateApproverNum(t *testing.T) {
 	ac := &fakeAPIClient{

pipecd-bot · 2021-11-19T03:55:31Z

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

pkg/app/piped/executor/waitapproval/waitapproval_test.go

--- pkg/app/piped/executor/waitapproval/waitapproval_test.go.orig
+++ pkg/app/piped/executor/waitapproval/waitapproval_test.go
@@ -18,12 +18,13 @@
 	"context"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"google.golang.org/grpc"
+
 	"github.com/pipe-cd/pipe/pkg/app/api/service/pipedservice"
 	"github.com/pipe-cd/pipe/pkg/app/piped/executor"
 	"github.com/pipe-cd/pipe/pkg/app/piped/metadatastore"
 	"github.com/pipe-cd/pipe/pkg/model"
-	"github.com/stretchr/testify/assert"
-	"google.golang.org/grpc"
 )
 
 type fakeLogPersister struct{}

ono-max · 2021-11-19T03:56:25Z

/golinter fmt

pipecd-bot · 2021-11-19T04:03:46Z

Code coverage for golang is 31.88%. This pull request decreases coverage by -0.03%.

File	Function	Base	Head	Diff
pkg/app/piped/executor/waitapproval/waitapproval.go	Register	`--`	`66.67%`	`+66.67%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.Execute	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.checkApproval	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.reportApproved	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.reportRequiringApproval	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.getMentionedAccounts	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.validateApproverNum	`--`	`86.36%`	`+86.36%`

pkg/app/piped/executor/waitapproval/waitapproval.go

ono-max · 2021-11-19T09:00:27Z

@nghialv

Updated 👍
However, I could not implement only the line breaks among your suggestion. It would be great if you advise me.

pipecd-bot · 2021-11-19T09:09:16Z

Code coverage for golang is 31.84%. This pull request decreases coverage by -0.07%.

File	Function	Base	Head	Diff
pkg/app/piped/executor/waitapproval/waitapproval.go	Register	`--`	`66.67%`	`+66.67%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.Execute	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.checkApproval	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.reportApproved	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.reportRequiringApproval	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.getMentionedAccounts	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.validateApproverNum	`--`	`100.00%`	`+100.00%`

nghialv · 2021-11-19T09:28:27Z

Thank you. Let me check it now.

pkg/app/piped/executor/waitapproval/waitapproval.go

Co-authored-by: Le Van Nghia <[email protected]>

pipecd-bot · 2021-11-25T13:59:18Z

The golinter build is completed with FAILURE. The build will be triggered again when you push any other commits. Or you can trigger it manually by /golinter trigger command right now.

You can check the build log from here.

pipecd-bot · 2021-11-25T14:06:31Z

Code coverage for golang is 31.96%. This pull request increases coverage by 0.05%.

File	Function	Base	Head	Diff
pkg/app/piped/executor/waitapproval/waitapproval.go	Register	`--`	`66.67%`	`+66.67%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.Execute	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.checkApproval	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.reportApproved	`--`	`75.00%`	`+75.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.reportRequiringApproval	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.getMentionedAccounts	`--`	`42.86%`	`+42.86%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.validateApproverNum	`--`	`75.00%`	`+75.00%`
pkg/config/deployment.go	WaitApprovalStageOptions.Validate	`--`	`100.00%`	`+100.00%`

pkg/config/deployment.go

pkg/app/piped/executor/waitapproval/waitapproval.go

Co-authored-by: Khanh Tran <[email protected]>

pipecd-bot · 2021-11-26T08:55:18Z

The golinter build is completed with FAILURE. The build will be triggered again when you push any other commits. Or you can trigger it manually by /golinter trigger command right now.

You can check the build log from here.

pipecd-bot · 2021-11-26T08:55:18Z

The golinter build is completed with FAILURE. The build will be triggered again when you push any other commits. Or you can trigger it manually by /golinter trigger command right now.

You can check the build log from here.

Co-authored-by: Khanh Tran <[email protected]>

pipecd-bot · 2021-11-26T08:55:48Z

The golinter build is completed with FAILURE. The build will be triggered again when you push any other commits. Or you can trigger it manually by /golinter trigger command right now.

You can check the build log from here.

pipecd-bot · 2021-11-26T08:55:49Z

The golinter build is completed with FAILURE. The build will be triggered again when you push any other commits. Or you can trigger it manually by /golinter trigger command right now.

You can check the build log from here.

pipecd-bot · 2021-11-26T09:03:16Z

Code coverage for golang is 31.97%. This pull request increases coverage by 0.06%.

File	Function	Base	Head	Diff
pkg/app/piped/executor/waitapproval/waitapproval.go	Register	`--`	`66.67%`	`+66.67%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.Execute	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.checkApproval	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.reportApproved	`--`	`75.00%`	`+75.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.reportRequiringApproval	`--`	`0.00%`	`+0.00%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.getMentionedAccounts	`--`	`42.86%`	`+42.86%`
pkg/app/piped/executor/waitapproval/waitapproval.go	Executor.validateApproverNum	`--`	`75.00%`	`+75.00%`
pkg/config/deployment.go	WaitApprovalStageOptions.Validate	`--`	`100.00%`	`+100.00%`
pkg/config/deployment.go	GenericDeploymentSpec.Validate	`56.25%`	`57.89%`	`+1.64%`

khanhtc1202 · 2021-11-26T11:13:38Z

/golinter trigger

khanhtc1202 · 2021-11-26T11:28:47Z

Nice work, lets go with this 🚀
/lgtm

nghialv · 2021-11-26T13:36:15Z

Nice.
/approve

pipecd-bot · 2021-11-26T13:36:19Z

This pull request is APPROVED by nghialv.

Approvers can cancel the approval by writing /approve cancel in a comment. Any additional commits also will change this pull request to be not-approved.

nakabonne · 2021-11-26T13:42:49Z

Holy cow! It blew my mind definitely!

Support multiple manual approvals

4066174

pipecd-bot added size/L area/piped labels Nov 19, 2021

pipecd-bot requested review from khanhtc1202 and nakabonne November 19, 2021 01:12

pipecd-bot reviewed Nov 19, 2021

View reviewed changes

Fix unused variables

7d7c088

小野直人 added 2 commits November 19, 2021 10:17

Format codes

3a1981f

Fix test

fbeeb96

Fix formats

3643ad2

Fix formats

3bd9b03

Format unfmt-ed files

2c8ff73

nghialv reviewed Nov 19, 2021

View reviewed changes

pkg/app/piped/executor/waitapproval/waitapproval.go Show resolved Hide resolved

Update log messages

b6d5e99

Fix tests

ab87d54

nghialv reviewed Nov 19, 2021

View reviewed changes

pkg/app/piped/executor/waitapproval/waitapproval.go Outdated Show resolved Hide resolved

nghialv reviewed Nov 19, 2021

View reviewed changes

pkg/app/piped/executor/waitapproval/waitapproval.go Outdated Show resolved Hide resolved

nghialv reviewed Nov 19, 2021

View reviewed changes

pkg/app/piped/executor/waitapproval/waitapproval.go Outdated Show resolved Hide resolved

nghialv reviewed Nov 19, 2021

View reviewed changes

pkg/app/piped/executor/waitapproval/waitapproval.go Outdated Show resolved Hide resolved

Update pkg/app/piped/executor/waitapproval/waitapproval.go

e73e886

Co-authored-by: Le Van Nghia <[email protected]>

小野直人 added 4 commits November 25, 2021 22:55

fix test

7d60a88

fix

9449372

fix test

82c2d8e

fix

0c3dce1

ono-max requested review from khanhtc1202 and nakabonne November 26, 2021 02:12

khanhtc1202 reviewed Nov 26, 2021

View reviewed changes

pkg/config/deployment.go Show resolved Hide resolved

khanhtc1202 reviewed Nov 26, 2021

View reviewed changes

pkg/config/deployment.go Outdated Show resolved Hide resolved

khanhtc1202 reviewed Nov 26, 2021

View reviewed changes

pkg/app/piped/executor/waitapproval/waitapproval.go Outdated Show resolved Hide resolved

小野直人 and others added 2 commits November 26, 2021 17:54

Call validation for WaitApprovalStageOptions

ed37174

Update pkg/config/deployment.go

741101e

Co-authored-by: Khanh Tran <[email protected]>

Update pkg/app/piped/executor/waitapproval/waitapproval.go

2df64f6

Co-authored-by: Khanh Tran <[email protected]>

pipecd-bot added the lgtm label Nov 26, 2021

pipecd-bot added the approved label Nov 26, 2021

pipecd-bot merged commit e2b54a4 into master Nov 26, 2021

pipecd-bot deleted the fix-#1569 branch November 26, 2021 13:44

This was referenced Dec 1, 2021

Update doc for deployment triggering #2867

Merged

Temporarily disable appconfig reporter in Piped #2868

Merged

Support multiple manual approvals #2802

Support multiple manual approvals #2802

Uh oh!

Conversation

ono-max commented Nov 19, 2021

Uh oh!

pipecd-bot commented Nov 19, 2021

Uh oh!

pipecd-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pipecd-bot commented Nov 19, 2021

Uh oh!

pipecd-bot commented Nov 19, 2021

Uh oh!

pipecd-bot commented Nov 19, 2021

Uh oh!

pipecd-bot commented Nov 19, 2021

Uh oh!

ono-max commented Nov 19, 2021

Uh oh!

pipecd-bot commented Nov 19, 2021

Uh oh!

Uh oh!

ono-max commented Nov 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pipecd-bot commented Nov 19, 2021

Uh oh!

nghialv commented Nov 19, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pipecd-bot commented Nov 25, 2021

Uh oh!

pipecd-bot commented Nov 25, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pipecd-bot commented Nov 26, 2021

Uh oh!

pipecd-bot commented Nov 26, 2021

Uh oh!

pipecd-bot commented Nov 26, 2021

Uh oh!

pipecd-bot commented Nov 26, 2021

Uh oh!

pipecd-bot commented Nov 26, 2021

Uh oh!

khanhtc1202 commented Nov 26, 2021

Uh oh!

khanhtc1202 commented Nov 26, 2021

Uh oh!

nghialv commented Nov 26, 2021

Uh oh!

pipecd-bot commented Nov 26, 2021

Uh oh!

nakabonne commented Nov 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ono-max commented Nov 19, 2021 •

edited

Loading