Skip to content

Conversation

markpollack
Copy link
Member

If ampqs is used in the URI and no explicit port is declared, the port number 5671 will be used instead of 5672

Resolves #6401

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 29, 2019
@xyloman
Copy link
Contributor

xyloman commented Oct 29, 2019

@markpollack thank you for performing this PR this is something that I have had on my list of things to contribute for some time. I know other in the community will greatly benefit from this update. 🎉

@philwebb philwebb added for: team-attention An issue we'd like other members of the team to review type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 29, 2019
@philwebb philwebb added this to the 2.1.10 milestone Oct 29, 2019
…g/springframework/boot/autoconfigure/amqp/RabbitProperties.java

Co-Authored-By: Bryan Kelly <xyloman@gmail.com>
@mbhave
Copy link
Contributor

mbhave commented Oct 31, 2019

There's some room for improvement here where if an amqps address is provided, we can set spring.rabbitmq.ssl.enabled to true. This would remove the need for the user to set spring.rabbitmq.ssl.enabled: true in addition to setting an amqps address.

@snicoll
Copy link
Member

snicoll commented Nov 1, 2019

Good catch. I suppose we'd need to check that all adresses are either all using amqps or amqp. If the net effect of this PR is that users must set the URI and the SSL flag, I think we should reconsider this change to avoid that.

snicoll pushed a commit to snicoll/spring-boot that referenced this pull request Nov 1, 2019
See spring-projectsgh-18808

Co-Authored-By: Bryan Kelly <xyloman@gmail.com>
snicoll added a commit to snicoll/spring-boot that referenced this pull request Nov 1, 2019
@wilkinsona
Copy link
Member

I think we should determine whether or not to implicitly enable SSL based on the first address. This will align the behaviour with determining the host, port, etc that are set on the RabbitConnectionFactoryBean.

snicoll pushed a commit to snicoll/spring-boot that referenced this pull request Nov 5, 2019
See spring-projectsgh-18808

Co-Authored-By: Bryan Kelly <xyloman@gmail.com>
snicoll added a commit to snicoll/spring-boot that referenced this pull request Nov 5, 2019
@snicoll snicoll self-assigned this Nov 5, 2019
@snicoll snicoll removed the for: team-attention An issue we'd like other members of the team to review label Nov 5, 2019
snicoll pushed a commit that referenced this pull request Nov 5, 2019
See gh-18808

Co-Authored-By: Bryan Kelly <xyloman@gmail.com>
@snicoll snicoll closed this in 9b3d625 Nov 5, 2019
@snicoll
Copy link
Member

snicoll commented Nov 5, 2019

Thank you Mark. As suggested by @mbhave, I've polished your proposal to automatically enable ssl support when amqps is detected. That way, you can have SSL configuration and just use it (or not) based on the actual address.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support amqps:// URIs in spring.rabbitmq.addresses
7 participants