Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop deprecated in_maintenance #483

Merged
merged 2 commits into from
Feb 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
SuperSandro2000 marked this conversation as resolved.
Show resolved Hide resolved
// 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