Add new resource to manage SLM policies

[olksdr]

Add new SLM resource, which will allow to create and update the snapshot lifecycle policies.

closes #21

[Kushmaro]

If config block is Min 1 , Max 1, theres' not much reason to have it as a block IMHO
the entire resource is essentially the resrource's configuration.

terraform often has some bugs with keeping multiple blocks idempotent.. we have such bugs in our Elastic Cloud provider (that are quite hard to fix) can we flatten these properties out?

[olksdr]

If config block is Min 1 , Max 1, theres' not much reason to have it as a block IMHO the entire resource is essentially the resrource's configuration.

It is not a problem to remove the block if it creates a better user experience.

terraform often has some bugs with keeping multiple blocks idempotent.. we have such bugs in our Elastic Cloud provider (that are quite hard to fix) can we flatten these properties out?

Could you, please, share more info on this? What errors have you encountered while testing this resource? Or do you mean there are some open issues in the terraform plugin SDK regarding the blocks we might need to be aware of?

[Kushmaro]

Sure @olksdr , here's an example bug we have on the cloud provider, where multiple topology blocks (i.e, ES nodes with the same role and function) are not idempotent unless ordered in a certain way
elastic/terraform-provider-ec#336

It's a bug with terraform itself, which we can't fix unless we implement another means of addressing the state of such resources.

That's why I'd like to avoid blocks when we can, seems like TF isn't quite mature with those.

[olksdr]

Sure @olksdr , here's an example bug we have on the cloud provider, where multiple topology blocks (i.e, ES nodes with the same role and function) are not idempotent unless ordered in a certain way elastic/terraform-provider-ec#336

It's a bug with terraform itself, which we can't fix unless we implement another means of addressing the state of such resources.

That's why I'd like to avoid blocks when we can, seems like TF isn't quite mature with those.

@Kushmaro I still do not see how the linked issue with terraform-provider-ec is related to this provider and using blocks in this provider? From what I see it's just the sum of different things here, which together created an issue:

• first of all it is API itself, and the usage of it
• second, it's of course there are some limitation in the SDK, which might required a workaround to fix in the current SDK

Have you looked into the workarounds proposed in hashicorp/terraform-plugin-sdk#195 with CustomizeDiff? That workaround is used by AWS and Fastly providers and might work for EC provider as well.

Are there any issues in any resources related to using blocks in the terraform-provider-elasticstack which you have encountered while testing it?

[Kushmaro]

@olksdr it's not directly related, it's just an exemplifier of blocks misbehaving with terraform in a specific case.
I haven't yet tested this resource myself to find any issues with it.

In any case, from a UX perspective, the argument still holds that there's no value in having a dedicated config block, given it is mandatory, and cannot exceed a single block.

[Crazybus]

LGTM!

[Crazybus]

In any case, from a UX perspective, the argument still holds that there's no value in having a dedicated config block, given it is mandatory, and cannot exceed a single block.

I think a good argument is that it models the current request body being sent to Elasticsearch. Having it flattened opens up the potential for naming conflicts.

What do we do if the config or retention blocks ever get a parameter that has the same name?

I would personally vote that we should add the config back to protect ourselves from any conflicts in the future.

[Kushmaro]

I think a good argument is that it models the current request body being sent to Elasticsearch. Having it flattened opens up the potential for naming conflicts.

IMHO API clients (SDKs, UIs, CLIs) should be concerned with simplifying UX for the customers using them.

In the same way that the UI doesn't expose itself in the same exact manner that the API is written, so shouldn't the terraform provider. It should abstract some of the complexity.

If in the future there would be another "indices" setting that is not "config.indices" (i.e not nested) I'd argue that the API design is a bit flawed. We can tackle such issues when we encounter them. If we prep for any worst-case scenarios, we just increase the complexity of use in the present time, just for sake of avoiding unwanted, yet to happen, futures.