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

Add script resource #173

Merged
merged 12 commits into from
Nov 1, 2022
65 changes: 65 additions & 0 deletions docs/resources/elasticsearch_script.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
---
# generated by https://github.com/hashicorp/terraform-plugin-docs
page_title: "elasticstack_elasticsearch_script Resource - terraform-provider-elasticstack"
subcategory: ""
Copy link
Member

Choose a reason for hiding this comment

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

subcategory: "Cluster" (I think, not sure if this warrants it's own category)

Might need to add a template

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh totally missed it, will fix.
I want this😂
hashicorp/terraform-plugin-docs#156

Copy link
Member

Choose a reason for hiding this comment

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

Nice, yep very much want that one too :)

description: |-
Creates or updates a stored script or search template. See https://www.elastic.co/guide/en/elasticsearch/reference/current/create-stored-script-api.html
---

# elasticstack_elasticsearch_script (Resource)

Creates or updates a stored script or search template. See https://www.elastic.co/guide/en/elasticsearch/reference/current/create-stored-script-api.html

## Example Usage
Copy link
Member

Choose a reason for hiding this comment

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

Should have said this earlier sorry, having a search template example would be nice too.


```terraform
provider "elasticstack" {
elasticsearch {}
}

resource "elasticstack_elasticsearch_script" "my_script" {
script_id = "my_script"
source = "Math.log(_score * 2) + params['my_modifier']"
context = "score"
Copy link
Member

Choose a reason for hiding this comment

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

This example is no longer valid, and needs lang defined.

Copy link
Contributor Author

@k-yomo k-yomo Nov 1, 2022

Choose a reason for hiding this comment

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

Oh good catch, fixed🙏
5bde0d4

}
```

<!-- schema generated by tfplugindocs -->
## Schema

### Required

- `script_id` (String) Identifier for the stored script. Must be unique within the cluster.
- `source` (String) For scripts, a string containing the script. For search templates, an object containing the search template.

### Optional

