From 6ebbc0e1592b2116b0c26cf3823d37e32259f95e Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Thu, 8 Aug 2024 12:57:23 +1000 Subject: [PATCH 01/15] azurerm_redis_cache: access_keys_authentication_disabled property (#26797) --- .../services/redis/redis_cache_data_source.go | 7 +++ .../redis/redis_cache_data_source_test.go | 1 + .../services/redis/redis_cache_resource.go | 21 ++++++- .../redis/redis_cache_resource_test.go | 57 +++++++++++++++++++ .../redis/validate/access_keys_auth.go | 18 ++++++ .../redis/validate/access_keys_auth_test.go | 17 ++++++ website/docs/d/redis_cache.html.markdown | 2 + website/docs/r/redis_cache.html.markdown | 2 + 8 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 internal/services/redis/validate/access_keys_auth.go create mode 100644 internal/services/redis/validate/access_keys_auth_test.go diff --git a/internal/services/redis/redis_cache_data_source.go b/internal/services/redis/redis_cache_data_source.go index 792be98bb762..a584dec28f45 100644 --- a/internal/services/redis/redis_cache_data_source.go +++ b/internal/services/redis/redis_cache_data_source.go @@ -235,6 +235,11 @@ func dataSourceRedisCache() *pluginsdk.Resource { Sensitive: true, }, + "access_keys_authentication_disabled": { + Type: pluginsdk.TypeBool, + Computed: true, + }, + "tags": commonschema.TagsDataSource(), }, } @@ -344,6 +349,8 @@ func dataSourceRedisCacheRead(d *pluginsdk.ResourceData, meta interface{}) error d.Set("primary_connection_string", getRedisConnectionString(*props.HostName, *props.SslPort, *keys.Model.PrimaryKey, enableSslPort)) d.Set("secondary_connection_string", getRedisConnectionString(*props.HostName, *props.SslPort, *keys.Model.SecondaryKey, enableSslPort)) + d.Set("access_keys_authentication_disabled", *props.DisableAccessKeyAuthentication) + if err := tags.FlattenAndSet(d, model.Tags); err != nil { return fmt.Errorf("setting `tags`: %+v", err) } diff --git a/internal/services/redis/redis_cache_data_source_test.go b/internal/services/redis/redis_cache_data_source_test.go index ff753f9a1096..154e5a570821 100644 --- a/internal/services/redis/redis_cache_data_source_test.go +++ b/internal/services/redis/redis_cache_data_source_test.go @@ -30,6 +30,7 @@ func TestAccRedisCacheDataSource_standard(t *testing.T) { check.That(data.ResourceName).Key("tags.environment").HasValue("production"), check.That(data.ResourceName).Key("primary_connection_string").Exists(), check.That(data.ResourceName).Key("secondary_connection_string").Exists(), + check.That(data.ResourceName).Key("access_keys_authentication_disabled").Exists(), ), }, }) diff --git a/internal/services/redis/redis_cache_resource.go b/internal/services/redis/redis_cache_resource.go index d2072b15db07..9be7ed289da8 100644 --- a/internal/services/redis/redis_cache_resource.go +++ b/internal/services/redis/redis_cache_resource.go @@ -365,6 +365,11 @@ func resourceRedisCache() *pluginsdk.Resource { }, }, + "access_keys_authentication_disabled": { + Type: pluginsdk.TypeBool, + Optional: true, + }, + "tags": commonschema.Tags(), }, @@ -376,6 +381,12 @@ func resourceRedisCache() *pluginsdk.Resource { } return false }), + pluginsdk.CustomizeDiffShim(func(ctx context.Context, diff *pluginsdk.ResourceDiff, v interface{}) error { + return validate.ValidateAccessKeysAuth( + diff.Get("access_keys_authentication_disabled").(bool), + diff.Get("redis_configuration.0.active_directory_authentication_enabled").(bool), + ) + }), ), } @@ -499,7 +510,8 @@ func resourceRedisCacheCreate(d *pluginsdk.ResourceData, meta interface{}) error parameters := redis.RedisCreateParameters{ Location: location.Normalize(d.Get("location").(string)), Properties: redis.RedisCreateProperties{ - EnableNonSslPort: pointer.To(enableNonSslPort.(bool)), + DisableAccessKeyAuthentication: pointer.To(d.Get("access_keys_authentication_disabled").(bool)), + EnableNonSslPort: pointer.To(enableNonSslPort.(bool)), Sku: redis.Sku{ Capacity: int64(d.Get("capacity").(int)), Family: redis.SkuFamily(d.Get("family").(string)), @@ -613,8 +625,9 @@ func resourceRedisCacheUpdate(d *pluginsdk.ResourceData, meta interface{}) error parameters := redis.RedisUpdateParameters{ Properties: &redis.RedisUpdateProperties{ - MinimumTlsVersion: pointer.To(redis.TlsVersion(d.Get("minimum_tls_version").(string))), - EnableNonSslPort: pointer.To(enableNonSslPort.(bool)), + DisableAccessKeyAuthentication: pointer.To(d.Get("access_keys_authentication_disabled").(bool)), + MinimumTlsVersion: pointer.To(redis.TlsVersion(d.Get("minimum_tls_version").(string))), + EnableNonSslPort: pointer.To(enableNonSslPort.(bool)), Sku: &redis.Sku{ Capacity: int64(d.Get("capacity").(int)), Family: redis.SkuFamily(d.Get("family").(string)), @@ -837,6 +850,8 @@ func resourceRedisCacheRead(d *pluginsdk.ResourceData, meta interface{}) error { d.Set("primary_access_key", keysResp.Model.PrimaryKey) d.Set("secondary_access_key", keysResp.Model.SecondaryKey) + d.Set("access_keys_authentication_disabled", props.DisableAccessKeyAuthentication) + if err := tags.FlattenAndSet(d, model.Tags); err != nil { return fmt.Errorf("setting `tags`: %+v", err) } diff --git a/internal/services/redis/redis_cache_resource_test.go b/internal/services/redis/redis_cache_resource_test.go index a3d458980bdf..1ce24148f076 100644 --- a/internal/services/redis/redis_cache_resource_test.go +++ b/internal/services/redis/redis_cache_resource_test.go @@ -561,6 +561,35 @@ func TestAccRedisCache_SkuDowngrade(t *testing.T) { }) } +func TestAccRedisCache_AccessKeysAuthenticationEnabledDisabled(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_redis_cache", "test") + r := RedisCacheResource{} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.accessKeysAuthentication(data, false, false), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: r.accessKeysAuthentication(data, true, true), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: r.accessKeysAuthentication(data, false, false), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + func (t RedisCacheResource) Exists(ctx context.Context, clients *clients.Client, state *pluginsdk.InstanceState) (*bool, error) { id, err := redis.ParseRediID(state.ID) if err != nil { @@ -1594,3 +1623,31 @@ resource "azurerm_redis_cache" "test" { } `, data.RandomInteger, data.Locations.Primary, data.RandomInteger) } + +func (RedisCacheResource) accessKeysAuthentication(data acceptance.TestData, accessKeysAuthenticationDisabled bool, activeDirectoryAuthenticationEnabled bool) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_redis_cache" "test" { + name = "acctestRedis-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + capacity = 1 + family = "C" + sku_name = "Basic" + non_ssl_port_enabled = false + minimum_tls_version = "1.2" + access_keys_authentication_disabled = %t + + redis_configuration { + active_directory_authentication_enabled = %t + } +}`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, accessKeysAuthenticationDisabled, accessKeysAuthenticationDisabled) +} diff --git a/internal/services/redis/validate/access_keys_auth.go b/internal/services/redis/validate/access_keys_auth.go new file mode 100644 index 000000000000..496f82a687c9 --- /dev/null +++ b/internal/services/redis/validate/access_keys_auth.go @@ -0,0 +1,18 @@ +package validate + +import ( + "fmt" + "log" +) + +// Entra (AD) auth has to be set to disable access keys auth +// https://learn.microsoft.com/en-us/azure/azure-cache-for-redis/cache-azure-active-directory-for-authentication +func ValidateAccessKeysAuth(accessKeysAuthenticationDisabled bool, activeDirectoryAuthenticationEnabled bool) error { + log.Printf("[DEBUG] ValidateAccessKeysAuth: accessKeysAuthenticationDisabled: %v, activeDirectoryAuthenticationEnabled: %v", accessKeysAuthenticationDisabled, activeDirectoryAuthenticationEnabled) + + if accessKeysAuthenticationDisabled && !activeDirectoryAuthenticationEnabled { + return fmt.Errorf("microsoft entra authorization (active_directory_authentication_enabled) must be enabled in order to disable access key authentication (access_keys_authentication_disabled): https://learn.microsoft.com/en-us/azure/azure-cache-for-redis/cache-azure-active-directory-for-authentication#disable-access-key-authentication-on-your-cache") + } + + return nil +} diff --git a/internal/services/redis/validate/access_keys_auth_test.go b/internal/services/redis/validate/access_keys_auth_test.go new file mode 100644 index 000000000000..0f1f5962933c --- /dev/null +++ b/internal/services/redis/validate/access_keys_auth_test.go @@ -0,0 +1,17 @@ +package validate + +import ( + "testing" +) + +func TestValidateAccessKeysAuth_valid(t *testing.T) { + if err := ValidateAccessKeysAuth(true, true); err != nil { + t.Fatalf("Should be valid if accessKeysAuthenticationDisabled: true and activeDirectoryAuthenticationEnabled: true but got error: %v", err) + } +} + +func TestValidateAccessKeysAuth_invalid(t *testing.T) { + if err := ValidateAccessKeysAuth(true, false); err == nil { + t.Fatalf("Should return error if accessKeysAuthenticationDisabled: true and activeDirectoryAuthenticationEnabled: false") + } +} diff --git a/website/docs/d/redis_cache.html.markdown b/website/docs/d/redis_cache.html.markdown index 511a4ef99933..8b5ee43d9716 100644 --- a/website/docs/d/redis_cache.html.markdown +++ b/website/docs/d/redis_cache.html.markdown @@ -68,6 +68,8 @@ output "hostname" { * `secondary_connection_string` - The secondary connection string of the Redis Instance. +* `access_keys_authentication_disabled` - Whether access key authentication is disabled. + * `redis_configuration` - A `redis_configuration` block as defined below. * `zones` - A list of Availability Zones in which this Redis Cache is located. diff --git a/website/docs/r/redis_cache.html.markdown b/website/docs/r/redis_cache.html.markdown index 5f504153f6d9..1c4e6d25a060 100644 --- a/website/docs/r/redis_cache.html.markdown +++ b/website/docs/r/redis_cache.html.markdown @@ -59,6 +59,8 @@ The following arguments are supported: --- +* `access_keys_authentication_disabled` - (Optional) Disable access key authentication. Microsoft Entra (AAD) authentication (`active_directory_authentication_enabled`) must be enabled to disable this. Defaults to `false`. + * `enable_non_ssl_port` - (Optional) Enable the non-SSL port (6379) - disabled by default. * `identity` - (Optional) An `identity` block as defined below. From 9dcb9a38fd0f232861383d0db9e7b6e6affa85d7 Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Thu, 15 Aug 2024 00:14:55 +1000 Subject: [PATCH 02/15] Fix tflint and unused test func arg --- internal/services/redis/redis_cache_data_source.go | 2 +- internal/services/redis/redis_cache_resource_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/services/redis/redis_cache_data_source.go b/internal/services/redis/redis_cache_data_source.go index a584dec28f45..4d2d95cf242c 100644 --- a/internal/services/redis/redis_cache_data_source.go +++ b/internal/services/redis/redis_cache_data_source.go @@ -349,7 +349,7 @@ func dataSourceRedisCacheRead(d *pluginsdk.ResourceData, meta interface{}) error d.Set("primary_connection_string", getRedisConnectionString(*props.HostName, *props.SslPort, *keys.Model.PrimaryKey, enableSslPort)) d.Set("secondary_connection_string", getRedisConnectionString(*props.HostName, *props.SslPort, *keys.Model.SecondaryKey, enableSslPort)) - d.Set("access_keys_authentication_disabled", *props.DisableAccessKeyAuthentication) + d.Set("access_keys_authentication_disabled", props.DisableAccessKeyAuthentication) if err := tags.FlattenAndSet(d, model.Tags); err != nil { return fmt.Errorf("setting `tags`: %+v", err) diff --git a/internal/services/redis/redis_cache_resource_test.go b/internal/services/redis/redis_cache_resource_test.go index 1ce24148f076..4535112280bc 100644 --- a/internal/services/redis/redis_cache_resource_test.go +++ b/internal/services/redis/redis_cache_resource_test.go @@ -1649,5 +1649,5 @@ resource "azurerm_redis_cache" "test" { redis_configuration { active_directory_authentication_enabled = %t } -}`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, accessKeysAuthenticationDisabled, accessKeysAuthenticationDisabled) +}`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, accessKeysAuthenticationDisabled, activeDirectoryAuthenticationEnabled) } From 0055fb8f0357b11f7e141c9beadf0bd91e928f5a Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Fri, 16 Aug 2024 12:33:27 +1000 Subject: [PATCH 03/15] Update website/docs/d/redis_cache.html.markdown Co-authored-by: magodo --- website/docs/d/redis_cache.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/d/redis_cache.html.markdown b/website/docs/d/redis_cache.html.markdown index 8b5ee43d9716..eea8494fc2a4 100644 --- a/website/docs/d/redis_cache.html.markdown +++ b/website/docs/d/redis_cache.html.markdown @@ -68,7 +68,7 @@ output "hostname" { * `secondary_connection_string` - The secondary connection string of the Redis Instance. -* `access_keys_authentication_disabled` - Whether access key authentication is disabled. +* `access_keys_authentication_disabled` - Specifies if access key authentication is disabled. * `redis_configuration` - A `redis_configuration` block as defined below. From bf232ce70acb2c7ef9ab6e363feb640417cf82e4 Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Fri, 16 Aug 2024 12:30:59 +1000 Subject: [PATCH 04/15] Inlined access keys auth validation into CustomizeDiff --- .../services/redis/redis_cache_resource.go | 17 +++++++++++++---- .../redis/validate/access_keys_auth.go | 18 ------------------ .../redis/validate/access_keys_auth_test.go | 17 ----------------- 3 files changed, 13 insertions(+), 39 deletions(-) delete mode 100644 internal/services/redis/validate/access_keys_auth.go delete mode 100644 internal/services/redis/validate/access_keys_auth_test.go diff --git a/internal/services/redis/redis_cache_resource.go b/internal/services/redis/redis_cache_resource.go index 9be7ed289da8..e3d602666e00 100644 --- a/internal/services/redis/redis_cache_resource.go +++ b/internal/services/redis/redis_cache_resource.go @@ -382,10 +382,19 @@ func resourceRedisCache() *pluginsdk.Resource { return false }), pluginsdk.CustomizeDiffShim(func(ctx context.Context, diff *pluginsdk.ResourceDiff, v interface{}) error { - return validate.ValidateAccessKeysAuth( - diff.Get("access_keys_authentication_disabled").(bool), - diff.Get("redis_configuration.0.active_directory_authentication_enabled").(bool), - ) + // Entra (AD) auth has to be set to disable access keys auth + // https://learn.microsoft.com/en-us/azure/azure-cache-for-redis/cache-azure-active-directory-for-authentication + + accessKeysAuthenticationDisabled := diff.Get("access_keys_authentication_disabled").(bool) + activeDirectoryAuthenticationEnabled := diff.Get("redis_configuration.0.active_directory_authentication_enabled").(bool) + + log.Printf("[DEBUG] ValidateAccessKeysAuth: accessKeysAuthenticationDisabled: %v, activeDirectoryAuthenticationEnabled: %v", accessKeysAuthenticationDisabled, activeDirectoryAuthenticationEnabled) + + if accessKeysAuthenticationDisabled && !activeDirectoryAuthenticationEnabled { + return fmt.Errorf("microsoft entra authorization (active_directory_authentication_enabled) must be enabled in order to disable access key authentication (access_keys_authentication_disabled): https://learn.microsoft.com/en-us/azure/azure-cache-for-redis/cache-azure-active-directory-for-authentication#disable-access-key-authentication-on-your-cache") + } + + return nil }), ), } diff --git a/internal/services/redis/validate/access_keys_auth.go b/internal/services/redis/validate/access_keys_auth.go deleted file mode 100644 index 496f82a687c9..000000000000 --- a/internal/services/redis/validate/access_keys_auth.go +++ /dev/null @@ -1,18 +0,0 @@ -package validate - -import ( - "fmt" - "log" -) - -// Entra (AD) auth has to be set to disable access keys auth -// https://learn.microsoft.com/en-us/azure/azure-cache-for-redis/cache-azure-active-directory-for-authentication -func ValidateAccessKeysAuth(accessKeysAuthenticationDisabled bool, activeDirectoryAuthenticationEnabled bool) error { - log.Printf("[DEBUG] ValidateAccessKeysAuth: accessKeysAuthenticationDisabled: %v, activeDirectoryAuthenticationEnabled: %v", accessKeysAuthenticationDisabled, activeDirectoryAuthenticationEnabled) - - if accessKeysAuthenticationDisabled && !activeDirectoryAuthenticationEnabled { - return fmt.Errorf("microsoft entra authorization (active_directory_authentication_enabled) must be enabled in order to disable access key authentication (access_keys_authentication_disabled): https://learn.microsoft.com/en-us/azure/azure-cache-for-redis/cache-azure-active-directory-for-authentication#disable-access-key-authentication-on-your-cache") - } - - return nil -} diff --git a/internal/services/redis/validate/access_keys_auth_test.go b/internal/services/redis/validate/access_keys_auth_test.go deleted file mode 100644 index 0f1f5962933c..000000000000 --- a/internal/services/redis/validate/access_keys_auth_test.go +++ /dev/null @@ -1,17 +0,0 @@ -package validate - -import ( - "testing" -) - -func TestValidateAccessKeysAuth_valid(t *testing.T) { - if err := ValidateAccessKeysAuth(true, true); err != nil { - t.Fatalf("Should be valid if accessKeysAuthenticationDisabled: true and activeDirectoryAuthenticationEnabled: true but got error: %v", err) - } -} - -func TestValidateAccessKeysAuth_invalid(t *testing.T) { - if err := ValidateAccessKeysAuth(true, false); err == nil { - t.Fatalf("Should return error if accessKeysAuthenticationDisabled: true and activeDirectoryAuthenticationEnabled: false") - } -} From e7605cb608dc471b53ad93c653dbf9df68556f6d Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Fri, 16 Aug 2024 13:14:13 +1000 Subject: [PATCH 05/15] Improved docs grammar for access_keys_authentication_disabled --- website/docs/r/redis_cache.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/redis_cache.html.markdown b/website/docs/r/redis_cache.html.markdown index 1c4e6d25a060..bb5be90efb1c 100644 --- a/website/docs/r/redis_cache.html.markdown +++ b/website/docs/r/redis_cache.html.markdown @@ -59,7 +59,7 @@ The following arguments are supported: --- -* `access_keys_authentication_disabled` - (Optional) Disable access key authentication. Microsoft Entra (AAD) authentication (`active_directory_authentication_enabled`) must be enabled to disable this. Defaults to `false`. +* `access_keys_authentication_disabled` - (Optional) Whether access key authentication is disabled. `active_directory_authentication_enabled` must be set to `true` to disable access key authentication. Defaults to `false`. * `enable_non_ssl_port` - (Optional) Enable the non-SSL port (6379) - disabled by default. From c71024ae94721c9e52c39269bfe3f17104a99f60 Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Fri, 16 Aug 2024 13:45:52 +1000 Subject: [PATCH 06/15] Further improvement on website/docs/r/redis_cache.html.markdown --- website/docs/r/redis_cache.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/redis_cache.html.markdown b/website/docs/r/redis_cache.html.markdown index bb5be90efb1c..88b7fe509c66 100644 --- a/website/docs/r/redis_cache.html.markdown +++ b/website/docs/r/redis_cache.html.markdown @@ -59,7 +59,7 @@ The following arguments are supported: --- -* `access_keys_authentication_disabled` - (Optional) Whether access key authentication is disabled. `active_directory_authentication_enabled` must be set to `true` to disable access key authentication. Defaults to `false`. +* `access_keys_authentication_disabled` - (Optional) Whether access key authentication is disabled? Defaults to `false`. `active_directory_authentication_enabled` must be set to `true` to disable access key authentication. * `enable_non_ssl_port` - (Optional) Enable the non-SSL port (6379) - disabled by default. From bc869cc576998127ddb01fb662e99533fbdc16ed Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Fri, 16 Aug 2024 14:11:21 +1000 Subject: [PATCH 07/15] Improved debug log message Co-authored-by: magodo --- internal/services/redis/redis_cache_resource.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/services/redis/redis_cache_resource.go b/internal/services/redis/redis_cache_resource.go index e3d602666e00..af97b37160c4 100644 --- a/internal/services/redis/redis_cache_resource.go +++ b/internal/services/redis/redis_cache_resource.go @@ -388,7 +388,7 @@ func resourceRedisCache() *pluginsdk.Resource { accessKeysAuthenticationDisabled := diff.Get("access_keys_authentication_disabled").(bool) activeDirectoryAuthenticationEnabled := diff.Get("redis_configuration.0.active_directory_authentication_enabled").(bool) - log.Printf("[DEBUG] ValidateAccessKeysAuth: accessKeysAuthenticationDisabled: %v, activeDirectoryAuthenticationEnabled: %v", accessKeysAuthenticationDisabled, activeDirectoryAuthenticationEnabled) + log.Printf("[DEBUG] CustomizeDiff: access_keys_authentication_disabled: %v, active_directory_authentication_enabled: %v", accessKeysAuthenticationDisabled, activeDirectoryAuthenticationEnabled) if accessKeysAuthenticationDisabled && !activeDirectoryAuthenticationEnabled { return fmt.Errorf("microsoft entra authorization (active_directory_authentication_enabled) must be enabled in order to disable access key authentication (access_keys_authentication_disabled): https://learn.microsoft.com/en-us/azure/azure-cache-for-redis/cache-azure-active-directory-for-authentication#disable-access-key-authentication-on-your-cache") From c04488b86907aadce9e4126b30cdeada3c290363 Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Fri, 16 Aug 2024 14:12:52 +1000 Subject: [PATCH 08/15] Improved error message Co-authored-by: magodo --- internal/services/redis/redis_cache_resource.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/services/redis/redis_cache_resource.go b/internal/services/redis/redis_cache_resource.go index af97b37160c4..480dfb726d10 100644 --- a/internal/services/redis/redis_cache_resource.go +++ b/internal/services/redis/redis_cache_resource.go @@ -391,7 +391,7 @@ func resourceRedisCache() *pluginsdk.Resource { log.Printf("[DEBUG] CustomizeDiff: access_keys_authentication_disabled: %v, active_directory_authentication_enabled: %v", accessKeysAuthenticationDisabled, activeDirectoryAuthenticationEnabled) if accessKeysAuthenticationDisabled && !activeDirectoryAuthenticationEnabled { - return fmt.Errorf("microsoft entra authorization (active_directory_authentication_enabled) must be enabled in order to disable access key authentication (access_keys_authentication_disabled): https://learn.microsoft.com/en-us/azure/azure-cache-for-redis/cache-azure-active-directory-for-authentication#disable-access-key-authentication-on-your-cache") + return fmt.Errorf("`active_directory_authentication_enabled` must be enabled in order to enable `access_keys_authentication_disabled`") } return nil From 983a600787c6a059013ac055dee1dda38d7cd113 Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Sun, 18 Aug 2024 22:18:34 +1000 Subject: [PATCH 09/15] Flip prop into _enabled To comply with Hashicorp contrib guide --- .../services/redis/redis_cache_data_source.go | 4 ++-- .../redis/redis_cache_data_source_test.go | 2 +- internal/services/redis/redis_cache_resource.go | 15 ++++++++------- .../services/redis/redis_cache_resource_test.go | 12 ++++++------ website/docs/d/redis_cache.html.markdown | 2 +- website/docs/r/redis_cache.html.markdown | 2 +- 6 files changed, 19 insertions(+), 18 deletions(-) diff --git a/internal/services/redis/redis_cache_data_source.go b/internal/services/redis/redis_cache_data_source.go index 4d2d95cf242c..6600a666de47 100644 --- a/internal/services/redis/redis_cache_data_source.go +++ b/internal/services/redis/redis_cache_data_source.go @@ -235,7 +235,7 @@ func dataSourceRedisCache() *pluginsdk.Resource { Sensitive: true, }, - "access_keys_authentication_disabled": { + "access_keys_authentication_enabled": { Type: pluginsdk.TypeBool, Computed: true, }, @@ -349,7 +349,7 @@ func dataSourceRedisCacheRead(d *pluginsdk.ResourceData, meta interface{}) error d.Set("primary_connection_string", getRedisConnectionString(*props.HostName, *props.SslPort, *keys.Model.PrimaryKey, enableSslPort)) d.Set("secondary_connection_string", getRedisConnectionString(*props.HostName, *props.SslPort, *keys.Model.SecondaryKey, enableSslPort)) - d.Set("access_keys_authentication_disabled", props.DisableAccessKeyAuthentication) + d.Set("access_keys_authentication_enabled", !(*props.DisableAccessKeyAuthentication)) if err := tags.FlattenAndSet(d, model.Tags); err != nil { return fmt.Errorf("setting `tags`: %+v", err) diff --git a/internal/services/redis/redis_cache_data_source_test.go b/internal/services/redis/redis_cache_data_source_test.go index 154e5a570821..acdf1ebb17ce 100644 --- a/internal/services/redis/redis_cache_data_source_test.go +++ b/internal/services/redis/redis_cache_data_source_test.go @@ -30,7 +30,7 @@ func TestAccRedisCacheDataSource_standard(t *testing.T) { check.That(data.ResourceName).Key("tags.environment").HasValue("production"), check.That(data.ResourceName).Key("primary_connection_string").Exists(), check.That(data.ResourceName).Key("secondary_connection_string").Exists(), - check.That(data.ResourceName).Key("access_keys_authentication_disabled").Exists(), + check.That(data.ResourceName).Key("access_keys_authentication_enabled").Exists(), ), }, }) diff --git a/internal/services/redis/redis_cache_resource.go b/internal/services/redis/redis_cache_resource.go index 480dfb726d10..c14db6b517cb 100644 --- a/internal/services/redis/redis_cache_resource.go +++ b/internal/services/redis/redis_cache_resource.go @@ -365,9 +365,10 @@ func resourceRedisCache() *pluginsdk.Resource { }, }, - "access_keys_authentication_disabled": { + "access_keys_authentication_enabled": { Type: pluginsdk.TypeBool, Optional: true, + Default: true, }, "tags": commonschema.Tags(), @@ -385,13 +386,13 @@ func resourceRedisCache() *pluginsdk.Resource { // Entra (AD) auth has to be set to disable access keys auth // https://learn.microsoft.com/en-us/azure/azure-cache-for-redis/cache-azure-active-directory-for-authentication - accessKeysAuthenticationDisabled := diff.Get("access_keys_authentication_disabled").(bool) + accessKeysAuthenticationDisabled := !(diff.Get("access_keys_authentication_enabled").(bool)) activeDirectoryAuthenticationEnabled := diff.Get("redis_configuration.0.active_directory_authentication_enabled").(bool) - log.Printf("[DEBUG] CustomizeDiff: access_keys_authentication_disabled: %v, active_directory_authentication_enabled: %v", accessKeysAuthenticationDisabled, activeDirectoryAuthenticationEnabled) + log.Printf("[DEBUG] CustomizeDiff: access_keys_authentication_enabled: %v, active_directory_authentication_enabled: %v", accessKeysAuthenticationDisabled, activeDirectoryAuthenticationEnabled) if accessKeysAuthenticationDisabled && !activeDirectoryAuthenticationEnabled { - return fmt.Errorf("`active_directory_authentication_enabled` must be enabled in order to enable `access_keys_authentication_disabled`") + return fmt.Errorf("`active_directory_authentication_enabled` must be enabled in order to disable `access_keys_authentication_enabled`") } return nil @@ -519,7 +520,7 @@ func resourceRedisCacheCreate(d *pluginsdk.ResourceData, meta interface{}) error parameters := redis.RedisCreateParameters{ Location: location.Normalize(d.Get("location").(string)), Properties: redis.RedisCreateProperties{ - DisableAccessKeyAuthentication: pointer.To(d.Get("access_keys_authentication_disabled").(bool)), + DisableAccessKeyAuthentication: pointer.To(!(d.Get("access_keys_authentication_enabled").(bool))), EnableNonSslPort: pointer.To(enableNonSslPort.(bool)), Sku: redis.Sku{ Capacity: int64(d.Get("capacity").(int)), @@ -634,7 +635,7 @@ func resourceRedisCacheUpdate(d *pluginsdk.ResourceData, meta interface{}) error parameters := redis.RedisUpdateParameters{ Properties: &redis.RedisUpdateProperties{ - DisableAccessKeyAuthentication: pointer.To(d.Get("access_keys_authentication_disabled").(bool)), + DisableAccessKeyAuthentication: pointer.To(!(d.Get("access_keys_authentication_enabled").(bool))), MinimumTlsVersion: pointer.To(redis.TlsVersion(d.Get("minimum_tls_version").(string))), EnableNonSslPort: pointer.To(enableNonSslPort.(bool)), Sku: &redis.Sku{ @@ -859,7 +860,7 @@ func resourceRedisCacheRead(d *pluginsdk.ResourceData, meta interface{}) error { d.Set("primary_access_key", keysResp.Model.PrimaryKey) d.Set("secondary_access_key", keysResp.Model.SecondaryKey) - d.Set("access_keys_authentication_disabled", props.DisableAccessKeyAuthentication) + d.Set("access_keys_authentication_enabled", !(*props.DisableAccessKeyAuthentication)) if err := tags.FlattenAndSet(d, model.Tags); err != nil { return fmt.Errorf("setting `tags`: %+v", err) diff --git a/internal/services/redis/redis_cache_resource_test.go b/internal/services/redis/redis_cache_resource_test.go index 4535112280bc..0090e4e5c610 100644 --- a/internal/services/redis/redis_cache_resource_test.go +++ b/internal/services/redis/redis_cache_resource_test.go @@ -567,21 +567,21 @@ func TestAccRedisCache_AccessKeysAuthenticationEnabledDisabled(t *testing.T) { data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.accessKeysAuthentication(data, false, false), + Config: r.accessKeysAuthentication(data, true, false), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), }, data.ImportStep(), { - Config: r.accessKeysAuthentication(data, true, true), + Config: r.accessKeysAuthentication(data, false, true), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), }, data.ImportStep(), { - Config: r.accessKeysAuthentication(data, false, false), + Config: r.accessKeysAuthentication(data, true, false), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -1624,7 +1624,7 @@ resource "azurerm_redis_cache" "test" { `, data.RandomInteger, data.Locations.Primary, data.RandomInteger) } -func (RedisCacheResource) accessKeysAuthentication(data acceptance.TestData, accessKeysAuthenticationDisabled bool, activeDirectoryAuthenticationEnabled bool) string { +func (RedisCacheResource) accessKeysAuthentication(data acceptance.TestData, accessKeysAuthenticationEnabled bool, activeDirectoryAuthenticationEnabled bool) string { return fmt.Sprintf(` provider "azurerm" { features {} @@ -1644,10 +1644,10 @@ resource "azurerm_redis_cache" "test" { sku_name = "Basic" non_ssl_port_enabled = false minimum_tls_version = "1.2" - access_keys_authentication_disabled = %t + access_keys_authentication_enabled = %t redis_configuration { active_directory_authentication_enabled = %t } -}`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, accessKeysAuthenticationDisabled, activeDirectoryAuthenticationEnabled) +}`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, accessKeysAuthenticationEnabled, activeDirectoryAuthenticationEnabled) } diff --git a/website/docs/d/redis_cache.html.markdown b/website/docs/d/redis_cache.html.markdown index eea8494fc2a4..efac25cc237c 100644 --- a/website/docs/d/redis_cache.html.markdown +++ b/website/docs/d/redis_cache.html.markdown @@ -68,7 +68,7 @@ output "hostname" { * `secondary_connection_string` - The secondary connection string of the Redis Instance. -* `access_keys_authentication_disabled` - Specifies if access key authentication is disabled. +* `access_keys_authentication_enabled` - Specifies if access key authentication is enabled. * `redis_configuration` - A `redis_configuration` block as defined below. diff --git a/website/docs/r/redis_cache.html.markdown b/website/docs/r/redis_cache.html.markdown index 88b7fe509c66..4d5365c3ade8 100644 --- a/website/docs/r/redis_cache.html.markdown +++ b/website/docs/r/redis_cache.html.markdown @@ -59,7 +59,7 @@ The following arguments are supported: --- -* `access_keys_authentication_disabled` - (Optional) Whether access key authentication is disabled? Defaults to `false`. `active_directory_authentication_enabled` must be set to `true` to disable access key authentication. +* `access_keys_authentication_enabled` - (Optional) Whether access key authentication is enabled? Defaults to `true`. `active_directory_authentication_enabled` must be set to `true` to disable access key authentication. * `enable_non_ssl_port` - (Optional) Enable the non-SSL port (6379) - disabled by default. From 7e4a511261fcf67a4174980f3010d555e0ca8971 Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Mon, 19 Aug 2024 15:26:25 +1000 Subject: [PATCH 10/15] Fix linting error on redis_cache_resource_test.go --- .../services/redis/redis_cache_resource_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/services/redis/redis_cache_resource_test.go b/internal/services/redis/redis_cache_resource_test.go index 0090e4e5c610..94b0af7f5055 100644 --- a/internal/services/redis/redis_cache_resource_test.go +++ b/internal/services/redis/redis_cache_resource_test.go @@ -1636,14 +1636,14 @@ resource "azurerm_resource_group" "test" { } resource "azurerm_redis_cache" "test" { - name = "acctestRedis-%d" - location = azurerm_resource_group.test.location - resource_group_name = azurerm_resource_group.test.name - capacity = 1 - family = "C" - sku_name = "Basic" - non_ssl_port_enabled = false - minimum_tls_version = "1.2" + name = "acctestRedis-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + capacity = 1 + family = "C" + sku_name = "Basic" + non_ssl_port_enabled = false + minimum_tls_version = "1.2" access_keys_authentication_enabled = %t redis_configuration { From 882fecdf6c89a298bdeeab9cd59e6e94af080834 Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Mon, 19 Aug 2024 15:33:22 +1000 Subject: [PATCH 11/15] Added nil check on redis_cache_data_source.go Co-authored-by: magodo --- internal/services/redis/redis_cache_data_source.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/services/redis/redis_cache_data_source.go b/internal/services/redis/redis_cache_data_source.go index 6600a666de47..d58729cce981 100644 --- a/internal/services/redis/redis_cache_data_source.go +++ b/internal/services/redis/redis_cache_data_source.go @@ -349,7 +349,11 @@ func dataSourceRedisCacheRead(d *pluginsdk.ResourceData, meta interface{}) error d.Set("primary_connection_string", getRedisConnectionString(*props.HostName, *props.SslPort, *keys.Model.PrimaryKey, enableSslPort)) d.Set("secondary_connection_string", getRedisConnectionString(*props.HostName, *props.SslPort, *keys.Model.SecondaryKey, enableSslPort)) - d.Set("access_keys_authentication_enabled", !(*props.DisableAccessKeyAuthentication)) + accessKeyAuthEnabled := true + if props.DisableAccessKeyAuthentication != nil { + accessKeyAuthEnabled = !(*props.DisableAccessKeyAuthentication) + } + d.Set("access_keys_authentication_enabled", accessKeyAuthEnabled) if err := tags.FlattenAndSet(d, model.Tags); err != nil { return fmt.Errorf("setting `tags`: %+v", err) From b384de3dfb75b790a62b95f3d2170a49698a06a3 Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Mon, 19 Aug 2024 15:35:30 +1000 Subject: [PATCH 12/15] Formatting on redis_cache_data_source.go --- internal/services/redis/redis_cache_data_source.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/services/redis/redis_cache_data_source.go b/internal/services/redis/redis_cache_data_source.go index d58729cce981..27f0edd8077b 100644 --- a/internal/services/redis/redis_cache_data_source.go +++ b/internal/services/redis/redis_cache_data_source.go @@ -349,10 +349,10 @@ func dataSourceRedisCacheRead(d *pluginsdk.ResourceData, meta interface{}) error d.Set("primary_connection_string", getRedisConnectionString(*props.HostName, *props.SslPort, *keys.Model.PrimaryKey, enableSslPort)) d.Set("secondary_connection_string", getRedisConnectionString(*props.HostName, *props.SslPort, *keys.Model.SecondaryKey, enableSslPort)) - accessKeyAuthEnabled := true - if props.DisableAccessKeyAuthentication != nil { - accessKeyAuthEnabled = !(*props.DisableAccessKeyAuthentication) - } + accessKeyAuthEnabled := true + if props.DisableAccessKeyAuthentication != nil { + accessKeyAuthEnabled = !(*props.DisableAccessKeyAuthentication) + } d.Set("access_keys_authentication_enabled", accessKeyAuthEnabled) if err := tags.FlattenAndSet(d, model.Tags); err != nil { From a68b4561292cc8cfad0df1b8556fb381b3a07356 Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Mon, 19 Aug 2024 15:55:16 +1000 Subject: [PATCH 13/15] More nil check and boolean prop flipping on redis_cache_resource.go --- internal/services/redis/redis_cache_resource.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/services/redis/redis_cache_resource.go b/internal/services/redis/redis_cache_resource.go index c14db6b517cb..5a2b81736e51 100644 --- a/internal/services/redis/redis_cache_resource.go +++ b/internal/services/redis/redis_cache_resource.go @@ -386,12 +386,12 @@ func resourceRedisCache() *pluginsdk.Resource { // Entra (AD) auth has to be set to disable access keys auth // https://learn.microsoft.com/en-us/azure/azure-cache-for-redis/cache-azure-active-directory-for-authentication - accessKeysAuthenticationDisabled := !(diff.Get("access_keys_authentication_enabled").(bool)) + accessKeysAuthenticationEnabled := diff.Get("access_keys_authentication_enabled").(bool) activeDirectoryAuthenticationEnabled := diff.Get("redis_configuration.0.active_directory_authentication_enabled").(bool) - log.Printf("[DEBUG] CustomizeDiff: access_keys_authentication_enabled: %v, active_directory_authentication_enabled: %v", accessKeysAuthenticationDisabled, activeDirectoryAuthenticationEnabled) + log.Printf("[DEBUG] CustomizeDiff: access_keys_authentication_enabled: %v, active_directory_authentication_enabled: %v", accessKeysAuthenticationEnabled, activeDirectoryAuthenticationEnabled) - if accessKeysAuthenticationDisabled && !activeDirectoryAuthenticationEnabled { + if !accessKeysAuthenticationEnabled && !activeDirectoryAuthenticationEnabled { return fmt.Errorf("`active_directory_authentication_enabled` must be enabled in order to disable `access_keys_authentication_enabled`") } @@ -860,7 +860,11 @@ func resourceRedisCacheRead(d *pluginsdk.ResourceData, meta interface{}) error { d.Set("primary_access_key", keysResp.Model.PrimaryKey) d.Set("secondary_access_key", keysResp.Model.SecondaryKey) - d.Set("access_keys_authentication_enabled", !(*props.DisableAccessKeyAuthentication)) + accessKeyAuthEnabled := true + if props.DisableAccessKeyAuthentication != nil { + accessKeyAuthEnabled = !(*props.DisableAccessKeyAuthentication) + } + d.Set("access_keys_authentication_enabled", accessKeyAuthEnabled) if err := tags.FlattenAndSet(d, model.Tags); err != nil { return fmt.Errorf("setting `tags`: %+v", err) From 50de57ee587fe9e9d575cff105045da044f15726 Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Wed, 21 Aug 2024 10:57:51 +1000 Subject: [PATCH 14/15] PR feedback: use pointer.From --- internal/services/redis/redis_cache_data_source.go | 3 ++- internal/services/redis/redis_cache_resource.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/services/redis/redis_cache_data_source.go b/internal/services/redis/redis_cache_data_source.go index 27f0edd8077b..f783eb20cd86 100644 --- a/internal/services/redis/redis_cache_data_source.go +++ b/internal/services/redis/redis_cache_data_source.go @@ -7,6 +7,7 @@ import ( "fmt" "time" + "github.com/hashicorp/go-azure-helpers/lang/pointer" "github.com/hashicorp/go-azure-helpers/lang/response" "github.com/hashicorp/go-azure-helpers/resourcemanager/commonids" "github.com/hashicorp/go-azure-helpers/resourcemanager/commonschema" @@ -351,7 +352,7 @@ func dataSourceRedisCacheRead(d *pluginsdk.ResourceData, meta interface{}) error accessKeyAuthEnabled := true if props.DisableAccessKeyAuthentication != nil { - accessKeyAuthEnabled = !(*props.DisableAccessKeyAuthentication) + accessKeyAuthEnabled = !pointer.From(props.DisableAccessKeyAuthentication) } d.Set("access_keys_authentication_enabled", accessKeyAuthEnabled) diff --git a/internal/services/redis/redis_cache_resource.go b/internal/services/redis/redis_cache_resource.go index 5a2b81736e51..f50233451096 100644 --- a/internal/services/redis/redis_cache_resource.go +++ b/internal/services/redis/redis_cache_resource.go @@ -862,7 +862,7 @@ func resourceRedisCacheRead(d *pluginsdk.ResourceData, meta interface{}) error { accessKeyAuthEnabled := true if props.DisableAccessKeyAuthentication != nil { - accessKeyAuthEnabled = !(*props.DisableAccessKeyAuthentication) + accessKeyAuthEnabled = !pointer.From(props.DisableAccessKeyAuthentication) } d.Set("access_keys_authentication_enabled", accessKeyAuthEnabled) From 5341f2f58f32682660ee28bd03213bd862ba5948 Mon Sep 17 00:00:00 2001 From: Gerry Tan Date: Wed, 21 Aug 2024 18:24:30 +1000 Subject: [PATCH 15/15] Removed redundant nil check --- internal/services/redis/redis_cache_data_source.go | 7 +------ internal/services/redis/redis_cache_resource.go | 7 +------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/internal/services/redis/redis_cache_data_source.go b/internal/services/redis/redis_cache_data_source.go index f783eb20cd86..98f533b9b510 100644 --- a/internal/services/redis/redis_cache_data_source.go +++ b/internal/services/redis/redis_cache_data_source.go @@ -349,12 +349,7 @@ func dataSourceRedisCacheRead(d *pluginsdk.ResourceData, meta interface{}) error enableSslPort := !*props.EnableNonSslPort d.Set("primary_connection_string", getRedisConnectionString(*props.HostName, *props.SslPort, *keys.Model.PrimaryKey, enableSslPort)) d.Set("secondary_connection_string", getRedisConnectionString(*props.HostName, *props.SslPort, *keys.Model.SecondaryKey, enableSslPort)) - - accessKeyAuthEnabled := true - if props.DisableAccessKeyAuthentication != nil { - accessKeyAuthEnabled = !pointer.From(props.DisableAccessKeyAuthentication) - } - d.Set("access_keys_authentication_enabled", accessKeyAuthEnabled) + d.Set("access_keys_authentication_enabled", !pointer.From(props.DisableAccessKeyAuthentication)) if err := tags.FlattenAndSet(d, model.Tags); err != nil { return fmt.Errorf("setting `tags`: %+v", err) diff --git a/internal/services/redis/redis_cache_resource.go b/internal/services/redis/redis_cache_resource.go index f50233451096..21899bb40ca9 100644 --- a/internal/services/redis/redis_cache_resource.go +++ b/internal/services/redis/redis_cache_resource.go @@ -859,12 +859,7 @@ func resourceRedisCacheRead(d *pluginsdk.ResourceData, meta interface{}) error { d.Set("secondary_connection_string", getRedisConnectionString(*props.HostName, *props.SslPort, *keysResp.Model.SecondaryKey, true)) d.Set("primary_access_key", keysResp.Model.PrimaryKey) d.Set("secondary_access_key", keysResp.Model.SecondaryKey) - - accessKeyAuthEnabled := true - if props.DisableAccessKeyAuthentication != nil { - accessKeyAuthEnabled = !pointer.From(props.DisableAccessKeyAuthentication) - } - d.Set("access_keys_authentication_enabled", accessKeyAuthEnabled) + d.Set("access_keys_authentication_enabled", !pointer.From(props.DisableAccessKeyAuthentication)) if err := tags.FlattenAndSet(d, model.Tags); err != nil { return fmt.Errorf("setting `tags`: %+v", err)