Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DeletionProtectionEnabled to Redis Cluster #10367

Merged
merged 17 commits into from
Aug 16, 2024
Merged

Add DeletionProtectionEnabled to Redis Cluster #10367

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
5a186d8
Add DeletionProtectionEnabled to Redis Cluster
Harshita1603 Apr 3, 2024
ca7fceb
Add a new line character at the end of Cluster.yaml
Harshita1603 Apr 4, 2024
3b16d68
Merge branch 'GoogleCloudPlatform:main' into main
Harshita1603 Apr 4, 2024
0dba36e
set deletion protection to false before resource cleanup
Harshita1603 Apr 5, 2024
c5716aa
Modify failing tests
Harshita1603 Apr 23, 2024
1fa2878
Modify field description
Harshita1603 Apr 29, 2024
7cdb5b6
Merge branch 'main' into main
Harshita1603 Apr 29, 2024
4ec46dc
Resolve Lint errors
Harshita1603 Apr 29, 2024
b241432
Add missing character ','
Harshita1603 Apr 29, 2024
3ebe74f
Modify deletionProtectionEnabled field in createOrUpdateRedisCluster
Harshita1603 Apr 29, 2024
e51aa3d
Merge branch 'FEATURE-BRANCH-major-release-6.0.0' into main
NickElliot Aug 16, 2024
c9cbb8c
Update resource_redis_cluster_test.go.erb
NickElliot Aug 16, 2024
e23623e
Update resource_redis_cluster_test.go.erb
NickElliot Aug 16, 2024
743b721
Update resource_redis_cluster_test.go.erb
NickElliot Aug 16, 2024
95a4c6c
Update redis_cluster_ha_single_zone.tf.erb
NickElliot Aug 16, 2024
36d0020
Update Cluster.yaml
NickElliot Aug 16, 2024
a72c0e1
Apply suggestions from code review
NickElliot Aug 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions mmv1/products/redis/Cluster.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,11 @@ examples:
policy_name: "mypolicy"
subnet_name: "mysubnet"
network_name: "mynetwork"
prevent_destroy: 'true'
deletion_protection_enabled: 'true'
test_vars_overrides:
prevent_destroy: 'false'
deletion_protection_enabled: 'false'
oics_vars_overrides:
prevent_destroy: 'false'
deletion_protection_enabled: 'false'
properties:
- !ruby/object:Api::Type::Time
name: createTime
Expand Down Expand Up @@ -231,3 +231,9 @@ properties:
description: |
Required. Number of shards for the Redis cluster.
required: true
- !ruby/object:Api::Type::Boolean
name: deletionProtectionEnabled
description: |
Optional. Indicates if the cluster is deletion protected or not. Default value is true.
required: false
default_value: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might be a better fit for default_from_api if this is also a server-side default -- if this config was manually overridden for a pre-existing resource by a user via cloud console or pantheon, adding a field with a default value could potentially introduce a breaking change for users' configs (i.e. it potentially planning to do false -> true for users who don't manually add deletion_protection_enabled = false to their config)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @NickElliot , it is not the default value. The server-side default is false. It will also not be backward compatible for existing terraform plans. Is my understanding correct?

5 changes: 1 addition & 4 deletions mmv1/templates/terraform/examples/redis_cluster_ha.tf.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,10 @@ resource "google_redis_cluster" "<%= ctx[:primary_resource_id] %>" {
node_type = "REDIS_SHARED_CORE_NANO"
transit_encryption_mode = "TRANSIT_ENCRYPTION_MODE_DISABLED"
authorization_mode = "AUTH_MODE_DISABLED"
deletion_protection_enabled = <%= ctx[:vars]['deletion_protection_enabled'] == 'true' %>
depends_on = [
google_network_connectivity_service_connection_policy.default
]

lifecycle {
prevent_destroy = <%= ctx[:vars]['prevent_destroy'] %>
}
}

