Merge pull request #349 from sapcc/rbac-policy-simplification

internal/api/keppel/accounts.go

@@ -153,6 +153,14 @@ func (a *API) renderAccount(dbAccount keppel.Account) (Account, error) {
 		policies[idx] = renderRBACPolicy(p)
 	}
 
+	//use this opportunity to populate the new accounts.rbac_policies_json field if necessary
+	if (dbAccount.RBACPoliciesJSON == "" || dbAccount.RBACPoliciesJSON == "[]") && len(dbPolicies) > 0 {
+		err := a.fillNewStyleRBACPolicies(dbAccount, dbPolicies)
+		if err != nil {
+			return Account{}, err
+		}
+	}
+
 	metadata := make(map[string]string)
 	if dbAccount.MetadataJSON != "" {
 		err := json.Unmarshal([]byte(dbAccount.MetadataJSON), &metadata)
@@ -829,6 +837,44 @@ func (a *API) putRBACPolicies(account keppel.Account, policies []keppel.RBACPoli
 		})
 	}
 
+	//sync new accounts.rbac_policies_json field with the state of the rbac_policies table
+	return a.fillNewStyleRBACPolicies(account, policies)
+}
+
+func (a *API) fillNewStyleRBACPolicies(account keppel.Account, policies []keppel.RBACPolicy) error {
+	// convert from old format to new format
+	var newPolicies []keppel.NewRBACPolicy
+	for _, policy := range policies {
+		apiPolicy := renderRBACPolicy(policy)
+		newPolicies = append(newPolicies, keppel.NewRBACPolicy(apiPolicy))
+	}
+
+	// ensure same sorting as in SQL (ORDER BY account_name, match_repository, match_username, match_cidr)
+	slices.SortFunc(newPolicies, func(lhs, rhs keppel.NewRBACPolicy) int {
+		cmp := strings.Compare(string(lhs.RepositoryPattern), string(rhs.RepositoryPattern))
+		if cmp != 0 {
+			return cmp
+		}
+		cmp = strings.Compare(string(lhs.UserNamePattern), string(rhs.UserNamePattern))
+		if cmp != 0 {
+			return cmp
+		}
+		return strings.Compare(lhs.CidrPattern, rhs.CidrPattern)
+	})
+
+	newPoliciesStr := ""
+	if len(newPolicies) > 0 {
+		buf, err := json.Marshal(newPolicies)
+		if err != nil {
+			return err
+		}
+		newPoliciesStr = string(buf)
+	}
+
+	_, err := a.db.Exec(`UPDATE accounts SET rbac_policies_json = $1 WHERE name = $2`, newPoliciesStr, account.Name)
+	if err != nil {
+		return fmt.Errorf("while writing accounts.rbac_policies_json: %w", err)
+	}
 	return nil
 }
 

internal/api/keppel/accounts_test.go

@@ -332,7 +332,7 @@ func TestAccountsAPI(t *testing.T) {
 	}.Check(t, h)
 	tr.DBChanges().AssertEqual(`
 		INSERT INTO accounts (name, auth_tenant_id, metadata_json) VALUES ('first', 'tenant1', '{"bar":"barbar","foo":"foofoo"}');
-		INSERT INTO accounts (name, auth_tenant_id, gc_policies_json) VALUES ('second', 'tenant1', '[{"match_repository":".*/database","except_repository":"archive/.*","time_constraint":{"on":"pushed_at","newer_than":{"value":10,"unit":"d"}},"action":"protect"},{"match_repository":".*","only_untagged":true,"action":"delete"}]');
+		INSERT INTO accounts (name, auth_tenant_id, gc_policies_json, rbac_policies_json) VALUES ('second', 'tenant1', '[{"match_repository":".*/database","except_repository":"archive/.*","time_constraint":{"on":"pushed_at","newer_than":{"value":10,"unit":"d"}},"action":"protect"},{"match_repository":".*","only_untagged":true,"action":"delete"}]', '[{"match_repository":"library/.*","permissions":["anonymous_pull"]},{"match_repository":"library/alpine","match_username":".*@tenant2","permissions":["pull","push"]}]');
 		INSERT INTO rbac_policies (account_name, match_repository, match_username, can_anon_pull) VALUES ('second', 'library/.*', '', TRUE);
 		INSERT INTO rbac_policies (account_name, match_repository, match_username, can_pull, can_push) VALUES ('second', 'library/alpine', '.*@tenant2', TRUE, TRUE);
 	`)
