Add azurerm_storage_data_lake_gen2_path with support for folders and ACLs

[stuartleeks]

NOTE that this PR currently has a commit to add in the vendored code for this PR (this will be rebased out once the PR is merged)

This PR adds the start of the azurerm_storage_data_lake_gen2_path resource (#7118) with support for creating folders and ACLs as per this comment.

[stuartleeks]

Rebased and added support for setting folder ACLs (and updated the PR comment above)

Would welcome review of this PR to give time to make any changes so that it is ready for when the corresponding giovanni PR is merged :-)

[stuartleeks]

Rebased now that giovanni is updated to v0.11.0

[stuartleeks]

Rebased on latest master and fixed up CI errors

[jackofallops]

Hi @stuartleeks
Thanks for the PR, afraid I've only had chance to do a fairly quick review here, there are some comments below. I ran the tests and, for me, they all fail. It looks like the delete func either doesn't work as expected, or needs to poll/wait for the operation to complete:

TestAccAzureRMStorageDataLakeGen2Path_basic: testing.go:745: Error destroying resource! WARNING: Dangling resources
may exist. The full state and error is shown below.
Error: Check failed: Path still exists: {Response:{Response:0xc001cf0360} ETag: LastModified:0001-01-01 00:00:00 +0000 UTC ResourceType: Owner: Group: ACL:}

Additionally, there appears to be a permissions issue in setting the ACLs via SetAccessControl:

TestAccAzureRMStorageDataLakeGen2Path_withSimpleACL: testing.go:684: Step 2 error: errors during apply:
Error: Error setting access control for Path "fstest" in File System "testpath" in Storage Account "acctestacc79k41": datalakestore.Client#SetAccessControl: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: Service returned an error. Status=403 Code="AuthorizationPermissionMismatch" Message="This request is not authorized to perform this operation using this permission.\nRequestId:1698a1b8-201f-005e-0e29-5cf5a2000000\nTime:2020-07-17T11:02:39.7133752Z"

If you can address/investigate the above, I'll loop back asap to complete the review.

Thanks!

[stuartleeks]

@jackofallops - thanks for your review. Weird about the tests as they were working locally when I pushed the changes. I'll have to have a dig in and see what's happening there.

I'm on vacation the next two weeks (and likely starting a new project when I get back) but will take a look at this when I get chance.

[jackofallops]

@jackofallops - thanks for your review. Weird about the tests as they were working locally when I pushed the changes. I'll have to have a dig in and see what's happening there.

I'm on vacation the next two weeks (and likely starting a new project when I get back) but will take a look at this when I get chance.

Not a problem, it may be that there are permissions for your user/SP that are not implicit for a subscription owner / GA? It wouldn't be the first time we've had to go dig for explicit permissions for the testing account. If I get chance I'll look into it.

[stuartleeks]

Can you share the test error that you saw? I'm wondering whether the test failed and didn't clean up, or something like that?

The test user needs to have the Storage Blob Data Owner permission, I think.

[jackofallops]

Can you share the test error that you saw? I'm wondering whether the test failed and didn't clean up, or something like that?

The test user needs to have the Storage Blob Data Owner permission, I think.

2 of the 5 test results (_basic, and _withSimpleACL) are included in the review note above, I only kept the error responses, not the full output, sorry. I'll take another look at this next week though, head down in something else I need to complete at the moment. Hopefully have something more by the time you're back from vacation. (have a great time btw :) )

[tombuildsstuff]

@stuartleeks hope you don't mind but I've rebased this and pushed a commit to fix the build failure now the shim layer's been merged - I'll kick off the tests but this should otherwise be good to merge 👍

[stuartleeks]

Thanks for the rebase @tombuildsstuff! Looks like the tests have all passed :-)

[katbyte]

@stuartleeks - it seems the tests for us are failing with:

------- Stdout: -------
=== RUN   TestAccAzureRMStorageDataLakeGen2Path_withOwner
=== PAUSE TestAccAzureRMStorageDataLakeGen2Path_withOwner
=== CONT  TestAccAzureRMStorageDataLakeGen2Path_withOwner
    TestAccAzureRMStorageDataLakeGen2Path_withOwner: testing.go:684: Step 0 error: errors during apply:
        
        Error: Error setting access control for Path "testpath" in File System "fstest" in Storage Account "acctestaccu2vbs": datalakestore.Client#SetAccessControl: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: Service returned an error. Status=403 Code="AuthorizationPermissionMismatch" Message="This request is not authorized to perform this operation using this permission.\nRequestId:275ae0c4-a01f-0018-72c5-bd9a16000000\nTime:2020-11-18T16:12:51.1978201Z"
        
          on /opt/teamcity-agent/temp/buildTmp/tf-test372406367/main.tf line 50:
          (source code not available)
        
        
--- FAIL: TestAccAzureRMStorageDataLakeGen2Path_withOwner (143.75s)
FAIL

any idea why that could be?

[stuartleeks]

@katbyte - ah. Is it possible to assign the account running the tests the Storage Blob Data Owner role?

[tombuildsstuff]

Tests pass:

@stuartleeks as a heads up we ended up pushing a role assignment within the tests, rather than at the subscription level - to be able to differentiate between users who have Storage RP permissions and don't when the shim layer we've added recently is used (to toggle between Data Plane and Resource Manager resources)

[stuartleeks]

@tombuildsstuff - nice, I like the approach!

Thanks for getting this merged :-D

[ghost]

This has been released in version 2.37.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.37.0"
}
# ... other configuration ...

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!