[ISSUE-127] Only change password and password_hash when value changes

CHANGELOG.md

@@ -7,6 +7,8 @@
 - Correctly identify a missing security user ([#101](https://github.com/elastic/terraform-provider-elasticstack/issues/101))
 - Support **7.x** Elasticsearch < **7.15** by removing the default `media_type` attribute in the Append processor ([#118](https://github.com/elastic/terraform-provider-elasticstack/pull/118))
 - Add `allow_restricted_indices` setting to security role ([#125](https://github.com/elastic/terraform-provider-elasticstack/issues/125))
+- Add conditional to only set `password` and `password_hash` when a new value is defined ([#127](https://github.com/elastic/terraform-provider-elasticstack/pull/128))
+- Add support for ELASTICSEARCH_INSECURE environment variable as the default of the `insecure` config value ([#127](https://github.com/elastic/terraform-provider-elasticstack/pull/128))
 
 ## [0.3.3] - 2023-03-22
 ### Fixed

internal/acctest/acctest.go

@@ -4,6 +4,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/elastic/terraform-provider-elasticstack/internal/clients"
 	"github.com/elastic/terraform-provider-elasticstack/provider"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
@@ -35,3 +36,7 @@ func PreCheck(t *testing.T) {
 		t.Fatal("Either ELASTICSEARCH_USERNAME and ELASTICSEARCH_PASSWORD must be set, or ELASTICSEARCH_API_KEY must be set for acceptance tests to run")
 	}
 }
+
+func ApiClient() *clients.ApiClient {
+	return Provider.Meta().(*clients.ApiClient)
+}

internal/elasticsearch/security/user.go

@@ -115,11 +115,12 @@ func resourceSecurityUserPut(ctx context.Context, d *schema.ResourceData, meta i
 
 	var user models.User
 	user.Username = usernameId
-	if v, ok := d.GetOk("password"); ok {
+
+	if v, ok := d.GetOk("password"); ok && d.HasChange("password") {
 		password := v.(string)
 		user.Password = &password
 	}
-	if v, ok := d.GetOk("password_hash"); ok {
+	if v, ok := d.GetOk("password_hash"); ok && d.HasChange("password_hash") {
 		pass_hash := v.(string)
 		user.PasswordHash = &pass_hash
 	}

internal/elasticsearch/security/user_test.go

@@ -1,7 +1,11 @@
 package security_test
 
 import (
+	"context"
+	"encoding/base64"
 	"fmt"
+	"io"
+	"strings"
 	"testing"
 
 	"github.com/elastic/terraform-provider-elasticstack/internal/acctest"
@@ -29,13 +33,134 @@ func TestAccResourceSecurityUser(t *testing.T) {
 				),
 			},
 			{
-				Config: testAccResourceSecurityUpdate(username),
+				Config: testAccResourceSecurityUpdate(username, "kibana_user"),
 				Check:  resource.TestCheckResourceAttr("elasticstack_elasticsearch_security_user.test", "email", "[email protected]"),
 			},
 		},
 	})
 }
 
+func TestAccImportedUserDoesNotResetPassword(t *testing.T) {
+	username := sdkacctest.RandStringFromCharSet(10, sdkacctest.CharSetAlphaNum)
+	initialPassword := sdkacctest.RandStringFromCharSet(10, sdkacctest.CharSetAlphaNum)
+	userUpdatedPassword := sdkacctest.RandStringFromCharSet(10, sdkacctest.CharSetAlphaNum)
+
+	resource.ParallelTest(t, resource.TestCase{
+		IsUnitTest:        true,
+		PreCheck:          func() { acctest.PreCheck(t) },
+		CheckDestroy:      checkResourceSecurityUserDestroy,
+		ProviderFactories: acctest.Providers,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccResourceSecurityUserUpdateNoPassword(username),
+				SkipFunc: func() (bool, error) {
+					client := acctest.ApiClient()
+					body := fmt.Sprintf("{\"roles\": [\"kibana_admin\"], \"password\": \"%s\"}", initialPassword)
+
+					resp, err := client.GetESClient().Security.PutUser(username, strings.NewReader(body))
+					if err != nil {
+						return false, nil
+					}
+
+					if resp.IsError() {
+						body, err := io.ReadAll(resp.Body)
+						return false, fmt.Errorf("failed to manually create import test user [%s] %s %s", username, body, err)
+					}
+					return false, err
+				},
+				ResourceName: "elasticstack_elasticsearch_security_user.test",
+				ImportStateIdFunc: func(s *terraform.State) (string, error) {
+					client := acctest.ApiClient()
+					clusterId, diag := client.ClusterID(context.Background())
+					if diag.HasError() {
+						return "", fmt.Errorf("failed to get cluster uuid: %s", diag[0].Summary)
+					}
+
+					return fmt.Sprintf("%s/%s", *clusterId, username), nil
+				},
+				ImportState:        true,
+				ImportStatePersist: true,
+				ImportStateCheck: func(is []*terraform.InstanceState) error {
+					importedUsername := is[0].Attributes["username"]
+					if importedUsername != username {
+						return fmt.Errorf("expected imported username [%s] to equal [%s]", importedUsername, username)
+					}
+
+					if _, ok := is[0].Attributes["password"]; ok {
+						return fmt.Errorf("expected imported user to not have a password set - got [%s]", is[0].Attributes["password"])
+					}
+
+					return nil
+				},
+				Check: checkUserCanAuthenticate(username, initialPassword),
+			},
+			{
+				Config: testAccResourceSecurityUserUpdateNoPassword(username),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("elasticstack_elasticsearch_security_user.test", "username", username),
+					resource.TestCheckTypeSetElemAttr("elasticstack_elasticsearch_security_user.test", "roles.*", "kibana_admin"),
+					resource.TestCheckNoResourceAttr("elasticstack_elasticsearch_security_user.test", "password"),
+					resource.TestCheckResourceAttr("elasticstack_elasticsearch_security_user.test", "full_name", "Test User"),
+					checkUserCanAuthenticate(username, initialPassword),
+				),
+			},
+			{
+				Config: testAccResourceSecurityUpdate(username, "kibana_admin"),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("elasticstack_elasticsearch_security_user.test", "email", "[email protected]"),
+					resource.TestCheckResourceAttrSet("elasticstack_elasticsearch_security_user.test", "password"),
+				),
+			},
+			{
+				SkipFunc: func() (bool, error) {
+					client := acctest.ApiClient().GetESClient()
+					body := fmt.Sprintf("{\"password\": \"%s\"}", userUpdatedPassword)
+
+					req := client.API.Security.ChangePassword.WithUsername(username)
+					resp, err := client.Security.ChangePassword(strings.NewReader(body), req)
+					if err != nil {
+						return false, nil
+					}
+
+					if resp.IsError() {
+						body, err := io.ReadAll(resp.Body)
+						return false, fmt.Errorf("failed to manually change import test user password [%s] %s %s", username, body, err)
+					}
+					return false, err
+				},
+				Config: testAccResourceSecurityUpdate(username, "kibana_user"),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("elasticstack_elasticsearch_security_user.test", "email", "[email protected]"),
+					resource.TestCheckResourceAttrSet("elasticstack_elasticsearch_security_user.test", "password"),
+					resource.TestCheckTypeSetElemAttr("elasticstack_elasticsearch_security_user.test", "roles.*", "kibana_user"),
+					checkUserCanAuthenticate(username, userUpdatedPassword),
+				),
+			},
+		},
+	})
+}
+
+func checkUserCanAuthenticate(username string, password string) func(*terraform.State) error {
+	return func(s *terraform.State) error {
+		client := acctest.ApiClient().GetESClient()
+		credentials := fmt.Sprintf("%s:%s", username, password)
+		authHeader := fmt.Sprintf("Basic %s", base64.StdEncoding.EncodeToString([]byte(credentials)))
+
+		req := client.API.Security.Authenticate.WithHeader(map[string]string{"Authorization": authHeader})
+		resp, err := client.Security.Authenticate(req)
+		if err != nil {
+			return err
+		}
+
+		if resp.IsError() {
+			body, err := io.ReadAll(resp.Body)
+
+			return fmt.Errorf("failed to authenticate as test user [%s] %s %s", username, body, err)
+		}
+		return nil
+	}
+}
+
 func testAccResourceSecurityUserCreate(username string) string {
 	return fmt.Sprintf(`
 provider "elasticstack" {
@@ -51,24 +176,38 @@ resource "elasticstack_elasticsearch_security_user" "test" {
 	`, username)
 }
 
-func testAccResourceSecurityUpdate(username string) string {
+func testAccResourceSecurityUserUpdateNoPassword(username string) string {
 	return fmt.Sprintf(`
 provider "elasticstack" {
   elasticsearch {}
 }
 
 resource "elasticstack_elasticsearch_security_user" "test" {
   username  = "%s"
-  roles     = ["kibana_user"]
+  roles     = ["kibana_admin"]
+  full_name = "Test User"
+}
+	`, username)
+}
+
+func testAccResourceSecurityUpdate(username string, role string) string {
+	return fmt.Sprintf(`
+provider "elasticstack" {
+  elasticsearch {}
+}
+
+resource "elasticstack_elasticsearch_security_user" "test" {
+  username  = "%s"
+  roles     = ["%s"]
   full_name = "Test User"
   email     = "[email protected]"
   password  = "qwerty123"
 }
-	`, username)
+	`, username, role)
 }
 
 func checkResourceSecurityUserDestroy(s *terraform.State) error {
-	client := acctest.Provider.Meta().(*clients.ApiClient)
+	client := acctest.ApiClient()
 
 	for _, rs := range s.RootModule().Resources {
 		if rs.Type != "elasticstack_elasticsearch_security_user" {

provider/provider.go

@@ -60,7 +60,7 @@ func New(version string) func() *schema.Provider {
 								Description: "Disable TLS certificate validation",
 								Type:        schema.TypeBool,
 								Optional:    true,
-								Default:     false,
+								DefaultFunc: schema.EnvDefaultFunc("ELASTICSEARCH_INSECURE", false),
 							},
 							"ca_file": {
 								Description: "Path to a custom Certificate Authority certificate",