ENG-13251: Add optional MongoDBSettings 'flavor' field to repository resource #503

juniocezar · 2024-01-29T21:31:32Z

Description of the change

ENG-13251: Add Flavor as an Optional MongoDBSettings field. Repository resource.

Type of change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklists

Development

Lint rules pass locally
The code changed/added as part of this pull request has been covered with tests
All tests related to the changed code pass in development

Code review

This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
Jira issue referenced in commit message and/or PR title

Testing

The flavor field should be optional:

resource "cyral_repository" "document_db_standard" {
  connection_draining {
    auto      = false
    wait_time = 0
  }
  name = "mongodb_terraform_test"
  repo_node {
    dynamic = false
    host    = "www.value.com"
    port    = 27017
  }
  type = "mongodb"
  mongodb_settings {
    server_type      = "replicaset"
    replica_set_name = "rs1"
  }
}

└─ $ ▶ terraform apply --auto-approve
cyral_repository.document_db_standard: Refreshing state... [id=2bgc3gkCWVrBDpR8NM4nynSj3M7]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # cyral_repository.document_db_standard will be updated in-place
  ~ resource "cyral_repository" "document_db_standard" {
        id     = "2bgc3gkCWVrBDpR8NM4nynSj3M7"
        name   = "mongodb_terraform_test"
        # (2 unchanged attributes hidden)

      - mongodb_settings {
          - replica_set_name = "rs0" -> null
          - server_type      = "replicaset" -> null
        }
      + mongodb_settings {
          + replica_set_name = "rs1"
          + server_type      = "replicaset"
        }

        # (2 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.
cyral_repository.document_db_standard: Modifying... [id=2bgc3gkCWVrBDpR8NM4nynSj3M7]
cyral_repository.document_db_standard: Modifications complete after 2s [id=2bgc3gkCWVrBDpR8NM4nynSj3M7]

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

Ensure that invalid flavors are not allowed:

resource "cyral_repository" "document_db_standard" {
  connection_draining {
    auto      = false
    wait_time = 0
  }
  name = "mongodb_terraform_test"
  repo_node {
    dynamic = false
    host    = "www.value.com"
    port    = 27017
  }
  type = "mongodb"
  mongodb_settings {
    server_type      = "replicaset"
    replica_set_name = "rs0"
    flavor = "cosmosdb"
  }
}


└─ $ ▶ terraform apply --auto-approve
╷
│ Error: expected mongodb_settings.0.flavor to be one of ["mongodb" "documentdb"], got cosmosdb
│ 
│   with cyral_repository.document_db_standard,
│   on main.tf line 15, in resource "cyral_repository" "document_db_standard":
│   15: resource "cyral_repository" "document_db_standard" {
│

Flavor mongodb can be used with replicaset

└─ $ ▶ terraform apply --auto-approve
cyral_repository.document_db_standard: Refreshing state... [id=2bgc3gkCWVrBDpR8NM4nynSj3M7]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # cyral_repository.document_db_standard will be updated in-place
  ~ resource "cyral_repository" "document_db_standard" {
        id     = "2bgc3gkCWVrBDpR8NM4nynSj3M7"
        name   = "mongodb_terraform_test"
        # (2 unchanged attributes hidden)

      - mongodb_settings {
          - replica_set_name = "rs1" -> null
          - server_type      = "replicaset" -> null
        }
      + mongodb_settings {
          + flavor           = "mongodb"
          + replica_set_name = "rs1"
          + server_type      = "replicaset"
        }

        # (2 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.
cyral_repository.document_db_standard: Modifying... [id=2bgc3gkCWVrBDpR8NM4nynSj3M7]
cyral_repository.document_db_standard: Modifications complete after 2s [id=2bgc3gkCWVrBDpR8NM4nynSj3M7]

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

Flavor mongodb can be used with standalone

└─ $ ▶ terraform apply --auto-approve
cyral_repository.document_db_standard: Refreshing state... [id=2bgfOFDW4cYQ5X6LgJuQCe2Koac]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # cyral_repository.document_db_standard will be updated in-place
  ~ resource "cyral_repository" "document_db_standard" {
        id     = "2bgfOFDW4cYQ5X6LgJuQCe2Koac"
        name   = "mongodb_terraform_test"
        # (2 unchanged attributes hidden)

      - mongodb_settings {
          - flavor      = "documentdb" -> null
          - server_type = "standalone" -> null
        }
      + mongodb_settings {
          + flavor      = "mongodb"
          + server_type = "standalone"
        }

        # (2 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.
cyral_repository.document_db_standard: Modifying... [id=2bgfOFDW4cYQ5X6LgJuQCe2Koac]
cyral_repository.document_db_standard: Modifications complete after 1s [id=2bgfOFDW4cYQ5X6LgJuQCe2Koac]

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

Flavor mongodb can be used with sharded

└─ $ ▶ terraform apply --auto-approve

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # cyral_repository.document_db_standard will be created
  + resource "cyral_repository" "document_db_standard" {
      + id   = (known after apply)
      + name = "mongodb_terraform_test"
      + type = "mongodb"

      + connection_draining {
          + auto      = false
          + wait_time = 0
        }

      + mongodb_settings {
          + flavor      = "mongodb"
          + server_type = "sharded"
        }

      + repo_node {
          + dynamic = false
          + host    = "www.value.com"
          + port    = 27017
        }
    }

Plan: 1 to add, 0 to change, 0 to destroy.
cyral_repository.document_db_standard: Creating...
cyral_repository.document_db_standard: Creation complete after 2s [id=2bgfXcRULXD6Z2QGXyZSzj8p4zr]

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.

Flavor documentdb can be used with standalone

└─ $ ▶ terraform apply --auto-approve

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # cyral_repository.document_db_standard will be created
  + resource "cyral_repository" "document_db_standard" {
      + id   = (known after apply)
      + name = "mongodb_terraform_test"
      + type = "mongodb"

      + connection_draining {
          + auto      = false
          + wait_time = 0
        }

      + mongodb_settings {
          + flavor      = "documentdb"
          + server_type = "standalone"
        }

      + repo_node {
          + dynamic = false
          + host    = "www.value.com"
          + port    = 27017
        }
    }

Plan: 1 to add, 0 to change, 0 to destroy.
cyral_repository.document_db_standard: Creating...
cyral_repository.document_db_standard: Creation complete after 2s [id=2bgfOFDW4cYQ5X6LgJuQCe2Koac]

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.

Flavor documentdb can be used with replicaset

└─ $ ▶ terraform apply --auto-approve
cyral_repository.document_db_standard: Refreshing state... [id=2bgc3gkCWVrBDpR8NM4nynSj3M7]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # cyral_repository.document_db_standard will be updated in-place
  ~ resource "cyral_repository" "document_db_standard" {
        id     = "2bgc3gkCWVrBDpR8NM4nynSj3M7"
        name   = "mongodb_terraform_test"
        # (2 unchanged attributes hidden)

      + mongodb_settings {
          + flavor           = "documentdb"
          + replica_set_name = "rs1"
          + server_type      = "replicaset"
        }
      - mongodb_settings {
          - flavor           = "mongodb" -> null
          - replica_set_name = "rs1" -> null
          - server_type      = "replicaset" -> null
        }

        # (2 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.
cyral_repository.document_db_standard: Modifying... [id=2bgc3gkCWVrBDpR8NM4nynSj3M7]
cyral_repository.document_db_standard: Modifications complete after 1s [id=2bgc3gkCWVrBDpR8NM4nynSj3M7]

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

Flavor documentdb cannot be used with sharded.

└─ $ ▶ terraform apply --auto-approve
cyral_repository.document_db_standard: Refreshing state... [id=2bgfXcRULXD6Z2QGXyZSzj8p4zr]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # cyral_repository.document_db_standard will be updated in-place
  ~ resource "cyral_repository" "document_db_standard" {
        id     = "2bgfXcRULXD6Z2QGXyZSzj8p4zr"
        name   = "mongodb_terraform_test"
        # (2 unchanged attributes hidden)

      + mongodb_settings {
          + flavor      = "documentdb"
          + server_type = "sharded"
        }
      - mongodb_settings {
          - flavor      = "mongodb" -> null
          - server_type = "sharded" -> null
        }

        # (2 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.
cyral_repository.document_db_standard: Modifying... [id=2bgfXcRULXD6Z2QGXyZSzj8p4zr]
╷
│ Error: Unable to update resource RepositoryUpdate
│ 
│   with cyral_repository.document_db_standard,
│   on main.tf line 15, in resource "cyral_repository" "document_db_standard":
│   15: resource "cyral_repository" "document_db_standard" {
│ 
│ "documentdb" MongoDB flavor cannot be combined with server type: "sharded". For configuring DocumentDB Elastic clusters, please use the standalone server type
╵

ricardorey10

LGTM, but let's wait until we hear from Wilson

ricardorey10 · 2024-01-30T19:36:58Z

cyral/internal/repository/resource_cyral_repository.go

@@ -263,10 +279,18 @@ func (r *RepoInfo) MongoDBSettingsFromInterface(i []interface{}) error {
 			Standalone,
 		)
 	}
+	if serverType == Sharded && mongoFlavor == MongoDBFlavorDocumentDB {


We already have validation for this combination in crud, so I don't think we should do it here too. I feel like the terraform provider should not encode these validations, but rather they should be left to the gRPC/REST implementation.

That said, that's just my opinion, and I don't know what's the idiomatic thing to do. Let's see what @wcmjunior thinks.

I am fine with both, we can wait for wilson.

It is ok to have the validation only in the backend as long as it provides an error message that actually helps the user to solve the problem. If it returns something like Invalid parameter it does not help, but if it returns something in the lines of Parameter X must be 'Y' or 'Z' it is ok.

ricardorey10 · 2024-01-30T19:38:04Z

cyral/internal/repository/resource_cyral_repository.go

+	if serverType == Sharded && mongoFlavor == MongoDBFlavorDocumentDB {
+		return fmt.Errorf(
+			"%q MongoDB flavor cannot be combined with server type: %q. For configuring "+
+				"DocumentDB Elastic clusters, please use the %s server type",


Both standalone and replicaset work with DocumentDB, right?

ricardorey10

Based on Wilson's comment I'll approve now so that you're not blocked on me whenever you make the necessary changes.

juniocezar · 2024-01-30T22:39:43Z

@ricardorey10 I just removed the local validation, as requested. This is the new output:


└─ $ ▶ terraform apply --auto-approve

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # cyral_repository.document_db_standard will be created
  + resource "cyral_repository" "document_db_standard" {
      + id   = (known after apply)
      + name = "mongodb_terraform_test"
      + type = "mongodb"

      + connection_draining {
          + auto      = false
          + wait_time = 0
        }

      + mongodb_settings {
          + flavor      = "documentdb"
          + server_type = "sharded"
        }

      + repo_node {
          + dynamic = false
          + host    = "www.value.com"
          + port    = 27017
        }
    }

Plan: 1 to add, 0 to change, 0 to destroy.
cyral_repository.document_db_standard: Creating...
╷
│ Error: Unable to create resource RepositoryCreate
│ 
│   with cyral_repository.document_db_standard,
│   on main.tf line 15, in resource "cyral_repository" "document_db_standard":
│   15: resource "cyral_repository" "document_db_standard" {
│ 
│ error executing POST request; status code: 400; body: "{\"Message\":\"Rpc error: code = InvalidArgument desc = 'documentdb' flavor is not compatible with sharded clusters\"}\n"
╵

sonarqubecloud · 2024-01-30T22:39:55Z

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

wcmjunior

Changes LGTM. Please also point out (a comment is fine) which is the minimum version of CP required to use the new attribute so I can add to the change log.

juniocezar · 2024-01-30T23:54:45Z

v4.12.3 is the minimum Control Plane version to use the new attribute.

juniocezar added 4 commits January 29, 2024 18:03

ENG-13251: Add MongoDB Flavor to Repo Configuration

3b3761a

Run documentation script

1c88fa8

precommit changes

982edc7

Update some tests

26ce92e

juniocezar requested review from wcmjunior and ricardorey10 January 30, 2024 19:21

juniocezar marked this pull request as ready for review January 30, 2024 19:21

juniocezar changed the title ~~ENG-13251: Add MongoDB Flavor to Repo Configuration~~ ENG-13251: Add optional MongoDBSettings 'flavor' field to repository resource Jan 30, 2024

ricardorey10 reviewed Jan 30, 2024

View reviewed changes

ricardorey10 approved these changes Jan 30, 2024

View reviewed changes

Remove local validation, leave it to API

f1ca103

wcmjunior approved these changes Jan 30, 2024

View reviewed changes

wcmjunior merged commit 068c5d9 into main Jan 31, 2024
2 checks passed

wcmjunior deleted the ENG-13251 branch January 31, 2024 00:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENG-13251: Add optional MongoDBSettings 'flavor' field to repository resource #503

ENG-13251: Add optional MongoDBSettings 'flavor' field to repository resource #503

juniocezar commented Jan 29, 2024 •

edited

Loading

ricardorey10 left a comment

ricardorey10 Jan 30, 2024

juniocezar Jan 30, 2024

wcmjunior Jan 30, 2024

ricardorey10 Jan 30, 2024

ricardorey10 left a comment

juniocezar commented Jan 30, 2024

sonarqubecloud bot commented Jan 30, 2024

wcmjunior left a comment

juniocezar commented Jan 30, 2024

ENG-13251: Add optional MongoDBSettings 'flavor' field to repository resource #503

ENG-13251: Add optional MongoDBSettings 'flavor' field to repository resource #503

Conversation

juniocezar commented Jan 29, 2024 • edited Loading

Description of the change

Type of change

Checklists

Development

Code review

Testing

ricardorey10 left a comment

Choose a reason for hiding this comment

ricardorey10 Jan 30, 2024

Choose a reason for hiding this comment

juniocezar Jan 30, 2024

Choose a reason for hiding this comment

wcmjunior Jan 30, 2024

Choose a reason for hiding this comment

ricardorey10 Jan 30, 2024

Choose a reason for hiding this comment

ricardorey10 left a comment

Choose a reason for hiding this comment

juniocezar commented Jan 30, 2024

sonarqubecloud bot commented Jan 30, 2024

Quality Gate passed

wcmjunior left a comment

Choose a reason for hiding this comment

juniocezar commented Jan 30, 2024

juniocezar commented Jan 29, 2024 •

edited

Loading