resource "google_network_connectivity_service_connection_policy" "default" {
Expand Down
53 changes: 43 additions & 10 deletions mmv1/third_party/terraform/services/redis/resource_redis_cluster_test.go.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ func TestAccRedisCluster_createClusterWithNodeType(t *testing.T) {
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"psc_configs"},
},
{
// set deletion protection to false
Config: createOrUpdateRedisCluster(name, /* replicaCount = */ 0, /* shardCount = */ 3, false, /*nodeType = */ "REDIS_STANDARD_SMALL"),
},
{
ResourceName: "google_redis_cluster.test",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"psc_configs"},
},
{
// clean up the resource
Config: createOrUpdateRedisCluster(name, /* replicaCount = */ 0, /* shardCount = */ 3, false, /*nodeType = */ "REDIS_STANDARD_SMALL"),
Expand Down Expand Up @@ -69,6 +79,16 @@ func TestAccRedisCluster_updateReplicaCount(t *testing.T) {
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"psc_configs"},
},
{
// set deletion protection to false
Config: createOrUpdateRedisCluster(name, /* replicaCount = */ 2, /* shardCount = */ 3, false, /*nodeType = */ ""),
},
{
ResourceName: "google_redis_cluster.test",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"psc_configs"},
},
{
// clean up the resource
Config: createOrUpdateRedisCluster(name, /* replicaCount = */ 2, /* shardCount = */ 3, false, /*nodeType = */ ""),
Expand All @@ -83,6 +103,16 @@ func TestAccRedisCluster_updateReplicaCount(t *testing.T) {
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"psc_configs"},
},
{
// set deletion protection to false
Config: createOrUpdateRedisCluster(name /* replicaCount = */, 0 /* shardCount = */, 3, false, /*nodeType = */ ""),
},
{
ResourceName: "google_redis_cluster.test",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"psc_configs"},
},
{
// clean up the resource
Config: createOrUpdateRedisCluster(name /* replicaCount = */, 0 /* shardCount = */, 3, false, /*nodeType = */ ""),
Expand Down Expand Up @@ -123,6 +153,16 @@ func TestAccRedisCluster_updateShardCount(t *testing.T) {
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"psc_configs"},
},
{
// set deletion protection to false
Config: createOrUpdateRedisCluster(name /* replicaCount = */, 1 /* shardCount = */, 5, false, /* nodeType = */ ""),
},
{
ResourceName: "google_redis_cluster.test",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"psc_configs"},
},
{
// clean up the resource
Config: createOrUpdateRedisCluster(name /* replicaCount = */, 1 /* shardCount = */, 5, false, /* nodeType = */ ""),
Expand All @@ -131,29 +171,22 @@ func TestAccRedisCluster_updateShardCount(t *testing.T) {
})
}

func createOrUpdateRedisCluster(name string, replicaCount int, shardCount int, preventDestroy bool, nodeType string) string {
lifecycleBlock := ""
if preventDestroy {
lifecycleBlock = `
lifecycle {
prevent_destroy = true
}`
}
func createOrUpdateRedisCluster(name string, replicaCount int, shardCount int, deletionProtectionEnabled bool, nodeType string) string {
return fmt.Sprintf(`
resource "google_redis_cluster" "test" {
provider = google-beta
name = "%s"
replica_count = %d
shard_count = %d
node_type = "%s"
deletion_protection_enabled = %v
region = "us-central1"
NickElliot marked this conversation as resolved.
Show resolved Hide resolved
psc_configs {
network = google_compute_network.producer_net.id
}
depends_on = [
google_network_connectivity_service_connection_policy.default
]
%s
}

resource "google_network_connectivity_service_connection_policy" "default" {
Expand Down Expand Up @@ -181,6 +214,6 @@ resource "google_compute_network" "producer_net" {
name = "%s"
auto_create_subnetworks = false
}
`, name, replicaCount, shardCount, nodeType, lifecycleBlock, name, name, name)
`, name, replicaCount, shardCount, nodeType, deletionProtectionEnabled, name, name, name)
}
<% end -%>