Skip to content

Conversation

@andsel
Copy link
Contributor

@andsel andsel commented Sep 18, 2025

Release notes

[rn:skip]

What does this PR do?

Translates Password setting class into plain Java.

Why is it important/What is the impact to the user?

N/A

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation
  • [ ] I have made corresponding change to the default configuration files (and/or docker env variables)
  • I have added tests that prove my fix is effective or that my feature works

Author's Checklist

  • [ ]

How to test this PR locally

Enable http basic authentication and try to connect.
Update logstash.yml with:

api.auth.type: basic
api.auth.basic.username: "logstash-user"
api.auth.basic.password: "s3cUreP4$$w0rD

Try to connect without user credentials header and verify that you don't have access.
Try with credentials and it works:

curl -v -u 'logstash-user:s3cUreP4$$w0rD' http://localhost:9600/_node/stats | jq .pipelines.main

Related issues

@andsel andsel self-assigned this Sep 18, 2025
@github-actions
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@mergify
Copy link
Contributor

mergify bot commented Sep 18, 2025

This pull request does not have a backport label. Could you fix it @andsel? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit.
  • If no backport is necessary, please add the backport-skip label

@andsel andsel marked this pull request as ready for review September 18, 2025 13:55
@jsvd jsvd requested a review from Copilot September 18, 2025 14:49
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Migrates the Ruby Password setting class to a Java implementation to improve performance and maintainability. The change translates the existing Ruby LogStash::Setting::Password class into a plain Java PasswordSetting class while maintaining the same functionality and API.

  • Implemented PasswordSetting in Java with identical coercion and validation logic
  • Updated Ruby code to use the Java implementation via java_import
  • Added comprehensive test coverage for the new Java implementation

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
logstash-core/src/main/java/org/logstash/settings/PasswordSetting.java New Java implementation of password setting with coercion logic
logstash-core/src/test/java/org/logstash/settings/PasswordSettingTest.java Test suite covering password setting validation and coercion scenarios
logstash-core/lib/logstash/settings.rb Removed Ruby Password class and imported Java PasswordSetting
logstash-core/lib/logstash/environment.rb Updated references to use PasswordSetting instead of Password
logstash-core/spec/logstash/settings/password_spec.rb Updated test to expect IllegalArgumentException instead of ArgumentError

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Member

@jsvd jsvd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just address the whitespace and when typoes!

@elastic-sonarqube
Copy link

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @andsel

@andsel andsel merged commit 6aa265d into elastic:main Sep 18, 2025
13 checks passed
donoghuc added a commit that referenced this pull request Sep 22, 2025
andsel added a commit to andsel/logstash that referenced this pull request Sep 30, 2025
andsel added a commit that referenced this pull request Oct 2, 2025
Secoind attempt to translates Password setting class into plain Java, which exposes a logger used by the Ruby ValidatedPassword subclass. Fixes a bug happened in previous #18183.

Co-authored: @donoghuc
v1v pushed a commit that referenced this pull request Oct 21, 2025
Secoind attempt to translates Password setting class into plain Java, which exposes a logger used by the Ruby ValidatedPassword subclass. Fixes a bug happened in previous #18183.

Co-authored: @donoghuc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reimplement LogStash::Setting::Password to Java

3 participants