crossplane-contrib / provider-upjet-azure

Official Azure Provider for Crossplane by Upbound.
Apache License 2.0
53 stars 69 forks source link

[Bug]: Crossplane fails to observe existing SecurityCenterContact #646

Open bak-fts opened 5 months ago

bak-fts commented 5 months ago

Is there an existing issue for this?

Affected Resource(s)

Resource MRs required to reproduce the bug

Add a resource like this somewhere in a composition:

    - name: SecurityCenterContact
      base:
        apiVersion: security.azure.upbound.io/v1beta1
        kind: SecurityCenterContact
        metadata:
          metadata:
            annotations:
              crossplane.io/external-name: example-security-center-contact
        spec:
          forProvider:
            name: example-security-center-contact
            email: "test@test.com"
            phone: "+123456789"

Steps to Reproduce

  1. Let Crossplane create the managed resource.
  2. Set the deletion policy of the managed resource to Orphan and delete the managed resource.
  3. Wait for Crossplane to re-create the managed resource.
  4. Observe that the managed resource is stuck in unready state.

What happened?

Expected behaviour: The managed resource recognizes that the role assignment exists and observes it.

Actual behaviour: The managed resource is stuck in unready state, failing to import the existing role assignment.

Relevant Error Output Snippet

async create failed: failed to create the resource: [{0 A resource with the ID "/subscriptions/[REDACTED]/providers/Microsoft.Security/securityContacts/example-security-center-contact" already exists - to be managed via Terraform this resource needs to be imported into the State. Please see the resource documentation for "azurerm_security_center_contact" for more information.  []}]

Crossplane Version

1.14.5

Provider Version

0.42.0

Kubernetes Version

v1.27.7

Kubernetes Distribution

AKS

Additional Info

No response

turkenf commented 4 months ago

Changing the finalizer and deleting the resource may cause unexpected behavior. Could you please try setting deletionPolicy: Orphan and deleting the resource?

bak-fts commented 4 months ago

@turkenf Thank you for the suggestion. I'll be deleting my undeletable resources this way in the future.

The behaviour does not change whether I delete the managed resource via unsetting its finalizers or setting deletionPolicy: Orphan.

turkenf commented 4 months ago

Here is an information about deletionPolicy.

The behaviour does not change whether I delete the managed resource via unsetting its finalizers or setting deletionPolicy: Orphan.

Ok, have you tested the import for these resources regardless of composition?

bak-fts commented 4 months ago

Yes.

I also realize that between SecurityCenterContact and RoleAssignment there are two separate issues.

  1. The RoleAssignment should be observed after only specifying the principalId, roleDefinitionName and scope without needing to specify a name, because those three properties already uniquely identify a role assignment.
  2. Both SecurityCenterContacts and RoleAssigments, as well as some other resources, cannot be observed even when specifying uniquely identifying properties such as name. Crossplane attempts to and fails to create them without ever checking of their existence. Even when explicitly setting both spec.name and metadata.annotations[crossplane.io/external-name] on the SecurityCenterContact, it still fails to observe the existing resource before attempting to create it.

RoleAssignment.yaml

apiVersion: authorization.azure.upbound.io/v1beta1
kind: RoleAssignment
metadata:
  name: issue
spec:
  deletionPolicy: Orphan
  forProvider:
    principalId: <OMITTED>
    roleDefinitionName: Storage Blob Data Contributor
    scope: /subscriptions/<OMITTED>/resourceGroups/<OMITTED>/providers/Microsoft.Storage/storageAccounts/<OMITTED>

When I apply the manifest & a role assignment with the same principal ID, role definition name and scope already exists:

    Message:               async create failed: failed to create the resource: [{0 authorization.RoleAssignmentsClient#Create: Failure responding to request: StatusCode=409 -- Original Error: autorest/azure: Service returned an error. Status=409 Code="RoleAssignmentExists" Message="The role assignment already exists."  []}] 

SecurityCenterContact.yaml

apiVersion: security.azure.upbound.io/v1beta1
kind: SecurityCenterContact
metadata:
  name: issue
spec:
  deletionPolicy: Orphan
  forProvider:
    alertNotifications: true
    alertsToAdmins: true
    email: test@test.com
    name: scc-test
    phone: "<OMITTED>"

When I apply the manifest & a Security Center Contact with the same name already exists in the subscription:

    Message:               async create failed: failed to create the resource: [{0 A resource with the ID "/subscriptions/<OMITTED>/providers/Microsoft.Security/securityContacts/scc-test" already exists - to be managed via Terraform this resource needs to be imported into the State. Please see the resource documentation for "azurerm_security_center_contact" for more information.  []}]
bak-fts commented 4 months ago

Should I split this issue?

djeremiah commented 4 months ago

These should probably be split, because one is a bug and one is a request.

The SecurityCenterContact is a straightforward bug, caused by a hardcoded value in the external name config: https://github.com/upbound/provider-azure/blob/main/config/externalname.go#L1224

The RoleAssignment is a little more complex. Import should work as-is, but in order to import it, you need to specify the full resource id in the crossplane.io/external-name annotation. While it seems like principalId, roleDefinitionName, and scope are enough to uniquely identify a resource, we're doing the lookup using the full resource id. Not a bug, just not the preferred ergonomics. https://github.com/upbound/provider-azure/blob/main/config/externalname.go#L131

bak-fts commented 3 months ago

Split the issue. This issue is now only concerning the SecurityCenterContact resource. Created a new issue #685 for observing RoleAssignments without specifying their full resource ID in the crossplane.io/external-name annotation.