- `context` (String) Context in which the script or search template should run.
- `elasticsearch_connection` (Block List, Max: 1) Used to establish connection to Elasticsearch server. Overrides environment variables if present. (see [below for nested schema](#nestedblock--elasticsearch_connection))
- `lang` (String) Script language. For search templates, use `mustache`. Defaults to `painless`.
- `params` (String) Parameters for the script or search template.

### Read-Only

- `id` (String) The ID of this resource.

<a id="nestedblock--elasticsearch_connection"></a>
### Nested Schema for `elasticsearch_connection`

Optional:

- `api_key` (String, Sensitive) API Key to use for authentication to Elasticsearch
- `ca_data` (String) PEM-encoded custom Certificate Authority certificate
- `ca_file` (String) Path to a custom Certificate Authority certificate
- `endpoints` (List of String, Sensitive) A list of endpoints the Terraform provider will point to. They must include the http(s) schema and port number.
- `insecure` (Boolean) Disable TLS certificate validation
- `password` (String, Sensitive) A password to use for API authentication to Elasticsearch.
- `username` (String) A username to use for API authentication to Elasticsearch.

## Import

Import is supported using the following syntax:

```shell
terraform import elasticstack_elasticsearch_script.my_script <cluster_uuid>/<script id>
```
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
terraform import elasticstack_elasticsearch_script.my_script <cluster_uuid>/<script id>
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
provider "elasticstack" {
elasticsearch {}
}

resource "elasticstack_elasticsearch_script" "my_script" {
script_id = "my_script"
source = "Math.log(_score * 2) + params['my_modifier']"
context = "score"
}
56 changes: 56 additions & 0 deletions internal/clients/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,59 @@ func (a *ApiClient) GetElasticsearchSettings(ctx context.Context) (map[string]in
}
return clusterSettings, diags
}

func (a *ApiClient) GetElasticsearchScript(ctx context.Context, id string) (*models.Script, diag.Diagnostics) {
res, err := a.es.GetScript(id, a.es.GetScript.WithContext(ctx))
if err != nil {
return nil, diag.FromErr(err)
}
defer res.Body.Close()
if res.StatusCode == http.StatusNotFound {
return nil, nil
}
if diags := utils.CheckError(res, fmt.Sprintf("Unable to get the script: %s", id)); diags.HasError() {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if diags := utils.CheckError(res, fmt.Sprintf("Unable to get the script: %s", id)); diags.HasError() {
if diags := utils.CheckError(res, fmt.Sprintf("Unable to get saved script: %s", id)); diags.HasError() {

return nil, diags
}
type getScriptResponse = struct {
Script *models.Script `json:"script"`
}
var scriptResponse getScriptResponse
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
type getScriptResponse = struct {
Script *models.Script `json:"script"`
}
var scriptResponse getScriptResponse
var scriptResponse struct {
Script *models.Script `json:"script"`
}

Any benefit to the named type here?

Copy link
Contributor Author

@k-yomo k-yomo Oct 31, 2022

Choose a reason for hiding this comment

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

Oh no, unnamed type would be fine, fixed so!

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
type getScriptResponse = struct {
Script *models.Script `json:"script"`
}
var scriptResponse getScriptResponse
var scriptResponse struct {
Script *models.Script `json:"script"`
}

Any benefit to the named type here?

if err := json.NewDecoder(res.Body).Decode(&scriptResponse); err != nil {
return nil, diag.FromErr(err)
}

return scriptResponse.Script, nil
}

func (a *ApiClient) PutElasticsearchScript(ctx context.Context, script *models.Script) diag.Diagnostics {
req := struct {
Script *models.Script `json:"script"`
}{
Script: script,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Script: script,
script,

}
scriptBytes, err := json.Marshal(req)
if err != nil {
return diag.FromErr(err)
}
res, err := a.es.PutScript(script.ID, bytes.NewReader(scriptBytes), a.es.PutScript.WithContext(ctx), a.es.PutScript.WithScriptContext(script.Context))
if err != nil {
return diag.FromErr(err)
}
defer res.Body.Close()
if diags := utils.CheckError(res, "Unable to put the script"); diags.HasError() {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if diags := utils.CheckError(res, "Unable to put the script"); diags.HasError() {
if diags := utils.CheckError(res, "Unable to put saved script"); diags.HasError() {

return diags
}
return nil
}

func (a *ApiClient) DeleteElasticsearchScript(ctx context.Context, id string) diag.Diagnostics {
res, err := a.es.DeleteScript(id, a.es.DeleteScript.WithContext(ctx))
if err != nil {
return diag.FromErr(err)
}
defer res.Body.Close()
if diags := utils.CheckError(res, fmt.Sprintf("Unable to delete script: %s", id)); diags.HasError() {
return diags
}
return nil
}
151 changes: 151 additions & 0 deletions internal/elasticsearch/cluster/script.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
package cluster

import (
"context"
"encoding/json"
"fmt"

"github.com/elastic/terraform-provider-elasticstack/internal/clients"
"github.com/elastic/terraform-provider-elasticstack/internal/models"
"github.com/elastic/terraform-provider-elasticstack/internal/utils"
"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
)

func ResourceScript() *schema.Resource {
scriptSchema := map[string]*schema.Schema{
"script_id": {
Description: "Identifier for the stored script. Must be unique within the cluster.",
Type: schema.TypeString,
Required: true,
ForceNew: true,
},
"lang": {
Description: "Script language. For search templates, use `mustache`. Defaults to `painless`.",
Type: schema.TypeString,
Optional: true,
Default: "painless",
Copy link
Member

Choose a reason for hiding this comment

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

This default makes sense in the context of a stored script, but not when used to manage a search template. IMO it would be better to mirror the API and have this as a required field without a default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh that's true, fixed to make it required field.

ValidateFunc: validation.StringInSlice([]string{"painless", "expression", "mustache", "java"}, false),
},
"source": {
Description: "For scripts, a string containing the script. For search templates, an object containing the search template.",
Type: schema.TypeString,
Required: true,
},
"params": {
Description: "Parameters for the script or search template.",
Type: schema.TypeString,
Optional: true,
DiffSuppressFunc: utils.DiffJsonSuppress,
ValidateFunc: validation.StringIsJSON,
},
"context": {
Description: "Context in which the script or search template should run.",
Type: schema.TypeString,
Optional: true,
},
}
utils.AddConnectionSchema(scriptSchema)

return &schema.Resource{
Description: "Creates or updates a stored script or search template. See https://www.elastic.co/guide/en/elasticsearch/reference/current/create-stored-script-api.html",

CreateContext: resourceScriptPut,
UpdateContext: resourceScriptPut,
ReadContext: resourceScriptRead,
DeleteContext: resourceScriptDelete,

Importer: &schema.ResourceImporter{
StateContext: schema.ImportStatePassthroughContext,
},

Schema: scriptSchema,
}
}

func resourceScriptRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
var diags diag.Diagnostics
client, err := clients.NewApiClient(d, meta)
if err != nil {
return diag.FromErr(err)
}

id := d.Id()
compId, diags := clients.CompositeIdFromStr(id)
if diags.HasError() {
return diags
}

script, diags := client.GetElasticsearchScript(ctx, compId.ResourceId)
if !d.IsNewResource() && script == nil && diags == nil {
tflog.Warn(ctx, fmt.Sprintf(`Script "%s" not found, removing from state`, compId.ResourceId))
Copy link
Member

Choose a reason for hiding this comment

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

It doesn't look like we log a message in this case for the other resources. Any reason this should be a warning?

Copy link
Contributor Author

@k-yomo k-yomo Oct 31, 2022

Choose a reason for hiding this comment

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

Yes, the reason is to let the user know that the resource is removed from state when actually not found on API which is sometimes unexpected for users.
I think it's pretty common pattern which the other providers do too.
https://github.com/hashicorp/terraform-provider-aws/search?q=removing+from+state

It it makes sense, I'll add it to the other resources too with !d.IsNewResource() check.
hashicorp/terraform-provider-aws#16796

Copy link
Contributor Author

@k-yomo k-yomo Nov 1, 2022

Choose a reason for hiding this comment

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

I just realized that adding !d.IsNewResource() is not good here since it can cause null pointer error in the following code.
It might be better to return error when the resource is d.IsNewResource()) as aws provider does, but I simply removed to align with the other resources for now.

d.SetId("")
}
if diags.HasError() {
return diags
}

if err := d.Set("script_id", compId.ResourceId); err != nil {
return diag.FromErr(err)
}
if err := d.Set("lang", script.Language); err != nil {
return diag.FromErr(err)
}
if err := d.Set("source", script.Source); err != nil {
return diag.FromErr(err)
}

return diags
}

func resourceScriptPut(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
client, err := clients.NewApiClient(d, meta)
if err != nil {
return diag.FromErr(err)
}

scriptID := d.Get("script_id").(string)
id, diags := client.ID(ctx, scriptID)
if diags.HasError() {
return diags
}

script := models.Script{
ID: scriptID,
Language: d.Get("lang").(string),
Source: d.Get("source").(string),
}
if paramsJSON, ok := d.GetOk("params"); ok {
var params map[string]interface{}
bytes := []byte(paramsJSON.(string))
err = json.Unmarshal(bytes, &params)
if err != nil {
return diag.FromErr(err)
}
script.Params = params
}
if scriptContext, ok := d.GetOk("context"); ok {
script.Context = scriptContext.(string)
}
if diags := client.PutElasticsearchScript(ctx, &script); diags.HasError() {
return diags
}

d.SetId(id.String())
return resourceScriptRead(ctx, d, meta)
}

func resourceScriptDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
client, err := clients.NewApiClient(d, meta)
if err != nil {
return diag.FromErr(err)
}

compId, diags := clients.CompositeIdFromStr(d.Id())
if diags.HasError() {
return diags
}
return client.DeleteElasticsearchScript(ctx, compId.ResourceId)
}
93 changes: 93 additions & 0 deletions internal/elasticsearch/cluster/script_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package cluster_test

import (
"fmt"
"testing"

"github.com/elastic/terraform-provider-elasticstack/internal/acctest"
"github.com/elastic/terraform-provider-elasticstack/internal/clients"
sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
)

func TestAccResourceScript(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a test managing a search template as well?

scriptID := sdkacctest.RandStringFromCharSet(10, sdkacctest.CharSetAlphaNum)

resource.UnitTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
CheckDestroy: checkScriptDestroy,
ProviderFactories: acctest.Providers,
Steps: []resource.TestStep{
{
Config: testAccScriptCreate(scriptID),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("elasticstack_elasticsearch_script.test", "script_id", scriptID),
resource.TestCheckResourceAttr("elasticstack_elasticsearch_script.test", "lang", "painless"),
resource.TestCheckResourceAttr("elasticstack_elasticsearch_script.test", "source", "Math.log(_score * 2) + params['my_modifier']"),
resource.TestCheckResourceAttr("elasticstack_elasticsearch_script.test", "context", "score"),
),
},
{
Config: testAccScriptUpdate(scriptID),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("elasticstack_elasticsearch_script.test", "script_id", scriptID),
resource.TestCheckResourceAttr("elasticstack_elasticsearch_script.test", "lang", "painless"),
resource.TestCheckResourceAttr("elasticstack_elasticsearch_script.test", "source", "Math.log(_score * 4) + params['changed_modifier']"),
resource.TestCheckResourceAttr("elasticstack_elasticsearch_script.test", "params", `{"changed_modifier":2}`),
),
},
},
})
}

