It is not possible to import securitygroup which is created previously

[nujragan]

What happened?

This problem is best described with an example:

Create a managed resource with a securitygroup with deletionPolicy: Orphan

apiVersion: ec2.aws.upbound.io/v1beta1
kind: SecurityGroup
metadata:
  name: test-sg-rule-deletion
spec:
  deletionPolicy: Orphan
  forProvider:
    description: test security group import
    region: us-east-1
    name: testsg
    vpcId: vpcxxxx
  providerConfigRef:
    name: XXXX

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.

apply failed: creating Security Group (testsg): InvalidGroup.Duplicate:
      The security group 'testsgv3' already exists for VPC 'vpc-XXX'
      code: 400

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(crossplane-contrib/provider-aws#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?

• Crossplane Version: v0.12.1
• Provider Version: 0.37.0
• Kubernetes Version: 1.25
• Kubernetes Distribution: EKS

[jbw976]

@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 😇

[nujragan]

@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

[nujragan]

Just FYI: crossplane-contrib/provider-aws has already solved this issue: crossplane-contrib/provider-aws#1175

[turkenf]

Hi @nujragan,

Thank you for raising this issue but I could not reproduce the issue. Could you please try again?

• After deleting the resource in your terminal, can you try 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

• You can find the external-name:

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-.........

[nujragan]

@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 crossplane-contrib/provider-aws#1175. This stops me from migrating to the official provider.

[turkenf]

@nujragan then, can't you import using Security group ID in the AWS console here?

[nujragan]

@turkenf I can, but when using in a composition, deleting the composition and recreating it, runs into this issue

[turkenf]

@nujragan, let's leave the composition aside to fully understand your request, what exactly do you expect from provider-aws here?

[nujragan]

@turkenf something similar to this: crossplane-contrib/provider-aws#1175.

[ONordander]

@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.

[turkenf]

@ONordander, it is possible with using crossplane.io/external-name: #782 (comment)

[ONordander]

@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.

[turkenf]

@ONordander, the way you specified is currently not possible.

[nujragan]

@ONordander thanks for the explanation, but @turkenf that is the ask. I dont know if this is a bug or a feature request.

[ulucinar]

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?

[ONordander]

@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.

[nujragan]

@ulucinar can I work on this ? Can you assign this to me.

[nujragan]

@jbw976 @turkenf can I work on this ? Can you assign this to me.

[turkenf]

Assigned you @nujragan, you can start working on it, thanks in advance for your contribution. 🙏

[turkenf]

@nujragan, any progress here?

[github-actions]

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.

[GMartinez-Sisti]

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?

[github-actions]

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.

[GMartinez-Sisti]

/fresh

[kejne]

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 crossplane-contrib/provider-aws#1175 is not directly transferable to this provider?