hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.63k stars 9.01k forks source link

aws_elasticsearch_domain cognito_options cause Cycle Error #5557

Closed ovomason closed 1 year ago

ovomason commented 5 years ago

Community Note

Terraform Version

Terraform v0.11.7

Affected Resource(s)

aws_elasticsearch_domain aws_cognito_user_pool aws_cognito_user_pool_client aws_cognito_identity_pool

Terraform Configuration Files


resource "aws_elasticsearch_domain" "elasticsearch-cognito" {
  domain_name = "test-ovowebsite-${var.environment}"
  elasticsearch_version = "6.2"
  cluster_config {

    instance_count = "${var.elasticsearch_instance_count}"
    instance_type = "${var.elasticsearch_instance_type}"
    zone_awareness_enabled = "${var.elasticsearch_zone_awareness}"
    dedicated_master_enabled = "${var.elasticsearch_dedicated_master_enabled}"
    dedicated_master_count = "${var.elasticsearch_master_count}"
    dedicated_master_type = "${var.elasticsearch_master_type}"
  }

  access_policies = <<CONFIG
{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": {
              "AWS": ["*"]
            },
            "Action": ["es:*"],
            "Resource": "arn:aws:es:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:domain/test-ovowebsite-${var.environment}/*"
        }
    ]
}
  CONFIG

  ebs_options {
    ebs_enabled = true
    volume_type = "gp2"
    volume_size = "10"
  }
  encrypt_at_rest {
    enabled = true
    kms_key_id = "${aws_kms_key.es_key.id}"
  }
  snapshot_options {
    automated_snapshot_start_hour = "${var.elasticsearch_snapshot_time}"
  }
  vpc_options {
    subnet_ids = ["${aws_subnet.protected_subnet1.id}","${aws_subnet.protected_subnet2.id}"]
    security_group_ids = ["${aws_security_group.elasticsearch.id}"]
  }
  cognito_options {
    enabled = true
    user_pool_id = "${aws_cognito_user_pool.kibana.id}"
    identity_pool_id = "${aws_cognito_identity_pool.kibana.id}"
    role_arn = "${aws_iam_role.CustomESCognitoAccess.arn}"
  }
  tags {
    Name = "ovowebsite-${var.environment}"
    Team = "${var.team}"
  }
}

// cognito setup
// create user pool
resource "aws_cognito_user_pool" "kibana" {
  name = "kibana user pool"
  auto_verified_attributes = ["email"]
  admin_create_user_config = {
    allow_admin_create_user_only = false
  }
  schema {
    attribute_data_type = "String"
    name = "email"
    required = true
  }
  alias_attributes = ["email"]
}
// set user pool domain
resource "aws_cognito_user_pool_domain" "kibana" {
  domain = "ovo-kibana-login"
  user_pool_id = "${aws_cognito_user_pool.kibana.id}"
}

// set user pool client
resource "aws_cognito_user_pool_client" "kibana" {
  name = "elasticsearch"
  user_pool_id = "${aws_cognito_user_pool.kibana.id}"
  generate_secret = true
  callback_urls = ["${aws_elasticsearch_domain.elasticsearch-cognito.kibana_endpoint}"]
  logout_urls = ["${aws_elasticsearch_domain.elasticsearch-cognito.kibana_endpoint}"]
  allowed_oauth_flows = ["code", "implicit"]
  allowed_oauth_scopes = ["email", "openid"]
}

//create identity pool
resource "aws_cognito_identity_pool" "kibana" {
  identity_pool_name               = "kibana pool"
  allow_unauthenticated_identities = false

  cognito_identity_providers {
    client_id               = "${aws_cognito_user_pool_client.kibana.id}"
    provider_name           = "${aws_cognito_user_pool.kibana.endpoint}"
    server_side_token_check = false
  }
}

Debug Output

