From 9cce4304306b3d159ca442b30b97011bd4128822 Mon Sep 17 00:00:00 2001 From: Dmitry Rodionov Date: Tue, 15 Feb 2022 18:42:03 +0300 Subject: [PATCH] remove several obsolete management api commands from pageserver's libpq api these commands are now available via http api --- pageserver/src/page_service.rs | 74 +----------------- test_runner/batch_others/test_auth.py | 33 ++++---- .../batch_others/test_pageserver_api.py | 76 ------------------- test_runner/batch_others/test_zenith_cli.py | 39 +++++----- test_runner/fixtures/zenith_fixtures.py | 53 ++++++++----- 5 files changed, 73 insertions(+), 202 deletions(-) diff --git a/pageserver/src/page_service.rs b/pageserver/src/page_service.rs index 7b69779e9a..a607ba77f6 100644 --- a/pageserver/src/page_service.rs +++ b/pageserver/src/page_service.rs @@ -27,13 +27,10 @@ use zenith_utils::lsn::Lsn; use zenith_utils::postgres_backend::is_socket_read_timed_out; use zenith_utils::postgres_backend::PostgresBackend; use zenith_utils::postgres_backend::{self, AuthType}; -use zenith_utils::pq_proto::{ - BeMessage, FeMessage, RowDescriptor, HELLO_WORLD_ROW, SINGLE_COL_ROWDESC, -}; +use zenith_utils::pq_proto::{BeMessage, FeMessage, RowDescriptor, SINGLE_COL_ROWDESC}; use zenith_utils::zid::{ZTenantId, ZTimelineId}; use crate::basebackup; -use crate::branches; use crate::config::PageServerConf; use crate::relish::*; use crate::repository::Timeline; @@ -662,75 +659,6 @@ impl postgres_backend::Handler for PageServerHandler { walreceiver::launch_wal_receiver(self.conf, tenantid, timelineid, &connstr)?; pgb.write_message_noflush(&BeMessage::CommandComplete(b"SELECT 1"))?; - } else if query_string.starts_with("branch_create ") { - let err = || format!("invalid branch_create: '{}'", query_string); - - // branch_create - // TODO lazy static - // TODO: escaping, to allow branch names with spaces - let re = Regex::new(r"^branch_create ([[:xdigit:]]+) (\S+) ([^\r\n\s;]+)[\r\n\s;]*;?$") - .unwrap(); - let caps = re.captures(query_string).with_context(err)?; - - let tenantid = ZTenantId::from_str(caps.get(1).unwrap().as_str())?; - let branchname = caps.get(2).with_context(err)?.as_str().to_owned(); - let startpoint_str = caps.get(3).with_context(err)?.as_str().to_owned(); - - self.check_permission(Some(tenantid))?; - - let _enter = - info_span!("branch_create", name = %branchname, tenant = %tenantid).entered(); - - let branch = - branches::create_branch(self.conf, &branchname, &startpoint_str, &tenantid)?; - let branch = serde_json::to_vec(&branch)?; - - pgb.write_message_noflush(&SINGLE_COL_ROWDESC)? - .write_message_noflush(&BeMessage::DataRow(&[Some(&branch)]))? - .write_message_noflush(&BeMessage::CommandComplete(b"SELECT 1"))?; - } else if query_string.starts_with("branch_list ") { - // branch_list - let re = Regex::new(r"^branch_list ([[:xdigit:]]+)$").unwrap(); - let caps = re - .captures(query_string) - .with_context(|| format!("invalid branch_list: '{}'", query_string))?; - - let tenantid = ZTenantId::from_str(caps.get(1).unwrap().as_str())?; - - // since these handlers for tenant/branch commands are deprecated (in favor of http based ones) - // just use false in place of include non incremental logical size - let branches = crate::branches::get_branches(self.conf, &tenantid, false)?; - let branches_buf = serde_json::to_vec(&branches)?; - - pgb.write_message_noflush(&SINGLE_COL_ROWDESC)? - .write_message_noflush(&BeMessage::DataRow(&[Some(&branches_buf)]))? - .write_message_noflush(&BeMessage::CommandComplete(b"SELECT 1"))?; - } else if query_string.starts_with("tenant_list") { - let tenants = crate::tenant_mgr::list_tenants()?; - let tenants_buf = serde_json::to_vec(&tenants)?; - - pgb.write_message_noflush(&SINGLE_COL_ROWDESC)? - .write_message_noflush(&BeMessage::DataRow(&[Some(&tenants_buf)]))? - .write_message_noflush(&BeMessage::CommandComplete(b"SELECT 1"))?; - } else if query_string.starts_with("tenant_create") { - let err = || format!("invalid tenant_create: '{}'", query_string); - - // tenant_create - let re = Regex::new(r"^tenant_create ([[:xdigit:]]+)$").unwrap(); - let caps = re.captures(query_string).with_context(err)?; - - self.check_permission(None)?; - - let tenantid = ZTenantId::from_str(caps.get(1).unwrap().as_str())?; - - tenant_mgr::create_repository_for_tenant(self.conf, tenantid)?; - - pgb.write_message_noflush(&SINGLE_COL_ROWDESC)? - .write_message_noflush(&BeMessage::CommandComplete(b"SELECT 1"))?; - } else if query_string.starts_with("status") { - pgb.write_message_noflush(&SINGLE_COL_ROWDESC)? - .write_message_noflush(&HELLO_WORLD_ROW)? - .write_message_noflush(&BeMessage::CommandComplete(b"SELECT 1"))?; } else if query_string.to_ascii_lowercase().starts_with("set ") { // important because psycopg2 executes "SET datestyle TO 'ISO'" // on connect diff --git a/test_runner/batch_others/test_auth.py b/test_runner/batch_others/test_auth.py index 025f21346f..8c0179d958 100644 --- a/test_runner/batch_others/test_auth.py +++ b/test_runner/batch_others/test_auth.py @@ -1,8 +1,8 @@ from contextlib import closing from typing import Iterator -from uuid import uuid4 +from uuid import UUID, uuid4 import psycopg2 -from fixtures.zenith_fixtures import ZenithEnvBuilder +from fixtures.zenith_fixtures import ZenithEnvBuilder, ZenithPageserverApiException import pytest pytest_plugins = ("fixtures.zenith_fixtures") @@ -15,31 +15,38 @@ def test_pageserver_auth(zenith_env_builder: ZenithEnvBuilder): ps = env.pageserver tenant_token = env.auth_keys.generate_tenant_token(env.initial_tenant) + tenant_http_client = env.pageserver.http_client(tenant_token) + invalid_tenant_token = env.auth_keys.generate_tenant_token(uuid4().hex) + invalid_tenant_http_client = env.pageserver.http_client(invalid_tenant_token) + management_token = env.auth_keys.generate_management_token() + management_http_client = env.pageserver.http_client(management_token) # this does not invoke auth check and only decodes jwt and checks it for validity # check both tokens - ps.safe_psql("status", password=tenant_token) - ps.safe_psql("status", password=management_token) + ps.safe_psql("set FOO", password=tenant_token) + ps.safe_psql("set FOO", password=management_token) # tenant can create branches - ps.safe_psql(f"branch_create {env.initial_tenant} new1 main", password=tenant_token) + tenant_http_client.branch_create(UUID(env.initial_tenant), 'new1', 'main') # console can create branches for tenant - ps.safe_psql(f"branch_create {env.initial_tenant} new2 main", password=management_token) + management_http_client.branch_create(UUID(env.initial_tenant), 'new2', 'main') - # fail to create branch using token with different tenantid - with pytest.raises(psycopg2.DatabaseError, match='Tenant id mismatch. Permission denied'): - ps.safe_psql(f"branch_create {env.initial_tenant} new2 main", password=invalid_tenant_token) + # fail to create branch using token with different tenant_id + with pytest.raises(ZenithPageserverApiException, + match='Forbidden: Tenant id mismatch. Permission denied'): + invalid_tenant_http_client.branch_create(UUID(env.initial_tenant), "new3", "main") # create tenant using management token - ps.safe_psql(f"tenant_create {uuid4().hex}", password=management_token) + management_http_client.tenant_create(uuid4()) # fail to create tenant using tenant token with pytest.raises( - psycopg2.DatabaseError, - match='Attempt to access management api with tenant scope. Permission denied'): - ps.safe_psql(f"tenant_create {uuid4().hex}", password=tenant_token) + ZenithPageserverApiException, + match='Forbidden: Attempt to access management api with tenant scope. Permission denied' + ): + tenant_http_client.tenant_create(uuid4()) @pytest.mark.parametrize('with_wal_acceptors', [False, True]) diff --git a/test_runner/batch_others/test_pageserver_api.py b/test_runner/batch_others/test_pageserver_api.py index 82f63761c2..b9ebba0c64 100644 --- a/test_runner/batch_others/test_pageserver_api.py +++ b/test_runner/batch_others/test_pageserver_api.py @@ -9,82 +9,6 @@ from typing import cast pytest_plugins = ("fixtures.zenith_fixtures") -def test_status_psql(zenith_simple_env: ZenithEnv): - env = zenith_simple_env - assert env.pageserver.safe_psql('status') == [ - ('hello world', ), - ] - - -def test_branch_list_psql(zenith_simple_env: ZenithEnv): - env = zenith_simple_env - # Create a branch for us - env.zenith_cli(["branch", "test_branch_list_main", "empty"]) - - conn = env.pageserver.connect() - cur = conn.cursor() - - cur.execute(f'branch_list {env.initial_tenant}') - branches = json.loads(cur.fetchone()[0]) - # Filter out branches created by other tests - branches = [x for x in branches if x['name'].startswith('test_branch_list')] - - assert len(branches) == 1 - assert branches[0]['name'] == 'test_branch_list_main' - assert 'timeline_id' in branches[0] - assert 'latest_valid_lsn' in branches[0] - assert 'ancestor_id' in branches[0] - assert 'ancestor_lsn' in branches[0] - - # Create another branch, and start Postgres on it - env.zenith_cli(['branch', 'test_branch_list_experimental', 'test_branch_list_main']) - env.zenith_cli(['pg', 'create', 'test_branch_list_experimental']) - - cur.execute(f'branch_list {env.initial_tenant}') - new_branches = json.loads(cur.fetchone()[0]) - # Filter out branches created by other tests - new_branches = [x for x in new_branches if x['name'].startswith('test_branch_list')] - assert len(new_branches) == 2 - new_branches.sort(key=lambda k: k['name']) - - assert new_branches[0]['name'] == 'test_branch_list_experimental' - assert new_branches[0]['timeline_id'] != branches[0]['timeline_id'] - - # TODO: do the LSNs have to match here? - assert new_branches[1] == branches[0] - - conn.close() - - -def test_tenant_list_psql(zenith_env_builder: ZenithEnvBuilder): - # don't use zenith_simple_env, because there might be other tenants there, - # left over from other tests. - env = zenith_env_builder.init() - - res = env.zenith_cli(["tenant", "list"]) - res.check_returncode() - tenants = sorted(map(lambda t: t.split()[0], res.stdout.splitlines())) - assert tenants == [env.initial_tenant] - - conn = env.pageserver.connect() - cur = conn.cursor() - - # check same tenant cannot be created twice - with pytest.raises(psycopg2.DatabaseError, - match=f'repo for {env.initial_tenant} already exists'): - cur.execute(f'tenant_create {env.initial_tenant}') - - # create one more tenant - tenant1 = uuid4().hex - cur.execute(f'tenant_create {tenant1}') - - cur.execute('tenant_list') - - # compare tenants list - new_tenants = sorted(map(lambda t: cast(str, t['id']), json.loads(cur.fetchone()[0]))) - assert sorted([env.initial_tenant, tenant1]) == new_tenants - - def check_client(client: ZenithPageserverHttpClient, initial_tenant: str): client.check_status() diff --git a/test_runner/batch_others/test_zenith_cli.py b/test_runner/batch_others/test_zenith_cli.py index d05e803a14..254efc8721 100644 --- a/test_runner/batch_others/test_zenith_cli.py +++ b/test_runner/batch_others/test_zenith_cli.py @@ -3,21 +3,21 @@ import uuid import requests from psycopg2.extensions import cursor as PgCursor -from fixtures.zenith_fixtures import ZenithEnv, ZenithEnvBuilder +from fixtures.zenith_fixtures import ZenithEnv, ZenithEnvBuilder, ZenithPageserverHttpClient from typing import cast pytest_plugins = ("fixtures.zenith_fixtures") -def helper_compare_branch_list(page_server_cur: PgCursor, env: ZenithEnv, initial_tenant: str): +def helper_compare_branch_list(pageserver_http_client: ZenithPageserverHttpClient, + env: ZenithEnv, + initial_tenant: str): """ Compare branches list returned by CLI and directly via API. Filters out branches created by other tests. """ - - page_server_cur.execute(f'branch_list {initial_tenant}') - branches_api = sorted( - map(lambda b: cast(str, b['name']), json.loads(page_server_cur.fetchone()[0]))) + branches = pageserver_http_client.branch_list(uuid.UUID(initial_tenant)) + branches_api = sorted(map(lambda b: cast(str, b['name']), branches)) branches_api = [b for b in branches_api if b.startswith('test_cli_') or b in ('empty', 'main')] res = env.zenith_cli(["branch"]) @@ -38,21 +38,20 @@ def helper_compare_branch_list(page_server_cur: PgCursor, env: ZenithEnv, initia def test_cli_branch_list(zenith_simple_env: ZenithEnv): env = zenith_simple_env - page_server_conn = env.pageserver.connect() - page_server_cur = page_server_conn.cursor() + pageserver_http_client = env.pageserver.http_client() # Initial sanity check - helper_compare_branch_list(page_server_cur, env, env.initial_tenant) + helper_compare_branch_list(pageserver_http_client, env, env.initial_tenant) # Create a branch for us res = env.zenith_cli(["branch", "test_cli_branch_list_main", "empty"]) assert res.stderr == '' - helper_compare_branch_list(page_server_cur, env, env.initial_tenant) + helper_compare_branch_list(pageserver_http_client, env, env.initial_tenant) # Create a nested branch res = env.zenith_cli(["branch", "test_cli_branch_list_nested", "test_cli_branch_list_main"]) assert res.stderr == '' - helper_compare_branch_list(page_server_cur, env, env.initial_tenant) + helper_compare_branch_list(pageserver_http_client, env, env.initial_tenant) # Check that all new branches are visible via CLI res = env.zenith_cli(["branch"]) @@ -63,10 +62,9 @@ def test_cli_branch_list(zenith_simple_env: ZenithEnv): assert 'test_cli_branch_list_nested' in branches_cli -def helper_compare_tenant_list(page_server_cur: PgCursor, env: ZenithEnv): - page_server_cur.execute(f'tenant_list') - tenants_api = sorted( - map(lambda t: cast(str, t['id']), json.loads(page_server_cur.fetchone()[0]))) +def helper_compare_tenant_list(pageserver_http_client: ZenithPageserverHttpClient, env: ZenithEnv): + tenants = pageserver_http_client.tenant_list() + tenants_api = sorted(map(lambda t: cast(str, t['id']), tenants)) res = env.zenith_cli(["tenant", "list"]) assert res.stderr == '' @@ -77,11 +75,9 @@ def helper_compare_tenant_list(page_server_cur: PgCursor, env: ZenithEnv): def test_cli_tenant_list(zenith_simple_env: ZenithEnv): env = zenith_simple_env - page_server_conn = env.pageserver.connect() - page_server_cur = page_server_conn.cursor() - + pageserver_http_client = env.pageserver.http_client() # Initial sanity check - helper_compare_tenant_list(page_server_cur, env) + helper_compare_tenant_list(pageserver_http_client, env) # Create new tenant tenant1 = uuid.uuid4().hex @@ -89,7 +85,7 @@ def test_cli_tenant_list(zenith_simple_env: ZenithEnv): res.check_returncode() # check tenant1 appeared - helper_compare_tenant_list(page_server_cur, env) + helper_compare_tenant_list(pageserver_http_client, env) # Create new tenant tenant2 = uuid.uuid4().hex @@ -97,7 +93,7 @@ def test_cli_tenant_list(zenith_simple_env: ZenithEnv): res.check_returncode() # check tenant2 appeared - helper_compare_tenant_list(page_server_cur, env) + helper_compare_tenant_list(pageserver_http_client, env) res = env.zenith_cli(["tenant", "list"]) res.check_returncode() @@ -107,6 +103,7 @@ def test_cli_tenant_list(zenith_simple_env: ZenithEnv): assert tenant1 in tenants assert tenant2 in tenants + def test_cli_ipv4_listeners(zenith_env_builder: ZenithEnvBuilder): # Start with single sk zenith_env_builder.num_safekeepers = 1 diff --git a/test_runner/fixtures/zenith_fixtures.py b/test_runner/fixtures/zenith_fixtures.py index 142045c04d..5f7b524890 100644 --- a/test_runner/fixtures/zenith_fixtures.py +++ b/test_runner/fixtures/zenith_fixtures.py @@ -1,6 +1,7 @@ from __future__ import annotations from dataclasses import dataclass, field +import textwrap from cached_property import cached_property import asyncpg import os @@ -526,9 +527,9 @@ class ZenithEnv: self.initial_tenant = uuid.uuid4().hex # Create a config file corresponding to the options - toml = f""" -default_tenantid = '{self.initial_tenant}' - """ + toml = textwrap.dedent(f""" + default_tenantid = '{self.initial_tenant}' + """) # Create config for pageserver pageserver_port = PageserverPort( @@ -537,12 +538,12 @@ default_tenantid = '{self.initial_tenant}' ) pageserver_auth_type = "ZenithJWT" if config.pageserver_auth_enabled else "Trust" - toml += f""" -[pageserver] -listen_pg_addr = 'localhost:{pageserver_port.pg}' -listen_http_addr = 'localhost:{pageserver_port.http}' -auth_type = '{pageserver_auth_type}' - """ + toml += textwrap.dedent(f""" + [pageserver] + listen_pg_addr = 'localhost:{pageserver_port.pg}' + listen_http_addr = 'localhost:{pageserver_port.http}' + auth_type = '{pageserver_auth_type}' + """) # Create a corresponding ZenithPageserver object self.pageserver = ZenithPageserver(self, @@ -728,6 +729,10 @@ def zenith_env_builder(test_output_dir, port_distributor) -> Iterator[ZenithEnvB yield builder +class ZenithPageserverApiException(Exception): + pass + + class ZenithPageserverHttpClient(requests.Session): def __init__(self, port: int, auth_token: Optional[str] = None) -> None: super().__init__() @@ -737,22 +742,32 @@ class ZenithPageserverHttpClient(requests.Session): if auth_token is not None: self.headers['Authorization'] = f'Bearer {auth_token}' + def verbose_error(self, res: requests.Response): + try: + res.raise_for_status() + except requests.RequestException as e: + try: + msg = res.json()['msg'] + except: + msg = '' + raise ZenithPageserverApiException(msg) from e + def check_status(self): self.get(f"http://localhost:{self.port}/v1/status").raise_for_status() def timeline_attach(self, tenant_id: uuid.UUID, timeline_id: uuid.UUID): res = self.post( f"http://localhost:{self.port}/v1/timeline/{tenant_id.hex}/{timeline_id.hex}/attach", ) - res.raise_for_status() + self.verbose_error(res) def timeline_detach(self, tenant_id: uuid.UUID, timeline_id: uuid.UUID): res = self.post( f"http://localhost:{self.port}/v1/timeline/{tenant_id.hex}/{timeline_id.hex}/detach", ) - res.raise_for_status() + self.verbose_error(res) def branch_list(self, tenant_id: uuid.UUID) -> List[Dict[Any, Any]]: res = self.get(f"http://localhost:{self.port}/v1/branch/{tenant_id.hex}") - res.raise_for_status() + self.verbose_error(res) res_json = res.json() assert isinstance(res_json, list) return res_json @@ -764,7 +779,7 @@ class ZenithPageserverHttpClient(requests.Session): 'name': name, 'start_point': start_point, }) - res.raise_for_status() + self.verbose_error(res) res_json = res.json() assert isinstance(res_json, dict) return res_json @@ -773,14 +788,14 @@ class ZenithPageserverHttpClient(requests.Session): res = self.get( f"http://localhost:{self.port}/v1/branch/{tenant_id.hex}/{name}?include-non-incremental-logical-size=1", ) - res.raise_for_status() + self.verbose_error(res) res_json = res.json() assert isinstance(res_json, dict) return res_json def tenant_list(self) -> List[Dict[Any, Any]]: res = self.get(f"http://localhost:{self.port}/v1/tenant") - res.raise_for_status() + self.verbose_error(res) res_json = res.json() assert isinstance(res_json, list) return res_json @@ -792,12 +807,12 @@ class ZenithPageserverHttpClient(requests.Session): 'tenant_id': tenant_id.hex, }, ) - res.raise_for_status() + self.verbose_error(res) return res.json() def timeline_list(self, tenant_id: uuid.UUID) -> List[str]: res = self.get(f"http://localhost:{self.port}/v1/timeline/{tenant_id.hex}") - res.raise_for_status() + self.verbose_error(res) res_json = res.json() assert isinstance(res_json, list) return res_json @@ -805,14 +820,14 @@ class ZenithPageserverHttpClient(requests.Session): def timeline_detail(self, tenant_id: uuid.UUID, timeline_id: uuid.UUID): res = self.get( f"http://localhost:{self.port}/v1/timeline/{tenant_id.hex}/{timeline_id.hex}") - res.raise_for_status() + self.verbose_error(res) res_json = res.json() assert isinstance(res_json, dict) return res_json def get_metrics(self) -> str: res = self.get(f"http://localhost:{self.port}/metrics") - res.raise_for_status() + self.verbose_error(res) return res.text