Add opensearch serverless group resources to v1beta1 #1130
Conversation
|
/test-examples="examples/opensearchserverless/v1beta1/accesspolicy.yaml" |
|
/test-examples="examples/opensearchserverless/v1beta1/lifecyclepolicy.yaml" |
|
/test-examples="examples/opensearchserverless/v1beta1/securitypolicy.yaml" |
turkenf
force-pushed
the
issue-889
branch
2 times, most recently
from
e23918e to
16fdf87
Compare
config/externalname.go
Outdated
| "aws_opensearchserverless_access_policy": opensearchserverlessAccessPolicy(), | ||
| // Collection can be imported using the AWS-assigned collection ID. i.e. ch9rq91uv4yd8rff1f39 | ||
| "aws_opensearchserverless_collection": opensearchserverlessCollection(), | ||
| // LifecyclePolicy can be imported using the policy name | ||
| "aws_opensearchserverless_lifecycle_policy": opensearchserverlessLifecyclePolicy(), | ||
| // SecurityConfig can be imported using the AWS-assigned security config ID | ||
| "aws_opensearchserverless_security_config": config.TemplatedStringAsIdentifier("name", "{{ .parameters.type }}/{{ .setup.client_metadata.account_id }}/{{ .external_name }}"), | ||
| // SecurityPolicy can be imported using the policy name | ||
| "aws_opensearchserverless_security_policy": opensearchserverlessSecurityPolicy(), | ||
| // VPCEndpoint can be imported using the AWS-assigned VPC Endpoint ID, i.e. vpce-0a957ae9ed5aee308 | ||
| "aws_opensearchserverless_vpc_endpoint": opensearchserverlessVpcEndpoint(), |
There was a problem hiding this comment.
Looks like the TemplatedStringAsIdentifier external-name configuration works for the security_config resource. It prevents an empty external-name by having the external-name annotation initialized with metadata.name and it results in a normalized API where the spec.forProvider.name parameter is removed in addition to the external-name normalization. Could it also work for the other resources here?
There was a problem hiding this comment.
aws_opensearchserverless_access_policy , aws_opensearchserverless_lifecycle_policy and aws_opensearchserverless_security_policy works with NameAsIdentifier external name config. Updated 👍
aws_opensearchserverless_vpc_endpoint and aws_opensearchserverless_collection still have to use the "modified" IdentifierFromProvider as their IDs are AWS-assigned random values with some regex restrictions on their ID format.
As an enhancement, to generalize the new pattern, we might introduce a new external config type something like IdentifierFromProviderWithStub which sets a dummy external name for the resources that are unable to do the first Observe with empty IDs.
|
/test-examples="examples/iam/v1beta1/policy.yaml" |
|
/test-examples="examples/eks/v1beta1/podidentityassociation.yaml" |
|
/test-examples="examples/ec2/v1beta1/securitygroupingressrule.yaml,examples/cognitoidp/v1beta1/userpoolclient.yaml,examples/simpledb/v1beta1/domain.yaml,examples/appconfig/v1beta1/environment.yaml,examples/elasticache/v1beta1/replicationgroup.yaml" |
|
/test-examples="examples/opensearchserverless/v1beta1/accesspolicy.yaml,examples/opensearchserverless/v1beta1/lifecyclepolicy.yaml,examples/opensearchserverless/v1beta1/securitypolicy.yaml" |
|
/test-examples="examples/opensearchserverless/v1beta1/collection.yaml,examples/opensearchserverless/v1beta1/vpcendpoint.yaml" |
|
/test-examples="examples/opensearchserverless/v1beta1/securityconfig.yaml" |
|
/test-examples="examples/opensearchserverless/v1beta1/accesspolicy.yaml,examples/opensearchserverless/v1beta1/lifecyclepolicy.yaml,examples/opensearchserverless/v1beta1/securitypolicy.yaml" |
|
Upjet commit |
|
/test-examples="examples/opensearchserverless/v1beta1/securitypolicy.yaml" |
|
/test-examples="examples/opensearchserverless/v1beta1/securityconfig.yaml" |
|
/test-examples="examples/opensearchserverless/v1beta1/vpcendpoint.yaml" |
|
/test-examples="examples/opensearchserverless/v1beta1/accesspolicy.yaml" |
|
/test-examples="examples/opensearchserverless/v1beta1/lifecyclepolicy.yaml" |
|
/test-examples="examples/opensearchserverless/v1beta1/securitypolicy.yaml" |
|
/test-examples="examples/opensearchserverless/v1beta1/collection.yaml" |
|
/test-examples="examples/opensearchserverless/v1beta1/accesspolicy.yaml,examples/opensearchserverless/v1beta1/lifecyclepolicy.yaml,examples/opensearchserverless/v1beta1/securitypolicy.yaml,examples/opensearchserverless/v1beta1/securityconfig.yaml" |
|
/test-examples="examples/opensearchserverless/v1beta1/collection.yaml,examples/opensearchserverless/v1beta1/vpcendpoint.yaml" |
|
/test-examples="examples/ec2/v1beta1/securitygroupingressrule.yaml,examples/cognitoidp/v1beta1/userpoolclient.yaml,examples/simpledb/v1beta1/domain.yaml,examples/appconfig/v1beta1/environment.yaml,examples/elasticache/v1beta1/replicationgroup.yaml,examples/eks/v1beta1/podidentityassociation.yaml,examples/iam/v1beta1/policy.yaml" |
|
/test-examples="examples/ec2/v1beta1/securitygroupingressrule.yaml,examples/cognitoidp/v1beta1/userpoolclient.yaml,examples/simpledb/v1beta1/domain.yaml,examples/appconfig/v1beta1/environment.yaml,examples/elasticache/v1beta1/replicationgroup.yaml,examples/eks/v1beta1/podidentityassociation.yaml" |
Signed-off-by: Fatih Türken <turkenf@gmail.com>
…field as an embedded object. - Fix the example manifest examples/opensearchserverless/v1beta1/securityconfig.yaml - Use config.TemplatedStringAsIdentifier as the external-name configuration for aws_opensearchserverless_security_config Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
…ecurity}_policy resources Signed-off-by: Erhan Cagirici <erhan@upbound.io>
…olicy,SecurityPolicy}.opensearchserverless resources
- Update the example manifests examples/opensearchserverless/v1beta1/{lifecyclepolicy,accesspolicy,securitypolicy}.yaml
The "policy" parameters are now multi-line JSON strings.
Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
Signed-off-by: Erhan Cagirici <erhan@upbound.io>
erhancagirici
force-pushed
the
issue-889
branch
from
efa9d89 to
09e0c3a
Compare
|
/test-examples="examples/opensearchserverless/v1beta1/accesspolicy.yaml,examples/opensearchserverless/v1beta1/lifecyclepolicy.yaml,examples/opensearchserverless/v1beta1/securitypolicy.yaml,examples/opensearchserverless/v1beta1/securityconfig.yaml" |
|
/test-examples="examples/opensearchserverless/v1beta1/collection.yaml,examples/opensearchserverless/v1beta1/vpcendpoint.yaml" |
|
The following uptest runs |
| // opensearchserverless | ||
| // | ||
| // AccessPolicy can be imported using the policy name | ||
| "aws_opensearchserverless_access_policy": config.NameAsIdentifier, |
There was a problem hiding this comment.
When I check the tf registry doc, I observe that the import format is: name and type arguments separated by a slash (/). This also applies to other resources with NameAsIdentifier. I'm not proposing to change this since the uptests were successful (and the imports were successful) but I wanted to leave this comment for the sake of understanding the approach here.
There was a problem hiding this comment.
In the no-fork architecture, we never visit the Import path of the terraform implementation and directly start with an Observe with the ID+provided parameters. In many resources, especially in plugin framework resources, you see that the ID is transformed during import. Therefore, the import ID and the "regular" ID format is frequently different. See the below example of aws_opensearchserverless_lifecycle_policy resource import implementation at terraform provider, which is also similar at other NameAsIdentifier resources:
https://github.com/upbound/terraform-provider-aws/blob/14c3fe90f23bf825d5d7301f1a7d3303c59664fc/internal/service/opensearchserverless/lifecycle_policy.go#L242-L254
As you can see from the implementation, the incoming ID with slashes is split and the first part is then assigned to both name and id. After that point, resource state continues its lifecycle as such.
Also at the Read implementation: https://github.com/upbound/terraform-provider-aws/blob/14c3fe90f23bf825d5d7301f1a7d3303c59664fc/internal/service/opensearchserverless/lifecycle_policy.go#L145
we see that the ID and type are supplied separately, where ID corresponds to name. If we were to supply the ID as the slash-separated import format, then it would have failed to Read() the resource, hence the Observe would fail and the import would never happen.
There was a problem hiding this comment.
Thank you for the detailed explanation.
Signed-off-by: Erhan Cagirici <erhan@upbound.io>
Description of your changes
Add opensearch serverless group resources to v1beta1
Fixes #889
Depends on: crossplane/upjet#341, crossplane/upjet#342, crossplane/upjet#344
I have:
make reviewable testto ensure this PR is ready for review.How has this code been tested
Manually and uptest.