Cycle: module.common.aws_cognito_user_pool_client.kibana, module.common.aws_cognito_identity_pool.kibana, module.common.aws_elasticsearch_domain.elasticsearch-cognito

Expected Behavior

I need to create a user pool, identity pool and user client (with the Elasticsearch Kibana Endpoint as the callback_url) and create the elasticsearch resource. Then enable Elasticsearch Cognito with the user pool id and identity pool id.

Actual Behavior

Creates an Error: Cycle: module.common.aws_cognito_user_pool_client.kibana, module.common.aws_cognito_identity_pool.kibana, module.common.aws_elasticsearch_domain.elasticsearch-cognito

Steps to Reproduce

Create an 'aws_cognito_user_pool' resource Create an 'aws_cognito_identity_pool' resource with 'cognito_identity_providers' attribute Create an 'aws_elasticsearch_domain' resource with 'cognito_options' attribute Create an 'aws_cognito_user_pool_client' resource with the elasticsearch kibana endpoint

Important Factoids

We require the kibana endpoint to redirect users to kibana after login.

I propose abstracting the cognito options for elasticsearch into its own resource. This would be the minimum required to get this working i believe.

It may also be work creating a new resource that can attach a new cognito identity provider?

jleskovar-tyro commented 5 years ago

Hi there. Putting aside the cyclic dependency issue you mention, my experience with ElasticSearch and cognito so far is that it simply isn't possible to have Terraform manage cognito with ElasticSearch. The problem I've been having is that enabling Cognito auth for ES results in AWS going rogue and creating its own application client on the user pool you specify, as well as adding this client as an auth provider on your identity pool. My current workaround is to use the AWS CLI to configure cognito, after using Terraform to enable cognito on the ElasticSearch domain. Doing this, you'll need to use "ignore_changes" on your identity pool to prevent TF from blowing away the AWS-managed app client

Geartrixy commented 5 years ago

Hello,

as @jleskovar-tyro mentioned, when you enable cognito authentication on ES, AWS will automatically create a new cognito app client in the background.

This renders the app client you want created via Terraform to be used via ES useless, as the configuration on ES references the AWS created app client I think?

The only work around I can think of is to data source the app client that gets created via AWS.

But then this would require two deployments, as the app client cannot be data sourced until it is created, so you would have to comment out the "cognito_identity_providers" block in the "aws_cognito_identity_pool" resource first (and any roles attachment resources). Deploy to enable cognito authentication for kibana on ES. Then remove the commented out code (adding the data sourced app client to the configuration) and deploy again?

Geartrixy commented 5 years ago

ah, this is not possible: https://www.terraform.io/docs/providers/aws/d/cognito_user_pool_client.html

bnr242003 commented 5 years ago

I am facing the similar issues. Two user pools are being created - one by Terraform and another by ES while creating the cluster and unfortunately the callback and signout URL's getting assigned to the user pool created by ES and that user pool is got getting attached to the Cognito Identity provider.

bnr242003 commented 5 years ago

Some quick solution will be appreciated. Please let me know if you need anymore clarifications.

jleskovar-tyro commented 5 years ago

@bnr242003 What we ended up doing was using terraform to still manage our aws_elasticsearch_domain with Cognito enabled, together with our Cognito related resources (i.e. aws_cognito_user_pool, aws_cognito_user_pool_domain, aws_cognito_identity_provider, and aws_cognito_identity_pool).

After doing a tf plan/apply, our pipeline will run a post-apply.sh script, which uses the AWS CLI to directly modify the UserPoolClient AWS automatically creates for you, in order to support our SAML identity provider.

See this gist for an example of how that works. Obviously, your use case may be different: https://gist.github.com/jleskovar-tyro/051317370190394d551f8861fb760f68

If you use this approach, you'll need to be sure to add the appropriate lifecycle.ignore_changes to any TF resources that are modified by the post-apply.sh script, or AWS itself. In our case, we had to ignore changes to cognito_identity_providers on our TF-managed aws_cognito_identity_pool resource, as otherwise the next tf plan/apply would revert the UserPoolClient AWS automatically creates and attaches for you. Hope that makes sense

