crossplane-contrib / provider-upjet-azure

Official Azure Provider for Crossplane by Upbound.
Apache License 2.0
57 stars 74 forks source link

[Bug]: Storage account always showing diff detected #683

Open fherbert opened 5 months ago

fherbert commented 5 months ago

Is there an existing issue for this?

Affected Resource(s)

storage.azure.upbound.io/v1beta1 - Account

Resource MRs required to reproduce the bug

apiVersion: storage.azure.upbound.io/v1beta1
kind: Account
metadata:
  name: lab
spec:
  providerConfigRef:
    name: azure
  forProvider:
    resourceGroupName: testRG
    location: Australia East
    accountTier: Premium
    accountKind: BlockBlobStorage
    accountReplicationType: ZRS
    allowNestedItemsToBePublic: false
    minTlsVersion: TLS1_2
    networkRules:
      - bypass:
          - Logging
          - Metrics
          - AzureServices
        defaultAction: Deny
        virtualNetworkSubnetIds:
          - /subscriptions/tenantID/resourceGroups/vnRG/providers/Microsoft.Network/virtualNetworks/vnets/subnets/subnet
        ipRules:
          - 15.25.14.0/23
    tags:
      App: TestAPP

Steps to Reproduce

kubectl apply -f MR.yaml

What happened?

Diff detected is continuously being logged, crossplane applies changes, the (we presume) azure modifies it back, crossplane detecs diff, rinse and repeat. Nothing in the provider pod debug logs indicates what is different.

Relevant Error Output Snippet

package-runtime 2024-03-25T03:55:31Z    DEBUG    provider-azure    Diff detected    {"uid": "f1234567-1234-1234-1234-12345bc19469", "name": "lab","gvk": "storage.azure.upbound.io/v1beta1, Kind=Account", "instanceDiff": "*terraform.InstanceDiff{mu:sync.Mutex{state:0, sema:0x0}, Attributes:map[string]*terraform.ResourceAttrDiff{\"queue_properties.#\":*terraform.ResourceAttrDiff{Old:\"\", New:\"\", NewComputed:true, NewRemoved:false, NewExtra:interface {}(nil), RequiresNew:false, Sensitive:false, Type:0x0}, \"share_properties.#\":*terraform.ResourceAttrDiff{Old:\"\", New:\"\", NewComputed:true, NewRemoved:false, NewExtra:interface {}(nil), RequiresNew:false, Sensitive:false, Type:0x0}}, Destroy:false, DestroyDeposed:false, DestroyTainted:false, RawConfig:cty.NilVal, RawState:cty.NilVal, RawPlan:cty.NilVal, Meta:map[string]interface {}(nil)}"}

Crossplane Version

1.15.1

Provider Version

1.0.0

Kubernetes Version

1.27.9

Kubernetes Distribution

AKS

Additional Info

No response

turkenf commented 5 months ago

@fherbert, thank you for raising this issue.

The issue can be reproduced with the information provided, and the UpToDate condition does not come.

2024-03-26T17:39:09+03:00   DEBUG   provider-azure  Diff detected   {"uid": "ccc1f4cc-3fa5-40b4-865f-b22c6816b3bf", "name": "labtefst", "gvk": "storage.azure.upbound.io/v1beta1, Kind=Account", "instanceDiff": "*terraform.InstanceDiff{mu:sync.Mutex{state:0, sema:0x0}, Attributes:map[string]*terraform.ResourceAttrDiff{\"queue_properties.#\":*terraform.ResourceAttrDiff{Old:\"\", New:\"\", NewComputed:true, NewRemoved:false, NewExtra:interface {}(nil), RequiresNew:false, Sensitive:false, Type:0x0}, \"share_properties.#\":*terraform.ResourceAttrDiff{Old:\"\", New:\"\", NewComputed:true, NewRemoved:false, NewExtra:interface {}(nil), RequiresNew:false, Sensitive:false, Type:0x0}}, Destroy:false, DestroyDeposed:false, DestroyTainted:false, RawConfig:cty.NilVal, RawState:cty.NilVal, RawPlan:cty.NilVal, Meta:map[string]interface {}(nil)}"}
2024-03-26T17:39:09+03:00   DEBUG   provider-azure  Successfully requested update of external resource  {"controller": "managed/storage.azure.upbound.io/v1beta1, kind=account", "request": {"name":"labtefst"}, "uid": "ccc1f4cc-3fa5-40b4-865f-b22c6816b3bf", "version": "23939", "external-name": "labtefst", "requeue-after": "2024-03-26T17:49:36+03:00"}
aurel333 commented 3 months ago

I think I am encountering the same issue as the storage account is continuously being reconciled. I want to add that this causes a lot of requests on the Storage Azure API, up to the point that the Azure API request budget can be exhausted for the subscription, which impacts other people using it.

turkenf commented 3 months ago

@aurel33, thank you for your interest, are diff in the same fields? If the logs are recorded, can you share them?

aurel333 commented 3 months ago

I used the following manifest (I removed some values but they do not seem important for this issue):

apiVersion: storage.azure.upbound.io/v1beta2
kind: Account
metadata:
  annotations:
    crossplane.io/external-name: accounttest
  name: account-test
spec:
  deletionPolicy: Delete
  forProvider:
    accessTier: Hot
    accountKind: FileStorage
    accountReplicationType: LRS
    accountTier: Premium
    allowNestedItemsToBePublic: false
    crossTenantReplicationEnabled: true
    enableHttpsTrafficOnly: false
    largeFileShareEnabled: true
    location: northeurope
    minTlsVersion: TLS1_2
    networkRules:
      bypass:
      - AzureServices
      defaultAction: Deny
      ipRules:
      -  ...
      -  ...
      virtualNetworkSubnetIds:
      - ...
      - ...
    publicNetworkAccessEnabled: true
    queueEncryptionKeyType: Service
    resourceGroupName: myrg
    shareProperties:
      retentionPolicy:
        days: 7
      smb: {}
    sharedAccessKeyEnabled: true
    tableEncryptionKeyType: Service
  initProvider: {}

And in the logs I got the following diff:

2024-06-10T17:36:22Z       DEBUG   provider-azure  Diff detected   {"uid": "6c1c0ed4-06cf-4412-aac7-5d24d8eaf8c2", "name": "account-test", "gvk": "storage.azure.upbound.io/v1beta1, Kind=Account", "instanceDiff": "*terraform.InstanceDiff{mu:sync.Mutex{state:0, sema:0x0}, Attributes:map[string]*terraform.ResourceAttrDiff{\"blob_properties.#\":*terraform.ResourceAttrDiff{Old:\"\", New:\"\", NewComputed:true, NewRemoved:false, NewExtra:interface {}(nil), RequiresNew:false, Sensitive:false, Type:0x0}, \"queue_properties.#\":*terraform.ResourceAttrDiff{Old:\"\", New:\"\", NewComputed:true, NewRemoved:false, NewExtra:interface {}(nil), RequiresNew:false, Sensitive:false, Type:0x0}}, Destroy:false, DestroyDeposed:false, DestroyTainted:false, RawConfig:cty.NilVal, RawState:cty.NilVal, RawPlan:cty.NilVal, Meta:map[string]interface {}(nil)}"}

So the detected diffs are on the fields blob_properties and queue_properties. I think what we have in common with @fherbert is that we do not explicitly set the properties detected as drift (though it may depend on the accountKind of the SA).

Also I notice that in both cases it is the field something_properties that is the issue.