IAM user secret bugfixes

docker-compose.yml

@@ -1,13 +1,14 @@
 version: "3"
 services:
   minio:
-    image: minio/minio:RELEASE.2021-04-06T23-11-00Z
+    image: minio/minio:RELEASE.2022-02-07T08-17-33Z
     ports:
       - "9000:9000"
+      - "9001:9001"
     environment:
       MINIO_ACCESS_KEY: minio
       MINIO_SECRET_KEY: minio123
-    command: server /data{0...3}
+    command: server --console-address :9001 /data{0...3}
   adminio-ui:
     image: rzrbld/adminio-ui:v1.93
     environment:

go.mod

@@ -4,6 +4,7 @@ go 1.16
 
 require (
 	github.com/aws/aws-sdk-go v1.42.51
+	github.com/google/go-cmp v0.5.7
 	github.com/hashicorp/terraform-plugin-sdk/v2 v2.10.1
 	github.com/jen20/awspolicyequivalence v1.1.0
 	github.com/minio/madmin-go v1.3.1

minio/new_client.go

@@ -12,7 +12,7 @@ import (
 //NewClient returns a new minio client
 func (config *S3MinioConfig) NewClient() (interface{}, error) {
 
-	minioClient := new(minio.Client)
+	var minioClient *minio.Client
 
 	var err error
 	if config.S3APISignature == "v2" {

minio/resource_minio_iam_policy_test.go

@@ -2,10 +2,12 @@ package minio
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"regexp"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"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"
@@ -25,7 +27,7 @@ func TestAccMinioIAMPolicy_basic(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckMinioIAMPolicyExists(resourceName),
 					resource.TestCheckResourceAttr(resourceName, "name", rName),
-					resource.TestCheckResourceAttr(resourceName, "policy", `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":["s3:ListBucket"],"Resource":["arn:aws:s3:::*"]}]}`),
+					testCheckJSONResourceAttr(resourceName, "policy", `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":["s3:ListBucket"],"Resource":["arn:aws:s3:::*"]}]}`),
 				),
 			},
 			{
@@ -103,14 +105,14 @@ func TestAccMinioIAMPolicy_policy(t *testing.T) {
 				Config: testAccMinioIAMPolicyConfigPolicy(rName1, policy1),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckMinioIAMPolicyExists(resourceName),
-					resource.TestCheckResourceAttr(resourceName, "policy", policy1),
+					testCheckJSONResourceAttr(resourceName, "policy", policy1),
 				),
 			},
 			{
 				Config: testAccMinioIAMPolicyConfigPolicy(rName2, policy2),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckMinioIAMPolicyExists(resourceName),
-					resource.TestCheckResourceAttr(resourceName, "policy", policy2),
+					testCheckJSONResourceAttr(resourceName, "policy", policy2),
 				),
 			},
 			{
@@ -172,6 +174,30 @@ func testAccCheckMinioIAMPolicyDisappears(resource string) resource.TestCheckFun
 	}
 }
 
+func testCheckJSONResourceAttr(name, key, value string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		rs, ok := s.RootModule().Resources[name]
+		if !ok {
+			return fmt.Errorf("Not found: %s", name)
+		}
+
+		v, ok := rs.Primary.Attributes[key]
+		if !ok {
+			return fmt.Errorf("%s: Attribute '%s' not found", name, key)
+		}
+
+		var actual, expected interface{}
+		_ = json.Unmarshal([]byte(value), &expected)
+		_ = json.Unmarshal([]byte(v), &actual)
+		diff := cmp.Diff(actual, expected)
+		if diff != "" {
+			return fmt.Errorf("%s: mismatch (-want +got):\n%s", name, diff)
+		}
+
+		return nil
+	}
+}
+
 func testAccMinioIAMPolicyConfigDescription(rName, description string) string {
 	return fmt.Sprintf(`
 resource "minio_iam_policy" "test" {

minio/resource_minio_iam_user.go

@@ -51,8 +51,10 @@ func resourceMinioIAMUser() *schema.Resource {
 				Computed: true,
 			},
 			"secret": {
-				Type:     schema.TypeString,
-				Computed: true,
+				Type:      schema.TypeString,
+				Computed:  true,
+				Optional:  true,
+				Sensitive: true,
 			},
 			"tags": tagsSchema(),
 		},
@@ -63,27 +65,45 @@ func minioCreateUser(ctx context.Context, d *schema.ResourceData, meta interface
 
 	iamUserConfig := IAMUserConfig(d, meta)
 
+	var err error
 	accessKey := iamUserConfig.MinioIAMName
-	secretKey, _ := generateSecretAccessKey()
+	secretKey := iamUserConfig.MinioSecret
 
-	err := iamUserConfig.MinioAdmin.AddUser(ctx, accessKey, string(secretKey))
+	if secretKey == "" {
+		if secretKey, err = generateSecretAccessKey(); err != nil {
+			return NewResourceError("Error creating user", accessKey, err)
+		}
+	}
+
+	err = iamUserConfig.MinioAdmin.AddUser(ctx, accessKey, secretKey)
 	if err != nil {
-		return NewResourceError("creating user failed", d.Id(), err)
+		return NewResourceError("Error creating user", accessKey, err)
 	}
 
 	d.SetId(aws.StringValue(&accessKey))
-	_ = d.Set("secret", string(secretKey))
+	_ = d.Set("secret", secretKey)
+
+	if iamUserConfig.MinioDisableUser {
+		err = iamUserConfig.MinioAdmin.SetUserStatus(ctx, accessKey, madmin.AccountDisabled)
+		if err != nil {
+			return NewResourceError("Error disabling IAM User %s: %s", d.Id(), err)
+		}
+	}
+
 	return minioReadUser(ctx, d, meta)
 }
 
 func minioUpdateUser(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 
 	iamUserConfig := IAMUserConfig(d, meta)
+
+	var err error
 	secretKey := iamUserConfig.MinioSecret
 
 	if secretKey == "" || iamUserConfig.MinioUpdateKey {
-		secretKeyBytes, _ := generateSecretAccessKey()
-		secretKey = string(secretKeyBytes)
+		if secretKey, err = generateSecretAccessKey(); err != nil {
+			return NewResourceError("Error creating user", d.Id(), err)
+		}
 	}
 
 	if d.HasChange(iamUserConfig.MinioIAMName) {
@@ -106,31 +126,32 @@ func minioUpdateUser(ctx context.Context, d *schema.ResourceData, meta interface
 	userStatus := UserStatus{
 		AccessKey: iamUserConfig.MinioIAMName,
 		SecretKey: secretKey,
-		Status:    madmin.AccountStatus(statusUser(false)),
+		Status:    madmin.AccountEnabled,
 	}
 
-	output, _ := iamUserConfig.MinioAdmin.GetUserInfo(ctx, iamUserConfig.MinioIAMName)
+	if iamUserConfig.MinioDisableUser {
+		userStatus.Status = madmin.AccountDisabled
+	}
 
-	if iamUserConfig.MinioDisableUser || output.Status == madmin.AccountStatus(statusUser(false)) && !iamUserConfig.MinioForceDestroy {
-		userStatus.Status = madmin.AccountStatus(statusUser(true))
-	} else if output.Status == madmin.AccountStatus(statusUser(true)) && !iamUserConfig.MinioForceDestroy {
-		userStatus.Status = madmin.AccountStatus(statusUser(false))
+	if iamUserConfig.MinioForceDestroy {
+		return minioDeleteUser(ctx, d, meta)
 	}
 
-	err := iamUserConfig.MinioAdmin.SetUserStatus(ctx, userStatus.AccessKey, userStatus.Status)
-	if err != nil {
-		return NewResourceError("Error to disable IAM User %s: %s", d.Id(), err)
+	userServerInfo, _ := iamUserConfig.MinioAdmin.GetUserInfo(ctx, iamUserConfig.MinioIAMName)
+	if userServerInfo.Status != userStatus.Status {
+		err := iamUserConfig.MinioAdmin.SetUserStatus(ctx, userStatus.AccessKey, userStatus.Status)
+		if err != nil {
+			return NewResourceError("Error to disable IAM User %s: %s", d.Id(), err)
+		}
 	}
 
 	if iamUserConfig.MinioUpdateKey {
 		err := iamUserConfig.MinioAdmin.SetUser(ctx, userStatus.AccessKey, userStatus.SecretKey, userStatus.Status)
 		if err != nil {
 			return NewResourceError("Error updating IAM User Key %s: %s", d.Id(), err)
 		}
-	}
 
-	if iamUserConfig.MinioForceDestroy {
-		_ = minioDeleteUser(ctx, d, meta)
+		_ = d.Set("secret", secretKey)
 	}
 
 	return minioReadUser(ctx, d, meta)
@@ -169,22 +190,22 @@ func minioDeleteUser(ctx context.Context, d *schema.ResourceData, meta interface
 
 	// IAM Users must be removed from all groups before they can be deleted
 	if err := deleteMinioIamUserGroupMemberships(ctx, iamUserConfig); err != nil {
-		return NewResourceError("Error removing IAM User (%s) group memberships: %s", d.Id(), err)
+		if iamUserConfig.MinioForceDestroy {
+			// Ignore errors when deleting group memberships, continue deleting user
+		} else {
+			return NewResourceError("Error removing IAM User (%s) group memberships: %s", d.Id(), err)
+		}
 	}
 
 	err := deleteMinioIamUser(ctx, iamUserConfig)
 	if err != nil {
 		return NewResourceError("Error deleting IAM User %s: %s", d.Id(), err)
 	}
 
-	return nil
-}
+	// Actively set resource as deleted as the update path might force a deletion via MinioForceDestroy
+	d.SetId("")
 
-func statusUser(status bool) string {
-	if status {
-		return "disabled"
-	}
-	return "enabled"
+	return nil
 }
 
 func validateMinioIamUserName(v interface{}, k string) (ws []string, errors []error) {