phr3nzii commented 4 years ago

Any updates on this issue? We're experiencing this, I'd rather not ignore changes on resources or use a script to make changes after tf apply as part of CI.

umeat commented 4 years ago

I tried to be cheeky and work around the behaviour of AWS creating those resources.

If you only give the role (aws_elasticsearch_domain.cognito_options.role_arn) access to cognito-idp:Describe and cognito-identity:Describe, the AWS API accepts the configuration without issue and doesn't go on to create any resources in the background.

However, now my Domain Status is stuck in Processing state and it hasn't actually updated the Cognito settings (I assume it's stuck retrying the calls to create the Client and Identity Provider).

I guess this half of the issue is with AWS.

You would still have the circular dependency issue in terraform.

denibertovic commented 4 years ago

@jleskovar-tyro Still using the post install approach? Do you happen to have an example of the terraform code as well?

lapkritinis commented 4 years ago

Looks like I was able to work around it. Only downside, that you need to hardcode domain name:

data "aws_elasticsearch_domain" "es-domain" { domain_name = "your-elasticsearch-domain" }

callback_urls = [ "https://${data.aws_elasticsearch_domain.es-domain.endpoint}app/kibana" ]

jleskovar-tyro commented 4 years ago

@denibertovic still using the post-apply script. Sorry for late reply. Let me distill a working sample for you in a gist

@lapkritinis the main issue though is when you want to have Kibana enabled with cognito auth, that’s when you start running into problems

lapkritinis commented 4 years ago

Thats what I have - elastic search with cognito auth.

On 2020-05-17, Sun at 22:02, jleskovar-tyro notifications@github.com wrote:

@denibertovic https://github.com/denibertovic still using the post-apply script. Sorry for late reply. Let me distill a working sample for you in a gist

@lapkritinis https://github.com/lapkritinis the main issue though is when you want to have Kibana enabled with cognito auth, that’s when you start running into problems

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/terraform-providers/terraform-provider-aws/issues/5557#issuecomment-629844738, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADVYNANZNT7IXTWVI57K3ZLRSAYEZANCNFSM4FPYTJGQ .

jleskovar-tyro commented 4 years ago

Hmm, were you using a custom user pool client, or just relying on the AWS managed user pool? The problem occurs due to AWS creating and attaching a user pool for you, which is a problem if you want a user pool backed by a custom identity provider (e.g. SAML provider)

msneller commented 3 years ago

I think this issue could be solved if terraform had a data source for Cognito client app (with support for regex filter on name) and an independent resource for adding Cognito identity providers to an existing identity pool.

KarstenSiemer commented 3 years ago

We also do have this problem. Most logical solution for me would be to be able to reference the the client at the elastic resource. Shall we open an issue at aws? They would have to patch this on aws side also

dcepelik commented 3 years ago

@radeksimko Hello again, I can confirm we're also affected by this issue. Is there anything we can do to move forward with this?

leoskyrocker commented 3 years ago

@msneller @KarstenSiemer even in that case you'd end up with having to applying twice right? Since data source cannot be "not found" it will complain the very first time you're enabling Cognito. Refer to https://github.com/hashicorp/terraform-provider-aws/issues/5557#issuecomment-478562921

lukasz-kaniowski commented 3 years ago

This seems to do the trick for me

resource "aws_elasticsearch_domain" "main" {
 ...
  cognito_options {
    enabled = true
    identity_pool_id = aws_cognito_identity_pool.es-pool.id
    role_arn = aws_iam_role.es-service.arn
    user_pool_id = aws_cognito_user_pool.es-pool.id
  }

}

resource "aws_cognito_identity_pool" "es-pool" {
...
  lifecycle {
    ignore_changes = [
      cognito_identity_providers
    ]
  }
}

