feat(redshift-alpha): implement IGrantable with a default service IAM Role

[dontirun]

Have Redshift Clusters implement the IGrantable interface with a default Service Role. This allows for easier permissioning for features like Redshift Spectrum.

Closes #28013

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

[dontirun]

Somewhat conflicted on whether this should even be a prop, but I am leaning towards having it since other services follow a similar pattern.

[dontirun]

Exemption Request for a new integration test.

1. All the old integration tests were affected by this change and they successfully ran.
2. The unit tests should capture this functionality well

[lpizzinidev]

Thanks for the contribution!
I left some suggestions for improvements on documentation and tests.
Also, I think that an integration test would be a good addition in this case.
Finally, the module in the title should be just redshift: feat(redshift): ....

[lpizzinidev]

Suggested change

	/**
	* A default AWS Identity and Access Management (IAM) role to be used by the cluster to access other AWS services.
	*/
	public readonly serviceRole: iam.IRole;
	/**
	* A default AWS Identity and Access Management (IAM) role to be used by the cluster to access other AWS services.
	*/
	private readonly serviceRole: iam.IRole;

No need to make this public.

[dontirun]

I think it's useful to either make this public or add additional methods to allow users to add permissions to the role directly.

It's convenient for adding additional permissions beyond what grants provide. For example if I wanted to allow my cluster to read from a Glue Data Catalog.

[lpizzinidev]

I think we need unit tests for:

• A Cluster with serviceRole specified via props
• A Cluster with serviceRole and roles specified via props

[dontirun]

I think that an integration test would be a good addition in this case.

I can add one, but I'm not entirely sure what value an integration test specifically for this feature would add. The feature is already captured in all the other integration tests as the serviceRole gets created by default and added to the cluster. From a CloudFormation perspective the implementation just adds an additional role to the already existing list of roles.

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 0fef007
• Result: FAILED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[lpizzinidev]

Thanks for the changes, a couple of notes.

• feat(redshift-alpha): implement IGrantable with a default service IAM Role #28018 (comment) - Let's keep it public and see what maintainers say, but the indentation in the documentation still needs to be adjusted.
• feat(redshift-alpha): implement IGrantable with a default service IAM Role #28018 (comment) - Tests are still needed.
• Finally, the module in the title should be just redshift: feat(redshift): .... - The title still needs to be updated.
• feat(redshift-alpha): implement IGrantable with a default service IAM Role #28018 (comment) - An integration test that specifies a custom serviceRole and grants permissions on the cluster should be added since not covered by the existing tests.
  I know that it's a borderline case, but it's better to have extra coverage, and its good practice when adding features.

[aws-cdk-automation]

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

[natalie-white-aws]

Per Kendra's request, I reviewed these changes and can confirm that these changes do not conflict with the glue-alpha L2 work in flight. Safe to merge pending all other CDK service team checks without impacting our work.

[aws-cdk-automation]

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

[aws-cdk-automation]

The pull request linter fails with the following errors:

❌ Features must contain a change to an integration test file and the resulting snapshot.

PRs must pass status checks before we can provide a meaningful review.

If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing Exemption Request and/or Clarification Request.