configstore: add ability to org members to restart stop cancel a project run #446

alessandro-sorint · 2023-10-10T12:23:42Z

This patch adds a project option to permit org members to execute run actions (restart/stop/cancel)
The MembersCanPerformRunActions option has been added to the project type. The API will permit to set it to true on project create/update only for projects belonging to an organization and will return an error when on user's projects.

fix #428

sgotti

@alessandro-sorint Thanks for the PR.

The commit title and description aren't detailing what it does (do not write the code changes, but the new behavior with more details).
Tests for this new behavior aren't implemented.

sgotti · 2023-10-11T07:30:15Z

internal/services/configstore/action/project.go

+	SkipSSHHostKeyCheck         bool
+	PassVarsToForkedPR          bool
+	DefaultBranch               string
+	CanMembersPerformRunActions bool


MembersCanPerformRunActions

sgotti · 2023-10-11T07:30:37Z

internal/services/configstore/action/project.go

+	SkipSSHHostKeyCheck         bool
+	PassVarsToForkedPR          bool
+	DefaultBranch               string
+	CanMembersPerformRunActions bool


Detail what "Run Actions" are.

sgotti · 2023-10-11T07:31:43Z

internal/services/configstore/db/migrate.go

+
+func (d *DB) migrateV3(tx *sql.Tx) error {
+	var ddlPostgres = []string{
+		"CREATE TABLE new_project (id varchar NOT NULL, revision bigint NOT NULL, creation_time timestamptz NOT NULL, update_time timestamptz NOT NULL, name varchar NOT NULL, parent_kind varchar NOT NULL, parent_id varchar NOT NULL, secret varchar NOT NULL, visibility varchar NOT NULL, remote_repository_config_type varchar NOT NULL, remote_source_id varchar NOT NULL, linked_account_id varchar NOT NULL, repository_id varchar NOT NULL, repository_path varchar NOT NULL, ssh_private_key varchar NOT NULL, skip_ssh_host_key_check boolean NOT NULL, webhook_secret varchar NOT NULL, pass_vars_to_forked_pr boolean NOT NULL, default_branch varchar NOT NULL, can_members_perform_run_actions boolean NOT NULL, PRIMARY KEY (id))",


Is a new table creation and copy really needed for postgres?

sgotti · 2023-10-11T07:33:51Z

internal/services/gateway/action/auth.go

+func (h *ActionHandler) CanAuthUserDeleteLogs(ctx context.Context, groupType scommon.GroupType, ref string) (bool, string, error) {
+	var ownerType cstypes.ObjectKind
+	var refID string
+	var ownerID string
+	switch groupType {
+	case scommon.GroupTypeProject:
+		p, _, err := h.configstoreClient.GetProject(ctx, ref)
+		if err != nil {
+			return false, "", util.NewAPIError(util.KindFromRemoteError(err), err)
+		}
+		refID = p.ID
+		ownerType = p.OwnerType
+		ownerID = p.OwnerID
+	case scommon.GroupTypeUser:
+		u, _, err := h.configstoreClient.GetUser(ctx, ref)
+		if err != nil {
+			return false, "", util.NewAPIError(util.KindFromRemoteError(err), err)
+		}
+
+		// user direct runs
+		refID = u.ID
+		ownerType = cstypes.ObjectKindUser
+		ownerID = u.ID
+	}
+
+	isProjectOwner, err := h.IsAuthUserProjectOwner(ctx, ownerType, ownerID)
+	if err != nil {
+		return false, "", errors.Wrapf(err, "failed to determine ownership")
+	}
+	if !isProjectOwner {
+		return false, "", nil
+	}
+	return true, refID, nil
+}
+
 func (h *ActionHandler) CanAuthUserDoRunActions(ctx context.Context, groupType scommon.GroupType, ref string) (bool, string, error) {
+	var ownerType cstypes.ObjectKind
+	var refID string
+	var ownerID string
+	var p *csapitypes.Project
+	switch groupType {
+	case scommon.GroupTypeProject:
+		var err error
+		p, _, err = h.configstoreClient.GetProject(ctx, ref)
+		if err != nil {
+			return false, "", util.NewAPIError(util.KindFromRemoteError(err), err)
+		}
+		refID = p.ID
+		ownerType = p.OwnerType
+		ownerID = p.OwnerID
+	case scommon.GroupTypeUser:
+		u, _, err := h.configstoreClient.GetUser(ctx, ref)
+		if err != nil {
+			return false, "", util.NewAPIError(util.KindFromRemoteError(err), err)
+		}
+
+		// user direct runs
+		refID = u.ID
+		ownerType = cstypes.ObjectKindUser
+		ownerID = u.ID
+	}
+
+	isAdmin := common.IsUserAdmin(ctx)
+	if isAdmin {
+		return true, refID, nil
+	}
+
+	if ownerType == cstypes.ObjectKindOrg && p.CanMembersPerformRunActions {
+		userID := common.CurrentUserID(ctx)
+		isUserMember, err := h.IsUserOrgMember(ctx, userID, ownerID)
+		if err != nil {
+			return false, "", errors.Wrapf(err, "failed to determine ownership")
+		}
+		return isUserMember, "", nil
+	}
+
+	isProjectOwner, err := h.IsAuthUserProjectOwner(ctx, ownerType, ownerID)
+	if err != nil {
+		return false, "", errors.Wrapf(err, "failed to determine ownership")
+	}
+	if !isProjectOwner {
+		return false, "", nil
+	}
+	return true, refID, nil
+}


Is it really needed to duplicate all of this code?

sgotti · 2023-10-11T07:37:32Z

internal/services/gateway/action/run.go

@@ -167,7 +167,7 @@ type DeleteLogsRequest struct {
 }

 func (h *ActionHandler) DeleteLogs(ctx context.Context, req *DeleteLogsRequest) error {
-	canDoRunActions, groupID, err := h.CanAuthUserDoRunActions(ctx, req.GroupType, req.Ref)
+	canDoRunActions, groupID, err := h.CanAuthUserDeleteLogs(ctx, req.GroupType, req.Ref)


Why have you split CanAuthUserDoRunActions in two functions if they do the same thing?

sgotti · 2023-10-11T07:40:17Z

internal/services/configstore/db/tests/fixtures/migrate/dbv3.jsonc

You should test the migration to v3. So at least explicitly define the new field added with the default value.

alessandro-sorint · 2023-10-11T13:57:00Z

Tests for this new behavior aren't implemented.

@sgotti I implemented TestProjectRunActions in integration test, do you mean other?

sgotti · 2023-10-11T15:55:41Z

tests/setup_test.go

@@ -1080,7 +1081,7 @@ func TestUpdateProject(t *testing.T) {
 	}
 }

-func createProject(ctx context.Context, t *testing.T, giteaClient *gitea.Client, gwClient *gwclient.Client) (*gitea.Repository, *gwapitypes.ProjectResponse) {
+func createProject(ctx context.Context, t *testing.T, giteaClient *gitea.Client, gwClient *gwclient.Client, parentRef string, membersCanPerformRunActions bool) (*gitea.Repository, *gwapitypes.ProjectResponse) {


Instead of increasing the number of function arguments I'll use the With... pattern

sgotti · 2023-10-11T15:59:19Z

internal/services/configstore/db/migrate.go

+func (d *DB) migrateV3(tx *sql.Tx) error {
+	var ddlPostgres = []string{
+		"ALTER TABLE project ADD COLUMN members_can_perform_run_actions boolean NOT NULL DEFAULT false",
+		"ALTER TABLE project ALTER COLUMN members_can_perform_run_actions DROP DEFAULT",


Is this statement really needed?

@sgotti in the first statement if I remove DEFAULT false the error is pq: column "can_members_perform_run_actions" of relation "project" contains null values, because we have already sono project rows in db.
In the second statement I remove DEFAULT to have the same shema of V3.

Ok, It just doesn't looks very clear. I'll prefer to do this in the standard 3 ways steps: Add column without non null constraint, update column with default values, add null constraint to the column.

sgotti · 2023-10-11T16:01:59Z

internal/services/configstore/db/migrate.go

+
+	var ddlSqlite3 = []string{
+		"CREATE TABLE new_project (id varchar NOT NULL, revision bigint NOT NULL, creation_time timestamp NOT NULL, update_time timestamp NOT NULL, name varchar NOT NULL, parent_kind varchar NOT NULL, parent_id varchar NOT NULL, secret varchar NOT NULL, visibility varchar NOT NULL, remote_repository_config_type varchar NOT NULL, remote_source_id varchar NOT NULL, linked_account_id varchar NOT NULL, repository_id varchar NOT NULL, repository_path varchar NOT NULL, ssh_private_key varchar NOT NULL, skip_ssh_host_key_check integer NOT NULL, webhook_secret varchar NOT NULL, pass_vars_to_forked_pr integer NOT NULL, default_branch varchar NOT NULL, members_can_perform_run_actions integer NOT NULL, PRIMARY KEY (id))",
+		"INSERT INTO new_project SELECT *,0 AS members_can_perform_run_actions FROM project",


sqlite3 should also recognize true and false.

sgotti · 2023-10-11T16:02:13Z

internal/services/configstore/db/migrate.go

+
+	var ddlSqlite3 = []string{
+		"CREATE TABLE new_project (id varchar NOT NULL, revision bigint NOT NULL, creation_time timestamp NOT NULL, update_time timestamp NOT NULL, name varchar NOT NULL, parent_kind varchar NOT NULL, parent_id varchar NOT NULL, secret varchar NOT NULL, visibility varchar NOT NULL, remote_repository_config_type varchar NOT NULL, remote_source_id varchar NOT NULL, linked_account_id varchar NOT NULL, repository_id varchar NOT NULL, repository_path varchar NOT NULL, ssh_private_key varchar NOT NULL, skip_ssh_host_key_check integer NOT NULL, webhook_secret varchar NOT NULL, pass_vars_to_forked_pr integer NOT NULL, default_branch varchar NOT NULL, members_can_perform_run_actions integer NOT NULL, PRIMARY KEY (id))",
+		"INSERT INTO new_project SELECT *,0 AS members_can_perform_run_actions FROM project",


Add a space after select *,

services/configstore/api/types/project.go

tests/setup_test.go

sgotti · 2023-10-11T16:06:40Z

@sgotti I implemented TestProjectRunActions in integration test, do you mean other?

Sorry, I missed them. Can you also add something in configstore tests for TestProjectUpdate (also once the behavior of an user project has been clarified)?

sgotti · 2023-10-11T16:11:14Z

internal/services/gateway/action/auth.go

@@ -207,6 +218,22 @@ func (h *ActionHandler) CanAuthUserDoRunActions(ctx context.Context, groupType s
 		ownerID = u.ID
 	}

+	if authType == authTypeRunAction {


you aren't checking the other action types

sgotti · 2023-10-11T16:15:17Z

internal/services/gateway/action/auth.go

+		isAdmin := common.IsUserAdmin(ctx)
+		if isAdmin {
+			return true, refID, nil
+		}
+
+		if ownerType == cstypes.ObjectKindOrg && p.MembersCanPerformRunActions {
+			userID := common.CurrentUserID(ctx)
+			isUserMember, err := h.IsUserOrgMember(ctx, userID, ownerID)
+			if err != nil {
+				return false, "", errors.Wrapf(err, "failed to determine ownership")
+			}
+			return isUserMember, "", nil
+		}
+	}


This logic is creating its own branch, but the branch can continue. It also uses IsUserAdmin instead of IsAuthUserProjectOwner like done after. So it looks confusing and wrong.

Why don't you just use this check? `isProjectOwner || (isUserOrgMember && p.MembersCanPerformRunActions) when the actionType is RunAction?

sgotti · 2023-10-11T16:15:31Z

internal/services/gateway/action/auth.go

@@ -182,13 +183,23 @@ func (h *ActionHandler) CanAuthUserGetRun(ctx context.Context, groupType scommon
 	return true, refID, nil
 }

-func (h *ActionHandler) CanAuthUserDoRunActions(ctx context.Context, groupType scommon.GroupType, ref string) (bool, string, error) {
+type authType string


alessandro-sorint · 2023-10-12T09:40:50Z

For user project the behavioir in CanAuthUserDoRunActions func is to ignore MembersCanPerformRunActions.
Should I also add a check where is not possible to set true in MembersCanPerformRunActions field for a user project? (with relative tests)

Yes, let's avoid changing this to true for user projects.

sgotti · 2023-10-12T13:31:33Z

internal/services/configstore/action/project.go

@@ -59,6 +59,29 @@ func (h *ActionHandler) ValidateProjectReq(ctx context.Context, req *CreateUpdat
 			return util.NewAPIError(util.ErrBadRequest, errors.Errorf("empty remote repository path"))
 		}
 	}
+
+	err := h.d.Do(ctx, func(tx *sql.Tx) error {


This shouldn't be done here creating an additional transaction but inside Create and Update project.

sgotti · 2023-10-12T13:34:05Z

internal/services/gateway/action/project.go

+	}
+
+	if p.OwnerType == cstypes.ObjectKindUser && p.MembersCanPerformRunActions {
+		return nil, util.NewAPIError(util.ErrBadRequest, errors.Errorf("can't set MembersCanPerformRunActions true for user projects"))


This check should be done in the configstore. (Like already done above in this patch, also if in the wrong place).

sgotti · 2023-10-12T13:34:53Z

internal/services/configstore/action/project.go

+			return errors.WithStack(err)
+		}
+		if ownerType == types.ObjectKindUser && req.MembersCanPerformRunActions {
+			return util.NewAPIError(util.ErrBadRequest, errors.Errorf("can't set MembersCanPerformRunActions true for user projects"))


Cannot set MembersCanPerformRunActions on an user project.

sgotti · 2023-10-12T13:35:32Z

internal/services/configstore/action/project.go

@@ -93,6 +116,8 @@ type CreateUpdateProjectRequest struct {
 	SkipSSHHostKeyCheck        bool
 	PassVarsToForkedPR         bool
 	DefaultBranch              string
+	//MembersCanPerformRunActions define if users of the organization can restart stop cancel a project run


MembersCanPerformRunActions defines if project organization members can restart/stop/cancel a project run

sgotti · 2023-10-12T13:46:50Z

tests/setup_test.go

-	remoteErrorBadRequest = "remote error badrequest"
+	remoteErrorNotExist         = "remote error notexist"
+	remoteErrorBadRequest       = "remote error badrequest"
+	remoteErrorForbiddenRequest = "remote error forbidden"


Also change the already existing inline strings.

sgotti · 2023-10-12T13:50:24Z

internal/services/gateway/action/auth.go

+	} else {
+		if !isProjectOwner {
+			return false, "", nil
+		}
 	}
 	return true, refID, nil


You have both a default return and a return at the end of the first branch
You should remove the default return or remove the return at the of the first branch.

internal/services/gateway/action/auth.go

sgotti · 2023-10-12T15:54:19Z

internal/services/configstore/action/project.go

@@ -93,6 +93,8 @@ type CreateUpdateProjectRequest struct {
 	SkipSSHHostKeyCheck        bool
 	PassVarsToForkedPR         bool
 	DefaultBranch              string
+	//MembersCanPerformRunActions define if users of the organization can restart/stop/cancel a project run


// MembersCanPerformRunActions defines if project organization members can restart/stop/cancel a project run

sgotti · 2023-10-12T15:55:15Z

internal/services/configstore/action/project.go

+			return errors.WithStack(err)
+		}
+		if ownerType == types.ObjectKindUser && req.MembersCanPerformRunActions {
+			return util.NewAPIError(util.ErrBadRequest, errors.Errorf("Cannot set MembersCanPerformRunActions on an user project."))


obviously the first letter of errors has never been capital so
s/Cannot/cannot/

sgotti · 2023-10-13T06:48:19Z

tests/setup_test.go

+	remoteErrorUnauthorized     = "remote error unauthorized"
+	remoteErrorUnknown          = "unknown api error (status: 403)"
+	remoteErrorInternal         = "remote error internal"


I meant only the strings for the remoteErrorForbidden you added. The others should be done in another PR...

sgotti · 2023-10-13T06:48:50Z

tests/setup_test.go

-	remoteErrorBadRequest = "remote error badrequest"
+	remoteErrorNotExist         = "remote error notexist"
+	remoteErrorBadRequest       = "remote error badrequest"
+	remoteErrorForbiddenRequest = "remote error forbidden"


remoteErrorForbidden

sgotti · 2023-10-13T06:49:53Z

tests/setup_test.go

+
+		push(t, config, giteaRepo.CloneURL, giteaToken, "commit", false)
+
+		// test org run actions user owner


// test org run actions executed by an user that's organization owner

sgotti · 2023-10-13T06:50:29Z

tests/setup_test.go

+			t.Fatalf("unexpected err: %v", err)
+		}
+
+		// test org run actions user member


// test org run actions executed by an user that's organization moember with project MembersCanPerformRunActions set to true

sgotti · 2023-10-13T06:50:54Z

tests/setup_test.go

+			t.Fatalf("unexpected err: %v", err)
+		}
+
+		// test org run actions user not member


// test org run actions executed by an user that isn't organization member

sgotti · 2023-10-13T06:51:51Z

tests/setup_test.go

+	`
+	expectedErr := remoteErrorForbiddenRequest
+
+	t.Run("test org run actions", func(t *testing.T) {


// test run actions on org's project

sgotti · 2023-10-13T06:52:10Z

tests/setup_test.go

+		}
+	})
+
+	t.Run("test user run actions", func(t *testing.T) {


// test run actions on user's project

tests/setup_test.go

sgotti · 2023-10-13T07:11:09Z

@alessandro-sorint
The commit and PR (since it's a single commit) description isn't detailing what it does and the behavior. Please add more details.

sgotti · 2023-10-16T07:03:22Z

tests/setup_test.go

 	tests := []struct {
-		name               string
-		passVarsToForkedPR bool
-		expectedPre        bool
-		expectedPost       bool
+		name                        string
+		passVarsToForkedPR          bool
+		expectedPre                 bool
+		expectedPost                bool
+		membersCanPerformRunActions bool
 	}{
 		{
-			name:               "test project update with pass-vars-to-forked-pr true",
-			passVarsToForkedPR: true,
-			expectedPre:        false,
-			expectedPost:       true,
+			name:                        "test project update with pass-vars-to-forked-pr true and membersCanPerformRunActions false ",
+			passVarsToForkedPR:          true,
+			expectedPre:                 false,
+			expectedPost:                true,
+			membersCanPerformRunActions: false,
 		},
 		{
-			name:               "test project update with pass-vars-to-forked-pr false",
-			passVarsToForkedPR: false,
-			expectedPre:        false,
-			expectedPost:       false,
+			name:                        "test project update with pass-vars-to-forked-pr false and membersCanPerformRunActions false",
+			passVarsToForkedPR:          false,
+			expectedPre:                 false,
+			expectedPost:                false,
+			membersCanPerformRunActions: false,
 		},
 	}

 	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			dir := t.TempDir()
+
+			ctx, cancel := context.WithCancel(context.Background())
+			defer cancel()
+
+			sc := setup(ctx, t, dir, withGitea(true))
+			defer sc.stop()
+
+			giteaAPIURL := fmt.Sprintf("http://%s:%s", sc.gitea.HTTPListenAddress, sc.gitea.HTTPPort)
+
+			giteaToken, token := createLinkedAccount(ctx, t, sc.gitea, sc.config)
+
+			giteaClient, err := gitea.NewClient(giteaAPIURL, gitea.SetToken(giteaToken))
+			if err != nil {
+				t.Fatalf("unexpected err: %v", err)
+			}
+
+			gwClient := gwclient.NewClient(sc.config.Gateway.APIExposedURL, token)
+
+			_, project := createProject(ctx, t, giteaClient, gwClient)
+			if project.PassVarsToForkedPR != tt.expectedPre {
+				t.Fatalf("expected PassVarsToForkedPR %v, got %v (pre-update)", tt.expectedPre, project.PassVarsToForkedPR)
+			}
+			if project.MembersCanPerformRunActions != tt.membersCanPerformRunActions {
+				t.Fatalf("expected MembersCanPerformRunActions %v, got %v (pre-update)", tt.membersCanPerformRunActions, project.MembersCanPerformRunActions)
+			}
+			project = updateProject(ctx, t, giteaClient, gwClient, project.ID, tt.passVarsToForkedPR)
+			if project.PassVarsToForkedPR != tt.expectedPost {
+				t.Fatalf("expected PassVarsToForkedPR %v, got %v (port-update)", tt.expectedPost, project.PassVarsToForkedPR)
+			}
+			if project.MembersCanPerformRunActions != tt.membersCanPerformRunActions {
+				t.Fatalf("expected MembersCanPerformRunActions %v, got %v (port-update)", tt.membersCanPerformRunActions, project.MembersCanPerformRunActions)
+			}
+		})
+	}


This mix of tests matrix and single sub tests is confusing. Also the pre/post stuff could be removed. Probably the 2 tests in the matrix could be just changed to standard sub tests.

sgotti · 2023-10-16T07:04:37Z

tests/setup_test.go

+			t.Fatalf("unexpected err: %v", err)
+		}
+
+		// test org run actions executed by an user that's organization moember with project MembersCanPerformRunActions set to true


moember typo

sgotti · 2023-10-16T07:05:01Z

tests/setup_test.go

+			t.Fatalf("expected err %v, got err: %v", expectedErr, err)
+		}
+
+		// test MembersCanPerformRunActions false user member


sgotti · 2023-10-16T07:05:41Z

tests/setup_test.go

+		}
+	})
+
+	t.Run("// test run actions on user's project", func(t *testing.T) {


starting // ?

sgotti · 2023-10-16T07:07:43Z

tests/setup_test.go

+		_, _, err = gwAdminClient.CreateUser(ctx, &gwapitypes.CreateUserRequest{UserName: agolaUser03})
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+
+		token, _, err = gwAdminClient.CreateUserToken(ctx, agolaUser03, &gwapitypes.CreateUserTokenRequest{TokenName: "testtoken"})
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+
+		gwUser03Client := gwclient.NewClient(sc.config.Gateway.APIExposedURL, token.Token)


users's creation and related stuffs at the start of the test.

sgotti · 2023-10-16T07:08:17Z

tests/setup_test.go

+		_, _, err = gwAdminClient.CreateUser(ctx, &gwapitypes.CreateUserRequest{UserName: agolaUser02})
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+
+		token, _, err := gwAdminClient.CreateUserToken(ctx, agolaUser02, &gwapitypes.CreateUserTokenRequest{TokenName: "testtoken"})
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+
+		gwUser02Client := gwclient.NewClient(sc.config.Gateway.APIExposedURL, token.Token)


users's creation and related stuffs at the start of the test.

sgotti · 2023-10-16T07:08:23Z

tests/setup_test.go

+		gwAdminClient := gwclient.NewClient(sc.config.Gateway.APIExposedURL, sc.config.Gateway.AdminToken)
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+


users's creation and related stuffs at the start of the test.

tests/setup_test.go

sgotti · 2023-10-16T08:00:26Z

tests/setup_test.go

+
+		_, project := createProject(ctx, t, giteaClient, gwClient)
+		if project.PassVarsToForkedPR != false {
+			t.Fatalf("expected PassVarsToForkedPR false, got %v (pre-update)", project.PassVarsToForkedPR)


remove (pre-update)

sgotti · 2023-10-16T08:00:31Z

tests/setup_test.go

+
+		project = updateProject(ctx, t, giteaClient, gwClient, project.ID, false)
+		if project.PassVarsToForkedPR != false {
+			t.Fatalf("expected PassVarsToForkedPR false, got %v (pre-update)", project.PassVarsToForkedPR)


remove (pre-update)

sgotti · 2023-10-16T08:00:35Z

tests/setup_test.go

+
+		project = updateProject(ctx, t, giteaClient, gwClient, project.ID, true)
+		if project.PassVarsToForkedPR != true {
+			t.Fatalf("expected PassVarsToForkedPR false, got %v (pre-update)", project.PassVarsToForkedPR)


remove (pre-update)

sgotti · 2023-10-16T08:00:50Z

tests/setup_test.go

-		if project.PassVarsToForkedPR != tt.expectedPre {
-			t.Fatalf("expected PassVarsToForkedPR %v, got %v (pre-update)", tt.expectedPre, project.PassVarsToForkedPR)
+		if project.MembersCanPerformRunActions != false {
+			t.Fatalf("expected MembersCanPerformRunActions false, got %v (pre-update)", project.MembersCanPerformRunActions)


remove (pre-update)

tests/setup_test.go

sgotti · 2023-10-16T11:15:04Z

tests/setup_test.go

+			t.Fatalf("expected PassVarsToForkedPR false, got %v (pre-update)", project.PassVarsToForkedPR)
+		}
+
+		project = updateProject(ctx, t, giteaClient, gwClient, project.ID, true)


remove updateProject function and directly call the api here as done in other tests.

…ect run This patch adds a project option to permit org members to execute run actions (restart/stop/cancel) The `MembersCanPerformRunActions` option has been added to the project type. The API will permit to set it to true on project create/update only for projects belonging to an organization and will return an error when on user's projects.

add an ui checkbox to let the project owner enable the project option to let members perform run actions as implemented in agola-io/agola#446

sgotti requested changes Oct 11, 2023

View reviewed changes

alessandro-sorint changed the title ~~configstore: add project run actions auth setting for org members~~ configstore: add ability to org members to restart stop cancel a project run Oct 11, 2023

alessandro-sorint force-pushed the issue-428 branch from eaf1329 to a017f67 Compare October 11, 2023 12:28

sgotti requested changes Oct 11, 2023

View reviewed changes

alessandro-sorint force-pushed the issue-428 branch from a017f67 to 90db6db Compare October 12, 2023 09:23

alessandro-sorint force-pushed the issue-428 branch from 90db6db to adf13b6 Compare October 12, 2023 12:28

alessandro-sorint requested a review from sgotti October 12, 2023 13:05

sgotti requested changes Oct 12, 2023

View reviewed changes

alessandro-sorint force-pushed the issue-428 branch from adf13b6 to 6cee32d Compare October 12, 2023 15:14

alessandro-sorint requested a review from sgotti October 12, 2023 15:56

sgotti requested changes Oct 13, 2023

View reviewed changes

alessandro-sorint force-pushed the issue-428 branch 3 times, most recently from 06ad852 to 93fd5d1 Compare October 13, 2023 10:08

alessandro-sorint requested a review from sgotti October 13, 2023 13:05

sgotti requested changes Oct 16, 2023

View reviewed changes

alessandro-sorint force-pushed the issue-428 branch from 93fd5d1 to 6c40d49 Compare October 16, 2023 07:57

alessandro-sorint requested a review from sgotti October 16, 2023 09:47

sgotti requested changes Oct 16, 2023

View reviewed changes

alessandro-sorint force-pushed the issue-428 branch from 6c40d49 to 987c613 Compare October 16, 2023 12:26

alessandro-sorint requested a review from sgotti October 16, 2023 13:05

sgotti approved these changes Oct 16, 2023

View reviewed changes

sgotti merged commit 7c80cf2 into agola-io:master Oct 16, 2023

alessandro-sorint deleted the issue-428 branch October 17, 2023 07:00

sgotti mentioned this pull request Dec 4, 2023

projectsettings: add ability to set membersCanPerformRunAction agola-io/agola-web#104

Merged


		push(t, config, giteaRepo.CloneURL, giteaToken, "commit", false)

		// test org run actions user owner

configstore: add ability to org members to restart stop cancel a project run #446

configstore: add ability to org members to restart stop cancel a project run #446

Conversation

alessandro-sorint commented Oct 10, 2023 • edited Loading

sgotti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgotti Oct 11, 2023 • edited Loading

Choose a reason for hiding this comment

alessandro-sorint commented Oct 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgotti commented Oct 11, 2023

sgotti Oct 11, 2023 • edited Loading

Choose a reason for hiding this comment

sgotti Oct 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alessandro-sorint commented Oct 12, 2023 • edited by sgotti Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgotti Oct 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgotti commented Oct 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alessandro-sorint commented Oct 10, 2023 •

edited

Loading

sgotti Oct 11, 2023 •

edited

Loading

sgotti Oct 11, 2023 •

edited

Loading

sgotti Oct 11, 2023 •

edited

Loading

alessandro-sorint commented Oct 12, 2023 •

edited by sgotti

Loading

sgotti Oct 12, 2023 •

edited

Loading