Skip to content

Commit

Permalink
Merge pull request #483 from sapcc/drop_maintenance
Browse files Browse the repository at this point in the history
Drop deprecated in_maintenance
  • Loading branch information
majewsky authored Feb 3, 2025
2 parents de7bdf5 + 147b197 commit 2e55ea6
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 79 deletions.
83 changes: 13 additions & 70 deletions internal/api/keppel/accounts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ func TestAccountsAPI(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"rbac_policies": []assert.JSONObject{},
},
Expand Down Expand Up @@ -118,7 +117,6 @@ func TestAccountsAPI(t *testing.T) {
"accounts": []assert.JSONObject{{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"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,
"metadata": nil,
"rbac_policies": []assert.JSONObject{},
},
Expand Down Expand Up @@ -207,7 +204,6 @@ func TestAccountsAPI(t *testing.T) {
"name": "second",
"auth_tenant_id": "tenant1",
"gc_policies": gcPoliciesJSON,
"in_maintenance": false,
"metadata": nil,
"rbac_policies": rbacPoliciesJSON,
},
Expand Down Expand Up @@ -257,15 +253,13 @@ func TestAccountsAPI(t *testing.T) {
{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"rbac_policies": []assert.JSONObject{},
},
{
"name": "second",
"auth_tenant_id": "tenant1",
"gc_policies": gcPoliciesJSON,
"in_maintenance": false,
"metadata": nil,
"rbac_policies": rbacPoliciesJSON,
},
Expand All @@ -282,7 +276,6 @@ func TestAccountsAPI(t *testing.T) {
"name": "second",
"auth_tenant_id": "tenant1",
"gc_policies": gcPoliciesJSON,
"in_maintenance": false,
"metadata": nil,
"rbac_policies": rbacPoliciesJSON,
},
Expand Down Expand Up @@ -324,7 +317,6 @@ func TestAccountsAPI(t *testing.T) {
"account": assert.JSONObject{
"name": "second",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"rbac_policies": newRBACPoliciesJSON,
},
Expand Down Expand Up @@ -368,7 +360,6 @@ func TestAccountsAPI(t *testing.T) {
"account": assert.JSONObject{
"name": "second",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"rbac_policies": newRBACPoliciesJSON,
"validation": assert.JSONObject{
Expand Down Expand Up @@ -397,7 +388,6 @@ func TestAccountsAPI(t *testing.T) {
"account": assert.JSONObject{
"name": "second",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"rbac_policies": newRBACPoliciesJSON,
},
Expand All @@ -419,36 +409,6 @@ func TestAccountsAPI(t *testing.T) {
`)
}

func TestPutAccountInMaintenanceFlag(t *testing.T) {
s := test.NewSetup(t, test.WithKeppelAPI)
h := s.Handler

// TODO: remove the writing capability for accounts.in_maintenance once the Elektra UI does not depend on it anymore
for _, inMaintenance := range []bool{false, true, false} {
assert.HTTPRequest{
Method: "PUT",
Path: "/keppel/v1/accounts/first",
Header: map[string]string{"X-Test-Perms": "change:tenant1"},
Body: assert.JSONObject{
"account": assert.JSONObject{
"auth_tenant_id": "tenant1",
"in_maintenance": inMaintenance,
},
},
ExpectStatus: http.StatusOK,
ExpectBody: assert.JSONObject{
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": inMaintenance,
"metadata": nil,
"rbac_policies": []assert.JSONObject{},
},
},
}.Check(t, h)
}
}

func TestGetAccountsErrorCases(t *testing.T) {
s := test.NewSetup(t, test.WithKeppelAPI)
h := s.Handler
Expand Down Expand Up @@ -507,7 +467,6 @@ func TestPutAccountErrorCases(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"rbac_policies": []assert.JSONObject{},
},
Expand Down Expand Up @@ -1000,7 +959,6 @@ func TestPutAccountErrorCases(t *testing.T) {
ExpectBody: assert.JSONObject{
"account": assert.JSONObject{
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"name": "first",
"rbac_policies": []assert.JSONObject{{
Expand All @@ -1021,7 +979,6 @@ func TestPutAccountErrorCases(t *testing.T) {
ExpectBody: assert.JSONObject{
"account": assert.JSONObject{
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"name": "first",
"rbac_policies": []assert.JSONObject{{
Expand Down Expand Up @@ -1161,20 +1118,19 @@ func TestPutAccountErrorCases(t *testing.T) {
ExpectBody: assert.StringData("no permission for keppel_account:unknown:change\n"),
}.Check(t, h)

// TODO: reenable once we completely remove support for the accounts.in_maintenance flag
// assert.HTTPRequest{
// Method: "PUT",
// Path: "/keppel/v1/accounts/first",
// Header: map[string]string{"X-Test-Perms": "change:tenant1"},
// Body: assert.JSONObject{
// "account": assert.JSONObject{
// "auth_tenant_id": "tenant1",
// "in_maintenance": true,
// },
// },
// ExpectStatus: http.StatusUnprocessableEntity,
// ExpectBody: assert.StringData("malformed attribute \"account.in_maintenance\" in request body is not allowed here\n"),
// }.Check(t, h)
assert.HTTPRequest{
Method: "PUT",
Path: "/keppel/v1/accounts/first",
Header: map[string]string{"X-Test-Perms": "change:tenant1"},
Body: assert.JSONObject{
"account": assert.JSONObject{
"auth_tenant_id": "tenant1",
"in_maintenance": true, // this field used to be supported, but support for it was removed
},
},
ExpectStatus: http.StatusBadRequest,
ExpectBody: assert.StringData("request body is not valid JSON: json: unknown field \"in_maintenance\"\n"),
}.Check(t, h)

assert.HTTPRequest{
Method: "PUT",
Expand Down Expand Up @@ -1228,7 +1184,6 @@ func TestGetPutAccountReplicationOnFirstUse(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"rbac_policies": []assert.JSONObject{},
},
Expand Down Expand Up @@ -1326,7 +1281,6 @@ func TestGetPutAccountReplicationOnFirstUse(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"rbac_policies": []assert.JSONObject{},
"replication": assert.JSONObject{
Expand All @@ -1353,7 +1307,6 @@ func TestGetPutAccountReplicationOnFirstUse(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"rbac_policies": []assert.JSONObject{},
"replication": assert.JSONObject{
Expand Down Expand Up @@ -1388,7 +1341,6 @@ func TestGetPutAccountReplicationOnFirstUse(t *testing.T) {
"account": assert.JSONObject{
"name": "second",
"auth_tenant_id": "tenant2",
"in_maintenance": false,
"metadata": nil,
"rbac_policies": []assert.JSONObject{},
},
Expand Down Expand Up @@ -1524,7 +1476,6 @@ func TestGetPutAccountReplicationFromExternalOnFirstUse(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"rbac_policies": []assert.JSONObject{},
"replication": assert.JSONObject{
Expand Down Expand Up @@ -1554,7 +1505,6 @@ func TestGetPutAccountReplicationFromExternalOnFirstUse(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"rbac_policies": []assert.JSONObject{},
"replication": assert.JSONObject{
Expand Down Expand Up @@ -1591,7 +1541,6 @@ func TestGetPutAccountReplicationFromExternalOnFirstUse(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"rbac_policies": []assert.JSONObject{},
"replication": assert.JSONObject{
Expand Down Expand Up @@ -1633,7 +1582,6 @@ func TestGetPutAccountReplicationFromExternalOnFirstUse(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"rbac_policies": []assert.JSONObject{},
"replication": assert.JSONObject{
Expand Down Expand Up @@ -1717,7 +1665,6 @@ func TestGetPutAccountReplicationFromExternalOnFirstUse(t *testing.T) {
"account": assert.JSONObject{
"name": "second",
"auth_tenant_id": "tenant2",
"in_maintenance": false,
"metadata": nil,
"rbac_policies": []assert.JSONObject{},
},
Expand Down Expand Up @@ -1900,7 +1847,6 @@ func TestReplicaAccountsInheritPlatformFilter(t *testing.T) {
"account": assert.JSONObject{
"name": name,
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"rbac_policies": []assert.JSONObject{},
"replication": assert.JSONObject{
Expand Down Expand Up @@ -1938,7 +1884,6 @@ func TestReplicaAccountsInheritPlatformFilter(t *testing.T) {
"account": assert.JSONObject{
"name": "first",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"platform_filter": testPlatformFilter,
"rbac_policies": []assert.JSONObject{},
Expand All @@ -1961,7 +1906,6 @@ func TestReplicaAccountsInheritPlatformFilter(t *testing.T) {
Body: assert.JSONObject{
"account": assert.JSONObject{
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"platform_filter": []assert.JSONObject{{
"os": "linux",
Expand All @@ -1978,7 +1922,6 @@ func TestReplicaAccountsInheritPlatformFilter(t *testing.T) {
"account": assert.JSONObject{
"name": "second",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": nil,
"platform_filter": testPlatformFilter,
"rbac_policies": []assert.JSONObject{},
Expand Down
6 changes: 1 addition & 5 deletions internal/keppel/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,7 @@ type Account struct {
State string `json:"state,omitempty"`
ValidationPolicy *ValidationPolicy `json:"validation,omitempty"`
PlatformFilter models.PlatformFilter `json:"platform_filter,omitempty"`

// TODO: deprecated, and remove
InMaintenance bool `json:"in_maintenance"`
Metadata *map[string]string `json:"metadata"`
Metadata *map[string]string `json:"metadata"`
}

// RenderAccount converts an account model from the DB into the API representation.
Expand Down Expand Up @@ -66,6 +63,5 @@ func RenderAccount(dbAccount models.Account) (Account, error) {
ReplicationPolicy: RenderReplicationPolicy(dbAccount),
ValidationPolicy: RenderValidationPolicy(dbAccount.Reduced()),
PlatformFilter: dbAccount.PlatformFilter,
InMaintenance: dbAccount.InMaintenance,
}, nil
}
8 changes: 8 additions & 0 deletions internal/keppel/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,14 @@ var sqlMigrations = map[string]string{
ALTER TABLE accounts
DROP COLUMN in_maintenance;
`,
"045_remove_accounts_in_maintenance_dummy.up.sql": `
ALTER TABLE accounts
DROP COLUMN in_maintenance;
`,
"045_remove_in_maintenance_dummy.down.sql": `
ALTER TABLE accounts
ADD COLUMN in_maintenance BOOLEAN NOT NULL DEFAULT FALSE;
`,
}

// DB adds convenience functions on top of gorp.DbMap.
Expand Down
3 changes: 0 additions & 3 deletions internal/models/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,6 @@ type Account struct {
NextEnforcementAt *time.Time `db:"next_enforcement_at"` // see tasks.CreateManagedAccountsJob
NextStorageSweepedAt *time.Time `db:"next_storage_sweep_at"` // see tasks.StorageSweepJob
NextFederationAnnouncementAt *time.Time `db:"next_federation_announcement_at"` // see tasks.AnnounceAccountToFederationJob

// TODO: remove once the Elektra UI has been updated to not require this flag to proceed with account deletion
InMaintenance bool `db:"in_maintenance"`
}

// Reduced converts an Account into a ReducedAccount.
Expand Down
1 change: 0 additions & 1 deletion internal/processor/accounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ func (p *Processor) CreateOrUpdateAccount(ctx context.Context, account keppel.Ac

// validate and update fields as requested
targetAccount.IsDeleting = account.State == "deleting"
targetAccount.InMaintenance = account.InMaintenance

// validate GC policies
if len(account.GCPolicies) == 0 {
Expand Down

0 comments on commit 2e55ea6

Please sign in to comment.