Skip to content

Enable logger usage checks in security subprojects#6210

Merged
cwperks merged 4 commits into
opensearch-project:mainfrom
cwperks:enable-logger-usage-check
Jun 16, 2026
Merged

Enable logger usage checks in security subprojects#6210
cwperks merged 4 commits into
opensearch-project:mainfrom
cwperks:enable-logger-usage-check

Conversation

@cwperks

@cwperks cwperks commented Jun 13, 2026

Copy link
Copy Markdown
Member

Description

Enables the logger usage precommit check for the Security repo subprojects where it can pass today:

  • sample-resource-plugin
  • bwc-test

The sample resource plugin had two existing logger callsites that passed a throwable alongside parameterized arguments. Those now use ParameterizedMessage, which matches the pattern accepted by OpenSearch's logger usage checker.

The duplicate root-level disable/comment was also removed because the logger usage checker is now published through OpenSearch build-tools. The root project still keeps its existing disable near the other root precommit toggles because enabling it currently surfaces a larger backlog of existing logger usage violations in main/test code.

This also updates Security's GitHub Actions Gradle steps from the old pinned gradle/gradle-build-action reference to gradle/actions/setup-gradle pinned to the full v3.5.0 commit SHA. The repository now rejects nested tag-based action references, and the old Gradle action delegates through gradle/actions/setup-gradle@v3.5.0, which causes jobs to fail before checkout/build starts.

Local composite BWC actions are also pinned so those jobs do not hit the same organization policy when they run.

The reusable opensearch-build workflow references were also advanced to a newer full commit SHA. The older pinned opensearch-build commit still used tag-based nested actions in get-ci-image-tag and the code diff workflows, which caused Get-CI-Image-Tag and Code-Diff-Analyzer to fail before their jobs could run.

Testing

  • ./gradlew loggerUsageCheck opensearch-sample-resource-plugin:loggerUsageCheck opensearch-security-bwc-test:loggerUsageCheck
  • ./gradlew spotlessApply

./gradlew precommit currently reaches the sample resource plugin and then fails on existing forbiddenApisMain violations for URL#openStream() in the sample plugin. That is unrelated to this logger usage change.

Signed-off-by: Craig Perkins <craig5008@gmail.com>
Signed-off-by: Craig Perkins <craig5008@gmail.com>
@github-actions

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 No relevant tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@github-actions

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Fix incorrect error message text

The error message says "Failed to update resource" but this is in
UpdateResourceGroupTransportAction. The message should say "Failed to update
resource group" to accurately reflect the operation being performed and avoid
confusion during debugging.

sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/transport/UpdateResourceGroupTransportAction.java [82]

-log.error(() -> new ParameterizedMessage("Failed to update resource: {}", request.getResourceId()), e);
+log.error(() -> new ParameterizedMessage("Failed to update resource group: {}", request.getResourceId()), e);
Suggestion importance[1-10]: 8

__

Why: The error message incorrectly states "Failed to update resource" in UpdateResourceGroupTransportAction, when it should say "Failed to update resource group" to accurately reflect the operation and improve debugging clarity.

Medium

@cwperks cwperks merged commit 996aa43 into opensearch-project:main Jun 16, 2026
62 of 63 checks passed
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.

2 participants