feat: support iam related arguments for ServiceAccountBinding #121
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for two new IAM-related fields (enable_iam_account_creation and aws_assume_role_arns) to the ServiceAccountBinding Terraform resource and data source, updates documentation, provider descriptions, and bumps the cloud-api-server dependency.
- Introduce
enable_iam_account_creation(bool) andaws_assume_role_arns(list of strings) in resource and data source schemas and map them to the CR spec - Update provider descriptions and Terraform docs for both resource and data source
- Bump
github.com/streamnative/cloud-api-serverto v1.36.0
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Bumped cloud-api-server module from v1.35.2 to v1.36.0 |
| docs/resources/service_account_binding.md | Documented new optional fields aws_assume_role_arns and enable_iam_account_creation |
| docs/data-sources/service_account_binding.md | Documented new read-only attributes for IAM support |
| cloud/resource_service_account_binding.go | Added schema entries, create/read logic, and CR spec mapping for new fields |
| cloud/provider.go | Added descriptions for the new parameters |
| cloud/data_source_service_account_binding.go | Added computed schema entries and read mapping for new fields |
Comments suppressed due to low confidence (1)
cloud/resource_service_account_binding.go:102
- New schema fields enable_iam_account_creation and aws_assume_role_arns lack corresponding unit or acceptance tests. Consider adding tests to verify their behavior.
"enable_iam_account_creation": {
jiangpengcheng
force-pushed
the
update_sab
branch
from
3542997 to
b816350
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ### Optional | ||
|
|
||
| - `aws_assume_role_arns` (List of String) A list of AWS IAM roles' arn which can be assumed by the AWS IAM role created for the service account binding |
There was a problem hiding this comment.
The description should use "role ARNs" (plural and capitalized) instead of "roles' arn". ARN stands for Amazon Resource Name and should be capitalized. The plural form should be "ARNs" not "arn".
| - `aws_assume_role_arns` (List of String) A list of AWS IAM roles' arn which can be assumed by the AWS IAM role created for the service account binding | |
| - `aws_assume_role_arns` (List of String) A list of AWS IAM role ARNs which can be assumed by the AWS IAM role created for the service account binding |
|
|
||
| ### Read-Only | ||
|
|
||
| - `aws_assume_role_arns` (List of String) A list of AWS IAM roles' arn which can be assumed by the AWS IAM role created for the service account binding |
There was a problem hiding this comment.
The description should use "role ARNs" (plural and capitalized) instead of "roles' arn". ARN stands for Amazon Resource Name and should be capitalized. The plural form should be "ARNs" not "arn".
| - `aws_assume_role_arns` (List of String) A list of AWS IAM roles' arn which can be assumed by the AWS IAM role created for the service account binding | |
| - `aws_assume_role_arns` (List of String) A list of AWS IAM role ARNs which can be assumed by the AWS IAM role created for the service account binding |
cloud/provider.go
Outdated
| "principal_name": "The principal name of apikey, it is the principal name of the service account that the apikey is associated with, it is used to grant permission on pulsar side", | ||
| "customized_metadata": "The custom metadata in the api key token", | ||
| "enable_iam_account_creation": "Whether to create an IAM account for the service account binding", | ||
| "aws_assume_role_arns": "A list of AWS IAM roles' arn which can be assumed by the AWS IAM role created for the service account binding", |
There was a problem hiding this comment.
The description should use "role ARNs" (plural and capitalized) instead of "roles' arn". ARN stands for Amazon Resource Name and should be capitalized. The plural form should be "ARNs" not "arn".
| "aws_assume_role_arns": "A list of AWS IAM roles' arn which can be assumed by the AWS IAM role created for the service account binding", | |
| "aws_assume_role_arns": "A list of AWS IAM role ARNs which can be assumed by the AWS IAM role created for the service account binding", |
No description provided.