-
Notifications
You must be signed in to change notification settings - Fork 278
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.
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
opensearch_config: only send PATCH on create if values set. #1268
Conversation
Config: fmt.Sprintf(testAccCheckDigitalOceanDatabaseOpensearchConfigConfigBasic, dbConfig, false, 1, "9223372036854775807"), | ||
Config: fmt.Sprintf(testAccCheckDigitalOceanDatabaseOpensearchConfigConfigBasic, dbConfig, false, 1, "1000000000000000000"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're hitting a bug with upstream Terraform here. I've change this to another value to allow us to test the broader functionality despite this.
resource.TestCheckResourceAttr("digitalocean_database_opensearch_config.foobar", "enable_security_audit", "true"), | ||
resource.TestCheckResourceAttr("digitalocean_database_opensearch_config.foobar", "ism_enabled", "true"), | ||
resource.TestCheckResourceAttr("digitalocean_database_opensearch_config.foobar", "ism_history_enabled", "true"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently both ism_enabled
and ism_history_enabled
must be true when setting the other ism_history
settings or the API returns errors, e.g.
Error: Error updating Opensearch configuration: PATCH https://api.digitalocean.com/v2/databases/d6b65b4f-954d-4ee0-a29b-29eb2b4d228f/config: 422 (request "7f2461f9-6c49-4291-a304-364f1d5eb740") invalid 'user_config': invalid input for opensearch: 'ism_history_enabled' is a required property
I've added enable_security_audit
so that we have another bool value to test changes. This exposed the other bug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
As reported in #1266, "creating" a
digitalocean_database_opensearch_config
without any setting leads to an error. This change protects against that by preventing sending the PATCH on create if no other values besidecluster_id
are set.Additionally, I ran into some issue running the acceptance test. One is a bug. We need to use
GetOkExists
when checking the value for a bool. If not, it is not correctly detected when being set tofalse
.There are also a few changes to the test that do not impact functionality which I will note inline.