The downside is that resource "aws_cognito_user_pool_client" "kibana" is not managed by TF, so you loosing ability to tweak client options from code, you need to rely on defaults.

petewilcock commented 3 years ago

It feels like the solution here would be to split out the cognito_identity_providers {} block and replace it with a cognito_identity_provider_association resource that would work something like this:

cognito_identity_provider_association {
    identity_pool_id = aws_cognito_identity_pool.name.id
    client_id = ""
    provider_name = ""
}

Similar kind of issue that was seen with security groups/IAM roles that necessitated a separate associative resource to prevent these kinds of annoying overlaps you can't control in a singular definition.

KarstenSiemer commented 3 years ago

I am unable to find a satisfying solution to this. To post-apply.sh is a nice idea but we don't use pipelines for plan/apply in a general way. We use atlantis for automatic stuff with github pull requests and atlantis is running via docker in fargate and it's image is based off of alpine which is a pain to install the aws cli, since aws cli is compiled via glibc and alpine is based on musl libc. And i kinda don't want to temper with the atlantis image that much. For locally planning and applying it could work via local-exec, but I'd really hate that. It violates what we actually want with via gitops and terraform.

anilmujagic commented 3 years ago

I'm using the provisioner to set the callback urls, and the rest of the configuration, after the Elasticsearch domain is created. However, the proper solution (i.e. separate resource) would be highly appreciated.

resource "aws_cognito_user_pool_client" "es_user_pool_client" {
  lifecycle {
    ignore_changes = [
      supported_identity_providers,
      callback_urls,
      logout_urls,
      allowed_oauth_flows,
      allowed_oauth_scopes,
      allowed_oauth_flows_user_pool_client
    ]
  }
}

resource "aws_elasticsearch_domain" "es" {
  provisioner "local-exec" {
    interpreter = ["/bin/bash", "-c"]
    command = <<-COMMAND
      aws cognito-idp update-user-pool-client \
        --user-pool-id ${aws_cognito_user_pool.es_pool.id} \
        --client-id ${aws_cognito_user_pool_client.es_user_pool_client.id} \
        --supported-identity-providers "COGNITO" \
        --callback-urls "https://${self.kibana_endpoint}app/kibana" \
        --logout-urls  "https://${self.kibana_endpoint}app/kibana" \
        --allowed-o-auth-flows "code" \
        --allowed-o-auth-scopes "email" "openid" \
        --allowed-o-auth-flows-user-pool-client \
        --region ${data.aws_region.current.name}
    COMMAND
  }
}

data "aws_region" "current" {}
gdavison commented 2 years ago

Hi everyone. In the AWS documentation for Amazon OpenSearch (the AWS rebranding of Elastiseach), it states

You don't need to add external identity providers to the identity pool. When you configure OpenSearch Service to use Amazon Cognito authentication, it configures the identity pool to use the user pool that you just created.

From my understanding, in your aws_cognito_identity_pool, you should be able to leave out the cognito_identity_providers and AWS will configure it. Note that you will have to ignore changes to that field using lifecycle.ignore_changes.

In the acceptance tests for aws_elasticsearch_domain we do the following

resource "aws_elasticsearch_domain" "test" {
  ...

  cognito_options {
    enabled          = true
    user_pool_id     = aws_cognito_user_pool.test.id
    identity_pool_id = aws_cognito_identity_pool.test.id
    role_arn         = aws_iam_role.test.arn
  }

  ...
}

resource "aws_cognito_identity_pool" "test" {
  identity_pool_name               = %[1]q
  allow_unauthenticated_identities = false

  lifecycle {
    ignore_changes = [cognito_identity_providers]
  }
}

Unfortunately, this isn't well documented.

Does this address the problem for you?

vavdoshka commented 1 year ago

I'm using the provisioner to set the callback urls, and the rest of the configuration, after the Elasticsearch domain is created. However, the proper solution (i.e. separate resource) would be highly appreciated.