func testAccScriptCreate(id string) string {
return fmt.Sprintf(`
provider "elasticstack" {
elasticsearch {}
}

resource "elasticstack_elasticsearch_script" "test" {
script_id = "%s"
source = "Math.log(_score * 2) + params['my_modifier']"
context = "score"
}
`, id)
}

func testAccScriptUpdate(id string) string {
return fmt.Sprintf(`
provider "elasticstack" {
elasticsearch {}
}

resource "elasticstack_elasticsearch_script" "test" {
script_id = "%s"
source = "Math.log(_score * 4) + params['changed_modifier']"
params = jsonencode({
changed_modifier = 2
})
}
`, id)
}

func checkScriptDestroy(s *terraform.State) error {
client := acctest.Provider.Meta().(*clients.ApiClient)

for _, rs := range s.RootModule().Resources {
if rs.Type != "elasticstack_elasticsearch_script" {
continue
}

compId, _ := clients.CompositeIdFromStr(rs.Primary.ID)
res, err := client.GetESClient().GetScript(compId.ResourceId)
if err != nil {
return err
}

if res.StatusCode != 404 {
return fmt.Errorf("script (%s) still exists", compId.ResourceId)
}
}
return nil
}
8 changes: 8 additions & 0 deletions internal/models/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 +197,11 @@ type LogstashPipeline struct {
PipelineSettings map[string]interface{} `json:"pipeline_settings"`
Username string `json:"username"`
}

type Script struct {
ID string `json:"-"`
Language string `json:"lang"`
Source string `json:"source"`
Params map[string]interface{} `json:"params"`
Context string `json:"-"`
}