Add support for Immutable Storage on container

[xperiandri]

Implemented:

• ImmutableStorageWithVersioning
• RequireInfrastructureEncryptionThis PR closes #

I have read the contributing guidelines and have completed the following:

• Tested my code end-to-end against a live Azure subscription.
• Updated the documentation in the docs folder for the affected changes.
• Written unit tests against the modified code that I have made.
• Updated the release notes with a new entry for this PR.
• Checked the coding standards outlined in the contributions guide and ensured my code adheres to them.

If I haven't completed any of the tasks above, I include the reasons why here:
Which test would you recommend to modify or implement?

Below is a minimal example configuration that includes the new features, which can be used to deploy to Azure:

let storage = storageAccount {
    name "wormtest222"
    sku Farmer.Storage.Sku.Standard_GRS
    disable_blob_public_access
    add_private_container
        "container"
        (blobContainerImmutabilityPolicies {
            allow_protected_append_writes AllAppendAllowed
            immutability_period_since_creation (365<Days> * 5)
        })
}

[xperiandri]

@ninjarobot do you have any remarks?

[isaacabraham]

You want to write at least one test confirmation the JSON that is emitted in the generate ARM template is correct i.e. checking fields etc.

[isaacabraham]

From an API design, what happens if you don't want to set any policies - do you need to provide a policy regardless (bear in mind computation expressions don't allow optional arguments AFAIK, and overloads must all have the same number of arguments)? I think perhaps creating a dedicated container { } CE which can have members on it including e.g.:

• name
• accessibility (public / private)
• immutability_policy

would be a better way to go.

[isaacabraham]

Also can you make an issue first which clearly outlines what this PR is designed to solve - normally we create issues, get them approved, and then create a PR to resolve them (in that order :-))

[ninjarobot]

Looks great, thanks for adding this support for newer storage account functionality. I made a few minor suggestions you can take or leave as applicable.

[isaacabraham]

Before merging, we need to modify the API - the use of a CE in this regard isn't going to work, from what I can see, from a usability perspective.

[xperiandri]

From an API design, what happens if you don't want to set any policies - do you need to provide a policy regardless (bear in mind computation expressions don't allow optional arguments AFAIK, and overloads must all have the same number of arguments)?

@isaacabraham, actually it works fine as all the tests pass. I verified that it works both ways and allow that optional parameter to be specified and omitted

[isaacabraham]

@isaacabraham, actually it works fine as all the tests pass. I verified that it works both ways and allow that optional parameter to be specified and omitted

That's good :-) Nonetheless, I think moving to a dedicated container { } CE at this stage makes more sense.

This PR still needs tests for the new code that's been created as well as the other points mentioned (including an issue / issues that reflect the actual changes that are being made here - it's not so much about upgrading the versions but about immutable blobs etc.).

Happy to talk through with you the API choices and anything else that's needed.

[xperiandri]

I thought about that but considered immutable policy as not so big change to introduce container CE.
However further container customization support may need a CE

[xperiandri]

I've implemented tests. Let me know if I miss something

[xperiandri]

Before merging, we need to modify the API - the use of a CE in this regard isn't going to work, from what I can see, from a usability perspective.

@isaacabraham my idea was to add now and preserve in the future ability to specify immutability policy
And add blobContainer CE and operations to storageAccount CE accepting BlobContainerConfig along with supporting additional container settings.
I have no intent to add them right now as it widens the scope of this PR.
Do you think blobContainer should be coupled with these changes?

[isaacabraham]

I think so, yes. If you look elsewhere in the Farmer API, you won't really much in the way of CEs being supplied as secondary arguments to a custom keyword, and I really would like to retain the pattern that we have.

You can repurpose the CE you've already built to actually be the container one - just add in the extra name parameter. You can then add that as an overload for the top-level CE add_private_container member.

What do you think?

Thanks for adding the tests.

[xperiandri]

@isaacabraham I've implemented storageBlobContainer CE as you suggested.
Any other remarks?

[ninjarobot]

@isaacabraham can you please review this again?