resource "aws_cognito_user_pool_client" "es_user_pool_client" {
  lifecycle {
    ignore_changes = [
      supported_identity_providers,
      callback_urls,
      logout_urls,
      allowed_oauth_flows,
      allowed_oauth_scopes,
      allowed_oauth_flows_user_pool_client
    ]
  }
}

resource "aws_elasticsearch_domain" "es" {
  provisioner "local-exec" {
    interpreter = ["/bin/bash", "-c"]
    command = <<-COMMAND
      aws cognito-idp update-user-pool-client \
        --user-pool-id ${aws_cognito_user_pool.es_pool.id} \
        --client-id ${aws_cognito_user_pool_client.es_user_pool_client.id} \
        --supported-identity-providers "COGNITO" \
        --callback-urls "https://${self.kibana_endpoint}app/kibana" \
        --logout-urls  "https://${self.kibana_endpoint}app/kibana" \
        --allowed-o-auth-flows "code" \
        --allowed-o-auth-scopes "email" "openid" \
        --allowed-o-auth-flows-user-pool-client \
        --region ${data.aws_region.current.name}
    COMMAND
  }
}

data "aws_region" "current" {}

It does not seem like a working approach. When AWS Opensearch creates the app client in your User Pool it stores its client ID and secret, which it uses later during login process for the authentication redirect to Cognito User Pool. So Opensearch is locked to the app client it has created. And what happens if you clean up that original client app named AmazonOpenSearchService-bla-bla is that your Domain will be broken: CleanShot 2022-07-23 at 14 42 26@2x

And it seems there is no way to set ClientID and Client Secret through the Opensearch control API.

anilmujagic commented 1 year ago

I'm using the provisioner to set the callback urls, and the rest of the configuration, after the Elasticsearch domain is created. However, the proper solution (i.e. separate resource) would be highly appreciated.

resource "aws_cognito_user_pool_client" "es_user_pool_client" {
  lifecycle {
    ignore_changes = [
      supported_identity_providers,
      callback_urls,
      logout_urls,
      allowed_oauth_flows,
      allowed_oauth_scopes,
      allowed_oauth_flows_user_pool_client
    ]
  }
}

resource "aws_elasticsearch_domain" "es" {
  provisioner "local-exec" {
    interpreter = ["/bin/bash", "-c"]
    command = <<-COMMAND
      aws cognito-idp update-user-pool-client \
        --user-pool-id ${aws_cognito_user_pool.es_pool.id} \
        --client-id ${aws_cognito_user_pool_client.es_user_pool_client.id} \
        --supported-identity-providers "COGNITO" \
        --callback-urls "https://${self.kibana_endpoint}app/kibana" \
        --logout-urls  "https://${self.kibana_endpoint}app/kibana" \
        --allowed-o-auth-flows "code" \
        --allowed-o-auth-scopes "email" "openid" \
        --allowed-o-auth-flows-user-pool-client \
        --region ${data.aws_region.current.name}
    COMMAND
  }
}

data "aws_region" "current" {}

It does not seem like a working approach. When AWS Opensearch creates the app client in your User Pool it stores its client ID and secret, which it uses later during login process for the authentication redirect to Cognito User Pool. So Opensearch is locked to the app client it has created. And what happens if you clean up that original client app named AmazonOpenSearchService-bla-bla is that your Domain will be broken: CleanShot 2022-07-23 at 14 42 26@2x

And it seems there is no way to set ClientID and Client Secret through the Opensearch control API.

It's not ideal, but it is a workaround which works as described, at least until this issue is fixed. Obviously, if you're deleting stuff you can cause it to break. Why do you need to delete the original client?

Relevant, from the repo you referenced: https://github.com/aws-samples/opensearch-in-vpc/blob/main/opensearch-in-vpc-module/opensearch.tf#L68-L87

vavdoshka commented 1 year ago