@@ -557,7 +557,7 @@ func TestAccountsAPI(t *testing.T) {
 		ExpectBody:   assert.JSONObject{"sublease_token": makeSubleaseToken("second", "registry.example.org", "this-is-the-token")},
 	}.Check(t, h)
 	tr.DBChanges().AssertEqual(`
-		UPDATE accounts SET gc_policies_json = '[]' WHERE name = 'second';
+		UPDATE accounts SET gc_policies_json = '[]', rbac_policies_json = '[{"match_repository":"library/alpine","match_username":".*@tenant2","permissions":["pull"]},{"match_repository":"library/alpine","match_username":".*@tenant3","permissions":["pull","delete"]}]' WHERE name = 'second';
 		DELETE FROM rbac_policies WHERE account_name = 'second' AND match_repository = 'library/.*' AND match_username = '' AND match_cidr = '0.0.0.0/0';
 		UPDATE rbac_policies SET can_push = FALSE WHERE account_name = 'second' AND match_repository = 'library/alpine' AND match_username = '.*@tenant2' AND match_cidr = '0.0.0.0/0';
 		INSERT INTO rbac_policies (account_name, match_repository, match_username, can_pull, can_delete) VALUES ('second', 'library/alpine', '.*@tenant3', TRUE, TRUE);
@@ -1099,7 +1099,7 @@ func TestPutAccountErrorCases(t *testing.T) {
 		},
 	}.Check(t, h)
 	tr.DBChanges().AssertEqual(`
-		INSERT INTO accounts (name, auth_tenant_id) VALUES ('first', 'tenant1');
+		INSERT INTO accounts (name, auth_tenant_id, rbac_policies_json) VALUES ('first', 'tenant1', '[{"match_cidr":"1.2.0.0/16","permissions":["pull"]}]');
 		INSERT INTO rbac_policies (account_name, match_repository, match_username, can_pull, match_cidr) VALUES ('first', '', '', TRUE, '1.2.0.0/16');
 	`)
 	assert.HTTPRequest{

internal/keppel/database.go

@@ -224,6 +224,14 @@ var sqlMigrations = map[string]string{
 		DROP TABLE rbac_policies;
 		DROP TABLE accounts;
 	`,
+	"036_add_accounts_rbac_policies_json.up.sql": `
+		ALTER TABLE accounts
+			ADD COLUMN rbac_policies_json TEXT NOT NULL DEFAULT '';
+	`,
+	"036_add_accounts_rbac_policies_json.down.sql": `
+		ALTER TABLE accounts
+			DROP COLUMN rbac_policies_json;
+	`,
 }
 
 // DB adds convenience functions on top of gorp.DbMap.

internal/keppel/gc_policy.go

@@ -31,6 +31,7 @@ import (
 )
 
 // GCPolicy is a policy enabling optional garbage collection runs in an account.
+// It is stored in serialized form in the GCPoliciesJSON field of type Account.
 type GCPolicy struct {
 	RepositoryRx         regexpext.BoundedRegexp `json:"match_repository"`
 	NegativeRepositoryRx regexpext.BoundedRegexp `json:"except_repository,omitempty"`

internal/keppel/models.go

@@ -57,6 +57,8 @@ type Account struct {
 
 	//MetadataJSON contains a JSON string of a map[string]string, or the empty string.
 	MetadataJSON string `db:"metadata_json"`
+	//RBACPoliciesJSON contains a JSON string of []keppel.RBACPolicy, or the empty string.
+	RBACPoliciesJSON string `db:"rbac_policies_json"`
 	//GCPoliciesJSON contains a JSON string of []keppel.GCPolicy, or the empty string.
 	GCPoliciesJSON string `db:"gc_policies_json"`
 	//SecurityScanPoliciesJSON contains a JSON string of []keppel.SecurityScanPolicy, or the empty string.
@@ -88,6 +90,8 @@ func FindAccount(db gorp.SqlExecutor, name string) (*Account, error) {
 ////////////////////////////////////////////////////////////////////////////////
 
 // RBACPolicy contains a record from the `rbac_policies` table.
+//
+// TODO Replace this DB table with the new `accounts.rbac_policies_json` field.
 type RBACPolicy struct {
 	AccountName             string `db:"account_name"`
 	CidrPattern             string `db:"match_cidr"`

internal/keppel/rbac_policy.go

@@ -0,0 +1,35 @@
+/*******************************************************************************
+*
+* Copyright 2024 SAP SE
+*
+* Licensed under the Apache License, Version 2.0 (the "License");
+* you may not use this file except in compliance with the License.
+* You should have received a copy of the License along with this
+* program. If not, you may obtain a copy of the License at
+*
+*     http://www.apache.org/licenses/LICENSE-2.0
+*
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*
+*******************************************************************************/
+
+package keppel
+
+import (
+	"github.com/sapcc/go-bits/regexpext"
+)
+
+// NewRBACPolicy is a policy granting user-defined access to repos in an account.
+// It is stored in serialized form in the RBACPoliciesJSON field of type Account.
+//
+// TODO: rename to type RBACPolicy after the `rbac_policies` table has been removed
+type NewRBACPolicy struct {
+	CidrPattern       string                `json:"match_cidr,omitempty"`
+	RepositoryPattern regexpext.PlainRegexp `json:"match_repository,omitempty"`
+	UserNamePattern   regexpext.PlainRegexp `json:"match_username,omitempty"`
+	Permissions       []string              `json:"permissions"`
+}