Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aws-redshift-alpha: create method that adds IAM role to cluster #22632

Closed
2 tasks
sean-beath opened this issue Oct 25, 2022 · 8 comments · Fixed by #23791
Closed
2 tasks

aws-redshift-alpha: create method that adds IAM role to cluster #22632

sean-beath opened this issue Oct 25, 2022 · 8 comments · Fixed by #23791
Assignees
Labels
@aws-cdk/aws-redshift Related to Amazon Redshift effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@sean-beath
Copy link
Contributor

Describe the feature

Create a method that will allow users to add an IAM role to an already created cluster. This would allow a user to do the following:
myCluster.add_role(myRole)

Use Case

I'm always frustrated when I can't add a role to an already created cluster, and instead need to add it to the cluster on creation.

Proposed Solution

No response

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.25.0a0

Environment details (OS name and version, etc.)

MacOS Monterey 12.5

@sean-beath sean-beath added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Oct 25, 2022
@github-actions github-actions bot added the @aws-cdk/aws-redshift Related to Amazon Redshift label Oct 25, 2022
@indrora
Copy link
Contributor

indrora commented Nov 16, 2022

If I'm understanding you right, you want to import an extant cluster and add a role to it?

If that's the case, imported resources can't easily be modified with CloudFormation: CloudFormation can't directly modify an already extant, referenced resource. However, since AWS Redshift has an API call that you can use to modify an extant role set, you can use a Custom Resource to do your work.

Using the API Call custom resource and the ModifyCluster API method, you should be able to do what you're looking to do.

@indrora indrora added p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Nov 16, 2022
@Rizxcviii
Copy link
Contributor

@indrora I understood this differently. I think what @sean-beath is referring to is the ability to add roles, after initially declaring a new cluster WITHIN the code, not using an import function. I've come across the issue before to attach multiple roles to the cluster after declaring it within the code.

@sean-beath
Copy link
Contributor Author

The functionality I am referring to here more closely aligns with @Rizxcviii however I believe the functionality would need to be implemented using an AWSCustomResource as mentioned by @indrora

Desired functionaility:

role1 = iam.Role(self, "role1", assumed_by=iam.ServicePrincipal("redshift.amazonaws.com"))
myCluster = redshift.Cluster(self, "myCluster", roles[role1])

role2 = iam.Role(self, "role2", assumed_by=iam.ServicePrincipal("redshift.amazonaws.com"))
myCluster.add_iam_role(role2) # OR myCluster.add_iam_role([role2,...])

I've recently created a PR for default roles on Redshift so I believe it would follow a similar format.

Pseudocode for add_iam_role:

if number of roles > limit or roles on cluster
    then throw error
else
    Create new custom resource that will:
        add iam role to cluster on update
        remove iam role from cluster on delete

@comcalvi
Copy link
Contributor

comcalvi commented Jan 31, 2023

@sean-beath I'm not sure I understand the ask here.

You can already modify the IAM resources on a created Cluster, you just need to modify the constructor, eg:

const cluster = new Cluster(this, 'Redshift', {
  roles: [new iam.Role(...)],
  ...,
});

you can create the addRole() as a convenience, but it shouldn't be blocking.

More importantly, why the custom resource? Why do you need the custom resource to manage the roles?

@Rizxcviii
Copy link
Contributor

Rizxcviii commented Feb 1, 2023

@comcalvi I've personally had this issue, where within another stack, I would create an IAM role for another resource. However the resource would need to access the redshift cluster. Therefore, I would have to declare the role (for the other resource) within the stack where the redshift cluster resides. I would then have to import it into the second stack and use it for the resource. This is where the 'issue' resides for me personally. It would be good to have this method available to use, but only if it does not block.

In regards to the custom resource, I believe that would be the only way to allow for modification of the IAM roles that can interact with the redshift cluster. Unless there is a better way to achieve this?

@comcalvi
Copy link
Contributor

comcalvi commented Feb 1, 2023

why do you need to import the cluster? You can export the role you need to the cluster, eg:

lib:

export interface ClusterStackProps extends cdk.StackProps {
  role: iam.IRole,
}

export class LambdaStack extends cdk.Stack {
  public readonly role: iam.IRole;
  constructor(scope: Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    const func = new lambda.Function(this, 'function', {
      code: lambda.Code.fromInline(''),
      runtime: lambda.Runtime.DOTNET_6,
      handler: 'index.handler',
    });

    this.role = func.role!;
  }
}

export class ClusterStack extends cdk.Stack {
  constructor(scope: Construct, id: string, props: ClusterStackProps) {
    super(scope, id, props);

    const cluster = new redshift.Cluster(this, 'cluster', {
      vpc: new ec2.Vpc(this, 'vpc'),
      masterUser: {
        masterUsername: 'admin',
      },
      roles: [
        props.role
      ]
    });
  }
}

bin:

const app = new cdk.App();
const lambdaStack = new LambdaStack(app, 'ApiStack', );
const role = lambdaStack.role;

new ClusterStack(app, 'ClusterStack', {
  role,
});

@Rizxcviii
Copy link
Contributor

Interesting, would this implementation work if the ClusterStack was created first, and thereafter the LambdaStack was created after?

@mergify mergify bot closed this as completed in #23791 Feb 10, 2023
mergify bot pushed a commit that referenced this issue Feb 10, 2023
…#23791)

Created an `addIamRole` method that will allow attaching an IAM role to a cluster, post its creation.
closes #22632 

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Construct Runtime Dependencies:

* [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-redshift Related to Amazon Redshift effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants