Skip to content

fix(cli): validate arg count for artifact tags create/delete to preve… - #1071

Open
mantrapatel05 wants to merge 1 commit into
goharbor:mainfrom
mantrapatel05:fix/artifact-tags-args-panic-clean
Open

mantrapatel05 wants to merge 1 commit into
goharbor:mainfrom
mantrapatel05:fix/artifact-tags-args-panic-clean

Conversation

@mantrapatel05

Copy link
Copy Markdown
Contributor

Description

harbor artifact tags create and delete both panic with an "index out of range" error
when called with exactly one argument. Neither command had an Args validator on the
cobra.Command, so passing a single arg fell into the non-interactive branch and tried
to read args[1], which doesn't exist.

Type of Change

Please select the relevant type.

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

Changes

  • Added an Args validator to CreateTagsCmd that allows 0 args (interactive mode) or 2 args (non-interactive), rejects anything else with a clean error
  • Added the same Args validator to DeleteTagsCmd
  • Verified manually: 1 arg → clean error, no panic; 2 args → works as before; 0 args → interactive prompts still work; go build ./... passes

@mantrapatel05
mantrapatel05 force-pushed the fix/artifact-tags-args-panic-clean branch from 66c676f to 6ebf1a2 Compare July 27, 2026 16:23
@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

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

Files with missing lines Patch % Lines
cmd/harbor/root/artifact/tags/create.go 0.00% 4 Missing ⚠️
cmd/harbor/root/artifact/tags/delete.go 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             main   #1071      +/-   ##
=========================================
- Coverage   10.99%   9.54%   -1.45%     
=========================================
  Files         173     321     +148     
  Lines        8671   16097    +7426     
=========================================
+ Hits          953    1536     +583     
- Misses       7612   14427    +6815     
- Partials      106     134      +28     

☔ View full report in Codecov by Harness.
📢 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.

@mantrapatel05
mantrapatel05 force-pushed the fix/artifact-tags-args-panic-clean branch 2 times, most recently from 888d6b6 to 6c5b3e6 Compare July 27, 2026 16:37
…nt panic

Signed-off-by: Mantra Patel <patelmantra551@gmail.com>
@mantrapatel05
mantrapatel05 force-pushed the fix/artifact-tags-args-panic-clean branch from 6c5b3e6 to 94250f3 Compare July 27, 2026 16:53
@qcserestipy
qcserestipy requested review from qcserestipy and a balanced review from Copilot September 1, 2026 08:04
@qcserestipy qcserestipy added bug Something isn't working status/has-approved-pr Issue has a linked pull request that has been approved and is waiting to be merged. labels Sep 1, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Prevents artifact tag create/delete commands from panicking on invalid argument counts.

Changes:

  • Allows only 0 or 2 positional arguments.
  • Returns a validation error for all other counts.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
create.go Validates create-command arguments.
delete.go Validates delete-command arguments.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

if len(args) == 0 || len(args) == 2 {
return nil
}
return fmt.Errorf("requires either 0 arguments(interactive mode) or 2 arguments(<project>/<repository>/<reference> <tag>), got %d", len(args))

@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.

Thank you for your contribution! lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working status/has-approved-pr Issue has a linked pull request that has been approved and is waiting to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug]: artifact tags create and delete panic with index out of range when exactly one argument is provided

3 participants