Skip to content

Commit

Permalink
Drop in_maintenance in the API and replace it with state
Browse files Browse the repository at this point in the history
  • Loading branch information
@SuperSandro2000
SuperSandro2000 committed Oct 22, 2024
1 parent 99464dc commit 200cf20
Show file tree
Hide file tree
Showing 18 changed files with 99 additions and 108 deletions.
10 changes: 4 additions & 6 deletions docs/api-spec.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ The following fields may be returned:
| `accounts[].gc_policies[].time_constraint.oldest`<br>`accounts[].gc_policies[].time_constraint.newest` | integer or omitted | If set, the GC policy only applies to at most that many images within each repository, specifically to those that are oldest/newest ones when ordered by the timestamp attribute specified in the `time_constraint.on` key. These constraints are forbidden for policies with action "delete" to ensure that GC runs are idempotent. |
| `accounts[].gc_policies[].time_constraint.older_than`<br>`accounts[].gc_policies[].time_constraint.newer_than` | duration or omitted | If set, the GC policy only applies to at most images whose timestamp (as selected by the `time_constraint.on` key) is older/newer than the given age. Durations are given as a JSON object with the keys `value` (integer) and `unit` (string), e.g. `{"value": 4, "unit": "d"}` for 4 days. The units `s` (second), `m` (minute), `h` (hour), `d` (day), `w` (7 days) and `y` (365 days) are understood. |
| `accounts[].gc_policies[].action` | string | One of: `delete` (to delete matching images) or `protect` (to not delete matching images, even if another policy with a lower priority would want to). |
| `accounts[].in_maintenance` | bool | Whether this account is in maintenance mode. [See below](#maintenance-mode) for details. |
| `accounts[].state` | string | The state of the account. Possible values are right now deleting [See below](#account-state) for details. |
| `accounts[].rbac_policies` | list of objects | Policies for rule-based access control (RBAC) to repositories in this account. RBAC policies are evaluated in addition to the permissions granted by the auth tenant. |
| `accounts[].rbac_policies[].match_cidr` | string | The RBAC policy applies to requests which originate from an IP address that matches the CIDR. |
| `accounts[].rbac_policies[].match_repository` | string | The RBAC policy applies to all repositories in this account whose name matches this regex. The leading account name and slash is stripped from the repository name before matching. The notes on regexes below apply. |
Expand Down Expand Up @@ -199,18 +199,16 @@ The following fields are shown on accounts configured with this strategy:

Note that the `accounts[].replication.upstream.password` field is omitted from GET responses for security reasons.

### Maintenance mode
### Account state

When `accounts[].in_maintenance` is true, the following differences in behavior apply to this account:
When `accounts[].state` is `deleting`, the following differences in behavior apply to this account:

- For primary accounts (i.e. accounts that are not replicas), no new blobs or manifests may be pushed. Only pulling and
deleting are allowed.
- For replica accounts, no new blobs or manifests will be replicated. Pulling is still allowed, but it becomes possible
to delete blobs and manifests.

Maintenance mode is a significant part of the account deletion workflow: Sending a DELETE request on an account is only
allowed while the account is in maintenance mode, and the caller must have deleted all manifests from the account before
attempting to DELETE it.
Sending a DELETE request on an account marks it as `state = "deleting"` and the janitor starts to delete everything that belongs to the account including manifests and blobs.

## GET /keppel/v1/accounts/:name

Expand Down
3 changes: 2 additions & 1 deletion internal/api/keppel/accounts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,9 @@ func (a *API) handleDeleteAccount(w http.ResponseWriter, r *http.Request) {

if account.IsDeleting {
respondwith.JSON(w, http.StatusConflict, struct {
Error string `json:"error,omitempty"`
Error string `json:"error"`
}{Error: "account is already set to be deleted"})
return
}

err := a.processor().MarkAccountForDeletion(*account, keppel.AuditContext{
Expand Down
82 changes: 31 additions & 51 deletions internal/api/keppel/accounts_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ func TestAccountsAPI(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"rbac_policies": []assert.JSONObject{},
},
},
Expand Down Expand Up @@ -119,7 +118,6 @@ func TestAccountsAPI(t *testing.T) {
"accounts": []assert.JSONObject{{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"rbac_policies": []assert.JSONObject{},
}},
},
Expand All @@ -133,7 +131,6 @@ func TestAccountsAPI(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"rbac_policies": []assert.JSONObject{},
},
},
Expand Down Expand Up @@ -206,7 +203,6 @@ func TestAccountsAPI(t *testing.T) {
"name": "second",
"auth_tenant_id": "tenant1",
"gc_policies": gcPoliciesJSON,
"in_maintenance": false,
"rbac_policies": rbacPoliciesJSON,
},
},
Expand Down Expand Up @@ -255,14 +251,12 @@ func TestAccountsAPI(t *testing.T) {
{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"rbac_policies": []assert.JSONObject{},
},
{
"name": "second",
"auth_tenant_id": "tenant1",
"gc_policies": gcPoliciesJSON,
"in_maintenance": false,
"rbac_policies": rbacPoliciesJSON,
},
},
Expand All @@ -278,7 +272,6 @@ func TestAccountsAPI(t *testing.T) {
"name": "second",
"auth_tenant_id": "tenant1",
"gc_policies": gcPoliciesJSON,
"in_maintenance": false,
"rbac_policies": rbacPoliciesJSON,
},
},
Expand All @@ -290,27 +283,40 @@ func TestAccountsAPI(t *testing.T) {

// check editing of InMaintenance flag (this also tests editing of GC policies
// since we don't give any and thus clear the field)
for _, inMaintenance := range []bool{true, false} {
for _, isDeleting := range []bool{true, false} {
account := assert.JSONObject{
"auth_tenant_id": "tenant1",
"rbac_policies": rbacPoliciesJSON,
}
accountExcept := assert.JSONObject{
"name": "second",
"auth_tenant_id": "tenant1",
"rbac_policies": rbacPoliciesJSON,
}
if isDeleting {
account = assert.JSONObject{
"auth_tenant_id": "tenant1",
"state": "deleting",
"rbac_policies": rbacPoliciesJSON,
}
accountExcept = assert.JSONObject{
"name": "second",
"auth_tenant_id": "tenant1",
"state": "deleting",
"rbac_policies": rbacPoliciesJSON,
}
}

assert.HTTPRequest{
Method: "PUT",
Path: "/keppel/v1/accounts/second",
Header: map[string]string{"X-Test-Perms": "change:tenant1"},
Body: assert.JSONObject{
"account": assert.JSONObject{
"auth_tenant_id": "tenant1",
"in_maintenance": inMaintenance,
"rbac_policies": rbacPoliciesJSON,
},
"account": account,
},
ExpectStatus: http.StatusOK,
ExpectBody: assert.JSONObject{
"account": assert.JSONObject{
"name": "second",
"auth_tenant_id": "tenant1",
"in_maintenance": inMaintenance,
"metadata": assert.JSONObject{},
"rbac_policies": rbacPoliciesJSON,
},
"account": accountExcept,
},
}.Check(t, h)

Expand All @@ -320,18 +326,12 @@ func TestAccountsAPI(t *testing.T) {
Header: map[string]string{"X-Test-Perms": "view:tenant1"},
ExpectStatus: http.StatusOK,
ExpectBody: assert.JSONObject{
"account": assert.JSONObject{
"name": "second",
"auth_tenant_id": "tenant1",
"in_maintenance": inMaintenance,
"metadata": assert.JSONObject{},
"rbac_policies": rbacPoliciesJSON,
},
"account": accountExcept,
},
}.Check(t, h)

// the first pass also generates an audit event since we're touching the GCPolicies
if inMaintenance {
if isDeleting {
s.Auditor.ExpectEvents(t,
cadf.Event{
RequestPath: "/keppel/v1/accounts/second",
Expand Down Expand Up @@ -386,7 +386,6 @@ func TestAccountsAPI(t *testing.T) {
"account": assert.JSONObject{
"name": "second",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"rbac_policies": newRBACPoliciesJSON,
},
},
Expand Down Expand Up @@ -429,7 +428,6 @@ func TestAccountsAPI(t *testing.T) {
"account": assert.JSONObject{
"name": "second",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"rbac_policies": newRBACPoliciesJSON,
"validation": assert.JSONObject{
"required_labels": []string{"foo", "bar"},
Expand Down Expand Up @@ -457,7 +455,6 @@ func TestAccountsAPI(t *testing.T) {
"account": assert.JSONObject{
"name": "second",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"rbac_policies": newRBACPoliciesJSON,
},
},
Expand Down Expand Up @@ -536,7 +533,6 @@ func TestPutAccountErrorCases(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"rbac_policies": []assert.JSONObject{},
},
},
Expand Down Expand Up @@ -1028,7 +1024,6 @@ func TestPutAccountErrorCases(t *testing.T) {
ExpectBody: assert.JSONObject{
"account": assert.JSONObject{
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"name": "first",
"rbac_policies": []assert.JSONObject{{
"match_cidr": "1.2.0.0/16",
Expand All @@ -1048,7 +1043,6 @@ func TestPutAccountErrorCases(t *testing.T) {
ExpectBody: assert.JSONObject{
"account": assert.JSONObject{
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"name": "first",
"rbac_policies": []assert.JSONObject{{
"match_cidr": "1.2.0.0/16",
Expand Down Expand Up @@ -1223,7 +1217,6 @@ func TestGetPutAccountReplicationOnFirstUse(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"rbac_policies": []assert.JSONObject{},
},
},
Expand Down Expand Up @@ -1320,7 +1313,6 @@ func TestGetPutAccountReplicationOnFirstUse(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"rbac_policies": []assert.JSONObject{},
"replication": assert.JSONObject{
"strategy": "on_first_use",
Expand All @@ -1346,7 +1338,6 @@ func TestGetPutAccountReplicationOnFirstUse(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"rbac_policies": []assert.JSONObject{},
"replication": assert.JSONObject{
"strategy": "on_first_use",
Expand Down Expand Up @@ -1380,7 +1371,6 @@ func TestGetPutAccountReplicationOnFirstUse(t *testing.T) {
"account": assert.JSONObject{
"name": "second",
"auth_tenant_id": "tenant2",
"in_maintenance": false,
"rbac_policies": []assert.JSONObject{},
},
},
Expand Down Expand Up @@ -1515,7 +1505,6 @@ func TestGetPutAccountReplicationFromExternalOnFirstUse(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"rbac_policies": []assert.JSONObject{},
"replication": assert.JSONObject{
"strategy": "from_external_on_first_use",
Expand Down Expand Up @@ -1544,7 +1533,6 @@ func TestGetPutAccountReplicationFromExternalOnFirstUse(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"rbac_policies": []assert.JSONObject{},
"replication": assert.JSONObject{
"strategy": "from_external_on_first_use",
Expand Down Expand Up @@ -1580,7 +1568,6 @@ func TestGetPutAccountReplicationFromExternalOnFirstUse(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"rbac_policies": []assert.JSONObject{},
"replication": assert.JSONObject{
"strategy": "from_external_on_first_use",
Expand All @@ -1605,7 +1592,6 @@ func TestGetPutAccountReplicationFromExternalOnFirstUse(t *testing.T) {
Body: assert.JSONObject{
"account": assert.JSONObject{
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"rbac_policies": []assert.JSONObject{},
"replication": assert.JSONObject{
"strategy": "from_external_on_first_use",
Expand All @@ -1622,7 +1608,6 @@ func TestGetPutAccountReplicationFromExternalOnFirstUse(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"rbac_policies": []assert.JSONObject{},
"replication": assert.JSONObject{
"strategy": "from_external_on_first_use",
Expand All @@ -1644,7 +1629,6 @@ func TestGetPutAccountReplicationFromExternalOnFirstUse(t *testing.T) {
Body: assert.JSONObject{
"account": assert.JSONObject{
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"rbac_policies": []assert.JSONObject{},
"replication": assert.JSONObject{
"strategy": "from_external_on_first_use",
Expand Down Expand Up @@ -1706,7 +1690,6 @@ func TestGetPutAccountReplicationFromExternalOnFirstUse(t *testing.T) {
"account": assert.JSONObject{
"name": "second",
"auth_tenant_id": "tenant2",
"in_maintenance": false,
"rbac_policies": []assert.JSONObject{},
},
},
Expand Down Expand Up @@ -1790,9 +1773,9 @@ func TestDeleteAccount(t *testing.T) {
// setup test accounts and repositories
nextBlobSweepAt := time.Unix(200, 0)
accounts := []*models.Account{
{Name: "test1", AuthTenantID: "tenant1", InMaintenance: true, NextBlobSweepedAt: &nextBlobSweepAt, GCPoliciesJSON: "[]", SecurityScanPoliciesJSON: "[]"},
{Name: "test2", AuthTenantID: "tenant2", InMaintenance: true, GCPoliciesJSON: "[]", SecurityScanPoliciesJSON: "[]"},
{Name: "test3", AuthTenantID: "tenant3", InMaintenance: true, GCPoliciesJSON: "[]", SecurityScanPoliciesJSON: "[]"},
{Name: "test1", AuthTenantID: "tenant1", IsDeleting: true, NextBlobSweepedAt: &nextBlobSweepAt, GCPoliciesJSON: "[]", SecurityScanPoliciesJSON: "[]"},
{Name: "test2", AuthTenantID: "tenant2", IsDeleting: true, GCPoliciesJSON: "[]", SecurityScanPoliciesJSON: "[]"},
{Name: "test3", AuthTenantID: "tenant3", IsDeleting: true, GCPoliciesJSON: "[]", SecurityScanPoliciesJSON: "[]"},
}
for _, account := range accounts {
mustInsert(t, s.DB, account)
Expand Down Expand Up @@ -1989,7 +1972,6 @@ func TestReplicaAccountsInheritPlatformFilter(t *testing.T) {
"account": assert.JSONObject{
"name": name,
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"rbac_policies": []assert.JSONObject{},
"replication": assert.JSONObject{
"strategy": "from_external_on_first_use",
Expand Down Expand Up @@ -2026,7 +2008,6 @@ func TestReplicaAccountsInheritPlatformFilter(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"platform_filter": testPlatformFilter,
"rbac_policies": []assert.JSONObject{},
"replication": assert.JSONObject{
Expand Down Expand Up @@ -2063,7 +2044,6 @@ func TestReplicaAccountsInheritPlatformFilter(t *testing.T) {
"account": assert.JSONObject{
"name": "second",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"platform_filter": testPlatformFilter,
"rbac_policies": []assert.JSONObject{},
"replication": assert.JSONObject{
Expand Down
6 changes: 3 additions & 3 deletions internal/api/keppel/fixtures/delete-account-000.sql
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
INSERT INTO accounts (name, auth_tenant_id, next_blob_sweep_at, in_maintenance) VALUES ('test1', 'tenant1', 200, TRUE);
INSERT INTO accounts (name, auth_tenant_id, in_maintenance) VALUES ('test2', 'tenant2', TRUE);
INSERT INTO accounts (name, auth_tenant_id, in_maintenance) VALUES ('test3', 'tenant3', TRUE);
INSERT INTO accounts (name, auth_tenant_id, next_blob_sweep_at, is_deleting) VALUES ('test1', 'tenant1', 200, TRUE);
INSERT INTO accounts (name, auth_tenant_id, is_deleting) VALUES ('test2', 'tenant2', TRUE);
INSERT INTO accounts (name, auth_tenant_id, is_deleting) VALUES ('test3', 'tenant3', TRUE);

INSERT INTO blob_mounts (blob_id, repo_id) VALUES (1, 1);
INSERT INTO blob_mounts (blob_id, repo_id) VALUES (2, 1);
Expand Down
6 changes: 3 additions & 3 deletions internal/api/keppel/fixtures/delete-account-001.sql
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
INSERT INTO accounts (name, auth_tenant_id, next_blob_sweep_at, in_maintenance, is_deleting, next_deletion_attempt_at) VALUES ('test1', 'tenant1', 200, TRUE, TRUE, 0);
INSERT INTO accounts (name, auth_tenant_id, in_maintenance) VALUES ('test2', 'tenant2', TRUE);
INSERT INTO accounts (name, auth_tenant_id, in_maintenance) VALUES ('test3', 'tenant3', TRUE);
INSERT INTO accounts (name, auth_tenant_id, next_blob_sweep_at, is_deleting, next_deletion_attempt_at) VALUES ('test1', 'tenant1', 200, TRUE, 0);
INSERT INTO accounts (name, auth_tenant_id) VALUES ('test2', 'tenant2');
INSERT INTO accounts (name, auth_tenant_id) VALUES ('test3', 'tenant3');

INSERT INTO blob_mounts (blob_id, repo_id) VALUES (1, 1);
INSERT INTO blob_mounts (blob_id, repo_id) VALUES (2, 1);
Expand Down
Loading

0 comments on commit 200cf20

Please sign in to comment.