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

Support for RabbitMQ in AmazonMQ #16030

Closed
yardensachs opened this issue Nov 4, 2020 · 24 comments · Fixed by #16108
Closed

Support for RabbitMQ in AmazonMQ #16030

yardensachs opened this issue Nov 4, 2020 · 24 comments · Fixed by #16108
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/mq Issues and PRs that pertain to the mq service.
Milestone

Comments

@yardensachs
Copy link
Contributor

yardensachs commented Nov 4, 2020

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

AmazonMQ now support RabbitMQ as an engine. This is the first time AWS adds another type of engine to AMQ service. Looking at the Go package - the changes are minimal.

New or Affected Resource(s)

  • aws_mq_broker

Potential Terraform Configuration

resource "aws_mq_broker" "example" {
  broker_name        = "example"

  engine_type        = "RabbitMQ"
  engine_version     = "3.8.6"
  host_instance_type = "mq.t2.micro"
  security_groups    = [aws_security_group.test.id]

  user {
    username = "ExampleUser"
    password = "MindTheGap"
  }
}

References

@yardensachs yardensachs added the enhancement Requests to existing resources that expand the functionality or scope. label Nov 4, 2020
@ghost ghost added the service/mq Issues and PRs that pertain to the mq service. label Nov 4, 2020
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Nov 4, 2020
@yardensachs
Copy link
Contributor Author

I'm going to take crack at this today

@yardensachs
Copy link
Contributor Author

blocked by #16021

@yardensachs
Copy link
Contributor Author

yardensachs commented Nov 5, 2020

AWS has some issue with their MQ API - it does not return list of users in DescribeBroker's payload if it's an AMQ instance with RabbitMQ engine.

@ewbankkit
Copy link
Contributor

@ewbankkit ewbankkit removed the needs-triage Waiting for first response or review from a maintainer. label Nov 5, 2020
@yardensachs
Copy link
Contributor Author

yardensachs commented Nov 6, 2020

From my support convo with AWS, It looks like for RabbitMQ engine type, AWS is not planning on sending the users list in the DescribeBroker API call:
Screen Shot 2020-11-06 at 7 11 04 AM

The issue that I am getting is that if the users list does not come back - terraform will assume it needs to add them again, because the mq broker state has that user list empty (its failing the tests, saying that nothing was changed, but a plan came back with an update). This only happens with RabbitMQ, It does return on the user list if the engine type is ApacheMQ.

@ewbankkit I need some advice on this resource.
How should I proceed?
Is there a way to ignore that user list? (based on the engine type)
Should username and password be only be effective on broker creation? (based on the engine type)

@DmytroMorhulDevPro
Copy link

Waiting, waiting, waiting ... 👍

@yardensachs
Copy link
Contributor Author

I am happy to finish ASAP, but I am blocked by my question. If anyone can help me with my question here #16108 (comment)

@Spareo
Copy link

Spareo commented Nov 11, 2020

@yardensachs thanks for getting this done so quickly, hopefully someone can help you get unblocked ASAP.

@ienders
Copy link

ienders commented Nov 13, 2020

There are a couple services which have similar issues. For example, DMS replication_task_settings contains read-only properties that need to be ignored, as re-submitting existing state as-is doesn't work; see: #13476.

I'd argue that it would be better to unblock the feature and work to improve in the future, and note to folks that they should do something like the following to ignore the limitation of the API (if it's the change detection here that's the issue):

  lifecycle {
    ignore_changes = [user]
  }

I've been eagerly awaiting an AWS managed drop-in RMQ solution for years, so if this isn't OK with the community, I'll be fine waiting another little bit.

Thank you for your contribution!

@yardensachs
Copy link
Contributor Author

@ienders I was able to figure it out without help!
Hopefully I will get #16108 reviewed quickly, I have seen PRs take long (and short) time til the team had time to review.

@steverukuts
Copy link

Some of you might be thinking that you can work around this by creating a broker in CloudFormation temporarily and referencing it with a data block, importing the resource later when this is supported by Terraform.

This crashes as (I think) the result from the AWS API is slightly different to what the data block is expecting. This is described further in #16047 and is fixed by PR #16108 (the same PR linked to this issue).

The only way to make this work while we wait is to create the broker using the console or the CLI and adding variables to your Terraform configuration or SSM parameters.

So, hopefully I can save others some time here if you're thinking along the same lines as me.

@mikearruda
Copy link

Any update on this one?

1 similar comment
@sunilkumarvytla
Copy link

Any update on this one?

@elbart0washere
Copy link

+1

@yardensachs
Copy link
Contributor Author

yardensachs commented Jan 24, 2021

I don't know when this will be merged.

I can't imagine the amount of work the maintainers have, and the speed at which AWS is now releasing changes - I am sure its crazy.

Once it gets reviewed I will update the PR.

