From d8b124fbf4a641508cb54ecc77c53c0b2cf8e622 Mon Sep 17 00:00:00 2001 From: captainkerk Date: Sun, 28 Jan 2018 03:06:57 +0000 Subject: [PATCH 1/4] added: enable/disable/modify redshift snapshot copy methods --- moto/redshift/exceptions.py | 19 ++++++++ moto/redshift/models.py | 47 +++++++++++++++++- moto/redshift/responses.py | 55 +++++++++++++++++++++ tests/test_redshift/test_redshift.py | 72 ++++++++++++++++++++++++++++ 4 files changed, 191 insertions(+), 2 deletions(-) diff --git a/moto/redshift/exceptions.py b/moto/redshift/exceptions.py index a89ed5a0..138afd44 100644 --- a/moto/redshift/exceptions.py +++ b/moto/redshift/exceptions.py @@ -93,3 +93,22 @@ class ResourceNotFoundFaultError(RedshiftClientError): msg = message super(ResourceNotFoundFaultError, self).__init__( 'ResourceNotFoundFault', msg) + + +class SnapshotCopyDisabledFaultError(RedshiftClientError): + def __init__(self, cluster_identifier): + super(SnapshotCopyDisabledFaultError, self).__init__( + 'SnapshotCopyDisabledFault', + "Cannot modify retention period because snapshot copy is disabled on Cluster {0}.".format(cluster_identifier)) + +class SnapshotCopyAlreadyDisabledFaultError(RedshiftClientError): + def __init__(self, cluster_identifier): + super(SnapshotCopyAlreadyDisabledFaultError, self).__init__( + 'SnapshotCopyAlreadyDisabledFault', + "Snapshot Copy is already disabled on Cluster {0}.".format(cluster_identifier)) + +class SnapshotCopyAlreadyEnabledFaultError(RedshiftClientError): + def __init__(self, cluster_identifier): + super(SnapshotCopyAlreadyEnabledFaultError, self).__init__( + 'SnapshotCopyAlreadyEnabledFault', + "Snapshot Copy is already enabled on Cluster {0}.".format(cluster_identifier)) diff --git a/moto/redshift/models.py b/moto/redshift/models.py index fa642ef0..2bab77f6 100644 --- a/moto/redshift/models.py +++ b/moto/redshift/models.py @@ -17,7 +17,10 @@ from .exceptions import ( ClusterSubnetGroupNotFoundError, InvalidParameterValueError, InvalidSubnetError, - ResourceNotFoundFaultError + ResourceNotFoundFaultError, + SnapshotCopyDisabledFaultError, + SnapshotCopyAlreadyDisabledFaultError, + SnapshotCopyAlreadyEnabledFaultError, ) @@ -80,6 +83,7 @@ class Cluster(TaggableResourceMixin, BaseModel): self.cluster_subnet_group_name = cluster_subnet_group_name self.publicly_accessible = publicly_accessible self.encrypted = encrypted + self.cluster_snapshot_copy_status = {} self.allow_version_upgrade = allow_version_upgrade if allow_version_upgrade is not None else True self.cluster_version = cluster_version if cluster_version else "1.0" @@ -194,7 +198,7 @@ class Cluster(TaggableResourceMixin, BaseModel): return self.cluster_identifier def to_json(self): - return { + json_response = { "MasterUsername": self.master_username, "MasterUserPassword": "****", "ClusterVersion": self.cluster_version, @@ -223,6 +227,7 @@ class Cluster(TaggableResourceMixin, BaseModel): "NodeType": self.node_type, "ClusterIdentifier": self.cluster_identifier, "AllowVersionUpgrade": self.allow_version_upgrade, + "Endpoint": { "Address": self.endpoint, "Port": self.port @@ -231,6 +236,10 @@ class Cluster(TaggableResourceMixin, BaseModel): "Tags": self.tags } + if self.cluster_snapshot_copy_status: + json_response['ClusterSnapshotCopyStatus'] = self.cluster_snapshot_copy_status + return json_response + class SubnetGroup(TaggableResourceMixin, BaseModel): @@ -417,6 +426,40 @@ class RedshiftBackend(BaseBackend): self.__dict__ = {} self.__init__(ec2_backend, region_name) + def enable_snapshot_copy(self, **kwargs): + cluster_identifier = kwargs['cluster_identifier'] + cluster = self.clusters[cluster_identifier] + if not cluster.cluster_snapshot_copy_status: + status = { + 'DestinationRegion': kwargs['destination_region'], + 'RetentionPeriod': kwargs['retention_period'], + 'SnapshotCopyGrantName': kwargs['snapshot_copy_grant_name'], + } + cluster.cluster_snapshot_copy_status = status + return cluster + + else: + raise SnapshotCopyAlreadyEnabledFaultError(cluster_identifier) + + + def disable_snapshot_copy(self, **kwargs): + cluster_identifier = kwargs['cluster_identifier'] + cluster = self.clusters[cluster_identifier] + if cluster.cluster_snapshot_copy_status: + cluster.cluster_snapshot_copy_status = {} + else: + raise SnapshotCopyAlreadyDisabledFaultError(cluster_identifier) + return cluster + + + def modify_snapshot_copy_retention_period(self, cluster_identifier, retention_period): + cluster = self.clusters[cluster_identifier] + if cluster.cluster_snapshot_copy_status: + cluster.cluster_snapshot_copy_status['RetentionPeriod'] = retention_period + else: + raise SnapshotCopyDisabledFaultError(cluster_identifier) + return cluster + def create_cluster(self, **cluster_kwargs): cluster_identifier = cluster_kwargs['cluster_identifier'] cluster = Cluster(self, **cluster_kwargs) diff --git a/moto/redshift/responses.py b/moto/redshift/responses.py index a320f9ca..bd7223c8 100644 --- a/moto/redshift/responses.py +++ b/moto/redshift/responses.py @@ -501,3 +501,58 @@ class RedshiftResponse(BaseResponse): } } }) + + def enable_snapshot_copy(self): + snapshot_copy_kwargs = { + 'cluster_identifier': self._get_param('ClusterIdentifier'), + 'destination_region': self._get_param('DestinationRegion'), + 'retention_period': self._get_param('RetentionPeriod'), + 'snapshot_copy_grant_name': self._get_param('SnapshotCopyGrantName'), + } + cluster = self.redshift_backend.enable_snapshot_copy(**snapshot_copy_kwargs) + + return self.get_response({ + "EnableSnapshotCopyResponse": { + "EnableSnapshotCopyResult": { + "Cluster": cluster.to_json() + }, + "ResponseMetadata": { + "RequestId": "384ac68d-3775-11df-8963-01868b7c937a", + } + } + }) + + def disable_snapshot_copy(self): + snapshot_copy_kwargs = { + 'cluster_identifier': self._get_param('ClusterIdentifier'), + } + cluster = self.redshift_backend.disable_snapshot_copy(**snapshot_copy_kwargs) + + return self.get_response({ + "DisableSnapshotCopyResponse": { + "DisableSnapshotCopyResult": { + "Cluster": cluster.to_json() + }, + "ResponseMetadata": { + "RequestId": "384ac68d-3775-11df-8963-01868b7c937a", + } + } + }) + + def modify_snapshot_copy_retention_period(self): + snapshot_copy_kwargs = { + 'cluster_identifier': self._get_param('ClusterIdentifier'), + 'retention_period': self._get_param('RetentionPeriod'), + } + cluster = self.redshift_backend.modify_snapshot_copy_retention_period(**snapshot_copy_kwargs) + + return self.get_response({ + "ModifySnapshotCopyRetentionPeriodResponse": { + "ModifySnapshotCopyRetentionPeriodResult": { + "Clusters": [cluster.to_json()] + }, + "ResponseMetadata": { + "RequestId": "384ac68d-3775-11df-8963-01868b7c937a", + } + } + }) diff --git a/tests/test_redshift/test_redshift.py b/tests/test_redshift/test_redshift.py index cebaa3ec..46400d34 100644 --- a/tests/test_redshift/test_redshift.py +++ b/tests/test_redshift/test_redshift.py @@ -1042,3 +1042,75 @@ def test_tagged_resource_not_found_error(): ResourceName='bad:arn' ).should.throw(ClientError, "Tagging is not supported for this type of resource") + +@mock_redshift +def test_enable_snapshot_copy(): + client = boto3.client('redshift', region_name='us-east-1') + client.create_cluster( + DBName='test', + ClusterIdentifier='test', + ClusterType='single-node', + NodeType='ds2.xlarge', + MasterUsername='user', + MasterUserPassword='password', + ) + client.enable_snapshot_copy( + ClusterIdentifier='test', + DestinationRegion='us-west-2', + RetentionPeriod=3, + SnapshotCopyGrantName='copy-us-east-1-to-us-west-2' + ) + response = client.describe_clusters(ClusterIdentifier='test') + cluster_snapshot_copy_status = response['Clusters'][0]['ClusterSnapshotCopyStatus'] + cluster_snapshot_copy_status['RetentionPeriod'].should.equal(3) + cluster_snapshot_copy_status['DestinationRegion'].should.equal('us-west-2') + cluster_snapshot_copy_status['SnapshotCopyGrantName'].should.equal('copy-us-east-1-to-us-west-2') + + +@mock_redshift +def test_disable_snapshot_copy(): + client = boto3.client('redshift', region_name='us-east-1') + client.create_cluster( + DBName='test', + ClusterIdentifier='test', + ClusterType='single-node', + NodeType='ds2.xlarge', + MasterUsername='user', + MasterUserPassword='password', + ) + client.enable_snapshot_copy( + ClusterIdentifier='test', + DestinationRegion='us-west-2', + RetentionPeriod=3, + SnapshotCopyGrantName='copy-us-east-1-to-us-west-2', + ) + client.disable_snapshot_copy( + ClusterIdentifier='test', + ) + response = client.describe_clusters(ClusterIdentifier='test') + response['Clusters'][0].shouldnt.contain('ClusterSnapshotCopyStatus') + +@mock_redshift +def test_modify_snapshot_copy_retention_period(): + client = boto3.client('redshift', region_name='us-east-1') + client.create_cluster( + DBName='test', + ClusterIdentifier='test', + ClusterType='single-node', + NodeType='ds2.xlarge', + MasterUsername='user', + MasterUserPassword='password', + ) + client.enable_snapshot_copy( + ClusterIdentifier='test', + DestinationRegion='us-west-2', + RetentionPeriod=3, + SnapshotCopyGrantName='copy-us-east-1-to-us-west-2', + ) + client.modify_snapshot_copy_retention_period( + ClusterIdentifier='test', + RetentionPeriod=5, + ) + response = client.describe_clusters(ClusterIdentifier='test') + cluster_snapshot_copy_status = response['Clusters'][0]['ClusterSnapshotCopyStatus'] + cluster_snapshot_copy_status['RetentionPeriod'].should.equal(5) From 92798b9a9fef52f5a79d5cd0b3ea48d2310d2ee7 Mon Sep 17 00:00:00 2001 From: captainkerk Date: Sun, 28 Jan 2018 03:27:06 +0000 Subject: [PATCH 2/4] improve error handling --- moto/redshift/models.py | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/moto/redshift/models.py b/moto/redshift/models.py index 2bab77f6..4975868d 100644 --- a/moto/redshift/models.py +++ b/moto/redshift/models.py @@ -83,7 +83,6 @@ class Cluster(TaggableResourceMixin, BaseModel): self.cluster_subnet_group_name = cluster_subnet_group_name self.publicly_accessible = publicly_accessible self.encrypted = encrypted - self.cluster_snapshot_copy_status = {} self.allow_version_upgrade = allow_version_upgrade if allow_version_upgrade is not None else True self.cluster_version = cluster_version if cluster_version else "1.0" @@ -236,8 +235,10 @@ class Cluster(TaggableResourceMixin, BaseModel): "Tags": self.tags } - if self.cluster_snapshot_copy_status: + try: json_response['ClusterSnapshotCopyStatus'] = self.cluster_snapshot_copy_status + except AttributeError: + pass return json_response @@ -429,7 +430,7 @@ class RedshiftBackend(BaseBackend): def enable_snapshot_copy(self, **kwargs): cluster_identifier = kwargs['cluster_identifier'] cluster = self.clusters[cluster_identifier] - if not cluster.cluster_snapshot_copy_status: + if not hasattr(cluster, 'cluster_snapshot_copy_status'): status = { 'DestinationRegion': kwargs['destination_region'], 'RetentionPeriod': kwargs['retention_period'], @@ -437,28 +438,25 @@ class RedshiftBackend(BaseBackend): } cluster.cluster_snapshot_copy_status = status return cluster - else: raise SnapshotCopyAlreadyEnabledFaultError(cluster_identifier) - def disable_snapshot_copy(self, **kwargs): cluster_identifier = kwargs['cluster_identifier'] cluster = self.clusters[cluster_identifier] - if cluster.cluster_snapshot_copy_status: - cluster.cluster_snapshot_copy_status = {} + if hasattr(cluster, 'cluster_snapshot_copy_status'): + del cluster.cluster_snapshot_copy_status + return cluster else: raise SnapshotCopyAlreadyDisabledFaultError(cluster_identifier) - return cluster - def modify_snapshot_copy_retention_period(self, cluster_identifier, retention_period): cluster = self.clusters[cluster_identifier] - if cluster.cluster_snapshot_copy_status: + if hasattr(cluster, 'cluster_snapshot_copy_status'): cluster.cluster_snapshot_copy_status['RetentionPeriod'] = retention_period + return cluster else: raise SnapshotCopyDisabledFaultError(cluster_identifier) - return cluster def create_cluster(self, **cluster_kwargs): cluster_identifier = cluster_kwargs['cluster_identifier'] From ed066582714d071e2db78ac96f3bd014a8b28a4a Mon Sep 17 00:00:00 2001 From: captainkerk Date: Sun, 28 Jan 2018 03:28:49 +0000 Subject: [PATCH 3/4] address spacing issues --- moto/redshift/exceptions.py | 2 ++ moto/redshift/models.py | 1 - moto/redshift/responses.py | 2 +- tests/test_redshift/test_redshift.py | 1 + 4 files changed, 4 insertions(+), 2 deletions(-) diff --git a/moto/redshift/exceptions.py b/moto/redshift/exceptions.py index 138afd44..865aaeab 100644 --- a/moto/redshift/exceptions.py +++ b/moto/redshift/exceptions.py @@ -101,12 +101,14 @@ class SnapshotCopyDisabledFaultError(RedshiftClientError): 'SnapshotCopyDisabledFault', "Cannot modify retention period because snapshot copy is disabled on Cluster {0}.".format(cluster_identifier)) + class SnapshotCopyAlreadyDisabledFaultError(RedshiftClientError): def __init__(self, cluster_identifier): super(SnapshotCopyAlreadyDisabledFaultError, self).__init__( 'SnapshotCopyAlreadyDisabledFault', "Snapshot Copy is already disabled on Cluster {0}.".format(cluster_identifier)) + class SnapshotCopyAlreadyEnabledFaultError(RedshiftClientError): def __init__(self, cluster_identifier): super(SnapshotCopyAlreadyEnabledFaultError, self).__init__( diff --git a/moto/redshift/models.py b/moto/redshift/models.py index 4975868d..44e944c3 100644 --- a/moto/redshift/models.py +++ b/moto/redshift/models.py @@ -226,7 +226,6 @@ class Cluster(TaggableResourceMixin, BaseModel): "NodeType": self.node_type, "ClusterIdentifier": self.cluster_identifier, "AllowVersionUpgrade": self.allow_version_upgrade, - "Endpoint": { "Address": self.endpoint, "Port": self.port diff --git a/moto/redshift/responses.py b/moto/redshift/responses.py index bd7223c8..724a61b6 100644 --- a/moto/redshift/responses.py +++ b/moto/redshift/responses.py @@ -555,4 +555,4 @@ class RedshiftResponse(BaseResponse): "RequestId": "384ac68d-3775-11df-8963-01868b7c937a", } } - }) + }) diff --git a/tests/test_redshift/test_redshift.py b/tests/test_redshift/test_redshift.py index 46400d34..79da9f19 100644 --- a/tests/test_redshift/test_redshift.py +++ b/tests/test_redshift/test_redshift.py @@ -1090,6 +1090,7 @@ def test_disable_snapshot_copy(): response = client.describe_clusters(ClusterIdentifier='test') response['Clusters'][0].shouldnt.contain('ClusterSnapshotCopyStatus') + @mock_redshift def test_modify_snapshot_copy_retention_period(): client = boto3.client('redshift', region_name='us-east-1') From 7130dd5239891a5ae2a9219e690c8c6f1c4e5906 Mon Sep 17 00:00:00 2001 From: captainkerk Date: Sun, 28 Jan 2018 03:53:32 +0000 Subject: [PATCH 4/4] rework to follow spec for encrypted/unencrypted clusters --- moto/redshift/models.py | 7 +++++++ moto/redshift/responses.py | 2 +- tests/test_redshift/test_redshift.py | 26 ++++++++++++++++++++++++-- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/moto/redshift/models.py b/moto/redshift/models.py index 44e944c3..f0ea9f5f 100644 --- a/moto/redshift/models.py +++ b/moto/redshift/models.py @@ -4,6 +4,7 @@ import copy import datetime import boto.redshift +from botocore.exceptions import ClientError from moto.compat import OrderedDict from moto.core import BaseBackend, BaseModel from moto.core.utils import iso_8601_datetime_with_milliseconds @@ -430,6 +431,12 @@ class RedshiftBackend(BaseBackend): cluster_identifier = kwargs['cluster_identifier'] cluster = self.clusters[cluster_identifier] if not hasattr(cluster, 'cluster_snapshot_copy_status'): + if cluster.encrypted == 'true' and kwargs['snapshot_copy_grant_name'] is None: + raise ClientError( + 'InvalidParameterValue', + 'SnapshotCopyGrantName is required for Snapshot Copy ' + 'on KMS encrypted clusters.' + ) status = { 'DestinationRegion': kwargs['destination_region'], 'RetentionPeriod': kwargs['retention_period'], diff --git a/moto/redshift/responses.py b/moto/redshift/responses.py index 724a61b6..63945c00 100644 --- a/moto/redshift/responses.py +++ b/moto/redshift/responses.py @@ -506,7 +506,7 @@ class RedshiftResponse(BaseResponse): snapshot_copy_kwargs = { 'cluster_identifier': self._get_param('ClusterIdentifier'), 'destination_region': self._get_param('DestinationRegion'), - 'retention_period': self._get_param('RetentionPeriod'), + 'retention_period': self._get_param('RetentionPeriod', 7), 'snapshot_copy_grant_name': self._get_param('SnapshotCopyGrantName'), } cluster = self.redshift_backend.enable_snapshot_copy(**snapshot_copy_kwargs) diff --git a/tests/test_redshift/test_redshift.py b/tests/test_redshift/test_redshift.py index 79da9f19..32deb74b 100644 --- a/tests/test_redshift/test_redshift.py +++ b/tests/test_redshift/test_redshift.py @@ -1047,12 +1047,13 @@ def test_tagged_resource_not_found_error(): def test_enable_snapshot_copy(): client = boto3.client('redshift', region_name='us-east-1') client.create_cluster( - DBName='test', ClusterIdentifier='test', ClusterType='single-node', - NodeType='ds2.xlarge', + DBName='test', + Encrypted=True, MasterUsername='user', MasterUserPassword='password', + NodeType='ds2.xlarge', ) client.enable_snapshot_copy( ClusterIdentifier='test', @@ -1067,6 +1068,27 @@ def test_enable_snapshot_copy(): cluster_snapshot_copy_status['SnapshotCopyGrantName'].should.equal('copy-us-east-1-to-us-west-2') +@mock_redshift +def test_enable_snapshot_copy_unencrypted(): + client = boto3.client('redshift', region_name='us-east-1') + client.create_cluster( + ClusterIdentifier='test', + ClusterType='single-node', + DBName='test', + MasterUsername='user', + MasterUserPassword='password', + NodeType='ds2.xlarge', + ) + client.enable_snapshot_copy( + ClusterIdentifier='test', + DestinationRegion='us-west-2', + ) + response = client.describe_clusters(ClusterIdentifier='test') + cluster_snapshot_copy_status = response['Clusters'][0]['ClusterSnapshotCopyStatus'] + cluster_snapshot_copy_status['RetentionPeriod'].should.equal(7) + cluster_snapshot_copy_status['DestinationRegion'].should.equal('us-west-2') + + @mock_redshift def test_disable_snapshot_copy(): client = boto3.client('redshift', region_name='us-east-1')