From f2d94e3cf3b5671018a63a2ae78a6190bc8c6809 Mon Sep 17 00:00:00 2001 From: Suhas Thalanki Date: Fri, 21 Feb 2025 17:18:13 -0500 Subject: [PATCH] fix: removed anon pg extension --- compute/compute-node.Dockerfile | 29 ------ compute_tools/src/compute.rs | 3 +- compute_tools/src/spec.rs | 121 +---------------------- compute_tools/src/spec_apply.rs | 93 ----------------- libs/compute_api/src/spec.rs | 3 - libs/compute_api/tests/cluster_spec.json | 8 -- 6 files changed, 2 insertions(+), 255 deletions(-) diff --git a/compute/compute-node.Dockerfile b/compute/compute-node.Dockerfile index ef4c22612d..f39ebacd12 100644 --- a/compute/compute-node.Dockerfile +++ b/compute/compute-node.Dockerfile @@ -1055,34 +1055,6 @@ RUN if [ -d pg_embedding-src ]; then \ make -j $(getconf _NPROCESSORS_ONLN) install; \ fi -######################################################################################### -# -# Layer "pg_anon-build" -# compile anon extension -# -######################################################################################### -FROM build-deps AS pg_anon-src -ARG PG_VERSION - -# This is an experimental extension, never got to real production. -# !Do not remove! It can be present in shared_preload_libraries and compute will fail to start if library is not found. -WORKDIR /ext-src -RUN case "${PG_VERSION:?}" in "v17") \ - echo "postgresql_anonymizer does not yet support PG17" && exit 0;; \ - esac && \ - wget https://github.com/neondatabase/postgresql_anonymizer/archive/refs/tags/neon_1.1.1.tar.gz -O pg_anon.tar.gz && \ - echo "321ea8d5c1648880aafde850a2c576e4a9e7b9933a34ce272efc839328999fa9 pg_anon.tar.gz" | sha256sum --check && \ - mkdir pg_anon-src && cd pg_anon-src && tar xzf ../pg_anon.tar.gz --strip-components=1 -C . - -FROM pg-build AS pg_anon-build -COPY --from=pg_anon-src /ext-src/ /ext-src/ -WORKDIR /ext-src -RUN if [ -d pg_anon-src ]; then \ - cd pg_anon-src && \ - make -j $(getconf _NPROCESSORS_ONLN) install && \ - echo 'trusted = true' >> /usr/local/pgsql/share/extension/anon.control; \ - fi - ######################################################################################### # # Layer "pg build with nonroot user and cargo installed" @@ -1672,7 +1644,6 @@ COPY --from=pg_roaringbitmap-build /usr/local/pgsql/ /usr/local/pgsql/ COPY --from=pg_semver-build /usr/local/pgsql/ /usr/local/pgsql/ COPY --from=pg_embedding-build /usr/local/pgsql/ /usr/local/pgsql/ COPY --from=wal2json-build /usr/local/pgsql /usr/local/pgsql -COPY --from=pg_anon-build /usr/local/pgsql/ /usr/local/pgsql/ COPY --from=pg_ivm-build /usr/local/pgsql/ /usr/local/pgsql/ COPY --from=pg_partman-build /usr/local/pgsql/ /usr/local/pgsql/ COPY --from=pg_mooncake-build /usr/local/pgsql/ /usr/local/pgsql/ diff --git a/compute_tools/src/compute.rs b/compute_tools/src/compute.rs index d323ea3dcd..a3ea3a147f 100644 --- a/compute_tools/src/compute.rs +++ b/compute_tools/src/compute.rs @@ -47,7 +47,7 @@ use crate::spec_apply::ApplySpecPhase::{ }; use crate::spec_apply::PerDatabasePhase; use crate::spec_apply::PerDatabasePhase::{ - ChangeSchemaPerms, DeleteDBRoleReferences, DropLogicalSubscriptions, HandleAnonExtension, + ChangeSchemaPerms, DeleteDBRoleReferences, DropLogicalSubscriptions, }; use crate::spec_apply::{apply_operations, MutableApplyContext, DB}; use crate::sync_sk::{check_if_synced, ping_safekeeper}; @@ -1123,7 +1123,6 @@ impl ComputeNode { let mut phases = vec![ DeleteDBRoleReferences, ChangeSchemaPerms, - HandleAnonExtension, ]; if spec.drop_subscriptions_before_start && !drop_subscriptions_done { diff --git a/compute_tools/src/spec.rs b/compute_tools/src/spec.rs index 6f28bd9733..59df9bbcef 100644 --- a/compute_tools/src/spec.rs +++ b/compute_tools/src/spec.rs @@ -214,123 +214,4 @@ pub async fn handle_migrations(client: &mut Client) -> Result<()> { .await?; Ok(()) -} - -/// Connect to the database as superuser and pre-create anon extension -/// if it is present in shared_preload_libraries -#[instrument(skip_all)] -pub async fn handle_extension_anon( - spec: &ComputeSpec, - db_owner: &str, - db_client: &mut Client, - grants_only: bool, -) -> Result<()> { - info!("handle extension anon"); - - if let Some(libs) = spec.cluster.settings.find("shared_preload_libraries") { - if libs.contains("anon") { - if !grants_only { - // check if extension is already initialized using anon.is_initialized() - let query = "SELECT anon.is_initialized()"; - match db_client.query(query, &[]).await { - Ok(rows) => { - if !rows.is_empty() { - let is_initialized: bool = rows[0].get(0); - if is_initialized { - info!("anon extension is already initialized"); - return Ok(()); - } - } - } - Err(e) => { - warn!( - "anon extension is_installed check failed with expected error: {}", - e - ); - } - }; - - // Create anon extension if this compute needs it - // Users cannot create it themselves, because superuser is required. - let mut query = "CREATE EXTENSION IF NOT EXISTS anon CASCADE"; - info!("creating anon extension with query: {}", query); - match db_client.query(query, &[]).await { - Ok(_) => {} - Err(e) => { - error!("anon extension creation failed with error: {}", e); - return Ok(()); - } - } - - // check that extension is installed - query = "SELECT extname FROM pg_extension WHERE extname = 'anon'"; - let rows = db_client.query(query, &[]).await?; - if rows.is_empty() { - error!("anon extension is not installed"); - return Ok(()); - } - - // Initialize anon extension - // This also requires superuser privileges, so users cannot do it themselves. - query = "SELECT anon.init()"; - match db_client.query(query, &[]).await { - Ok(_) => {} - Err(e) => { - error!("anon.init() failed with error: {}", e); - return Ok(()); - } - } - } - - // check that extension is installed, if not bail early - let query = "SELECT extname FROM pg_extension WHERE extname = 'anon'"; - match db_client.query(query, &[]).await { - Ok(rows) => { - if rows.is_empty() { - error!("anon extension is not installed"); - return Ok(()); - } - } - Err(e) => { - error!("anon extension check failed with error: {}", e); - return Ok(()); - } - }; - - let query = format!("GRANT ALL ON SCHEMA anon TO {}", db_owner); - info!("granting anon extension permissions with query: {}", query); - db_client.simple_query(&query).await?; - - // Grant permissions to db_owner to use anon extension functions - let query = format!("GRANT ALL ON ALL FUNCTIONS IN SCHEMA anon TO {}", db_owner); - info!("granting anon extension permissions with query: {}", query); - db_client.simple_query(&query).await?; - - // This is needed, because some functions are defined as SECURITY DEFINER. - // In Postgres SECURITY DEFINER functions are executed with the privileges - // of the owner. - // In anon extension this it is needed to access some GUCs, which are only accessible to - // superuser. But we've patched postgres to allow db_owner to access them as well. - // So we need to change owner of these functions to db_owner. - let query = format!(" - SELECT 'ALTER FUNCTION '||nsp.nspname||'.'||p.proname||'('||pg_get_function_identity_arguments(p.oid)||') OWNER TO {};' - from pg_proc p - join pg_namespace nsp ON p.pronamespace = nsp.oid - where nsp.nspname = 'anon';", db_owner); - - info!("change anon extension functions owner to db owner"); - db_client.simple_query(&query).await?; - - // affects views as well - let query = format!("GRANT ALL ON ALL TABLES IN SCHEMA anon TO {}", db_owner); - info!("granting anon extension permissions with query: {}", query); - db_client.simple_query(&query).await?; - - let query = format!("GRANT ALL ON ALL SEQUENCES IN SCHEMA anon TO {}", db_owner); - info!("granting anon extension permissions with query: {}", query); - db_client.simple_query(&query).await?; - } - } - - Ok(()) -} +} \ No newline at end of file diff --git a/compute_tools/src/spec_apply.rs b/compute_tools/src/spec_apply.rs index c4416480d8..1913baab01 100644 --- a/compute_tools/src/spec_apply.rs +++ b/compute_tools/src/spec_apply.rs @@ -46,7 +46,6 @@ impl Debug for DB { pub enum PerDatabasePhase { DeleteDBRoleReferences, ChangeSchemaPerms, - HandleAnonExtension, /// This is a shared phase, used for both i) dropping dangling LR subscriptions /// before dropping the DB, and ii) dropping all subscriptions after creating /// a fresh branch. @@ -588,98 +587,6 @@ async fn get_operations<'a>( ] .into_iter(); - Ok(Box::new(operations)) - } - // TODO: remove this completely https://github.com/neondatabase/cloud/issues/22663 - PerDatabasePhase::HandleAnonExtension => { - // Only install Anon into user databases - let db = match &db { - DB::SystemDB => return Ok(Box::new(empty())), - DB::UserDB(db) => db, - }; - // Never install Anon when it's not enabled as feature - if !spec.features.contains(&ComputeFeature::AnonExtension) { - return Ok(Box::new(empty())); - } - - // Only install Anon when it's added in preload libraries - let opt_libs = spec.cluster.settings.find("shared_preload_libraries"); - - let libs = match opt_libs { - Some(libs) => libs, - None => return Ok(Box::new(empty())), - }; - - if !libs.contains("anon") { - return Ok(Box::new(empty())); - } - - let db_owner = db.owner.pg_quote(); - - let operations = vec![ - // Create anon extension if this compute needs it - // Users cannot create it themselves, because superuser is required. - Operation { - query: String::from("CREATE EXTENSION IF NOT EXISTS anon CASCADE"), - comment: Some(String::from("creating anon extension")), - }, - // Initialize anon extension - // This also requires superuser privileges, so users cannot do it themselves. - Operation { - query: String::from("SELECT anon.init()"), - comment: Some(String::from("initializing anon extension data")), - }, - Operation { - query: format!("GRANT ALL ON SCHEMA anon TO {}", db_owner), - comment: Some(String::from( - "granting anon extension schema permissions", - )), - }, - Operation { - query: format!( - "GRANT ALL ON ALL FUNCTIONS IN SCHEMA anon TO {}", - db_owner - ), - comment: Some(String::from( - "granting anon extension schema functions permissions", - )), - }, - // We need this, because some functions are defined as SECURITY DEFINER. - // In Postgres SECURITY DEFINER functions are executed with the privileges - // of the owner. - // In anon extension this it is needed to access some GUCs, which are only accessible to - // superuser. But we've patched postgres to allow db_owner to access them as well. - // So we need to change owner of these functions to db_owner. - Operation { - query: format!( - include_str!("sql/anon_ext_fn_reassign.sql"), - db_owner = db_owner, - ), - comment: Some(String::from( - "change anon extension functions owner to database_owner", - )), - }, - Operation { - query: format!( - "GRANT ALL ON ALL TABLES IN SCHEMA anon TO {}", - db_owner, - ), - comment: Some(String::from( - "granting anon extension tables permissions", - )), - }, - Operation { - query: format!( - "GRANT ALL ON ALL SEQUENCES IN SCHEMA anon TO {}", - db_owner, - ), - comment: Some(String::from( - "granting anon extension sequences permissions", - )), - }, - ] - .into_iter(); - Ok(Box::new(operations)) } } diff --git a/libs/compute_api/src/spec.rs b/libs/compute_api/src/spec.rs index 8fffae92fb..2e7f501d4b 100644 --- a/libs/compute_api/src/spec.rs +++ b/libs/compute_api/src/spec.rs @@ -156,9 +156,6 @@ pub enum ComputeFeature { /// track short-lived connections as user activity. ActivityMonitorExperimental, - /// Pre-install and initialize anon extension for every database in the cluster - AnonExtension, - /// This is a special feature flag that is used to represent unknown feature flags. /// Basically all unknown to enum flags are represented as this one. See unit test /// `parse_unknown_features()` for more details. diff --git a/libs/compute_api/tests/cluster_spec.json b/libs/compute_api/tests/cluster_spec.json index ccd015ad19..37de24be5b 100644 --- a/libs/compute_api/tests/cluster_spec.json +++ b/libs/compute_api/tests/cluster_spec.json @@ -208,7 +208,6 @@ ], "remote_extensions": { "library_index": { - "anon": "anon", "postgis-3": "postgis", "libpgrouting-3.4": "postgis", "postgis_raster-3": "postgis", @@ -217,12 +216,6 @@ "address_standardizer-3": "postgis" }, "extension_data": { - "anon": { - "archive_path": "5834329303/v15/extensions/anon.tar.zst", - "control_data": { - "anon.control": "# PostgreSQL Anonymizer (anon) extension\ncomment = ''Data anonymization tools''\ndefault_version = ''1.1.0''\ndirectory=''extension/anon''\nrelocatable = false\nrequires = ''pgcrypto''\nsuperuser = false\nmodule_pathname = ''$libdir/anon''\ntrusted = true\n" - } - }, "postgis": { "archive_path": "5834329303/v15/extensions/postgis.tar.zst", "control_data": { @@ -238,7 +231,6 @@ } }, "custom_extensions": [ - "anon" ], "public_extensions": [ "postgis"