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

Mail health indicator fails when host is not set in properties #38007

Closed
ykartsev opened this issue Oct 24, 2023 · 2 comments
Closed

Mail health indicator fails when host is not set in properties #38007

ykartsev opened this issue Oct 24, 2023 · 2 comments
Assignees
Labels
type: regression A regression from a previous release
Milestone

Comments

@ykartsev
Copy link

ykartsev commented Oct 24, 2023

In what version(s) of Spring Integration are you seeing this issue?

5.5.19

Describe the bug
I started seeing it after I have updated my Spring Integration based project from Spring Boot v2.6.15 to v2.7.17. After that my /health endpoint started returning "DOWN", showing mail component as culprit. Debugging showed that this happens due to an updated doHealthCheck method in MailHealthIndicator class:
Version in v2.6.15 looks like this:

	@Override
	protected void doHealthCheck(Builder builder) throws Exception {
		builder.withDetail("location", this.mailSender.getHost() + ":" + this.mailSender.getPort());
		this.mailSender.testConnection();
		builder.up();
	}

Version in v2.7.17 looks like this:

	@Override
	protected void doHealthCheck(Builder builder) throws Exception {
		int port = this.mailSender.getPort();
		builder.withDetail("location", (port != JavaMailSenderImpl.DEFAULT_PORT)
				? this.mailSender.getHost() + ":" + this.mailSender.getPort() : this.mailSender.getHost());
		this.mailSender.testConnection();
		builder.up();
	}

In both cases this.mailSender.getHost() is null, but in the latter one builder.withDetail throws IllegalArgumentException due to value argument being null. In first case it was just setting the location value to null:-1.

So, despite JavaMailSenderImpl works fine (sends e-mails successfully), /health for mail component fails due to host being null. And looks like such JavaMailSenderImpl initialization existed before, it just it surfaced with an update of MailHealthIndicator class.

To Reproduce

  1. Initialize a mail outbound-channel-adapter using java-mail-properties as documentation instructs:

Starting with version 5.1.3, the host, username ane mail-sender can be omitted, if java-mail-properties is provided. However, the host and username has to be configured with appropriate Java mail properties, e.g. for SMTP

  <int-mail:outbound-channel-adapter channel="outboundEmailChannel"
                                     java-mail-properties="javaMailProperties"
                                     username="${email.smtp.username}"
                                     password="${email.smtp.password}"/>

  <util:properties id="javaMailProperties">
    <prop key="mail.smtp.host">${email.smtp.server.host}</prop>
    <prop key="mail.smtp.port">${email.smtp.server.port}</prop>
    <prop key="mail.debug">${email.smtp.debug}</prop>
    <prop key="mail.debug">${email.smtp.debug}</prop>
    <prop key="mail.smtp.auth">${email.smtp.auth.enable}</prop>
    <prop key="mail.smtp.starttls.enable">${email.smtp.starttls.enable}</prop>
  </util:properties>
  1. Start up the application
  2. Access /health endpoint with details (using management.endpoint.health.show-details=always)
  3. Observe "DOWN" status with the following mail field:
"mail": {
            "status": "DOWN",
            "details": {
                "error": "java.lang.IllegalArgumentException: Value must not be null"
            }
        },

Expected behavior

/health endpoint should return UP if JavaMailSenderImpl is correctly initialized via javaMailProperties. I would assume that either MailHealthIndicator should be updated or initialization of the JavaMailSenderImpl should set host/port from javaMailProperties.

Sample

I can come up with something if really needed in this case, but at the moment I hope that information above should be enough. Please let me know if otherwise. Thank you.

@ykartsev ykartsev added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Oct 24, 2023
@artembilan
Copy link
Member

Even if Spring Integration creates that JavaMailSenderImpl internally based on those javaMailProperties, the problem is in Spring Boot.
Your suggestion about updating MailHealthIndicator in regard of consulting JavaMailSenderImpl.javaMailProperties is correct.
Moving the issue into Spring Boot project...

@philwebb philwebb transferred this issue from spring-projects/spring-integration Oct 24, 2023
@wilkinsona
Copy link
Member

This is a regression that was introduced by the changes for #35247.

@wilkinsona wilkinsona added type: regression A regression from a previous release and removed type: bug A general bug status: waiting-for-triage An issue we've not yet triaged labels Oct 24, 2023
@wilkinsona wilkinsona added this to the 2.7.x milestone Oct 24, 2023
@scottfrederick scottfrederick self-assigned this Oct 24, 2023
@scottfrederick scottfrederick changed the title JavaMailSender initialization via java-mail-properties does not set host/port causing /health mail actuator to fail Mail health indicator fails when host is not set in properties Oct 25, 2023
@scottfrederick scottfrederick modified the milestones: 2.7.x, 2.7.18 Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: regression A regression from a previous release
Projects
None yet
Development

No branches or pull requests

4 participants