Skip to content

Conversation

mxm-tr
Copy link

@mxm-tr mxm-tr commented Apr 16, 2025

No description provided.

Copy link

@thcdrt thcdrt left a comment

Choose a reason for hiding this comment

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

LGTM

@reasonerjt reasonerjt requested a review from stonezdj April 21, 2025 08:42
2. Define its lifetime to a value that would prevent rate limiting from being triggered (10s?) in the [proxy-controller](https://github.com/goharbor/harbor/blob/bfc29904f96e17248a4e6204d12058c1d7d05ab8/src/controller/proxy/controller.go#L41-L48)

```golang
manifestExistsCacheInterval = 10 * time.Second
Copy link

@renmaosheng renmaosheng Apr 21, 2025

Choose a reason for hiding this comment

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

Hi Maxime, by given such internal, there would be a a chance the client will pull different artifacts with the same binary from the source repo, if your pipeline is using this, you will have different testing results depending on when you run the tests. Such ambiguity will lead to difficulty when you trouble-shooting some issues. In my opinion, we should encourage the user to use sha digest instead of tag to reduce the API calling number.

Copy link
Author

Choose a reason for hiding this comment

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

Indeed the best practice is to use the sha digest, unfortunately some big open source projects (rancher for example) use hundreds of images from various origins and only allow the user to specify repository proxy urls, this proposal was made to ensure we don't send a HEAD request every time we pull an artifact.

The risk of pulling different artifacts using a tag exists indeed, maybe this behavior could be configurable?

Choose a reason for hiding this comment

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

Hi Maxime, I don't know how to make it configurable, but we should highlight to the users about the possibility of pulling different binaries in different time, thanks.

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.

6 participants