Open nujragan opened 1 year ago
@nujragan did you try this scenario according to the documentation in https://docs.crossplane.io/knowledge-base/guides/import-existing-resources/? The crossplane.io/external-name
annotation is key to the import flow, but I didn't see it mentioned in this issue, hence why I'm asking 😇
@jbw976 crossplane.io/external-name
for a securitygroup is the security group id, which is generated by aws which is not known by the composition
Just FYI: crossplane-contrib/provider-aws has already solved this issue: https://github.com/crossplane-contrib/provider-aws/issues/1175
Hi @nujragan,
Thank you for raising this issue but I could not reproduce the issue. Could you please try again?
crossplane.io/external-name
annotation as below:
apiVersion: ec2.aws.upbound.io/v1beta1
kind: SecurityGroup
metadata:
annotations:
crossplane.io/external-name: sg-01234567891234567
meta.upbound.io/example-id: ec2/v1beta1/securitygroup
labels:
testing.upbound.io/example-name: example
name: example
spec:
deletionPolicy: Orphan
forProvider:
region: us-east-1
description: test security group import
name: testsg
vpcIdRef:
name: sample-vpc
NAME READY SYNCED EXTERNAL-NAME AGE
securitygroup.ec2.aws.upbound.io/example True True sg-01234567891234567 7m13s
or:
AWS Console->EC2->Security Groups->Security group ID->sg-.........
@turkenf you are right, I can import it with having external name as the security group Id, I am trying to use this in a composition and there seems to be no way to get the security group Id from aws in the composition, FYI: this issue is already solved in crossplane-contrib/provider-aws https://github.com/crossplane-contrib/provider-aws/issues/1175. This stops me from migrating to the official provider.
@nujragan then, can't you import using Security group ID
in the AWS console here?
@turkenf I can, but when using in a composition, deleting the composition and recreating it, runs into this issue
@nujragan, let's leave the composition aside to fully understand your request, what exactly do you expect from provider-aws here?
@turkenf something similar to this: https://github.com/crossplane-contrib/provider-aws/issues/1175.
@turkenf if I can chime in.
If we create a SecurityGroup using deletionPolicy: Orphan
, then delete it and re-apply it, I expect the provider to assume ownership of the already created SecurityGroup.
@ONordander, it is possible with using crossplane.io/external-name
: https://github.com/upbound/provider-aws/issues/782#issuecomment-1630581703
@ONordander, it is possible with using
crossplane.io/external-name
: #782 (comment)
Yes, sorry if it was unclear but I mean without manual intervention, with crossplane-contrib/provider-aws it assumed ownership without adding crossplane.io/external-name
.
@ONordander, the way you specified is currently not possible.
@ONordander thanks for the explanation, but @turkenf that is the ask. I dont know if this is a bug or a feature request.
Hi @ONordander,
Because when we set the external-name annotation (crossplane.io/external-name
) to the security group's ID, importing succeeds, I would classify this issue as a feature request. The requested feature is being able to import security groups using their names, i.e., being able to import them via spec.forProvider.name
.
The upbound/provider-aws
provider is an upjet-based provider, meaning that we rely on Terraform to manage the external security group resource. According to the Terraform documentation for the corresponding Terraform resource, it's only possible to import them using the security group ID. Thus, unless importing via the security group name works out of the box with Terraform, it's not straightforward for us to add this new feature.
Looking at how the importing via the security group name feature was implemented in crossplane-contrib/provider-aws
, I wonder whether choosing the first security group with the given name is safe:
func (e *external) getSecurityGroupByName(ctx context.Context, groupName string) (*string, error) {
groups, err := e.sg.DescribeSecurityGroups(ctx, &awsec2.DescribeSecurityGroupsInput{
Filters: []awsec2types.Filter{
{Name: aws.String("group-name"), Values: []string{groupName}},
},
})
if err != nil || len(groups.SecurityGroups) == 0 {
return nil, err
}
return groups.SecurityGroups[0].GroupId, nil
}
Is it guaranteed to return an array of length at most 1? Or in other words, are security group names unique per region per account?
@ulucinar I haven't invested the time to understand the underlying details of the Terrajet providers yet, so thank you for the explanation. It would be really nice to see this feature added, not only for security groups. Since it sounds like this functionality will be bigger than just this single change, is it tracked in some other issue?
I think you are right, it looks like the VPC Id should be part of the query as well to make it fully unique:
A security group name must be unique within the VPC.
@ulucinar can I work on this ? Can you assign this to me.
@jbw976 @turkenf can I work on this ? Can you assign this to me.
Assigned you @nujragan, you can start working on it, thanks in advance for your contribution. 🙏
@nujragan, any progress here?
This provider repo does not have enough maintainers to address every issue. Since there has been no activity in the last 90 days it is now marked as stale
. It will be closed in 14 days if no further activity occurs. Leaving a comment starting with /fresh
will mark this issue as not stale.
Comment to remove stale. We are too seeing this issue. If for some reason the resource is removed and set to orphan, we won't be able to automatically adopt it since we can't figure out the security group id.
With terraform we can use name_prefix and deal with the left over groups using another ad-hoc method.
Is there any way we can mimic name_prefix
in crossplane?
This provider repo does not have enough maintainers to address every issue. Since there has been no activity in the last 90 days it is now marked as stale
. It will be closed in 14 days if no further activity occurs. Leaving a comment starting with /fresh
will mark this issue as not stale.
/fresh
Also having this issue. One of the blockers from managing other clusters from a platform cluster without using some kind of cluster backup solution. Would definitely be a great addition!
I guess the solution from https://github.com/crossplane-contrib/provider-aws/issues/1175 is not directly transferable to this provider?
What happened?
This problem is best described with an example:
Create a managed resource with a securitygroup with deletionPolicy: Orphan
Apply the securitygroup Delete the securitygroup Now the securitygroup stays behind in AWS which is as expected. Apply the securitygroup again Now the security will have a reconcile error and cannot create a duplicate groupname.
Tried using
crossplane.io/external-name
annotation but the external name for a securitygroup is a securitygroup ID which is random and generated by aws.We expect that it can reconcile the existing securitygroup.
We want to import this created securitygroup but this is not possible this way.
This also prevents us from migrating from crossplane-contrib/provider-aws to the official provider.
This was an issue with crossplane-contrib/provider-aws(https://github.com/crossplane-contrib/provider-aws/issues/1175) but they seemed to have fixed it.
Expected behavior is for the securitygroup to reconcile based on name instead of id which is generated by aws(which we have no control over or means to get it)
How can we reproduce it?
Above steps should help to reproduce the issue
What environment did it happen in?