I'm using the provisioner to set the callback urls, and the rest of the configuration, after the Elasticsearch domain is created. However, the proper solution (i.e. separate resource) would be highly appreciated.

resource "aws_cognito_user_pool_client" "es_user_pool_client" {
  lifecycle {
    ignore_changes = [
      supported_identity_providers,
      callback_urls,
      logout_urls,
      allowed_oauth_flows,
      allowed_oauth_scopes,
      allowed_oauth_flows_user_pool_client
    ]
  }
}

resource "aws_elasticsearch_domain" "es" {
  provisioner "local-exec" {
    interpreter = ["/bin/bash", "-c"]
    command = <<-COMMAND
      aws cognito-idp update-user-pool-client \
        --user-pool-id ${aws_cognito_user_pool.es_pool.id} \
        --client-id ${aws_cognito_user_pool_client.es_user_pool_client.id} \
        --supported-identity-providers "COGNITO" \
        --callback-urls "https://${self.kibana_endpoint}app/kibana" \
        --logout-urls  "https://${self.kibana_endpoint}app/kibana" \
        --allowed-o-auth-flows "code" \
        --allowed-o-auth-scopes "email" "openid" \
        --allowed-o-auth-flows-user-pool-client \
        --region ${data.aws_region.current.name}
    COMMAND
  }
}

data "aws_region" "current" {}

It does not seem like a working approach. When AWS Opensearch creates the app client in your User Pool it stores its client ID and secret, which it uses later during login process for the authentication redirect to Cognito User Pool. So Opensearch is locked to the app client it has created. And what happens if you clean up that original client app named AmazonOpenSearchService-bla-bla is that your Domain will be broken: CleanShot 2022-07-23 at 14 42 26@2x And it seems there is no way to set ClientID and Client Secret through the Opensearch control API.

It's not ideal, but it is a workaround which works as described, at least until this issue is fixed. Obviously, if you're deleting stuff you can cause it to break. Why do you need to delete the original client?

Relevant, from the repo you referenced: https://github.com/aws-samples/opensearch-in-vpc/blob/main/opensearch-in-vpc-module/opensearch.tf#L68-L87

So the point here is not about deleting things but rather about the fact that OpenSearch is oblivious to whatever side application client one would try to force it to use. It creates its application client in Cognito (automatically), and it saves its Client ID and Secret ID internally in the cluster and uses them to redirect the user to Cognito to its own application client. So there is no way to make it redirect the user to the custom-created application client as described approach relies on. And as a proof, one can set things up the way described and just remove the original client application. OpenSearch won't try to use the other application client as that setup suggests. It will just fail to authenticate.

anilmujagic commented 1 year ago

So the point here is not about deleting things but rather about the fact that OpenSearch is oblivious to whatever side application client one would try to force it to use. It creates its application client in Cognito (automatically), and it saves its Client ID and Secret ID internally in the cluster and uses them to redirect the user to Cognito to its own application client. So there is no way to make it redirect the user to the custom-created application client as described approach relies on. And as a proof, one can set things up the way described and just remove the original client application. OpenSearch won't try to use the other application client as that setup suggests. It will just fail to authenticate.

Ok, I'm not sure if I understand what you want to say, but what you describe is the problem this issue is pretty much about. So yes, the problem with the default client exists. However, I have demonstrated in the linked repo that there is a workaround that makes it possible to have automated deployment with Terraform. Hopefully, this issue here will be solved and the workaround can be removed to have cleaner Terraform resource configuration code, but until then the workaround does enable deployment without manual intervention.

strelok899 commented 1 year ago

@gdavison is this bug going to be fixed?

matthewbga commented 1 year ago

I am curious on this one as well. It seems that the configuration options/capabilities in terraform aren't "covering" the options, etc. AWS provides in terms of Cognito. Is there an improvement in the works?

github-actions[bot] commented 1 year ago

This functionality has been released in v4.60.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

github-actions[bot] commented 1 year 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.