-
Notifications
You must be signed in to change notification settings - Fork 8
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
First PoC of Harbor Satellite #21
Conversation
Included 2 possible implementations : - using sync.Map optimised for high concurrency read/writes - using base map with RWMutex lock/unlock better performing for low-concurrency read/writes
Removed concurrency handling Added hash map comparison of local + fetched images Added cli to ask user to choose between remote fetcher and file fetcher Added barebones files of remote fetcher and file fetcher with test image lists
Modified image list comparison to work via hash map comparison Setup of skeleton files for file fetcher and http fetcher
Updated user feedback to be more concise Moved List call in-memory-store to not proc when hashes are equal
Reworked main's user interface to automatically recognise and validate URL or filepath Reworked http-fetcher to accept full URLs Added images.json test file
http-fetcher : Now using correct Harbor v2 API Local image list is based on tag list retrieved via API file-fetcher : Created JSON struct with name, digest and repository URL This data, with optional tag name, can be used to make docker pull commands using only url + digest
Features : - Check user input for local filepath or URL - Retrieve list of images via URL or local file and store them in-memory - Check for changes in remote URL or local file and update in-memory store accordingly - Launch local Zot registry - Replicate images using in-memory store list to local registry Main missing features/elements : - Delete images from local repository if removed from in-memory store - Unit + integration tests - Optimization and refactoring
Important Review SkippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Hi @OneFlyingBanana, Can I work on supporting structured logging using log or logrus? |
fmt.Println("Source:", source) | ||
|
||
if source != "" { | ||
CopyImage(source) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not ignore the error returned.
func getPullSource(image string) string { | ||
input := os.Getenv("USER_INPUT") | ||
if os.Getenv("SCHEME") == "https://" { | ||
url := os.Getenv("HOST") + "/" + os.Getenv("REGISTRY") + "/" + image |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would avoid getting environment variables here and rather do it at startup and pass then as a parameter. Makes it easier to unit test and debug in the future.
req.Header.Set("Authorization", "Basic "+auth) | ||
|
||
// Send the request | ||
httpClient := &http.Client{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create an http client for the struct rather than creating one for every function call.
|
||
// Check if the received image exists in the store | ||
for storeDigest, storeImage := range s.images { | ||
if storeImage == image { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to inverse the check and call continue instead. This is just to avoid the amount of statement nesting that is done. Improves readability.
return ®istryInfo, nil | ||
} | ||
|
||
func CopyImage(imageName string) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does CopyImage need to be exported outside the package.
close in favor of the changed code |
Features :
Main missing features/elements :