harrystech / arthur-redshift-etl

ELT Code for your Data Warehouse
MIT License
26 stars 11 forks source link

Update the Redshift CFn template to include additional IAM roles #797

Closed jwisdom-harrys closed 1 year ago

jwisdom-harrys commented 1 year ago

Description

We currently have evolved to need 6 additional IAM roles for the Redshift cluster, and the current template only supports 5. Add an additional role.

Also, add in more parameters to the Redshift parameter group that weren't explicitly declared before.

User-visible Changes

None

Internal Changes

Update to cloudformation template that defines the Redshift clusters.

Links

https://harrys.atlassian.net/browse/DENG-2129

Testing

Created a change set to verify the changes I expect are being propagated:

-->./do_cloudformation.sh create-change-set dw_cluster dev \
    MasterUserPassword=UsePreviousValue \
    MasterUsername=UsePreviousValue \
    NodeType=UsePreviousValue \
    PreferredMaintenanceWindow=UsePreviousValue \
    QueryConcurrency=UsePreviousValue \
    SnapshotIdentifier=UsePreviousValue \
    VpcStackName=UsePreviousValue \
AdditionalClusterIAMRole1=UsePreviousValue\
AdditionalClusterIAMRole2=UsePreviousValue\
AdditionalClusterIAMRole3=UsePreviousValue\
AdditionalClusterIAMRole4=arn:aws:iam::367733367673:role/fivetran-reshift \
AdditionalClusterIAMRole5=arn:aws:iam::367733367673:role/HarrysLakeEtlTasks \
AdditionalClusterIAMRole6=arn:aws:iam::367733367673:role/RedshiftSpectrumCrossAccount-dev \
NumberOfNodes=10
jwisdom-harrys commented 1 year ago

New change set, looks much more sensical https://us-east-1.console.aws.amazon.com/cloudformation/home?region=us-east-1#/stacks/changesets/json?stackId=arn%3Aaws%3Acloudformation%3Aus-east-1%3A367733367673%3Astack%2Fdw-cluster-dev%2F5b028cc0-a52b-11e7-87d0-50d5cad95262&changeSetId=arn%3Aaws%3Acloudformation%3Aus-east-1%3A367733367673%3AchangeSet%2FDW-2022-12-09-17-14-49%2F62b30deb-a01f-4eba-b217-ff570e8f558f

thomas-vogels commented 1 year ago

While this will do what you want ... sometimes constraints are a good thing :-) We have three roles that seemingly do the same thing? Maybe it's time to clean up the roles we have instead of adding more?

jwisdom-harrys commented 1 year ago

While this will do what you want ... sometimes constraints are a good thing :-) We have three roles that seemingly do the same thing? Maybe it's time to clean up the roles we have instead of adding more?

Definitely agreed. We are explicitly punting on as much as possible (e.g. a refactor to terraform) for now to get this change set out as quickly as possible so we can effectively utilize our RIs. https://harrys.atlassian.net/browse/DENG-2220