terraform-aws-modules / terraform-aws-opensearch

Terraform module to create AWS OpenSearch resources 🇺🇦
https://registry.terraform.io/modules/terraform-aws-modules/opensearch/aws
Apache License 2.0
15 stars 20 forks source link

Bug: Incorrect evaluation of master_user_options when master_user_arn is set #9

Closed marksansome closed 2 months ago

marksansome commented 4 months ago

Description

In the current implementation, there seems to be a bug related to the evaluation of the master_user_options field when the master_user_arn is set to a non-null string. This results in master_user_arn being incorrectly evaluated to null.

Versions

Reproduction Code [Required]

data "aws_iam_policy_document" "opensearch_assume_role" {
  statement {
    actions = [
      "sts:AssumeRole"
    ]

    principals {
      type        = "AWS"
      identifiers = ["arn:aws:iam::${data.aws_caller_identity.root.account_id}:root"]
    }
  }
}

resource "aws_iam_role" "opensearch_master" {
  name               = "OpenSearch_Master_staging01"
  assume_role_policy = data.aws_iam_policy_document.opensearch_assume_role.json
}

module "opensearch" {
  source  = "terraform-aws-modules/opensearch/aws"
  version = "1.1.2"

  advanced_security_options = {
    enabled                        = false
    anonymous_auth_enabled         = false
    internal_user_database_enabled = false

    master_user_options = {
      master_user_arn = aws_iam_role.opensearch_master.arn
    }
  }

  cluster_config = {
    instance_count           = 3
    dedicated_master_enabled = true
    dedicated_master_type    = "m6g.large.search"
    instance_type            = "r6g.large.search"

    zone_awareness_config = {
      availability_zone_count = 3
    }

    zone_awareness_enabled = true
  }

  domain_name = "staging01"

  engine_version = "OpenSearch_2.11"

  depends_on = [
    aws_iam_role.opensearch_master
  ]
}

Steps to reproduce the behavior:

Run terraform plan

Expected behavior

When var.advanced_security_options.master_user_options.master_user_arn is set, we should see that the arn is passed to the resource

Actual behavior

Running plan for the above code shows that no master_user_options are configured as part of aws_opensearch_domain. See resource reference

      + advanced_security_options {
          + anonymous_auth_enabled         = false
          + enabled                        = false
          + internal_user_database_enabled = false
        }

Additional context

Looking at the module I believe the issue is in this line in main.tf

master_user_arn      = try(master_user_options.value.master_user_arn, null) == null ? try(master_user_options.value.master_user_arn, data.aws_iam_session_context.current[0].issuer_arn) : null

My understanding is that if master_user_options.value.master_user_arn has a value the try will evaluate to true making the outcome of the ternary expression null

github-actions[bot] commented 3 months ago

This issue has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 10 days

github-actions[bot] commented 2 months ago

This issue was automatically closed because of stale in 10 days

RLashofRegas commented 2 months ago

@bryantbiggs this is not stale and still exists in version 1.2.0. I saw you commenting on similar issues/PRs, so tagging you. This conditional needs to be flipped: Current (if master_user_options.value.master_user_arn is null, set the value to master_user_options.value.master_user_arn, otherwise set it to null):

          master_user_arn      = try(master_user_options.value.master_user_arn, null) == null ? try(master_user_options.value.master_user_arn, data.aws_iam_session_context.current[0].issuer_arn) : null

Corrected (if master_user_options.value.master_user_arn is null, set the value to null, otherwise set it to master_user_options.value.master_user_arn):

          master_user_arn      = try(master_user_options.value.master_user_arn, null) == null ? null : try(master_user_options.value.master_user_arn, data.aws_iam_session_context.current[0].issuer_arn)
fcoelho commented 2 months ago

Here's a self contained example that triggers this issue:

provider "aws" {
  region = "eu-west-1"
}

module "opensearch" {
  source  = "terraform-aws-modules/opensearch/aws"
  version = "1.2.0"

  advanced_security_options = {
    enabled                        = true
    anonymous_auth_enabled         = false
    internal_user_database_enabled = true

    master_user_options = {
      master_user_name     = "example"
      master_user_password = "Barbarbarbar1!"
    }
  }

  auto_tune_options = {
    desired_state = "DISABLED"
  }

  cluster_config = {
    instance_count           = 2
    dedicated_master_enabled = false
    instance_type            = "t3.small.search"
  }

  domain_name = "test-os-master-user"

  engine_version = "OpenSearch_2.13"
}

Upon running terraform apply, I get this (some parts redacted):

Terraform will perform the following actions:

  # module.opensearch.aws_opensearch_domain.this[0] will be created
  + resource "aws_opensearch_domain" "this" {
      ...

      + advanced_security_options {
          + anonymous_auth_enabled         = false
          + enabled                        = true
          + internal_user_database_enabled = true

          + master_user_options {
              + master_user_arn      = "arn:aws:iam::000000000000:role/xxxxxxxxxx"
              + master_user_name     = "example"
              + master_user_password = (sensitive value)
            }
        }

        ...
    }

Plan: 1 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

module.opensearch.aws_opensearch_domain.this[0]: Creating...
╷
│ Error: creating OpenSearch Domain: ValidationException: You must provide either a master username or a master user ARN but not together.
│ 
│   with module.opensearch.aws_opensearch_domain.this[0],
│   on .terraform/modules/opensearch/main.tf line 29, in resource "aws_opensearch_domain" "this":
│   29: resource "aws_opensearch_domain" "this" {
│ 
╵

Modifying the module directly with the suggestion in the comment above fixes the issue and creates the cluster correctly.

keep-calm-and-make-devops commented 2 months ago

Got the same issue that described above. It also exists in version 1.2.1.

dim-ops commented 2 months ago

@bryantbiggs I have the same issue too

moritzzimmer commented 2 months ago

same here

github-actions[bot] commented 1 week ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.