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": {
| import ( | ||
| "context" | ||
| "fmt" | ||
| "github.com/streamnative/cloud-api-server/pkg/apis/cloud/v1alpha1" |
There was a problem hiding this comment.
[nitpick] The external package import is placed between standard library imports. Consider grouping external and local imports separately for readability.
| apierrors "k8s.io/apimachinery/pkg/api/errors" | ||
|
|
||
| "github.com/hashicorp/terraform-plugin-sdk/v2/diag" | ||
| "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" |
There was a problem hiding this comment.
[nitpick] The moved import disrupts conventional import grouping. Consider aligning standard library, external, and local imports for consistency.
| apierrors "k8s.io/apimachinery/pkg/api/errors" | |
| "github.com/hashicorp/terraform-plugin-sdk/v2/diag" | |
| "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" | |
| "github.com/hashicorp/terraform-plugin-sdk/v2/diag" | |
| "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" | |
| apierrors "k8s.io/apimachinery/pkg/api/errors" |
| _ = d.Set("pool_member_name", serviceAccountBinding.Spec.PoolMemberRef.Name) | ||
| _ = d.Set("pool_member_namespace", serviceAccountBinding.Spec.PoolMemberRef.Namespace) | ||
| _ = d.Set("enable_iam_account_creation", serviceAccountBinding.Spec.EnableIAMAccountCreation) | ||
| _ = d.Set("aws_assume_role_arns", serviceAccountBinding.Spec.AWSAssumeRoleARNs) |
There was a problem hiding this comment.
Directly setting a []string on a TypeList may lead to type mismatches. Use a helper like flattenStringSlice to convert to []interface{}.
| _ = d.Set("aws_assume_role_arns", serviceAccountBinding.Spec.AWSAssumeRoleARNs) | |
| _ = d.Set("aws_assume_role_arns", flattenStringSlice(serviceAccountBinding.Spec.AWSAssumeRoleARNs)) |
No description provided.