Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.
vole
Clone

Read only, no credentials:

https://rodent.software/git/vole.git

Write over SSH with a registered key:

git clone ssh://git@0.0.0.0:2222/vole.git

A free and open-source forge that lives in your Git repo.

Unify account terminology and clarify ACL commands

2e024cc96a391c0959ad2914caeed78b600fcfc1 · Keenan Nemetz · 14d ago · parent ab326d3a 

Unify account terminology and clarify ACL commands

Use account consistently in the CLI, help, docs, and remote admin messages. Rename the CLI account command from user to account while retaining the old user spelling as an alias.

Replace positional ACL mutations with explicit verbs such as set-role, protect add, runners remove, and checks enable. Update local and remote admin parsing, hints, docs, and tests.

Tidy repository filter alignment and remove duplicated issue/change counts from page subtitles.

server/integration_test.go +102−24

@@ -29,6 +29,7 @@ import (
29 29 "github.com/rodent-software/vole/ci"
30 30 "github.com/rodent-software/vole/ci/runs"
31 31 "github.com/rodent-software/vole/core/git"
32 "github.com/rodent-software/vole/forge/access"
32 33 "github.com/rodent-software/vole/forge/config"
33 34 "github.com/rodent-software/vole/forge/identity"
34 35 "github.com/rodent-software/vole/forge/index"
@@ -1607,7 +1608,7 @@ func TestIssueAssigneesRenderedFilteredAndValidated(t *testing.T) {
1607 1608 t.Fatalf("assignee is not rendered as a filter link:\n%s", body)
1608 1609 }
1609 1610 // The list page offers the assignee as a filter and shows it on the row.
1610 if _, body := f.get("/r/" + f.repo + "/issues"); !strings.Contains(body, "Assignees:") {
1611 if _, body := f.get("/r/" + f.repo + "/issues"); !strings.Contains(body, "Assignees") {
1611 1612 t.Fatalf("list has no assignee filters:\n%s", body)
1612 1613 }
1613 1614 // And the filter excludes issues with a different assignee.
@@ -3313,7 +3314,7 @@ func TestOnlyAllowedRunnersGreenLightABranch(t *testing.T) {
3313 3314 }
3314 3315
3315 3316 // Allow the account, through the command a host admin would use.
3316 out, errs, err := f.cli("", "acl", "runners", "--add", f.repo, "wilbur", "--config", f.cfgFile(t))
3317 out, errs, err := f.cli("", "acl", "runners", "add", f.repo, "wilbur", "--config", f.cfgFile(t))
3317 3318 if err != nil {
3318 3319 t.Fatalf("acl runners: %v\n%s", err, errs)
3319 3320 }
@@ -3350,7 +3351,7 @@ func TestOnlyAllowedRunnersGreenLightABranch(t *testing.T) {
3350 3351 }
3351 3352
3352 3353 // Allowing it again is enough: nothing else changed, and nothing was lost.
3353 if _, errs, err := f.cli("", "acl", "runners", "--add", f.repo, "wilbur", "--config", f.cfgFile(t)); err != nil {
3354 if _, errs, err := f.cli("", "acl", "runners", "add", f.repo, "wilbur", "--config", f.cfgFile(t)); err != nil {
3354 3355 t.Fatalf("acl runners: %v\n%s", err, errs)
3355 3356 }
3356 3357 if _, errs, err := f.gitWithKey(writerKey, feature, "push", f.sshURL(f.repo), "HEAD:refs/heads/main"); err != nil {
@@ -3358,8 +3359,8 @@ func TestOnlyAllowedRunnersGreenLightABranch(t *testing.T) {
3358 3359 }
3359 3360 }
3360 3361
3361 // TestACLRunnersEditsTheList: three shapes, because replacing a list to add a sixth
3362 // name is how a list gets shortened by mistake.
3362 // TestACLRunnersEditsTheList: add, set, remove and clear are separate verbs,
3363 // because replacing a list to add a sixth name is how a list gets shortened by mistake.
3363 3364 func TestACLRunnersEditsTheList(t *testing.T) {
3364 3365 t.Parallel()
3365 3366 f := newFixture(t)
@@ -3375,33 +3376,33 @@ func TestACLRunnersEditsTheList(t *testing.T) {
3375 3376 return a.Runners
3376 3377 }
3377 3378
3378 if out, errs, err := f.cli("", "acl", "runners", "--add", f.repo, "wilbur", "--config", cfg); err != nil {
3379 if out, errs, err := f.cli("", "acl", "runners", "add", f.repo, "wilbur", "--config", cfg); err != nil {
3379 3380 t.Fatalf("add: %v\n%s", err, errs)
3380 3381 } else if !strings.Contains(out, "wilbur") {
3381 3382 t.Fatalf("add did not report the account:\n%s", out)
3382 3383 }
3383 3384 if got := runners(); len(got) != 1 || got[0] != f.accountName("wilbur") {
3384 t.Fatalf("runners after --add wilbur = %#v", got)
3385 t.Fatalf("runners after add wilbur = %#v", got)
3385 3386 }
3386 3387
3387 3388 // Adding again is a no-op rather than a duplicate in the file.
3388 if _, errs, err := f.cli("", "acl", "runners", "--add", f.repo, "wilbur", "--config", cfg); err != nil {
3389 if _, errs, err := f.cli("", "acl", "runners", "add", f.repo, "wilbur", "--config", cfg); err != nil {
3389 3390 t.Fatalf("second add: %v\n%s", err, errs)
3390 3391 }
3391 3392 if got := runners(); len(got) != 1 {
3392 t.Fatalf("a repeated --add duplicated the account: %#v", got)
3393 t.Fatalf("a repeated add duplicated the account: %#v", got)
3393 3394 }
3394 3395
3395 3396 // The owner is an admin, so they already count; naming them is legal but adds
3396 3397 // nothing, and an empty list is rendered as what it means rather than as "none".
3397 if out, _, err := f.cli("", "acl", "runners", f.repo, "--config", cfg); err != nil {
3398 if out, _, err := f.cli("", "acl", "runners", "list", f.repo, "--config", cfg); err != nil {
3398 3399 t.Fatalf("show: %v", err)
3399 3400 } else if !strings.Contains(out, "wilbur") {
3400 3401 t.Fatalf("show did not list the runner:\n%s", out)
3401 3402 }
3402 3403
3403 // Bare names replace the list: a repo with one runner stays a repo with one.
3404 if _, errs, err := f.cli("", "acl", "runners", f.repo, "keenan", "--config", cfg); err != nil {
3404 // set replaces the list: a repo with one runner stays a repo with one.
3405 if _, errs, err := f.cli("", "acl", "runners", "set", f.repo, "keenan", "--config", cfg); err != nil {
3405 3406 t.Fatalf("replace: %v\n%s", err, errs)
3406 3407 }
3407 3408 if got := runners(); len(got) != 1 || got[0] != f.ownerID {
@@ -3411,7 +3412,7 @@ func TestACLRunnersEditsTheList(t *testing.T) {
3411 3412 // Removing somebody who is not on the list is an error, the way `git config
3412 3413 // --unset` refuses a key that is not set: a revocation that never happened
3413 3414 // should not look like one that did.
3414 if _, errs, err := f.cli("", "acl", "runners", "--remove", f.repo, "wilbur", "--config", cfg); err == nil {
3415 if _, errs, err := f.cli("", "acl", "runners", "remove", f.repo, "wilbur", "--config", cfg); err == nil {
3415 3416 t.Fatalf("removing a non-runner was accepted:\n%s", errs)
3416 3417 } else if !strings.Contains(errs, "is not a runner for") {
3417 3418 t.Fatalf("the refusal does not say what happened:\n%s", errs)
@@ -3420,37 +3421,114 @@ func TestACLRunnersEditsTheList(t *testing.T) {
3420 3421 t.Fatalf("a refused removal changed the list: %#v", got)
3421 3422 }
3422 3423
3423 // Removal is a flag, like `git config --unset`.
3424 if _, errs, err := f.cli("", "acl", "runners", "--remove", f.repo, "keenan", "--config", cfg); err != nil {
3424 // Removal is a subcommand, not a whole-list replacement.
3425 if _, errs, err := f.cli("", "acl", "runners", "remove", f.repo, "keenan", "--config", cfg); err != nil {
3425 3426 t.Fatalf("remove: %v\n%s", err, errs)
3426 3427 }
3427 3428 if got := runners(); len(got) != 0 {
3428 t.Fatalf("runners after --remove = %#v", got)
3429 t.Fatalf("runners after remove = %#v", got)
3429 3430 }
3430 3431
3431 // And --clear returns to admins-only, which is where a new repository starts.
3432 if _, errs, err := f.cli("", "acl", "runners", "--add", f.repo, "wilbur", "--config", cfg); err != nil {
3432 // And clear returns to admins-only, which is where a new repository starts.
3433 if _, errs, err := f.cli("", "acl", "runners", "add", f.repo, "wilbur", "--config", cfg); err != nil {
3433 3434 t.Fatalf("re-add: %v\n%s", err, errs)
3434 3435 }
3435 if _, errs, err := f.cli("", "acl", "runners", "--clear", f.repo, "--config", cfg); err != nil {
3436 if _, errs, err := f.cli("", "acl", "runners", "clear", f.repo, "--config", cfg); err != nil {
3436 3437 t.Fatalf("clear: %v\n%s", err, errs)
3437 3438 }
3438 3439 if got := runners(); len(got) != 0 {
3439 t.Fatalf("runners after --clear = %#v", got)
3440 t.Fatalf("runners after clear = %#v", got)
3440 3441 }
3441 // Two edits at once is a mistake, not a sequence.
3442 if _, errs, err := f.cli("", "acl", "runners", "--add", "--clear", f.repo, "wilbur", "--config", cfg); err == nil {
3443 t.Fatalf("--add --clear was accepted:\n%s", errs)
3442 // Modes are subcommands now; the old flag form is gone rather than accepted
3443 // silently with different semantics.
3444 if _, errs, err := f.cli("", "acl", "runners", "--add", f.repo, "wilbur", "--config", cfg); err == nil {
3445 t.Fatalf("the old --add form was accepted:\n%s", errs)
3444 3446 }
3445 3447
3446 3448 // An unknown account is refused rather than written as a typo that silently
3447 3449 // permits nobody.
3448 _, errs, err := f.cli("", "acl", "runners", "--add", f.repo, "nobody", "--config", cfg)
3450 _, errs, err := f.cli("", "acl", "runners", "add", f.repo, "nobody", "--config", cfg)
3449 3451 if err == nil {
3450 3452 t.Fatalf("an unknown account was accepted:\n%s", errs)
3451 3453 }
3452 3454 }
3453 3455
3456 // TestACLCommandsNameTheFieldTheyChange: each mutation says what it changes,
3457 // and list edits have explicit verbs instead of a mode flag or bare names.
3458 func TestACLCommandsNameTheFieldTheyChange(t *testing.T) {
3459 t.Parallel()
3460 f := newFixture(t)
3461 f.addWriter(t)
3462 f.publish(f.repo)
3463 cfg := f.cfgFile(t)
3464 cli := func(args ...string) (string, string) {
3465 t.Helper()
3466 args = append(args, "--config", cfg)
3467 out, errs, err := f.cli("", args...)
3468 if err != nil {
3469 t.Fatalf("%v: %v\n%s", args, err, errs)
3470 }
3471 return out, errs
3472 }
3473 acl := func() *access.ACL {
3474 t.Helper()
3475 a, err := mustRepo(t, f).ACL()
3476 if err != nil {
3477 t.Fatal(err)
3478 }
3479 return a
3480 }
3481
3482 cli("acl", "set-role", f.repo, "wilbur", "read")
3483 if got := acl().Entries[f.accountName("wilbur")]; got != "read" {
3484 t.Fatalf("role after set-role = %q", got)
3485 }
3486
3487 cli("acl", "set-default-role", f.repo, "contribute")
3488 if got := acl().Default; got != "contribute" {
3489 t.Fatalf("default after set-default-role = %q", got)
3490 }
3491
3492 cli("acl", "set-approvals", f.repo, "2")
3493 if got := acl().RequiredApprovals(); got != 2 {
3494 t.Fatalf("approvals after set-approvals = %d", got)
3495 }
3496
3497 cli("acl", "checks", "disable", f.repo)
3498 if acl().RequiresChecks() {
3499 t.Fatal("checks disable left checks required")
3500 }
3501 cli("acl", "checks", "enable", f.repo)
3502 if !acl().RequiresChecks() {
3503 t.Fatal("checks enable left checks optional")
3504 }
3505
3506 cli("acl", "protect", "add", f.repo, "refs/heads/main", "refs/heads/release")
3507 if got := strings.Join(acl().Protected, ","); got != "refs/heads/main,refs/heads/release" {
3508 t.Fatalf("protected after add = %q", got)
3509 }
3510 cli("acl", "protect", "remove", f.repo, "refs/heads/main")
3511 if got := strings.Join(acl().Protected, ","); got != "refs/heads/release" {
3512 t.Fatalf("protected after remove = %q", got)
3513 }
3514 cli("acl", "protect", "set", f.repo, "refs/heads/only")
3515 if got := strings.Join(acl().Protected, ","); got != "refs/heads/only" {
3516 t.Fatalf("protected after set = %q", got)
3517 }
3518 if out, _ := cli("acl", "protect", "list", f.repo); !strings.Contains(out, "refs/heads/only") {
3519 t.Fatalf("protect list did not show the list:\n%s", out)
3520 }
3521 cli("acl", "protect", "clear", f.repo)
3522 if got := acl().Protected; len(got) != 0 {
3523 t.Fatalf("protected after clear = %#v", got)
3524 }
3525
3526 // The old positional command is gone rather than silently mapped.
3527 if _, errs, err := f.cli("", "acl", "set", f.repo, "wilbur", "read", "--config", cfg); err == nil {
3528 t.Fatalf("old acl set form was accepted:\n%s", errs)
3529 }
3530 }
3531
3454 3532 // TestFailingCheckBlocksTheMerge: a green approval does not outvote a red build.
3455 3533 func TestFailingCheckBlocksTheMerge(t *testing.T) {
3456 3534 t.Parallel()