@alex-bes
Copy link

Some of you might be thinking that you can work around this by creating a broker in CloudFormation temporarily and referencing it with a data block, importing the resource later when this is supported by Terraform.

This crashes as (I think) the result from the AWS API is slightly different to what the data block is expecting. This is described further in #16047 and is fixed by PR #16108 (the same PR linked to this issue).

The only way to make this work while we wait is to create the broker using the console or the CLI and adding variables to your Terraform configuration or SSM parameters.

So, hopefully I can save others some time here if you're thinking along the same lines as me.

Similar to what @steverukuts describes, the viable workaround is to set up the broker via aws_cloudformation_stack terraform resource and reference cloud formation outputs instead of a data block.

This is obviously a hack but well, it works. Also, one could hide the details in a terraform module so variables/outputs will not change while migrating to a native resource once the respective PR is merged.

https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudformation_stack
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-amazonmq-broker.html

@yardensachs
Copy link
Contributor Author

yardensachs commented Feb 13, 2021

Some of you might be thinking that you can work around this by creating a broker in CloudFormation temporarily and referencing it with a data block, importing the resource later when this is supported by Terraform.
This crashes as (I think) the result from the AWS API is slightly different to what the data block is expecting. This is described further in #16047 and is fixed by PR #16108 (the same PR linked to this issue).
The only way to make this work while we wait is to create the broker using the console or the CLI and adding variables to your Terraform configuration or SSM parameters.
So, hopefully I can save others some time here if you're thinking along the same lines as me.

Similar to what @steverukuts describes, the viable workaround is to set up the broker via aws_cloudformation_stack terraform resource and reference cloud formation outputs instead of a data block.

This is obviously a hack but well, it works. Also, one could hide the details in a terraform module so variables/outputs will not change while migrating to a native resource once the respective PR is merged.

https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudformation_stack
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-amazonmq-broker.html

Ended up doing this as a temporary solution baed on that idea:

locals {
  template_body = {
    "Parameters" = {},
    "Resources" = {
      "RabbitBroker" = {
        "Type" = "AWS::AmazonMQ::Broker",
        "Properties" = {
          "AuthenticationStrategy"  = "simple",
          "AutoMinorVersionUpgrade" = "true",
          "BrokerName"              = "name",
          "DeploymentMode"          = "SINGLE_INSTANCE",
          "EngineType"              = "RabbitMQ",
          "EngineVersion"           = "3.8.6",
          "HostInstanceType"        = "mq.t3.micro",
          "Logs" = {
            "General" = "false"
          },
          "MaintenanceWindowStartTime" = {
            "DayOfWeek" = "WEDNESDAY",
            "TimeOfDay" = "00:00",
            "TimeZone"  = "UTC"
          },
          "PubliclyAccessible" = "false",
          "SecurityGroups"     = [""],
          "StorageType"        = "ebs",
          "SubnetIds"          = [""],
          "Tags" = [
            {
              "Key"   = "key",
              "Value" = "value"
            }
          ],
          "Users" = [
            {
              "Username" = "username",
              "Password" = "password",
            },
          ]
        }
      }
    }

    "Outputs" = {
      "Endpoints" = {
        "Value" = {
          "Fn::Select" = [
            0,
            {
              "Fn::GetAtt" = [
                "RabbitBroker",
                "AmqpEndpoints"
              ]
            }
          ]
        },
        "Description" = "RabbitBroker AmqpEndpoints"
      }
    }

  }
}


resource "aws_cloudformation_stack" "rabbitmq" {
  name = "rabbitmq-stack"

  template_body = jsonencode(local.template_body)
}

output "endpoint" {
  value       = aws_cloudformation_stack.rabbitmq.outputs["Endpoints"]
  description = "RabbitMQ endpoint"
}

@mrtristan
Copy link

@yardensachs what's your thought on how cleanly this will convert once the provider is capable of managing it? i was going to manually create things in the console and then import once capable, but this seems better as the interim step.

@yardensachs
Copy link
Contributor Author

@mrtristan Good question, you can ask CF to keep the resources, even if the CF stack is delete AFAIK:
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-attribute-deletionpolicy.html

And then you can just import the broker to terraform.

@HaroonSaid
Copy link

Any one looking at the PR ?
4 months seems a really long time for code review

@github-actions github-actions bot added this to the v3.32.0 milestone Mar 5, 2021
@ghost
Copy link

ghost commented Mar 12, 2021

This has been released in version 3.32.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 for triage. Thanks!

@nsharma-fy
Copy link

Native RabbitMQ works in 3.32, thank you!

@ghost
Copy link

ghost commented Apr 5, 2021

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked as resolved and limited conversation to collaborators Apr 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/mq Issues and PRs that pertain to the mq service.
Projects
None yet
Development

Successfully merging a pull request may close this issue.