diff --git a/docs/providers/snowflake.md b/docs/providers/snowflake.md index 8cb5a45..c54ba5e 100644 --- a/docs/providers/snowflake.md +++ b/docs/providers/snowflake.md @@ -171,7 +171,7 @@ GRANT CONNECT ON DATABASE your_database TO planetscale_discovery; GRANT pg_read_all_stats, pg_read_all_settings, pg_monitor TO planetscale_discovery; ``` -This login cannot read table rows. Discovery still reports the same tables, indexes, extensions, and settings as `snowflake_admin`. The one difference is per-column `n_distinct` / `correlation`, which Postgres only shows for tables the user can `SELECT`. Drop the login after the run: +This login cannot read table rows. Discovery still reports the same tables, indexes, extensions, and settings as `snowflake_admin`. The one difference is per-column `n_distinct` / `null_frac` / `correlation`, which Postgres only shows for tables the user can `SELECT`. Drop the login after the run: ```sql REVOKE pg_read_all_stats, pg_read_all_settings, pg_monitor FROM planetscale_discovery; @@ -311,7 +311,7 @@ Use `name` for the instance display name. Use `host` for the connection endpoint - Table contents or row data - Application code - Passwords, password hashes, private keys, or other credentials (used for the connection, never written to the report). Postgres roles appear only as names with a `has_password` flag -- Column values. Column statistics are limited to `n_distinct` and `correlation` +- Column values. Column statistics are limited to `n_distinct`, `null_frac` and `correlation` The report does include hostnames, instance names, the account locator, Postgres role names, and table, column, and index names. diff --git a/planetscale_discovery/cloud/analyzers/aws_analyzer.py b/planetscale_discovery/cloud/analyzers/aws_analyzer.py index ec10fce..a6e0f8a 100644 --- a/planetscale_discovery/cloud/analyzers/aws_analyzer.py +++ b/planetscale_discovery/cloud/analyzers/aws_analyzer.py @@ -48,6 +48,40 @@ ) +def _rds_instance_replication(db_instance: Dict[str, Any]) -> Dict[str, Any]: + return { + "read_replica_source_db_instance_identifier": db_instance.get( + "ReadReplicaSourceDBInstanceIdentifier" + ), + "read_replica_db_instance_identifiers": db_instance.get( + "ReadReplicaDBInstanceIdentifiers", [] + ), + "read_replica_source_db_cluster_identifier": db_instance.get( + "ReadReplicaSourceDBClusterIdentifier" + ), + "read_replica_db_cluster_identifiers": db_instance.get( + "ReadReplicaDBClusterIdentifiers", [] + ), + "replica_mode": db_instance.get("ReplicaMode"), + } + + +def _rds_instance_billing(db_instance: Dict[str, Any]) -> Dict[str, Any]: + return { + "storage_throughput": db_instance.get("StorageThroughput"), + "max_allocated_storage": db_instance.get("MaxAllocatedStorage"), + "dedicated_log_volume": db_instance.get("DedicatedLogVolume", False), + "engine_lifecycle_support": db_instance.get("EngineLifecycleSupport"), + } + + +def _aurora_cluster_replication(db_cluster: Dict[str, Any]) -> Dict[str, Any]: + return { + "replication_source_identifier": db_cluster.get("ReplicationSourceIdentifier"), + "read_replica_identifiers": db_cluster.get("ReadReplicaIdentifiers", []), + } + + class AWSAnalyzer(CloudAnalyzer): """AWS cloud database environment analyzer.""" @@ -470,6 +504,8 @@ def _analyze_rds_instances(self, region: str) -> List[Dict[str, Any]]: "region": region, "db_cluster_identifier": db_instance.get("DBClusterIdentifier"), "dbi_resource_id": db_instance.get("DbiResourceId"), + **_rds_instance_replication(db_instance), + **_rds_instance_billing(db_instance), } instances.append(instance_analysis) @@ -532,6 +568,7 @@ def _analyze_aurora_clusters(self, region: str) -> List[Dict[str, Any]]: ), "backtrack_window": db_cluster.get("BacktrackWindow", 0), "region": region, + **_aurora_cluster_replication(db_cluster), } # Get cluster members @@ -2058,6 +2095,7 @@ def _analyze_single_rds_instance( "db_instance_class": instance_data.get("DBInstanceClass"), "allocated_storage": instance_data.get("AllocatedStorage"), "storage_type": instance_data.get("StorageType"), + "iops": instance_data.get("Iops"), "storage_encrypted": instance_data.get("StorageEncrypted", False), "kms_key_id": instance_data.get("KmsKeyId"), "multi_az": instance_data.get("MultiAZ", False), @@ -2092,6 +2130,8 @@ def _analyze_single_rds_instance( "monitoring_interval": instance_data.get("MonitoringInterval", 0), "db_cluster_identifier": instance_data.get("DBClusterIdentifier"), "dbi_resource_id": instance_data.get("DbiResourceId"), + **_rds_instance_replication(instance_data), + **_rds_instance_billing(instance_data), } self._collect_region_metrics(region, [analysis], []) @@ -2144,6 +2184,7 @@ def _analyze_single_aurora_cluster( } for member in cluster_data.get("DBClusterMembers", []) ], + **_aurora_cluster_replication(cluster_data), } if analysis["engine_mode"] == "serverless" or cluster_data.get( diff --git a/planetscale_discovery/database/analyzers/schema_analyzer.py b/planetscale_discovery/database/analyzers/schema_analyzer.py index fb3e9c4..39ea41f 100644 --- a/planetscale_discovery/database/analyzers/schema_analyzer.py +++ b/planetscale_discovery/database/analyzers/schema_analyzer.py @@ -320,7 +320,11 @@ def _get_table_analysis(self) -> List[Dict[str, Any]]: for row in rows: table_info = dict(row) - # Get column information + # Get column information. + # default_sequence is the sequence that the column default + # calls when the default is one nextval() call and nothing + # else. The default text stays in the database: the query + # selects only the sequence name, from pg_depend. try: with self.connection.cursor() as detail_cursor: detail_cursor.execute( @@ -348,7 +352,18 @@ def _get_table_analysis(self) -> List[Dict[str, Any]]: AND dep.refobjsubid = a.attnum AND dep.deptype IN ('a', 'i') LIMIT 1 - ) as owned_sequence + ) as owned_sequence, + ( + SELECT dep.refobjid::regclass::text + FROM pg_depend dep + JOIN pg_class seq ON seq.oid = dep.refobjid AND seq.relkind = 'S' + WHERE dep.classid = 'pg_attrdef'::regclass + AND dep.objid = d.oid + AND dep.refclassid = 'pg_class'::regclass + AND pg_get_expr(d.adbin, d.adrelid) + ~ '^nextval\\(''(?:[^'']|'''')+''::regclass\\)$' + LIMIT 1 + ) as default_sequence FROM pg_attribute a LEFT JOIN pg_attrdef d ON (a.attrelid = d.adrelid AND a.attnum = d.adnum) WHERE a.attrelid = %s @@ -378,6 +393,7 @@ def _get_table_analysis(self) -> List[Dict[str, Any]]: tablename, attname, n_distinct, + null_frac, correlation FROM pg_stats WHERE schemaname = %s AND tablename = %s diff --git a/tests/unit/test_aws_rds_billing_fields.py b/tests/unit/test_aws_rds_billing_fields.py new file mode 100644 index 0000000..04d5bd8 --- /dev/null +++ b/tests/unit/test_aws_rds_billing_fields.py @@ -0,0 +1,70 @@ +import logging + +import pytest + +from planetscale_discovery.cloud.analyzers.aws_analyzer import AWSAnalyzer +from planetscale_discovery.config.config_manager import AWSConfig + +REGION = "us-east-1" + +BILLED_INSTANCE = { + "DBInstanceIdentifier": "app-primary", + "Engine": "postgres", + "EngineVersion": "13.20", + "DBInstanceClass": "db.r6g.4xlarge", + "AllocatedStorage": 2000, + "StorageType": "gp3", + "Iops": 12000, + "StorageThroughput": 500, + "MaxAllocatedStorage": 4000, + "DedicatedLogVolume": True, + "EngineLifecycleSupport": "open-source-rds-extended-support", +} + +UNBILLED_INSTANCE = { + "DBInstanceIdentifier": "app-small", + "Engine": "postgres", + "EngineVersion": "17.4", + "DBInstanceClass": "db.t3.micro", +} + + +def _analyzer(mocker) -> AWSAnalyzer: + analyzer = AWSAnalyzer( + AWSConfig(enabled=True, regions=[REGION]), logging.getLogger("t") + ) + analyzer.session = mocker.Mock() + mocker.patch.object(analyzer, "_collect_region_metrics") + return analyzer + + +def _full_path(analyzer, mocker, instance): + mocker.patch.object(analyzer, "_fetch_rds_db_instances", return_value=[instance]) + [analysis] = analyzer._analyze_rds_instances(REGION) + return analysis + + +def _focused_path(analyzer, mocker, instance): + return analyzer._analyze_single_rds_instance(instance, REGION) + + +@pytest.mark.parametrize("path", [_full_path, _focused_path]) +class TestRdsBillingFields: + def test_billed_storage_and_support_fields_are_kept(self, mocker, path): + analysis = path(_analyzer(mocker), mocker, BILLED_INSTANCE) + + assert analysis["iops"] == 12000 + assert analysis["storage_throughput"] == 500 + assert analysis["max_allocated_storage"] == 4000 + assert analysis["dedicated_log_volume"] is True + assert analysis["engine_lifecycle_support"] == ( + "open-source-rds-extended-support" + ) + + def test_absent_fields_stay_unset(self, mocker, path): + analysis = path(_analyzer(mocker), mocker, UNBILLED_INSTANCE) + + assert analysis["storage_throughput"] is None + assert analysis["max_allocated_storage"] is None + assert analysis["dedicated_log_volume"] is False + assert analysis["engine_lifecycle_support"] is None diff --git a/tests/unit/test_aws_rds_replicas.py b/tests/unit/test_aws_rds_replicas.py new file mode 100644 index 0000000..9dc1758 --- /dev/null +++ b/tests/unit/test_aws_rds_replicas.py @@ -0,0 +1,144 @@ +import logging + +import boto3 +import pytest +from moto import mock_aws + +from planetscale_discovery.cloud.analyzers.aws_analyzer import AWSAnalyzer +from planetscale_discovery.config.config_manager import AWSConfig + +REGION = "us-east-1" +PW = "password123" + + +@pytest.fixture +def aws_env(aws_credentials): + with mock_aws(): + yield boto3.client("rds", region_name=REGION) + + +def _analyzer() -> AWSAnalyzer: + cfg = AWSConfig(enabled=True, regions=[REGION], discover_all=True) + analyzer = AWSAnalyzer(cfg, logging.getLogger("rds-replica-test")) + assert analyzer.authenticate() is True + return analyzer + + +class TestRdsReadReplicaLinks: + def test_primary_and_replica_name_each_other(self, aws_env): + aws_env.create_db_instance( + DBInstanceIdentifier="app-primary", + DBInstanceClass="db.r6g.large", + Engine="postgres", + MasterUsername="admin", + MasterUserPassword=PW, + AllocatedStorage=100, + ) + aws_env.create_db_instance_read_replica( + DBInstanceIdentifier="app-replica", + SourceDBInstanceIdentifier="app-primary", + ) + + result = _analyzer().analyze()["resources"][REGION] + by_id = {i["db_instance_identifier"]: i for i in result["rds_instances"]} + + primary = by_id["app-primary"] + replica = by_id["app-replica"] + assert primary["read_replica_db_instance_identifiers"] == ["app-replica"] + assert primary["read_replica_source_db_instance_identifier"] is None + assert replica["read_replica_source_db_instance_identifier"] == "app-primary" + assert replica["read_replica_db_instance_identifiers"] == [] + + def test_standalone_instance_has_empty_links(self, aws_env): + aws_env.create_db_instance( + DBInstanceIdentifier="solo", + DBInstanceClass="db.t3.micro", + Engine="postgres", + MasterUsername="admin", + MasterUserPassword=PW, + AllocatedStorage=20, + ) + + [solo] = _analyzer().analyze()["resources"][REGION]["rds_instances"] + + assert solo["read_replica_source_db_instance_identifier"] is None + assert solo["read_replica_db_instance_identifiers"] == [] + assert solo["read_replica_source_db_cluster_identifier"] is None + assert solo["read_replica_db_cluster_identifiers"] == [] + assert solo["replica_mode"] is None + + +class TestAuroraClusterReplicaLinks: + def test_replica_cluster_names_its_source(self, aws_env): + west = boto3.client("rds", region_name="us-west-2") + source_arn = west.create_db_cluster( + DBClusterIdentifier="app-cluster", + Engine="aurora-postgresql", + MasterUsername="admin", + MasterUserPassword=PW, + )["DBCluster"]["DBClusterArn"] + aws_env.create_db_cluster( + DBClusterIdentifier="app-cluster-east", + Engine="aurora-postgresql", + MasterUsername="admin", + MasterUserPassword=PW, + ReplicationSourceIdentifier=source_arn, + ) + aws_env.create_db_cluster( + DBClusterIdentifier="app-cluster-solo", + Engine="aurora-postgresql", + MasterUsername="admin", + MasterUserPassword=PW, + ) + + result = _analyzer().analyze()["resources"][REGION] + by_id = {c["identifier"]: c for c in result["aurora_clusters"]} + + assert by_id["app-cluster-east"]["replication_source_identifier"] == source_arn + assert by_id["app-cluster-solo"]["replication_source_identifier"] is None + assert by_id["app-cluster-solo"]["read_replica_identifiers"] == [] + + +class TestFocusedPathReplicaLinks: + def test_cross_region_replica_keeps_the_source_arn(self, mocker): + analyzer = AWSAnalyzer( + AWSConfig(enabled=True, regions=[REGION]), logging.getLogger("t") + ) + analyzer.session = mocker.Mock() + mocker.patch.object(analyzer, "_collect_region_metrics") + source_arn = "arn:aws:rds:us-west-2:123456789012:db:app-primary" + + analysis = analyzer._analyze_single_rds_instance( + { + "DBInstanceIdentifier": "app-replica-east", + "Engine": "postgres", + "DBInstanceClass": "db.r6g.large", + "ReadReplicaSourceDBInstanceIdentifier": source_arn, + "ReplicaMode": "open-read-only", + }, + REGION, + ) + + assert analysis["read_replica_source_db_instance_identifier"] == source_arn + assert analysis["replica_mode"] == "open-read-only" + assert analysis["read_replica_db_instance_identifiers"] == [] + + def test_aurora_replica_cluster_names_its_source(self, mocker): + analyzer = AWSAnalyzer( + AWSConfig(enabled=True, regions=[REGION]), logging.getLogger("t") + ) + mocker.patch.object(analyzer, "_collect_region_metrics") + source_arn = "arn:aws:rds:us-west-2:123456789012:cluster:app-cluster" + + analysis = analyzer._analyze_single_aurora_cluster( + { + "DBClusterIdentifier": "app-cluster-east", + "Engine": "aurora-postgresql", + "EngineMode": "provisioned", + "ReplicationSourceIdentifier": source_arn, + }, + REGION, + ) + + assert analysis["replication_source_identifier"] == source_arn + assert analysis["read_replica_identifiers"] == [] diff --git a/tests/unit/test_schema_analyzer.py b/tests/unit/test_schema_analyzer.py index d61c99e..03f4a34 100644 --- a/tests/unit/test_schema_analyzer.py +++ b/tests/unit/test_schema_analyzer.py @@ -540,6 +540,49 @@ def cursor_factory(): assert result[0]["columns"][0]["column_name"] == "id" assert len(result[0]["statistics"]) == 1 + def test_get_table_analysis_statistics_select_null_frac(self, mock_connection): + """The pg_stats query selects null_frac. + + Without it a column with no NULLs reads the same as one whose NULL + share was never measured. + """ + connection, _ = mock_connection + version_cursor, main_cursor, detail_cursor = ( + MagicMock(), + MagicMock(), + MagicMock(), + ) + cursors = iter([version_cursor, main_cursor]) + + def cursor_factory(): + ctx = MagicMock() + ctx.__enter__ = MagicMock(return_value=next(cursors, detail_cursor)) + ctx.__exit__ = MagicMock(return_value=None) + return ctx + + connection.cursor.side_effect = cursor_factory + version_cursor.fetchone.return_value = {"server_version_num": "140011"} + main_cursor.fetchall.return_value = [ + { + "schema_name": "public", + "table_name": "users", + "table_oid": 16385, + "table_type": "r", + "is_partition": False, + } + ] + detail_cursor.fetchall.return_value = [] + + SchemaAnalyzer(connection)._get_table_analysis() + + stats_sql = [ + c.args[0] + for c in detail_cursor.execute.call_args_list + if "FROM pg_stats" in c.args[0] + ] + assert len(stats_sql) == 1 + assert "null_frac" in stats_sql[0] + def test_get_table_analysis_empty(self, mock_connection): """Test table analysis with no tables returns empty list""" connection, _ = mock_connection @@ -794,6 +837,35 @@ def test_get_table_analysis_column_identity_and_generated( for column in absent: assert column not in column_sql + def test_get_table_analysis_column_default_sequence( + self, analyzer, mock_connection + ): + """Column query names the sequence a nextval() default calls, never the default text""" + _, cursor = mock_connection + cursor.fetchall.return_value = [ + {"table_oid": 16385, "table_name": "users", "table_type": "r"} + ] + + with patch.object(analyzer, "_get_server_version_num", return_value=170000): + analyzer._get_table_analysis() + + column_sql = next( + c.args[0] + for c in cursor.execute.call_args_list + if "FROM pg_attribute a" in c.args[0] + ) + assert "as default_sequence" in column_sql + # The sequence comes from pg_depend, through the column's pg_attrdef row. + assert "dep.classid = 'pg_attrdef'::regclass" in column_sql + assert "dep.objid = d.oid" in column_sql + # Only a default that is one nextval() call names a sequence. + assert r"~ '^nextval\(''(?:[^'']|'''')+''::regclass\)$'" in column_sql + # The default text appears only in that filter. It is never a + # selected column, so it cannot reach the report. + assert column_sql.count("pg_get_expr") == 1 + assert "AND pg_get_expr(d.adbin, d.adrelid)" in column_sql + assert "adsrc" not in column_sql + # --------------------------------------------------------------- # _get_index_analysis() # ---------------------------------------------------------------