feat: add governance_subscription_resource [GOV-630] #2072
Conversation
| func resourceGovernanceSubscriptionCreate(ctx context.Context, d *schema.ResourceData, client avngen.Client) error { | ||
| var req governance.OrganizationGovernanceSubscriptionCreateIn | ||
|
|
||
| req.SubscriptionName = d.Get("subscription_name").(string) |
There was a problem hiding this comment.
The reason why I did not use the preferred schemautil.ResourceDataGet and schemautil.ResourceDataSet was because:
- in the tf schema, the subscription_data is a list and acorn API wants single object.
- There we also some optional fields that needed to be excluded if not defined.
- schemautil.RenameAlias (project_name -> project) seemed to not reach inside subscription_data, only top level.
There was a problem hiding this comment.
Hey. It is better to stick to the schema. I would flatten subscription_data field. Is there any reason why it is turned into a property?
There was a problem hiding this comment.
Agreed, I wanted to keep it 1:1 with the schema as well thus I did not flatten it. The subscription_data is a property in Acorn API because the relevant fields could be different for other subscription_type (we don't know yet).
Though one option could be to make the resource based on subscription_type. In this case, aiven_governance_kafka_subscription. While it would not be 1:1 with API schema, it would provide more flat structure and perhaps reduce congitive complexity.
There was a problem hiding this comment.
Regarding the API schema, I think there is room for improvement and we'll probably try to simplify it with some abstraction but I'm hoping the Beta mark will give us room for future improvements.
| Optional: true, | ||
| ForceNew: true, | ||
| ValidateFunc: validation.StringLenBetween(1, 256), | ||
| Description: userconfig.Desc("The IP address from which a principal is allowed or denied access to the resource. Use `*` for all hosts").ForceNew().MaxLen(256).Build(), |
There was a problem hiding this comment.
nit: there is IsCIDR validator organization_vpc.go
There was a problem hiding this comment.
Hey, I did not use this for now, since I realized it needs to support csv of ips (which matches the kafka-native-acl schema)
| func resourceGovernanceSubscriptionCreate(ctx context.Context, d *schema.ResourceData, client avngen.Client) error { | ||
| var req governance.OrganizationGovernanceSubscriptionCreateIn | ||
|
|
||
| req.SubscriptionName = d.Get("subscription_name").(string) |
There was a problem hiding this comment.
Hey. It is better to stick to the schema. I would flatten subscription_data field. Is there any reason why it is turned into a property?
internal/sdkprovider/service/governance/governance_subscription.go
Outdated
Show resolved
Hide resolved
internal/sdkprovider/service/governance/governance_subscription.go
Outdated
Show resolved
Hide resolved
roope-kar
force-pushed
the
rkar-add-governance-subscription-resource
branch
4 times, most recently
from
531e329 to
9889554
Compare
for managing governance subscriptions, beta as prone to change.
roope-kar
force-pushed
the
rkar-add-governance-subscription-resource
branch
from
9889554 to
2e1af34
Compare
for managing governance subscriptions, marked as beta as the API and resource are prone to change.
About this change—what it does
Resolves: #xxxxx.
Why this way