diff --git a/barman/cloud_providers/__init__.py b/barman/cloud_providers/__init__.py index 2a77bae3a..9e2279b52 100644 --- a/barman/cloud_providers/__init__.py +++ b/barman/cloud_providers/__init__.py @@ -307,11 +307,16 @@ def get_snapshot_interface_from_backup_info(backup_info, config=None): elif backup_info.snapshots_info.provider == "aws": from barman.cloud_providers.aws_s3 import AwsCloudSnapshotInterface + # When creating a snapshot interface for existing backups we use the region + # from the backup_info, unless a region is set in the config in which case the + # config region takes precedence. region = None profile = None if config is not None and hasattr(config, "aws_region"): region = config.aws_region profile = config.aws_profile + if region is None: + region = backup_info.snapshots_info.region return AwsCloudSnapshotInterface(profile, region) else: raise CloudProviderUnsupported( diff --git a/doc/manual/28-snapshots.en.md b/doc/manual/28-snapshots.en.md index 629f530c4..493334f96 100644 --- a/doc/manual/28-snapshots.en.md +++ b/doc/manual/28-snapshots.en.md @@ -92,6 +92,7 @@ The following additional prerequisites apply to snapshot backups on AWS: The following permissions are required: - `ec2:CreateSnapshot` +- `ec2:CreateTags` - `ec2:DeleteSnapshot` - `ec2:DescribeSnapshots` - `ec2:DescribeInstances` @@ -147,6 +148,10 @@ azure_resource_group = AZURE_RESOURCE_GROUP #### Configuration for AWS snapshots +When specifying `snapshot_instance` or `snapshot_disks`, Barman will accept either the instance/volume ID which was assigned to the resource by AWS *or* a name. +If a name is used then Barman will query AWS to find resources with a matching `Name` tag. +If zero or multiple matching resources are found then Barman will exit with an error. + The following optional parameters can be set when using AWS: ``` ini diff --git a/tests/test_cloud_snapshot_interface.py b/tests/test_cloud_snapshot_interface.py index 89d129656..7b23f8f0f 100644 --- a/tests/test_cloud_snapshot_interface.py +++ b/tests/test_cloud_snapshot_interface.py @@ -61,6 +61,7 @@ class TestGetSnapshotInterface(object): ("aws", AwsCloudSnapshotInterface), ("azure", AzureCloudSnapshotInterface), ("gcp", GcpCloudSnapshotInterface), + ("unsupportedcloud", None), ], ) @mock.patch("barman.cloud_providers._get_azure_credential") @@ -136,6 +137,7 @@ def test_from_config_azure_no_subscription_id(self): ("aws", AwsCloudSnapshotInterface), ("azure", AzureCloudSnapshotInterface), ("gcp", GcpCloudSnapshotInterface), + ("unsupportedcloud", None), ], ) @mock.patch("barman.cloud_providers.aws_s3.boto3") @@ -205,7 +207,7 @@ def test_from_backup_info_azure_no_subscription_id(self): mock_backup_info = mock.Mock( snapshots_info=mock.Mock(provider="azure", subscription_id=None) ) - # WHEN get snapshot_interface_from_server_config is called + # WHEN get snapshot_interface_from_backup_info is called with pytest.raises(ConfigurationException) as exc: get_snapshot_interface_from_backup_info(mock_backup_info, mock_config) # THEN the expected exception is raised @@ -214,6 +216,46 @@ def test_from_backup_info_azure_no_subscription_id(self): in str(exc.value) ) + @pytest.mark.parametrize( + ("config_region", "backup_info_region", "expected_region"), + ( + # If neither config nor backup_info have a region we expect None + (None, None, None), + # If the config has a region but backup_info does not then we expect the + # region in the config + ("config-region", None, "config-region"), + # If the backup_info has a region but the config does not then we expect + # the region in the backup_info + (None, "backup-info-region", "backup-info-region"), + # If both config and backup_info have a region we expect the region in the + # config + ("config-region", "backup-info-region", "config-region"), + ), + ) + @mock.patch("barman.cloud_providers.aws_s3.boto3") + def test_from_backup_info_aws_region( + self, mock_boto3, config_region, backup_info_region, expected_region + ): + """ + Verify that the region is taken from the backup_info but can be overridden by + the config. + """ + # GIVEN a server config with the aws snapshot provider and the specified region + mock_config = mock.Mock(snapshot_provider="aws", aws_region=config_region) + # AND a backup info with the specified region + mock_backup_info = mock.Mock( + snapshots_info=mock.Mock(provider="aws", region=backup_info_region) + ) + # AND the session has no default region name + mock_boto3.Session.return_value.region_name = None + + # WHEN get snapshot_interface_from_backup_info is called + snapshot_interface = get_snapshot_interface_from_backup_info( + mock_backup_info, mock_config + ) + # THEN the snapshot interface has the expected region + assert snapshot_interface.region == expected_region + @pytest.mark.parametrize( ("cloud_provider", "interface_cls"), [ @@ -223,6 +265,7 @@ def test_from_backup_info_azure_no_subscription_id(self): AzureCloudSnapshotInterface, ), ("google-cloud-storage", GcpCloudSnapshotInterface), + ("unsupportedcloud", None), ], ) @mock.patch("barman.cloud_providers._get_azure_credential") @@ -2051,7 +2094,7 @@ def test_get_attached_volumes_disk_not_found(self): expected_volumes = [d["name"] for d in disks[:-1]] assert set(attached_volumes.keys()) == set(expected_volumes) - def test_get_attached_volumes_disk_not_attaached(self): + def test_get_attached_volumes_disk_not_attached(self): """ Verify that a SnapshotBackupException is raised if a disk is not attached to the instance. @@ -2235,7 +2278,7 @@ class TestAzureVolumeMetadata(object): # If no attachment metadata or disk metadata is passed then we expect init # to succeed but the lun and location to be None. (None, None, None, None), - # If the lun is set in the attachment metadata then we eexpect it to be + # If the lun is set in the attachment metadata then we expect it to be # set in the VolumeMetadata instance. ( mock.Mock(lun="10"), @@ -2243,7 +2286,7 @@ class TestAzureVolumeMetadata(object): "10", None, ), - # If the location is set in the attachment metadata then we eexpect it to + # If the location is set in the attachment metadata then we expect it to # be set in the VolumeMetadata instance. (None, mock.Mock(location="uksouth"), None, "uksouth"), # If lun and location are set in the attachment metadata then we expect diff --git a/tests/test_executor.py b/tests/test_executor.py index ee9f877e9..0f2833717 100644 --- a/tests/test_executor.py +++ b/tests/test_executor.py @@ -1712,7 +1712,7 @@ def mock_resolve_mounted_volume(mock_volume, disk_name, _cmd): mock_resolve_mounted_volume, mock_volume, disk ) - # WHEN find_missing_and_unmounted_disks is called for all expected disksts + # WHEN find_missing_and_unmounted_disks is called for all expected disks instance_name = "instance1" snapshot_disks = ( expected_missing_disks + expected_unmounted_disks + expected_mounted_disks