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

Adding CREATE/DELETE Label for Robot Account #15815

Conversation

DandyDeveloper
Copy link
Contributor

@DandyDeveloper DandyDeveloper commented Oct 19, 2021

This is a basic implementation of adding CREATE/DELETE options for labels on both Artifacts AND Chartrepo charts.

Tests will follow, when I understand how your Python tests work :)

@DandyDeveloper DandyDeveloper force-pushed the improvements/dandydev/robot-account-support-label-create branch from 82dc6ed to 568cf5b Compare October 19, 2021 06:38
@DandyDeveloper
Copy link
Contributor Author

DandyDeveloper commented Oct 19, 2021

@YangJiao0817 @wy65701436 FYI, I'm a bit confused by your testing on the API Tests. I can't find ref

https://github.com/dandydeveloper/harbor/blob/068d1d46ca3f9f2d8cb3fca0fb6649bd9390c54d/tests/apitests/python/library/artifact.py#L57

In order to create a delete label for the client correctly.

@YangJiao0817
Copy link
Member

@DandyDeveloper Please execute make swagger_client in the harbor/ directory, this can solve your problem.

@heww heww requested a review from AllForNothing October 25, 2021 09:10
@codecov
Copy link

codecov bot commented Oct 25, 2021

Codecov Report

Merging #15815 (402f3d0) into master (2ec8a41) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #15815      +/-   ##
==========================================
+ Coverage   66.83%   66.86%   +0.02%     
==========================================
  Files         935      935              
  Lines       77679    77716      +37     
  Branches     2290     2290              
==========================================
+ Hits        51913    51961      +48     
+ Misses      21762    21751      -11     
  Partials     4004     4004              
Flag Coverage Δ
unittests 66.86% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ide-nav/system-robot-accounts/system-robot-util.ts 67.64% <100.00%> (ø)
src/common/utils/encrypt.go 66.66% <0.00%> (-2.78%) ⬇️
...tegration/tag-retention/tag-retention.component.ts 29.95% <0.00%> (-2.21%) ⬇️
src/controller/robot/controller.go 57.60% <0.00%> (-1.29%) ⬇️
src/common/utils/utils.go 72.86% <0.00%> (-0.62%) ⬇️
src/core/auth/authenticator.go 58.13% <0.00%> (ø)
src/pkg/reg/adapter/jfrog/adapter.go 59.89% <0.00%> (ø)
...lnerability-scanning/result-bar-chart.component.ts 33.65% <0.00%> (ø)
...ry/artifact/artifact-tag/artifact-tag.component.ts 30.92% <0.00%> (+0.65%) ⬆️
src/portal/src/app/shared/units/shared.utils.ts 24.35% <0.00%> (+1.28%) ⬆️
... and 4 more

@AllForNothing AllForNothing self-assigned this Oct 26, 2021
@AllForNothing
Copy link
Contributor

AllForNothing commented Oct 26, 2021

@DandyDeveloper Please also modify harbor/tests/resources/Harbor-Pages/Robot_Account.robot, and add "Delete Artifact label", "Create Helm Chart label" and "Delete Helm Chart label" as below, this will make sure e2e testing can run successfully
image

@DandyDeveloper DandyDeveloper force-pushed the improvements/dandydev/robot-account-support-label-create branch from a7a8996 to d213bbd Compare October 27, 2021 04:14
@DandyDeveloper
Copy link
Contributor Author

@AllForNothing All done. Thanks for the review.

Copy link
Contributor

@AllForNothing AllForNothing left a comment

Choose a reason for hiding this comment

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

LGTM

@AllForNothing
Copy link
Contributor

@DandyDeveloper One more thing, please squash your commits into one

Copy link
Contributor

@wy65701436 wy65701436 left a comment

Choose a reason for hiding this comment

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

lgtm, thanks

Signed-off-by: Aaron Layfield <aaron.layfield@gmail.com>
@DandyDeveloper DandyDeveloper force-pushed the improvements/dandydev/robot-account-support-label-create branch from d213bbd to 402f3d0 Compare October 27, 2021 04:44
@DandyDeveloper
Copy link
Contributor Author

@AllForNothing Look good?

@AllForNothing
Copy link
Contributor

@AllForNothing Look good?

Yes. Thanks for your contribution

@AllForNothing AllForNothing merged commit 7b75a45 into goharbor:master Oct 27, 2021
prahaladdarkin pushed a commit to prahaladdarkin/harbor that referenced this pull request Nov 12, 2021
Signed-off-by: Aaron Layfield <aaron.layfield@gmail.com>
stonezdj pushed a commit to stonezdj/harbor that referenced this pull request Jan 4, 2022
Signed-off-by: Aaron Layfield <aaron.layfield@gmail.com>
taejune pushed a commit to tmax-cloud/hyperregistry that referenced this pull request Feb 15, 2022
Signed-off-by: Aaron Layfield <aaron.layfield@gmail.com>
prahaladdarkin pushed a commit to prahaladdarkin/harbor that referenced this pull request Mar 13, 2022
Signed-off-by: Aaron Layfield <aaron.layfield@gmail.com>
@DandyDeveloper
Copy link
Contributor Author

@YangJiao0817 @AllForNothing @wy65701436 This has been skipped for several releases now.

Can I please get this included in a release?

@AllForNothing
Copy link
Contributor

@DandyDeveloper This is already included in the upcoming 2.5

@DandyDeveloper
Copy link
Contributor Author

@AllForNothing Oh, sorry I don't see a tag anywhere so I assume it wasn't. Thank you.

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.

Robot Account "403 FORBIDDEN" on "Add" or "Removal" or artifact (image) label
4 participants