Add support to disable authentication for Azure Redis caches

[code-haven]

By default, Azure Redis cache instances have authentication enabled. To disable it, we'll need to set authnotrequired=yes as part of redis_configuration. This PR adds the changes for that.

[tombuildsstuff]

hey @code-haven

Thanks for this PR :)

Taking a look through I believe it'd make more sense to invert this value so that it's a boolean field called authentication_enabled with a default value of true, and then cast to/from the yes/no required in the Azure API, to make this a more Terraform-ey user experience - what do you think?

Thanks!

[code-haven]

@tombuildsstuff Apologies for the force pushes -- wanted to keep the changes under a single commit. I have made the changes that were suggested.

[tombuildsstuff]

hey @code-haven

Thanks for pushing those updates - besides the test naming (which needs the underscore to match the other tests to ensure it's consistent for some tooling - which I'll push a commit for, I hope you don't mind) this LGTM 👍

Thanks!

[tombuildsstuff]

@code-haven no worries about the force pushes, I hope you don't mind I had to push a commit to master; for whatever reason we can push to rebases to branches of a fork, but if we do that to the master branch it breaks the PR, so this was simplest 🤷‍♂

[code-haven]

@tombuildsstuff That's fine. Thank you!

However, there is a problem(or bug ?) I discovered yesterday while testing this and was confirmed by the solutions architect from Microsoft I'm working with at work:

If we have a subnet that has a Redis cache launched withenable_authentication = true, we can't add a second one with enable_authentication = false under that subnet.

* second-redis: Error issuing create request for Redis Cache second-redis-delete (resource group redis-rg): redis.Client#Create: Failure sending request: StatusCode=400 -- Original Error: Code="BadRequest" Message="This subnet could not be used for the request because it contains resource navigation links, such as '<link to first-redis>'. Please try with a different subnet.

However, we can launch the second one with enable_authentication = true and then change it post creation to false. Do I need to document this?

[katbyte]

@code-haven,

looks like we are getting test failures with the new property:

------- Stdout: -------
=== RUN   TestAccAzureRMRedisCache_PatchSchedule
=== PAUSE TestAccAzureRMRedisCache_PatchSchedule
=== CONT  TestAccAzureRMRedisCache_PatchSchedule
--- FAIL: TestAccAzureRMRedisCache_PatchSchedule (68.77s)
    testing.go:568: Step 0 error: errors during apply:
        
        Error: Error issuing create request for Redis Cache acctestRedis-190510050856814028 (resource group acctestRG-190510050856814028): redis.Client#Create: Failure sending request: StatusCode=400 -- Original Error: Code="InvalidRequestBody" Message="The value of the parameter 'properties.redisConfiguration.authnotrequired' is invalid.\r\nRequestID=73d98b42-6902-4d52-bd85-3f118435952a"
        
          on /opt/teamcity-agent/temp/buildTmp/tf-test619733293/main.tf line 7:
          (source code not available)
        
        
FAIL

WRT to that bug there, it sounds like it is on the azure side. Perhaps it would be best to open an issue on the azure-sdk-for-go and link to it here and add a note to the docs explaining the limitation (as well as linking to the issue). WDYT?

[code-haven]

@katbyte @tombuildsstuff
It looks like Azure rejects authnotrequired parameter if subnet_id is not specified.

As a solution, I made enable_authentication optional and only available for caches launched within a subnet so that we'll end up sending the parameter only if the instance is launched within a subnet.

What do you guys think?

[tombuildsstuff]

hey @code-haven

Thanks for pushing those changes, that makes sense to me - aside from one minor fix for the documentation (to ensure it matches the other resources, which I hope you don't mind but I'll push a commit to fix) - this now LGTM 👍

Thanks!

[tombuildsstuff]

Tests pass:

dismissing since changes have been pushed

[tombuildsstuff]

LGTM 👍

[ghost]

This has been released in version 1.28.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 = "~> 1.28.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 👉 hashibot-feedback@hashicorp.com. Thanks!