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.

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

ec_deployment_elasticsearch_keystore: Add resource #364

Conversation

marclop
Copy link
Contributor

@marclop marclop commented Sep 2, 2021

Description

Adds a new ec_deployment_elasticsearch_keystore resource which allows
allows creating and updating Elasticsearch keystore settings.

Elasticsearch keystore settings can be created and updated through this
resource, each resource represents a single Elasticsearch Keystore
setting. After adding a key and its secret value to the keystore, you
can use the key name in place of the secret value when you configure
sensitive settings.

This resource offers weaker consistency guarantees and will not detect
and update keystore setting values that have been modified outside of
the scope of Terraform, usually referred to as drift. For instance,
examine the following scenario:
1. A keystore setting is created using this resource.
2. The keystore setting's value is modified to a different value
using the Elasticsearch Service API.
3. Running terraform apply will fail to detect the changes and
will not update the keystore setting to the value defined in the
terraform configuration.

To force the keystore setting to the value it is configured to hold,
you may want to taint the resource and force its recreation.

Thanks to @karencfv and @Crazybus for the initial work.

Related Issues

Closes #100

How Has This Been Tested?

Manually, unit and acceptance tested.

Types of Changes

  • New feature (non-breaking change which adds functionality)

karencfv and others added 8 commits September 1, 2021 17:12
Quick example of what
elastic#100 might look
like.
Updates the logic to match what the resource should do, in short:

Modifies the `create` function to use a hash of the deployment and the
setting name.
Modifies the `delete` function to unset the value of the keystore so the
keystore setting entry is deleted when sent to the `PATCH` api.
Simplifies the `expandModel` logic and allows it to `json.Unmarshal` the
specified string when it's JSON formatted, otherwise defaults to string.
Simplifies the `read` function to lookup the setting name as the key on
the returned secrets map and to only reset the ID since the setting does
not exist in the backing Elasticsearch cluster anymore.
Updates the schema so that when changing the `deployment_id` field, it
triggers a re-creation and correctly deletes the previous entry from the
previous deployment ID and creates a new entry on the new deployment.

Signed-off-by: Marc Lopez Rubio <[email protected]>
Adds an acceptance test that cycles through all the possible states that
the resource can go through. Also using the `as_file` field to simulate
the case where GCS snapshot credentials are set as a keystore setting.

Signed-off-by: Marc Lopez Rubio <[email protected]>
Signed-off-by: Marc Lopez Rubio <[email protected]>
Signed-off-by: Marc Lopez Rubio <[email protected]>
@marclop marclop added Team:Delivery new-resource New resource requests labels Sep 2, 2021
@marclop marclop self-assigned this Sep 2, 2021
@marclop marclop requested a review from a team as a code owner September 2, 2021 13:26
@marclop
Copy link
Contributor Author

marclop commented Sep 2, 2021

Reviewers can use https://registry.terraform.io/tools/doc-preview to verify that the docs display nicely and as intended.

@marclop
Copy link
Contributor Author

marclop commented Sep 6, 2021

@alaudazzi could I have a review on the docs from you folks, please?

Copy link
Contributor

@alaudazzi alaudazzi left a comment

Choose a reason for hiding this comment

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

Left a few editing suggestions, otherwise LGTM.

docs/resources/ec_deployment_elasticsearch_keystore.md Outdated Show resolved Hide resolved
docs/resources/ec_deployment_elasticsearch_keystore.md Outdated Show resolved Hide resolved
docs/resources/ec_deployment_elasticsearch_keystore.md Outdated Show resolved Hide resolved
docs/resources/ec_deployment_elasticsearch_keystore.md Outdated Show resolved Hide resolved
docs/resources/ec_deployment_elasticsearch_keystore.md Outdated Show resolved Hide resolved
docs/resources/ec_deployment_elasticsearch_keystore.md Outdated Show resolved Hide resolved
docs/resources/ec_deployment_elasticsearch_keystore.md Outdated Show resolved Hide resolved
docs/resources/ec_deployment_elasticsearch_keystore.md Outdated Show resolved Hide resolved
docs/resources/ec_deployment_elasticsearch_keystore.md Outdated Show resolved Hide resolved
docs/resources/ec_deployment_elasticsearch_keystore.md Outdated Show resolved Hide resolved
@marclop
Copy link
Contributor Author

marclop commented Sep 10, 2021

Thank you for the suggestions @alaudazzi. Looks much better now.

@marclop
Copy link
Contributor Author

marclop commented Sep 13, 2021

Failing tests are fixed in #378

@marclop marclop merged commit d897968 into elastic:master Sep 13, 2021
@marclop marclop deleted the f/add-ec_deployment_elasticsearch_keystore_setting branch September 13, 2021 09:26
marclop added a commit to marclop/terraform-provider-ec that referenced this pull request Sep 13, 2021
Adds the missing changelog entry for the change introduced in elastic#364

Signed-off-by: Marc Lopez Rubio <[email protected]>
marclop added a commit that referenced this pull request Sep 13, 2021
Adds the missing changelog entry for the change introduced in #364

Signed-off-by: Marc Lopez Rubio <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-resource New resource requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ec_deployment_elasticsearch_keystore: Add new resource
5 participants