Skip to content

fix(ldap): add url and scope validation to ldap ping command#804

Merged
bupd merged 1 commit into
goharbor:mainfrom
PrasunaEnumarthy:fix/ldap-ping-validation
May 7, 2026
Merged

fix(ldap): add url and scope validation to ldap ping command#804
bupd merged 1 commit into
goharbor:mainfrom
PrasunaEnumarthy:fix/ldap-ping-validation

Conversation

@PrasunaEnumarthy

@PrasunaEnumarthy PrasunaEnumarthy commented Apr 11, 2026

Copy link
Copy Markdown
Contributor

Description

Adds input validation for the harbor ldap ping command to prevent invalid inputs from being sent to the API.

  • Normalizes and validates --ldap-url using utils.FormatUrl() and utils.ValidateURL()
  • Restricts --ldap-scope to valid values: 0 (base), 1 (one level), 2 (subtree)

This ensures early CLI validation and clearer error messages instead of delayed API failures.

Type of Change

Please select the relevant type.

  • Bug fix
  • New feature
  • Refactor
  • Documentation update
  • Chore / maintenance

Changes

  • Added URL normalization and validation for --ldap-url
  • Added bounds checking for --ldap-scope (0–2 only)
  • Improved error handling with early validation before API call
image

@codecov

codecov Bot commented Apr 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 8.03%. Comparing base (60ad0bd) to head (f4593f4).
⚠️ Report is 131 commits behind head on main.

Files with missing lines Patch % Lines
cmd/harbor/root/ldap/ping.go 0.00% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             main    #804      +/-   ##
=========================================
- Coverage   10.99%   8.03%   -2.96%     
=========================================
  Files         173     270      +97     
  Lines        8671   13173    +4502     
=========================================
+ Hits          953    1058     +105     
- Misses       7612   12002    +4390     
- Partials      106     113       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: PrasunaEnumarthy <eswari.prasuna@gmail.com>
@PrasunaEnumarthy PrasunaEnumarthy force-pushed the fix/ldap-ping-validation branch from eca8b99 to f4593f4 Compare April 11, 2026 19:41
@qcserestipy

Copy link
Copy Markdown
Collaborator

Thank you for the contribution, in principle LGTM. Please add screenshots or a screen record of the new behavior in the description.

@PrasunaEnumarthy

PrasunaEnumarthy commented Apr 15, 2026

Copy link
Copy Markdown
Contributor Author

@qcserestipy
I've added screenshot demonstrating the new validation behavior. Please let me know if anything else needs to be improved.

@qcserestipy qcserestipy self-requested a review April 16, 2026 03:43

@qcserestipy qcserestipy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Lgtm

@bupd bupd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm

@bupd bupd merged commit 3dda0d4 into goharbor:main May 7, 2026
6 of 8 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.

[bug]: CLI validation missing: ldap ping command lacks URL normalization and scope bounds checking

3 participants