-
Notifications
You must be signed in to change notification settings - Fork 10
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
feat: Add E2E Test for Harbor Satellite #27
Conversation
Warning Rate limit exceeded@bupd has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 46 minutes and 9 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe updates introduce a new feature called "Harbor Satellite" aimed at decentralizing Harbor container registries for edge computing environments. Key changes include configuration and setup files, code implementations for building and running the satellite service, handling image replication and storage, and end-to-end tests. These modifications enable Harbor systems to function reliably in scenarios with intermittent internet connectivity by storing and serving container images locally. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Satellite
participant Registry
User->>Satellite: Start Satellite Service
activate Satellite
Satellite->>Registry: Fetch Initial Configurations
Registry-->>Satellite: Configurations
User->>Satellite: Request Image
Satellite->>Registry: Retrieve Image
Registry-->>Satellite: Image Data
Satellite-->>User: Serve Image
deactivate Satellite
Note over Satellite: Operates independently if registry is unavailable
Poem
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 (
|
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.
Actionable comments posted: 17
Outside diff range and nitpick comments (4)
internal/satellite/satellite.go (1)
Line range hint
39-65
: The error fromDeleteExtraImages
is not checked. Consider handling this error to ensure robust operation.- s.replicator.DeleteExtraImages(ctx, imgs) + if err := s.replicator.DeleteExtraImages(ctx, imgs); err != nil { + return err + }Tools
golangci-lint
65-65: Error return value of
s.replicator.DeleteExtraImages
is not checked (errcheck)main.go (1)
Line range hint
41-161
: Ensure proper error handling in the main execution flow.- if err := metricsSrv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { - return err - } + if err := metricsSrv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { + return fmt.Errorf("error starting metrics server: %w", err) + }This change ensures that errors from the server are properly wrapped with context, improving the traceability and debuggability of the application.
README.md (1)
Line range hint
1-103
: Update documentation to align with the latest project developments.Consider revising the "Non-Goals" section to provide more clarity on what the project does not aim to achieve. Currently, it is marked as "T.B.D.", which might leave readers with unanswered questions.
Tools
LanguageTool
[style] ~10-~10: The phrase ‘in recent years’ is used quite frequently. Consider using a less frequent alternative to set your writing apart from others. (IN_RECENT_STYLE)
Context: ...egistry is unavailable. ## Background In recent years, containers have extended beyond their ...
[uncategorized] ~27-~27: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...s in poor/no coverage areas could prove problematic since : - Harbor wasn't designed to ru...
[duplication] ~31-~31: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...ries is not operationally feasible with Harbor - Harbor would be too similar to a simple regist...
[grammar] ~34-~34: The usual collocation for “independently” is “of”, not “from”. Did you mean “independently of”? (INDEPENDENTLY_FROM_OF)
Context: ...ht and will be able to keep functioning independently from Harbor instances. ## Compatibility Co...
[uncategorized] ~59-~59: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ... then be accessible to other local edge devices who can pull required images directly f...
[grammar] ~60-~60: Uncountable nouns are usually not used with an indefinite article. Use simply “direct access”. (A_UNCOUNTABLE)
Context: ...ull required images directly from it. _(A direct access from edge device to the remote registry...
[uncategorized] ~64-~64: Do not mix variants of the same word (‘containerise’ and ‘containerize’) within a single text. (EN_WORD_COHERENCY)
Context: ...onnnectivity, these devices need to run containerised images but cannot pull from a central H...
[uncategorized] ~64-~64: It appears that hyphens are missing in the adjective “up-to-date”. (UP_TO_DATE_HYPHEN)
Context: ... central Harbor registry and thus store up to date images locally._ ![Use Case #1](docs/i...
[uncategorized] ~73-~73: Do not mix variants of the same word (‘containerise’ and ‘containerize’) within a single text. (EN_WORD_COHERENCY)
Context: ...nt amount of IoT devices needing to run containerised applications, a single local registry i...
[uncategorized] ~73-~73: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case. (AMOUNTOF_TO_NUMBEROF)
Context: ...ght not be able to handle the increased amount of demands from edge devices. The solut...
[style] ~73-~73: As a shorter alternative for ‘able to’, consider using “can”. (BE_ABLE_TO)
Context: ...several registries to several nodes who are able to automatically replicate images across e...
[style] ~79-~79: Consider using more formal and concise wording here. (RESPONSIBLE_FOR)
Context: ... The stateless satellite component will be in charge of configuring the local OCI compliant reg...
[style] ~92-~92: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...imize pull requests to the registry. 3. By directly referencing the registry. ...
[style] ~93-~93: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...directly referencing** the registry. 4. By directly referencing the registry a...go.mod (1)
12-12
: The comment indicates usinggo get
forzotregistry.dev/zot
. It's good practice to document such specific instructions, but ensure it is necessary asgo mod
should handle fetching the correct version.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (4)
.DS_Store
is excluded by!**/.DS_Store
docs/.DS_Store
is excluded by!**/.DS_Store
go.sum
is excluded by!**/*.sum
registry/.DS_Store
is excluded by!**/.DS_Store
Files selected for processing (15)
- .env (1 hunks)
- .gitignore (1 hunks)
- README.md (2 hunks)
- ci/main.go (2 hunks)
- config.toml (1 hunks)
- go.mod (1 hunks)
- image-list/images.json (1 hunks)
- internal/replicate/replicate.go (1 hunks)
- internal/satellite/satellite.go (2 hunks)
- internal/store/file-fetch.go (1 hunks)
- internal/store/http-fetch.go (1 hunks)
- internal/store/in-memory-store.go (1 hunks)
- main.go (4 hunks)
- registry/config.json (1 hunks)
- registry/launch-registry.go (1 hunks)
Files skipped from review due to trivial changes (5)
- .env
- .gitignore
- config.toml
- image-list/images.json
- registry/config.json
Additional context used
golangci-lint
internal/satellite/satellite.go
39-39: Error return value of
s.replicator.DeleteExtraImages
is not checked (errcheck)
65-65: Error return value of
s.replicator.DeleteExtraImages
is not checked (errcheck)internal/replicate/replicate.go
47-47: Error return value is not checked (errcheck)
internal/store/in-memory-store.go
58-58: Error return value of
s.AddImage
is not checked (errcheck)
75-75: Error return value of
s.RemoveImage
is not checked (errcheck)
133-133: Error return value of
s.Remove
is not checked (errcheck)
227-227: Error return value of
s.Remove
is not checked (errcheck)
228-228: Error return value of
s.Add
is not checked (errcheck)
152-152: ineffectual assignment to change (ineffassign)
LanguageTool
README.md
[style] ~10-~10: The phrase ‘in recent years’ is used quite frequently. Consider using a less frequent alternative to set your writing apart from others. (IN_RECENT_STYLE)
Context: ...egistry is unavailable. ## Background In recent years, containers have extended beyond their ...
[uncategorized] ~27-~27: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...s in poor/no coverage areas could prove problematic since : - Harbor wasn't designed to ru...
[duplication] ~31-~31: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...ries is not operationally feasible with Harbor - Harbor would be too similar to a simple regist...
[grammar] ~34-~34: The usual collocation for “independently” is “of”, not “from”. Did you mean “independently of”? (INDEPENDENTLY_FROM_OF)
Context: ...ht and will be able to keep functioning independently from Harbor instances. ## Compatibility Co...
[uncategorized] ~59-~59: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ... then be accessible to other local edge devices who can pull required images directly f...
[grammar] ~60-~60: Uncountable nouns are usually not used with an indefinite article. Use simply “direct access”. (A_UNCOUNTABLE)
Context: ...ull required images directly from it. _(A direct access from edge device to the remote registry...
[uncategorized] ~64-~64: Do not mix variants of the same word (‘containerise’ and ‘containerize’) within a single text. (EN_WORD_COHERENCY)
Context: ...onnnectivity, these devices need to run containerised images but cannot pull from a central H...
[uncategorized] ~64-~64: It appears that hyphens are missing in the adjective “up-to-date”. (UP_TO_DATE_HYPHEN)
Context: ... central Harbor registry and thus store up to date images locally._ ![Use Case #1](docs/i...
[uncategorized] ~73-~73: Do not mix variants of the same word (‘containerise’ and ‘containerize’) within a single text. (EN_WORD_COHERENCY)
Context: ...nt amount of IoT devices needing to run containerised applications, a single local registry i...
[uncategorized] ~73-~73: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case. (AMOUNTOF_TO_NUMBEROF)
Context: ...ght not be able to handle the increased amount of demands from edge devices. The solut...
[style] ~73-~73: As a shorter alternative for ‘able to’, consider using “can”. (BE_ABLE_TO)
Context: ...several registries to several nodes who are able to automatically replicate images across e...
[style] ~79-~79: Consider using more formal and concise wording here. (RESPONSIBLE_FOR)
Context: ... The stateless satellite component will be in charge of configuring the local OCI compliant reg...
[style] ~92-~92: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...imize pull requests to the registry. 3. By directly referencing the registry. ...
[style] ~93-~93: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...directly referencing** the registry. 4. By directly referencing the registry a...
Additional comments not posted (7)
internal/satellite/satellite.go (1)
17-22
: The constructor forSatellite
is well-implemented and initializes the struct members correctly.internal/store/file-fetch.go (1)
31-44
: TheFileImageListFetcher
function is correctly implemented to handle errors and construct the file path.ci/main.go (1)
Line range hint
13-45
: The main function is well-structured and handles initialization and error scenarios appropriately.go.mod (4)
7-8
: Added dependencies are crucial for the new functionalities. Ensure that the versions ofgithub.com/google/go-containerregistry
andgithub.com/joho/godotenv
are compatible with other project dependencies.
16-16
:github.com/spf13/viper
is added to manage application configuration. This is a standard choice for handling configurations in Go applications.
1-1
: Ensure the module name correction fromcontainer-registry.com/harbor-satelite
tocontainer-registry.com/harbor-satellite
is consistent across the entire project.Verification successful
The verification confirms that the module name correction from
container-registry.com/harbor-satelite
tocontainer-registry.com/harbor-satellite
has been applied consistently across the project.
main.go
go.mod
internal/replicate/replicate.go
internal/satellite/satellite.go
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the module name is updated across all Go files. rg --type go "container-registry.com/harbor-satelite"Length of output: 53
Script:
#!/bin/bash # Description: Verify that the module name is updated across all files. rg "container-registry.com/harbor-satelite" # Description: Verify that the correct module name is used across all files. rg "container-registry.com/harbor-satellite"Length of output: 682
10-10
: Updatedgolang.org/x/sync
tov0.7.0
. Verify that this update does not introduce any breaking changes or conflicts with existing code.
internal/store/in-memory-store.go
Outdated
// Iterate over in memory store and remove any image that is not found in imageMap | ||
for digest, image := range s.images { | ||
if _, exists := imageMap[image]; !exists { | ||
s.Remove(ctx, digest, 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.
Error handling for s.Remove
is missing in multiple places. Ignoring errors from a remove operation can lead to inconsistent state within the application.
- s.Remove(ctx, digest, image)
+ if err := s.Remove(ctx, digest, image); err != nil {
+ return nil, fmt.Errorf("failed to remove image: %s, digest: %s, error: %w", image, digest, err)
+ }
- s.Remove(context.Background(), storeDigest, storeImage)
+ if err := s.Remove(context.Background(), storeDigest, store
Image); err != nil {
+ fmt.Printf("Failed to remove image: %s, digest: %s, error: %s\n", storeImage, storeDigest, err)
+ }
Also applies to: 227-227
Tools
golangci-lint
133-133: Error return value of
s.Remove
is not checked (errcheck)
internal/store/in-memory-store.go
Outdated
} | ||
} | ||
if !found { | ||
s.RemoveImage(ctx, 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.
Error handling for s.RemoveImage
is missing. This can lead to silent failures which are hard to debug.
- s.RemoveImage(ctx, image)
+ if err := s.RemoveImage(ctx, image); err != nil {
+ return nil, fmt.Errorf("failed to remove image: %s, error: %w", image, err)
+ }
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
s.RemoveImage(ctx, image) | |
if err := s.RemoveImage(ctx, image); err != nil { | |
return nil, fmt.Errorf("failed to remove image: %s, error: %w", image, err) | |
} |
Tools
golangci-lint
75-75: Error return value of
s.RemoveImage
is not checked (errcheck)
internal/store/in-memory-store.go
Outdated
} | ||
if change { | ||
fmt.Println("Changes detected in the store") | ||
change = false |
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.
The variable change
is reset ineffectively, leading to potential logical errors in state management.
- change = false
+ // Removed as it is not necessary to reset 'change' here since it's already handled in the if-else logic
Committable suggestion was skipped due to low confidence.
Tools
golangci-lint
152-152: ineffectual assignment to change (ineffassign)
internal/store/in-memory-store.go
Outdated
} else { | ||
// Digest exists but does not match the current image reference | ||
s.Remove(context.Background(), storeDigest, storeImage) | ||
s.Add(context.Background(), digest, 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.
Error handling for s.Add
is missing. This is critical as adding an image is a key operation and failures should be managed properly.
- s.Add(context.Background(), digest, image)
+ if err := s.Add(context.Background(), digest, image); err != nil {
+ fmt.Printf("Failed to add image: %s, digest: %s, error: %s\n", image, digest, err)
+ }
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
s.Add(context.Background(), digest, image) | |
if err := s.Add(context.Background(), digest, image); err != nil { | |
fmt.Printf("Failed to add image: %s, digest: %s, error: %s\n", image, digest, err) | |
} |
Tools
golangci-lint
228-228: Error return value of
s.Add
is not checked (errcheck)
internal/store/http-fetch.go
Outdated
username := os.Getenv("HARBOR_USERNAME") | ||
password := os.Getenv("HARBOR_PASSWORD") | ||
auth := base64.StdEncoding.EncodeToString([]byte(username + ":" + password)) | ||
|
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.
Consider externalizing credentials management.
Using environment variables directly in the code can lead to security risks and inflexibility. Consider using a more secure and flexible credential management approach, such as Vault or AWS Secrets Manager.
ci/main.go
Outdated
// buildSatellite builds and runs the Satellite service | ||
func buildSatellite(client *dagger.Client, ctx context.Context) { | ||
slog.Info("Starting the Satellite build process...") | ||
|
||
// Get the directory of project located one level up from the current working directory | ||
parentDir, err := getProjectDir() | ||
if err != nil { | ||
log.Fatalf("Error getting parent directory: %v", err) | ||
} | ||
|
||
// Use the parent directory path in Dagger | ||
dir := client.Host().Directory(parentDir) | ||
|
||
// Configure and build the container | ||
container := client.Container(). | ||
From(imageVersion). | ||
WithDirectory(appDir, dir). | ||
WithWorkdir(appDir). | ||
WithExec([]string{"go", "build", "-o", appBinary, sourceFile}). | ||
WithExposedPort(containerPort). | ||
WithExec([]string{"./" + appBinary}) | ||
|
||
// Start the service | ||
_, err = container.AsService().Start(ctx) | ||
if err != nil { | ||
log.Fatalf("Error starting the Satellite service: %v", err) | ||
} | ||
} |
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.
Consider enhancing the error handling in buildSatellite
by not using log.Fatalf
, which exits the program. Instead, return an error and let the caller decide how to handle it.
- log.Fatalf("Error getting parent directory: %v", err)
- log.Fatalf("Error starting the Satellite service: %v", err)
+ return fmt.Errorf("error getting parent directory: %v", err)
+ return fmt.Errorf("error starting the Satellite service: %v", err)
Committable suggestion was skipped due to low confidence.
main.go
Outdated
bringOwnRegistry := viper.GetBool("bring_own_registry") | ||
if bringOwnRegistry { | ||
registryAdr := viper.GetString("own_registry_adr") | ||
|
||
// Validate registryAdr format | ||
matched, err := regexp.MatchString(`^127\.0\.0\.1:\d{1,5}$`, registryAdr) | ||
if err != nil { | ||
return fmt.Errorf("error validating registry address: %w", err) | ||
} | ||
if !matched { | ||
return fmt.Errorf("invalid registry address format: %s", registryAdr) | ||
} | ||
os.Setenv("ZOT_URL", registryAdr) | ||
fmt.Println("Registry URL set to:", registryAdr) | ||
} else { | ||
g.Go(func() error { | ||
launch, err := registry.LaunchRegistry() | ||
if launch { | ||
cancel() | ||
return nil | ||
} else { | ||
log.Println("Error launching registry :", err) | ||
cancel() | ||
return err | ||
} | ||
}) | ||
} |
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.
Refactor conditional logic for clarity and maintainability.
- if bringOwnRegistry {
- registryAdr := viper.GetString("own_registry_adr")
- ...
- } else {
- g.Go(func() error {
- launch, err := registry.LaunchRegistry()
- ...
- })
- }
+ if bringOwnRegistry {
+ handleOwnRegistry()
+ } else {
+ launchDefaultRegistry(g)
+ }
Extracting the conditional branches into separate functions can improve readability and maintainability of the code.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
bringOwnRegistry := viper.GetBool("bring_own_registry") | |
if bringOwnRegistry { | |
registryAdr := viper.GetString("own_registry_adr") | |
// Validate registryAdr format | |
matched, err := regexp.MatchString(`^127\.0\.0\.1:\d{1,5}$`, registryAdr) | |
if err != nil { | |
return fmt.Errorf("error validating registry address: %w", err) | |
} | |
if !matched { | |
return fmt.Errorf("invalid registry address format: %s", registryAdr) | |
} | |
os.Setenv("ZOT_URL", registryAdr) | |
fmt.Println("Registry URL set to:", registryAdr) | |
} else { | |
g.Go(func() error { | |
launch, err := registry.LaunchRegistry() | |
if launch { | |
cancel() | |
return nil | |
} else { | |
log.Println("Error launching registry :", err) | |
cancel() | |
return err | |
} | |
}) | |
} | |
bringOwnRegistry := viper.GetBool("bring_own_registry") | |
if bringOwnRegistry { | |
handleOwnRegistry() | |
} else { | |
launchDefaultRegistry(g) | |
} | |
func handleOwnRegistry() { | |
registryAdr := viper.GetString("own_registry_adr") | |
// Validate registryAdr format | |
matched, err := regexp.MatchString(`^127\.0\.0\.1:\d{1,5}$`, registryAdr) | |
if err != nil { | |
fmt.Errorf("error validating registry address: %w", err) | |
return | |
} | |
if !matched { | |
fmt.Errorf("invalid registry address format: %s", registryAdr) | |
return | |
} | |
os.Setenv("ZOT_URL", registryAdr) | |
fmt.Println("Registry URL set to:", registryAdr) | |
} | |
func launchDefaultRegistry(g *errgroup.Group) { | |
g.Go(func() error { | |
launch, err := registry.LaunchRegistry() | |
if launch { | |
cancel() | |
return nil | |
} else { | |
log.Println("Error launching registry :", err) | |
cancel() | |
return err | |
} | |
}) | |
} |
internal/replicate/replicate.go
Outdated
// Get the list of images from the local registry | ||
localImages, err := crane.ListTags(localRegistry) | ||
if err != nil { | ||
return fmt.Errorf("failed to get local registry catalog: %w", err) | ||
} | ||
|
||
// Create a map for quick lookup of the provided image list | ||
imageMap := make(map[string]struct{}) | ||
for _, img := range imgs { | ||
// Strip the "album-server:" prefix from the image name | ||
strippedName := stripPrefix(img.Name) | ||
imageMap[strippedName] = struct{}{} | ||
} | ||
|
||
// Iterate over the local images and delete those not in the provided image list | ||
for _, localImage := range localImages { | ||
if _, exists := imageMap[localImage]; !exists { | ||
// Image is not in the provided list, delete it | ||
fmt.Print("Deleting image: ", localRegistry+":"+localImage, " ... ") | ||
err := crane.Delete(fmt.Sprintf("%s:%s", localRegistry, localImage)) | ||
if err != nil { | ||
fmt.Printf("failed to delete image %s: %v\n", localImage, err) | ||
return nil | ||
} | ||
fmt.Printf("Deleted image: %s\n", localImage) | ||
} | ||
} | ||
|
||
return nil |
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.
Refactor deletion logic to improve readability and error handling.
- for _, localImage := range localImages {
- if _, exists := imageMap[localImage]; !exists {
- fmt.Print("Deleting image: ", localRegistry+":"+localImage, " ... ")
- err := crane.Delete(fmt.Sprintf("%s:%s", localRegistry, localImage))
- ...
- }
- }
+ deleteUnlistedImages(localImages, imageMap, localRegistry)
Extracting the deletion logic into a separate function can improve readability and make the code easier to maintain and test.
Committable suggestion was skipped due to low confidence.
internal/replicate/replicate.go
Outdated
func (r *BasicReplicator) Replicate(ctx context.Context, image string) error { | ||
|
||
source := getPullSource(image) | ||
|
||
if source != "" { | ||
CopyImage(source) | ||
} | ||
return nil | ||
} |
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.
Handle errors from CopyImage
function.
- CopyImage(source)
+ if err := CopyImage(source); err != nil {
+ return fmt.Errorf("failed to copy image: %w", err)
+ }
This change ensures that errors from the CopyImage
function are properly handled, improving the robustness of the replication process.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
func (r *BasicReplicator) Replicate(ctx context.Context, image string) error { | |
source := getPullSource(image) | |
if source != "" { | |
CopyImage(source) | |
} | |
return nil | |
} | |
func (r *BasicReplicator) Replicate(ctx context.Context, image string) error { | |
source := getPullSource(image) | |
if source != "" { | |
if err := CopyImage(source); err != nil { | |
return fmt.Errorf("failed to copy image: %w", err) | |
} | |
} | |
return nil | |
} |
Tools
golangci-lint
47-47: Error return value is not checked (errcheck)
README.md
Outdated
### Specific Use Cases | ||
|
||
### Replicating From a Remote Registry to the Edge Registry | ||
Harbor Satellite may be implemented following 1 or several of 3 different architectures depending on its use cases : | ||
|
||
In this use case, the stateless satellite component will handle pulling images from a remote registry and then pushing them to the local OCI registry. This local registry will then be accessible to other local edge devices, who can pull required images directly from it. | ||
1. **Replicating from a remote registry to a local registry.** | ||
In this basic use case, the stateless satellite component will handle pulling images from a remote registry and then pushing them to the local OCI compliant registry. This local registry will then be accessible to other local edge devices who can pull required images directly from it. | ||
_(A direct access from edge device to the remote registry is still possible when network conditions permit it)._ | ||
The satellite component may also handle updating container runtime configurations and fetching image lists from Ground Control, a part of Harbor. | ||
The stateful local regsitry will also need to handle storing and managing data on local volumes. | ||
A typical use case would work as follows : | ||
_In an edge computing environment where IoT devices are deployed to a location with limited or no internet connnectivity, these devices need to run containerised images but cannot pull from a central Harbor registry. A local Harbor Satellite instance can be deployed and take up this role while Internet connectivity is unreliable and distribute all required images. Once a reliable connection is re-established, the Harbor Satellite instance will be able to pull required images from its central Harbor registry and thus store up to date images locally._ | ||
|
||
![Use Case #1](docs/images/satellite_use_case_1.svg) | ||
<p align="center"><em>Use case #1</em></p> | ||
|
||
### Replicating From a Remote Registry to an Edge Kubernetes Registry | ||
2. **Replicating from a remote regsitry to a local Spegel Registry** | ||
The stateless satellite component send pull instructions to Spegel instances running with each node of a Kubernetes cluster. The node will then directly pull images from a remote registry and share it with other local nodes, removing the need for each of them to individually pull an image from a remote registry. | ||
The network interfaces (boundaries) represented in this use case should and will be the same as those represented in use case #1 | ||
A typical use case would work as follows : | ||
_In a larger scale edge computing environment with a significant amount of IoT devices needing to run containerised applications, a single local registry in might not be able to handle the increased amount of demands from edge devices. The solution is to deploy several registries to several nodes who are able to automatically replicate images across each other thanks to Spegel instances running together with each node. The Satellite component will use the same interface to instruct each node when, where and how to pull new images that need to be replicated across the cluster._ | ||
|
||
The stateless satellite component sends pull instructions to Spegel instances running on each Kubernetes node. The node container runtime will then directly pull images from a remote registry to its internal store. Building on Spegel images are now available for other local nodes, removing the need for each of them to individually pull an image from a remote registry. | ||
This use case only works in Kubernetes environments, the major advantage of such a setup compared to use case #1 is that it allows to operate a stateful registry on a stateless cluster. The only dependency satellite has is on spegel. | ||
![Use Case #2](docs/images/satellite_use_case_2.svg) | ||
<p align="center"><em>Use case #2</em></p> | ||
|
||
![Use Case #1](docs/images/satellite_use_case_2.svg) | ||
3. **Proxying from a remote regsitry over the local registry** | ||
The stateless satellite component will be in charge of configuring the local OCI compliant registry, which will be running in proxy mode only. This local registry will then handle pulling necessary images from the remote registry and serving them up for use by local edge devices. | ||
A typical use case would work as follows : | ||
_When, for a number of possible different reasons, the remote registry side of the diagram would not be able to produce a list of images to push down to the Harbor Satellite, the Satellite would then act as a proxy and forward all requests from edge devices to the remote registry. This ensures the availability of necessary images without the need for a pre-compiled list of images_ | ||
|
||
![Use Case #3](docs/images/satellite_use_case_3.svg) | ||
<p align="center"><em>Use case #3</em></p> |
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.
Clarify use case descriptions for better understanding.
In the use case descriptions, consider adding more detailed steps or flowcharts to visually represent the processes involved. This can help in better understanding the complex interactions and functionalities of the Harbor Satellite.
Tools
LanguageTool
[uncategorized] ~59-~59: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ... then be accessible to other local edge devices who can pull required images directly f...
[grammar] ~60-~60: Uncountable nouns are usually not used with an indefinite article. Use simply “direct access”. (A_UNCOUNTABLE)
Context: ...ull required images directly from it. _(A direct access from edge device to the remote registry...
[uncategorized] ~64-~64: Do not mix variants of the same word (‘containerise’ and ‘containerize’) within a single text. (EN_WORD_COHERENCY)
Context: ...onnnectivity, these devices need to run containerised images but cannot pull from a central H...
[uncategorized] ~64-~64: It appears that hyphens are missing in the adjective “up-to-date”. (UP_TO_DATE_HYPHEN)
Context: ... central Harbor registry and thus store up to date images locally._ ![Use Case #1](docs/i...
[uncategorized] ~73-~73: Do not mix variants of the same word (‘containerise’ and ‘containerize’) within a single text. (EN_WORD_COHERENCY)
Context: ...nt amount of IoT devices needing to run containerised applications, a single local registry i...
[uncategorized] ~73-~73: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case. (AMOUNTOF_TO_NUMBEROF)
Context: ...ght not be able to handle the increased amount of demands from edge devices. The solut...
[style] ~73-~73: As a shorter alternative for ‘able to’, consider using “can”. (BE_ABLE_TO)
Context: ...several registries to several nodes who are able to automatically replicate images across e...
[style] ~79-~79: Consider using more formal and concise wording here. (RESPONSIBLE_FOR)
Context: ... The stateless satellite component will be in charge of configuring the local OCI compliant reg...
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.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- ci/main.go (1 hunks)
- test/e2e/registry_test.go (1 hunks)
Additional context used
golangci-lint
ci/main.go
12-12: const
imageVersion
is unused (unused)
13-13: const
exposePort
is unused (unused)
14-14: const
containerPort
is unused (unused)
15-15: const
appDir
is unused (unused)
16-16: const
appBinary
is unused (unused)
17-17: const
sourceFile
is unused (unused)test/e2e/registry_test.go
37-37: Error return value of
tunnel.Stop
is not checked (errcheck)
93-93: SA4006: this value of
container
is never used (staticcheck)
Additional comments not posted (1)
test/e2e/registry_test.go (1)
14-18
: Constants usage approved.The constants
regUrl
,registryPort
, andimageToPush
are appropriately used within the file.
ci/main.go
Outdated
appBinary = "app" // Name of the built application | ||
sourceFile = "main.go" // Source file to build | ||
) | ||
|
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.
Improve error handling and correct Docker image version in comment.
- The Docker image version mentioned in the comment (
golang:1.19
) does not match the version used in the code (golang:1.21
). Consider aligning these for consistency. - Using
panic
for error handling is not advisable in production code as it leads to abrupt termination of the program. Consider returning errors and handling them gracefully.
- // use a golang:1.19 container
+ // use a golang:1.21 container
- panic(err)
+ if err != nil {
+ log.Printf("Failed to connect to Dagger: %v", err)
+ return
+ }
Also applies to: 25-25
ci/main.go
Outdated
const ( | ||
imageVersion = "golang:1.22" // Use a constant for the Go image version | ||
exposePort = 9090 // Client port to expose | ||
containerPort = 9090 // Container port to expose | ||
appDir = "/app" // Directory inside the container | ||
appBinary = "app" // Name of the built application | ||
sourceFile = "main.go" // Source file to build | ||
) |
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.
Remove unused constants or clarify their purpose.
The constants imageVersion
, exposePort
, containerPort
, appDir
, appBinary
, and sourceFile
are declared but not used within this file. If these are intended for future use, please add comments clarifying this. Otherwise, consider removing them to clean up the codebase.
- const (
- imageVersion = "golang:1.22" // Use a constant for the Go image version
- exposePort = 9090 // Client port to expose
- containerPort = 9090 // Container port to expose
- appDir = "/app" // Directory inside the container
- appBinary = "app" // Name of the built application
- sourceFile = "main.go" // Source file to build
- )
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const ( | |
imageVersion = "golang:1.22" // Use a constant for the Go image version | |
exposePort = 9090 // Client port to expose | |
containerPort = 9090 // Container port to expose | |
appDir = "/app" // Directory inside the container | |
appBinary = "app" // Name of the built application | |
sourceFile = "main.go" // Source file to build | |
) |
Tools
golangci-lint
12-12: const
imageVersion
is unused (unused)
13-13: const
exposePort
is unused (unused)
14-14: const
containerPort
is unused (unused)
15-15: const
appDir
is unused (unused)
16-16: const
appBinary
is unused (unused)
17-17: const
sourceFile
is unused (unused)
test/e2e/registry_test.go
Outdated
// expose HTTP service to host | ||
tunnel, err := client.Host().Tunnel(registry).Start(ctx) | ||
assert.NoError(t, err, "Failed to serve tunnel to host") | ||
defer tunnel.Stop(ctx) |
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.
Check the error return value of tunnel.Stop
.
The error return value from tunnel.Stop
is not checked, which could potentially ignore important cleanup failures. Consider handling this error appropriately.
- defer tunnel.Stop(ctx)
+ defer func() {
+ if err := tunnel.Stop(ctx); err != nil {
+ log.Printf("Failed to stop tunnel: %v", err)
+ }
+ }()
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
defer tunnel.Stop(ctx) | |
defer func() { | |
if err := tunnel.Stop(ctx); err != nil { | |
log.Printf("Failed to stop tunnel: %v", err) | |
} | |
}() |
Tools
golangci-lint
37-37: Error return value of
tunnel.Stop
is not checked (errcheck)
test/e2e/registry_test.go
Outdated
// push and then delete the images | ||
container = container.WithExec([]string{"docker", "push", imageTag}) | ||
container = container.WithExec([]string{"docker", "image", "rmi", imageTag}) | ||
container = container.WithExec([]string{"docker", "image", "rmi", imageToPush}) |
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.
Address unused variable issue in pushImageToRegistry
.
The variable container
is reassigned several times but its final value is never used. Consider removing unnecessary assignments or utilizing the variable effectively.
- container = container.WithExec([]string{"docker", "image", "rmi", imageTag})
- container = container.WithExec([]string{"docker", "image", "rmi", imageToPush})
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
container = container.WithExec([]string{"docker", "image", "rmi", imageToPush}) |
Tools
golangci-lint
93-93: SA4006: this value of
container
is never used (staticcheck)
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.
Actionable comments posted: 7
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- internal/replicate/replicate.go (1 hunks)
- main.go (3 hunks)
- test/e2e/registry_test.go (1 hunks)
- test/e2e/testdata/config.toml (1 hunks)
- test/e2e/testdata/harbor.yml (1 hunks)
Files skipped from review due to trivial changes (1)
- test/e2e/testdata/config.toml
Additional context used
golangci-lint
internal/replicate/replicate.go
46-46: Error return value is not checked (errcheck)
test/e2e/registry_test.go
132-132: func
pushImageToRegistry
is unused (unused)
257-257: func
generateConfigFile
is unused (unused)
78-78: ineffectual assignment to err (ineffassign)
176-176: SA4006: this value of
container
is never used (staticcheck)
yamllint
test/e2e/testdata/harbor.yml
[error] 5-5: trailing spaces (trailing-spaces)
[error] 57-57: syntax error: expected , but found '' (syntax)
[error] 205-205: trailing spaces (trailing-spaces)
[warning] 313-313: too many blank lines (1 > 0) (empty-lines)
main.go
Outdated
@@ -27,10 +37,20 @@ func main() { | |||
} | |||
|
|||
func run() error { | |||
var fetcher store.ImageFetcher |
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.
Refactor and optimize configuration and error handling logic.
- Configuration Loading: Consider consolidating the repeated calls to
viper.SetConfig*
into a helper function to improve code clarity and maintainability. - Error Handling in Registry Setup: The nested conditionals and repeated error handling logic can be simplified by using early returns or extracting methods.
- URL or File Input Parsing: The logic for handling input as a URL or file is overly complex and can be simplified. Additionally, the use of environment variables (
USER_INPUT
,SCHEME
, etc.) could be replaced with a more robust configuration management approach. - Resource Initialization: The creation of
storer
,replicator
, andsatellite
instances are tightly coupled within therun
function. Consider using a factory pattern or dependency injection for better testability and separation of concerns.
- viper.SetConfigName("config")
- viper.SetConfigType("toml")
- viper.AddConfigPath(".")
+ initializeViperConfig()
func initializeViperConfig() {
viper.SetConfigName("config")
viper.SetConfigType("toml")
viper.AddConfigPath(".")
}
Also applies to: 46-53, 82-108, 110-147, 154-157
main.go
Outdated
storer := store.NewInMemoryStore(fetcher) | ||
replicator := replicate.NewReplicator() | ||
s := satellite.NewSatellite(storer, replicator) | ||
|
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.
Encapsulate resource creation for better modularity and testability.
The direct instantiation of storer
, replicator
, and satellite
within the run
function limits the testability and modularity of the code. Consider using a factory pattern or dependency injection.
- storer := store.NewInMemoryStore(fetcher)
- replicator := replicate.NewReplicator()
- s := satellite.NewSatellite(storer, replicator)
+ s, err := initializeSatellite(fetcher)
+ if err != nil {
+ return err
+ }
func initializeSatellite(fetcher store.ImageFetcher) (*satellite.Satellite, error) {
storer := store.NewInMemoryStore(fetcher)
replicator := replicate.NewReplicator()
return satellite.NewSatellite(storer, replicator), nil
}
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
storer := store.NewInMemoryStore(fetcher) | |
replicator := replicate.NewReplicator() | |
s := satellite.NewSatellite(storer, replicator) | |
storer := store.NewInMemoryStore(fetcher) | |
replicator := replicate.NewReplicator() | |
s := satellite.NewSatellite(storer, replicator) | |
s, err := initializeSatellite(fetcher) | |
if err != nil { | |
return err | |
} | |
func initializeSatellite(fetcher store.ImageFetcher) (*satellite.Satellite, error) { | |
storer := store.NewInMemoryStore(fetcher) | |
replicator := replicate.NewReplicator() | |
return satellite.NewSatellite(storer, replicator), nil | |
} |
main.go
Outdated
input := viper.GetString("url_or_file") | ||
parsedURL, err := url.Parse(input) | ||
if err != nil || parsedURL.Scheme == "" { | ||
if strings.ContainsAny(input, "\\:*?\"<>|") { | ||
fmt.Println("Path contains invalid characters. Please check the configuration.") | ||
return fmt.Errorf("invalid file path") | ||
} | ||
dir, err := os.Getwd() | ||
if err != nil { | ||
fmt.Println("Error getting current directory:", err) | ||
return err | ||
} | ||
absPath := filepath.Join(dir, input) | ||
if _, err := os.Stat(absPath); os.IsNotExist(err) { | ||
fmt.Println("No URL or file found. Please check the configuration.") | ||
return fmt.Errorf("file not found") | ||
} | ||
fmt.Println("Input is a valid file path.") | ||
fetcher = store.FileImageListFetcher(input) | ||
os.Setenv("USER_INPUT", input) | ||
} else { | ||
fmt.Println("Input is a valid URL.") | ||
fetcher = store.RemoteImageListFetcher(input) | ||
os.Setenv("USER_INPUT", input) | ||
parts := strings.SplitN(input, "://", 2) | ||
scheme := parts[0] + "://" | ||
os.Setenv("SCHEME", scheme) | ||
hostAndPath := parts[1] | ||
hostParts := strings.Split(hostAndPath, "/") | ||
host := hostParts[0] | ||
os.Setenv("HOST", host) | ||
apiVersion := hostParts[1] | ||
os.Setenv("API_VERSION", apiVersion) | ||
registry := hostParts[2] | ||
os.Setenv("REGISTRY", registry) | ||
repository := hostParts[3] | ||
os.Setenv("REPOSITORY", repository) | ||
} |
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.
Optimize input parsing and environment variable management.
The current method of handling input parsing and setting environment variables is verbose and error-prone. Consider using a structured approach with a configuration object or struct.
- parts := strings.SplitN(input, "://", 2)
- scheme := parts[0] + "://"
- hostAndPath := parts[1]
- ...
+ u, err := url.Parse(input)
+ if err != nil {
+ return fmt.Errorf("invalid URL: %w", err)
+ }
+ os.Setenv("SCHEME", u.Scheme + "://")
+ os.Setenv("HOST", u.Host)
Committable suggestion was skipped due to low confidence.
main.go
Outdated
bringOwnRegistry := viper.GetBool("bring_own_registry") | ||
if bringOwnRegistry { | ||
registryAdr := viper.GetString("own_registry_adr") | ||
|
||
// Validate registryAdr format | ||
// matched, err := regexp.MatchString(`^127\.0\.0\.1:\d{1,5}$`, registryAdr) | ||
// if err != nil { | ||
// return fmt.Errorf("error validating registry address: %w", err) | ||
// } | ||
// if matched { | ||
// return fmt.Errorf("invalid registry address format: %s", registryAdr) | ||
// } | ||
os.Setenv("ZOT_URL", registryAdr) | ||
fmt.Println("Registry URL set to:", registryAdr) | ||
} else { | ||
g.Go(func() error { | ||
launch, err := registry.LaunchRegistry() | ||
if launch { | ||
cancel() | ||
return nil | ||
} else { | ||
log.Println("Error launching registry :", err) | ||
cancel() | ||
return err | ||
} | ||
}) | ||
} |
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.
Improve clarity in registry address validation and error handling.
The commented-out validation logic for registryAdr
could be crucial in preventing misconfigurations. Consider re-enabling and refining this code to ensure robustness. Also, the handling of the LaunchRegistry
method could be improved by separating the concerns into different functions or methods.
+ if err := validateRegistryAddress(registryAdr); err != nil {
+ return err
+ }
+ if err := handleRegistryLaunch(); err != nil {
+ return err
+ }
func validateRegistryAddress(address string) error {
matched, err := regexp.MatchString(`^127\.0\.0\.1:\d{1,5}$`, address)
if err != nil {
return fmt.Errorf("error validating registry address: %w", err)
}
if matched {
return fmt.Errorf("invalid registry address format: %s", address)
}
return nil
}
func handleRegistryLaunch() error {
launch, err := registry.LaunchRegistry()
if !launch || err != nil {
log.Println("Error launching registry :", err)
return err
}
return nil
}
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
bringOwnRegistry := viper.GetBool("bring_own_registry") | |
if bringOwnRegistry { | |
registryAdr := viper.GetString("own_registry_adr") | |
// Validate registryAdr format | |
// matched, err := regexp.MatchString(`^127\.0\.0\.1:\d{1,5}$`, registryAdr) | |
// if err != nil { | |
// return fmt.Errorf("error validating registry address: %w", err) | |
// } | |
// if matched { | |
// return fmt.Errorf("invalid registry address format: %s", registryAdr) | |
// } | |
os.Setenv("ZOT_URL", registryAdr) | |
fmt.Println("Registry URL set to:", registryAdr) | |
} else { | |
g.Go(func() error { | |
launch, err := registry.LaunchRegistry() | |
if launch { | |
cancel() | |
return nil | |
} else { | |
log.Println("Error launching registry :", err) | |
cancel() | |
return err | |
} | |
}) | |
} | |
bringOwnRegistry := viper.GetBool("bring_own_registry") | |
if bringOwnRegistry { | |
registryAdr := viper.GetString("own_registry_adr") | |
if err := validateRegistryAddress(registryAdr); err != nil { | |
return err | |
} | |
os.Setenv("ZOT_URL", registryAdr) | |
fmt.Println("Registry URL set to:", registryAdr) | |
} else { | |
if err := handleRegistryLaunch(); err != nil { | |
return err | |
} | |
} | |
func validateRegistryAddress(address string) error { | |
matched, err := regexp.MatchString(`^127\.0\.0\.1:\d{1,5}$`, address) | |
if err != nil { | |
return fmt.Errorf("error validating registry address: %w", err) | |
} | |
if matched { | |
return fmt.Errorf("invalid registry address format: %s", address) | |
} | |
return nil | |
} | |
func handleRegistryLaunch() error { | |
launch, err := registry.LaunchRegistry() | |
if !launch || err != nil { | |
log.Println("Error launching registry :", err) | |
return err | |
} | |
return nil | |
} |
internal/replicate/replicate.go
Outdated
func (r *BasicReplicator) Replicate(ctx context.Context, image string) error { | ||
source := getPullSource(image) | ||
|
||
if source != "" { | ||
CopyImage(source) | ||
} | ||
return nil | ||
} |
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.
Ensure error handling and improve logic separation in replication methods.
- Error Handling in
Replicate
Method: TheCopyImage
function is called without checking for errors. This could lead to silent failures that are hard to debug. - Refactor
DeleteExtraImages
Method: The method for deleting extra images is complex and mixes different levels of abstraction. Consider breaking it down into smaller, more focused methods.
- CopyImage(source)
+ if err := CopyImage(source); err != nil {
+ return fmt.Errorf("failed to copy image: %w", err)
+ }
func deleteUnlistedImages(localImages, imageMap, localRegistry) error {
for _, localImage := range localImages {
if _, exists := imageMap[localImage]; !exists {
fmt.Print("Deleting image: ", localRegistry+":"+localImage, " ... ")
err := crane.Delete(fmt.Sprintf("%s:%s", localRegistry, localImage))
if err != nil {
fmt.Printf("failed to delete image %s: %v\n", localImage, err)
return err
}
fmt.Printf("Deleted image: %s\n", localImage)
}
}
return nil
}
Also applies to: 58-95
Tools
golangci-lint
46-46: Error return value is not checked (errcheck)
test/e2e/registry_test.go
Outdated
func TestSetupContainerRegistry(t *testing.T) { | ||
ctx := context.Background() | ||
|
||
// Initialize Dagger client | ||
client, err := dagger.Connect(ctx, dagger.WithLogOutput(os.Stderr)) | ||
if err != nil { | ||
log.Fatalf("Failed to connect to Dagger: %v", err) | ||
} | ||
defer client.Close() | ||
|
||
// Set up remote Registry | ||
remote, err := setupRemoteRegistry(client, ctx) | ||
assert.NoError(t, err, "Failed to set up remote registry") | ||
// Set up the container registry | ||
registry, err := setupContainerRegistry(client, ctx) | ||
assert.NoError(t, err, "Failed to set up container registry") | ||
// reg, _ := registry.Hostname(ctx) | ||
// fmt.Println(reg) | ||
// | ||
// // expose HTTP service to host | ||
// tunnel, err := client.Host().Tunnel(registry).Start(ctx) | ||
// assert.NoError(t, err, "Failed to serve tunnel to host") | ||
// | ||
// // get HTTP service address | ||
// endpoint, err := tunnel.Endpoint(ctx, dagger.ServiceEndpointOpts{ | ||
// Scheme: "tcp", | ||
// }) | ||
// assert.NoError(t, err, "Failed to get registry endpoint") | ||
// | ||
// log.Println(endpoint, "\n\n\n\n\nthe tunnel endpoint", endpoint) | ||
|
||
// Push the image to the registry | ||
// pushImageToRegistry(ctx, client, registry, endpoint) | ||
// assert.NoError(t, err, "Failed to upload image to registry") | ||
|
||
// Implement the Satellite Testing | ||
buildSatellite(client, ctx, registry, remote) | ||
assert.NoError(t, err, "Failed to build Satellite") | ||
} |
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.
Refactor test setup and improve error handling in TestSetupContainerRegistry
.
The test setup is verbose and mixes setup logic with error handling. Consider extracting setup steps into separate functions and using more descriptive error messages.
- client, err := dagger.Connect(ctx, dagger.WithLogOutput(os.Stderr))
- if err != nil {
- log.Fatalf("Failed to connect to Dagger: %v", err)
- }
- defer client.Close()
+ client, err := initializeDaggerClient(ctx)
+ if err != nil {
+ log.Fatalf("Failed to initialize Dagger client: %v", err)
+ }
func initializeDaggerClient(ctx context.Context) (*dagger.Client, error) {
client, err := dagger.Connect(ctx, dagger.WithLogOutput(os.Stderr))
if err != nil {
return nil, err
}
return client, nil
}
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
func TestSetupContainerRegistry(t *testing.T) { | |
ctx := context.Background() | |
// Initialize Dagger client | |
client, err := dagger.Connect(ctx, dagger.WithLogOutput(os.Stderr)) | |
if err != nil { | |
log.Fatalf("Failed to connect to Dagger: %v", err) | |
} | |
defer client.Close() | |
// Set up remote Registry | |
remote, err := setupRemoteRegistry(client, ctx) | |
assert.NoError(t, err, "Failed to set up remote registry") | |
// Set up the container registry | |
registry, err := setupContainerRegistry(client, ctx) | |
assert.NoError(t, err, "Failed to set up container registry") | |
// reg, _ := registry.Hostname(ctx) | |
// fmt.Println(reg) | |
// | |
// // expose HTTP service to host | |
// tunnel, err := client.Host().Tunnel(registry).Start(ctx) | |
// assert.NoError(t, err, "Failed to serve tunnel to host") | |
// | |
// // get HTTP service address | |
// endpoint, err := tunnel.Endpoint(ctx, dagger.ServiceEndpointOpts{ | |
// Scheme: "tcp", | |
// }) | |
// assert.NoError(t, err, "Failed to get registry endpoint") | |
// | |
// log.Println(endpoint, "\n\n\n\n\nthe tunnel endpoint", endpoint) | |
// Push the image to the registry | |
// pushImageToRegistry(ctx, client, registry, endpoint) | |
// assert.NoError(t, err, "Failed to upload image to registry") | |
// Implement the Satellite Testing | |
buildSatellite(client, ctx, registry, remote) | |
assert.NoError(t, err, "Failed to build Satellite") | |
} | |
func TestSetupContainerRegistry(t *testing.T) { | |
ctx := context.Background() | |
// Initialize Dagger client | |
client, err := initializeDaggerClient(ctx) | |
if err != nil { | |
log.Fatalf("Failed to initialize Dagger client: %v", err) | |
} | |
defer client.Close() | |
// Set up remote Registry | |
remote, err := setupRemoteRegistry(client, ctx) | |
assert.NoError(t, err, "Failed to set up remote registry") | |
// Set up the container registry | |
registry, err := setupContainerRegistry(client, ctx) | |
assert.NoError(t, err, "Failed to set up container registry") | |
// reg, _ := registry.Hostname(ctx) | |
// fmt.Println(reg) | |
// | |
// // expose HTTP service to host | |
// tunnel, err := client.Host().Tunnel(registry).Start(ctx) | |
// assert.NoError(t, err, "Failed to serve tunnel to host") | |
// | |
// // get HTTP service address | |
// endpoint, err := tunnel.Endpoint(ctx, dagger.ServiceEndpointOpts{ | |
// Scheme: "tcp", | |
// }) | |
// assert.NoError(t, err, "Failed to get registry endpoint") | |
// | |
// log.Println(endpoint, "\n\n\n\n\nthe tunnel endpoint", endpoint) | |
// Push the image to the registry | |
// pushImageToRegistry(ctx, client, registry, endpoint) | |
// assert.NoError(t, err, "Failed to upload image to registry") | |
// Implement the Satellite Testing | |
buildSatellite(client, ctx, registry, remote) | |
assert.NoError(t, err, "Failed to build Satellite") | |
} | |
func initializeDaggerClient(ctx context.Context) (*dagger.Client, error) { | |
client, err := dagger.Connect(ctx, dagger.WithLogOutput(os.Stderr)) | |
if err != nil { | |
return nil, err | |
} | |
return client, nil | |
} |
test/e2e/testdata/harbor.yml
Outdated
# jobservice_db_index: 2 | ||
# trivy_db_index: 5 | ||
# idle_timeout_seconds: 30 | ||
# # it's optional, the db for harbor business misc, by default is 0, uncomment it if you want to change it. | ||
# # harbor_db_index: 6 | ||
# # it's optional, the db for harbor cache layer, by default is 0, uncomment it if you want to change it. | ||
# # cache_layer_db_index: 7 | ||
|
||
# Uncomment uaa for trusting the certificate of uaa instance that is hosted via self-signed cert. | ||
# uaa: | ||
# ca_file: /path/to/ca | ||
|
||
# Global proxy | ||
# Config http proxy for components, e.g. http://my.proxy.com:3128 | ||
# Components doesn't need to connect to each others via http proxy. | ||
# Remove component from `components` array if want disable proxy | ||
# for it. If you want use proxy for replication, MUST enable proxy | ||
# for core and jobservice, and set `http_proxy` and `https_proxy`. | ||
# Add domain to the `no_proxy` field, when you want disable proxy | ||
# for some special registry. | ||
proxy: | ||
http_proxy: | ||
https_proxy: | ||
no_proxy: | ||
components: | ||
- core | ||
- jobservice | ||
- trivy | ||
|
||
# metric: | ||
# enabled: false | ||
# port: 9090 | ||
# path: /metrics | ||
|
||
# Trace related config | ||
# only can enable one trace provider(jaeger or otel) at the same time, | ||
# and when using jaeger as provider, can only enable it with agent mode or collector mode. | ||
# if using jaeger collector mode, uncomment endpoint and uncomment username, password if needed | ||
# if using jaeger agetn mode uncomment agent_host and agent_port | ||
# trace: | ||
# enabled: true | ||
# # set sample_rate to 1 if you wanna sampling 100% of trace data; set 0.5 if you wanna sampling 50% of trace data, and so forth | ||
# sample_rate: 1 | ||
# # # namespace used to differenciate different harbor services | ||
# # namespace: | ||
# # # attributes is a key value dict contains user defined attributes used to initialize trace provider | ||
# # attributes: | ||
# # application: harbor | ||
# # # jaeger should be 1.26 or newer. | ||
# # jaeger: | ||
# # endpoint: http://hostname:14268/api/traces | ||
# # username: | ||
# # password: | ||
# # agent_host: hostname | ||
# # # export trace data by jaeger.thrift in compact mode | ||
# # agent_port: 6831 | ||
# # otel: | ||
# # endpoint: hostname:4318 | ||
# # url_path: /v1/traces | ||
# # compression: false | ||
# # insecure: true | ||
# # # timeout is in seconds | ||
# # timeout: 10 | ||
|
||
# Enable purge _upload directories | ||
upload_purging: | ||
enabled: true | ||
# remove files in _upload directories which exist for a period of time, default is one week. | ||
age: 168h | ||
# the interval of the purge operations | ||
interval: 24h | ||
dryrun: false | ||
|
||
# Cache layer configurations | ||
# If this feature enabled, harbor will cache the resource | ||
# `project/project_metadata/repository/artifact/manifest` in the redis | ||
# which can especially help to improve the performance of high concurrent | ||
# manifest pulling. | ||
# NOTICE | ||
# If you are deploying Harbor in HA mode, make sure that all the harbor | ||
# instances have the same behaviour, all with caching enabled or disabled, | ||
# otherwise it can lead to potential data inconsistency. | ||
cache: | ||
# not enabled by default | ||
enabled: false | ||
# keep cache for one day by default | ||
expire_hours: 24 | ||
|
||
# Harbor core configurations | ||
# Uncomment to enable the following harbor core related configuration items. | ||
# core: | ||
# # The provider for updating project quota(usage), there are 2 options, redis or db, | ||
# # by default is implemented by db but you can switch the updation via redis which | ||
# # can improve the performance of high concurrent pushing to the same project, | ||
# # and reduce the database connections spike and occupies. | ||
# # By redis will bring up some delay for quota usage updation for display, so only | ||
# # suggest switch provider to redis if you were ran into the db connections spike aroud | ||
# # the scenario of high concurrent pushing to same project, no improvment for other scenes. | ||
# quota_update_provider: redis # Or db | ||
|
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.
Correct syntax errors and improve formatting in harbor.yml
.
There are several syntax errors and formatting issues in the harbor.yml
file that could cause problems when parsing the file.
- hostname: 0.0.0.0
+ hostname: 0.0.0.0
- data_volume: /data
+ data_volume: /data
- # cache_layer_db_index: 7
+ # cache_layer_db_index: 7
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
# Configuration file of Harbor | |
# The IP address or hostname to access admin UI and registry service. | |
# DO NOT use localhost or 127.0.0.1, because Harbor needs to be accessed by external clients. | |
hostname: 0.0.0.0 | |
# http related config | |
http: | |
# port for http, default is 80. If https enabled, this port will redirect to https port | |
port: 80 | |
# # https related config | |
# https: | |
# # https port for harbor, default is 443 | |
# port: 443 | |
# # The path of cert and key files for nginx | |
# certificate: /etc/ssl/mycerts/mycert.crt | |
# private_key: /etc/ssl/mycerts/mykey.key | |
# # enable strong ssl ciphers (default: false) | |
# # strong_ssl_ciphers: false | |
# # Uncomment following will enable tls communication between all harbor components | |
# internal_tls: | |
# # set enabled to true means internal tls is enabled | |
# enabled: true | |
# # put your cert and key files on dir | |
# dir: /etc/harbor/tls/internal | |
# Uncomment external_url if you want to enable external proxy | |
# And when it enabled the hostname will no longer used | |
# external_url: https://reg.mydomain.com:8433 | |
# The initial password of Harbor admin | |
# It only works in first time to install harbor | |
# Remember Change the admin password from UI after launching Harbor. | |
harbor_admin_password: Harbor12345 | |
# Harbor DB configuration | |
database: | |
# The password for the root user of Harbor DB. Change this before any production use. | |
password: root123 | |
# The maximum number of connections in the idle connection pool. If it <=0, no idle connections are retained. | |
max_idle_conns: 100 | |
# The maximum number of open connections to the database. If it <= 0, then there is no limit on the number of open connections. | |
# Note: the default number of connections is 1024 for postgres of harbor. | |
max_open_conns: 900 | |
# The maximum amount of time a connection may be reused. Expired connections may be closed lazily before reuse. If it <= 0, connections are not closed due to a connection's age. | |
# The value is a duration string. A duration string is a possibly signed sequence of decimal numbers, each with optional fraction and a unit suffix, such as "300ms", "-1.5h" or "2h45m". Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". | |
conn_max_lifetime: 5m | |
# The maximum amount of time a connection may be idle. Expired connections may be closed lazily before reuse. If it <= 0, connections are not closed due to a connection's idle time. | |
# The value is a duration string. A duration string is a possibly signed sequence of decimal numbers, each with optional fraction and a unit suffix, such as "300ms", "-1.5h" or "2h45m". Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". | |
conn_max_idle_time: 0 | |
# The default data volume | |
# data_volume: /home/bupd/harbor/data | |
data_volume: /data | |
# Harbor Storage settings by default is using /data dir on local filesystem | |
# Uncomment storage_service setting If you want to using external storage | |
# storage_service: | |
# # ca_bundle is the path to the custom root ca certificate, which will be injected into the truststore | |
# # of registry's containers. This is usually needed when the user hosts a internal storage with self signed certificate. | |
# ca_bundle: | |
# # storage backend, default is filesystem, options include filesystem, azure, gcs, s3, swift and oss | |
# # for more info about this configuration please refer https://docs.docker.com/registry/configuration/ | |
# filesystem: | |
# maxthreads: 100 | |
# # set disable to true when you want to disable registry redirect | |
# redirect: | |
# disable: false | |
# Trivy configuration | |
# | |
# Trivy DB contains vulnerability information from NVD, Red Hat, and many other upstream vulnerability databases. | |
# It is downloaded by Trivy from the GitHub release page https://github.com/aquasecurity/trivy-db/releases and cached | |
# in the local file system. In addition, the database contains the update timestamp so Trivy can detect whether it | |
# should download a newer version from the Internet or use the cached one. Currently, the database is updated every | |
# 12 hours and published as a new release to GitHub. | |
trivy: | |
# ignoreUnfixed The flag to display only fixed vulnerabilities | |
ignore_unfixed: false | |
# skipUpdate The flag to enable or disable Trivy DB downloads from GitHub | |
# | |
# You might want to enable this flag in test or CI/CD environments to avoid GitHub rate limiting issues. | |
# If the flag is enabled you have to download the `trivy-offline.tar.gz` archive manually, extract `trivy.db` and | |
# `metadata.json` files and mount them in the `/home/scanner/.cache/trivy/db` path. | |
skip_update: false | |
# | |
# skipJavaDBUpdate If the flag is enabled you have to manually download the `trivy-java.db` file and mount it in the | |
# `/home/scanner/.cache/trivy/java-db/trivy-java.db` path | |
skip_java_db_update: false | |
# | |
# The offline_scan option prevents Trivy from sending API requests to identify dependencies. | |
# Scanning JAR files and pom.xml may require Internet access for better detection, but this option tries to avoid it. | |
# For example, the offline mode will not try to resolve transitive dependencies in pom.xml when the dependency doesn't | |
# exist in the local repositories. It means a number of detected vulnerabilities might be fewer in offline mode. | |
# It would work if all the dependencies are in local. | |
# This option doesn't affect DB download. You need to specify "skip-update" as well as "offline-scan" in an air-gapped environment. | |
offline_scan: false | |
# | |
# Comma-separated list of what security issues to detect. Possible values are `vuln`, `config` and `secret`. Defaults to `vuln`. | |
security_check: vuln | |
# | |
# insecure The flag to skip verifying registry certificate | |
insecure: false | |
# github_token The GitHub access token to download Trivy DB | |
# | |
# Anonymous downloads from GitHub are subject to the limit of 60 requests per hour. Normally such rate limit is enough | |
# for production operations. If, for any reason, it's not enough, you could increase the rate limit to 5000 | |
# requests per hour by specifying the GitHub access token. For more details on GitHub rate limiting please consult | |
# https://docs.github.com/rest/overview/resources-in-the-rest-api#rate-limiting | |
# | |
# You can create a GitHub token by following the instructions in | |
# https://help.github.com/en/github/authenticating-to-github/creating-a-personal-access-token-for-the-command-line | |
# | |
# github_token: xxx | |
jobservice: | |
# Maximum number of job workers in job service | |
max_job_workers: 10 | |
# The jobLoggers backend name, only support "STD_OUTPUT", "FILE" and/or "DB" | |
job_loggers: | |
- STD_OUTPUT | |
- FILE | |
# - DB | |
# The jobLogger sweeper duration (ignored if `jobLogger` is `stdout`) | |
logger_sweeper_duration: 1 #days | |
notification: | |
# Maximum retry count for webhook job | |
webhook_job_max_retry: 3 | |
# HTTP client timeout for webhook job | |
webhook_job_http_client_timeout: 3 #seconds | |
# Log configurations | |
log: | |
# options are debug, info, warning, error, fatal | |
level: info | |
# configs for logs in local storage | |
local: | |
# Log files are rotated log_rotate_count times before being removed. If count is 0, old versions are removed rather than rotated. | |
rotate_count: 50 | |
# Log files are rotated only if they grow bigger than log_rotate_size bytes. If size is followed by k, the size is assumed to be in kilobytes. | |
# If the M is used, the size is in megabytes, and if G is used, the size is in gigabytes. So size 100, size 100k, size 100M and size 100G | |
# are all valid. | |
rotate_size: 200M | |
# The directory on your host that store log | |
location: /data/hlogs | |
# Uncomment following lines to enable external syslog endpoint. | |
# external_endpoint: | |
# # protocol used to transmit log to external endpoint, options is tcp or udp | |
# protocol: tcp | |
# # The host of external endpoint | |
# host: localhost | |
# # Port of external endpoint | |
# port: 5140 | |
#This attribute is for migrator to detect the version of the .cfg file, DO NOT MODIFY! | |
_version: 2.10.0 | |
# Uncomment external_database if using external database. | |
# external_database: | |
# harbor: | |
# host: harbor_db_host | |
# port: harbor_db_port | |
# db_name: harbor_db_name | |
# username: harbor_db_username | |
# password: harbor_db_password | |
# ssl_mode: disable | |
# max_idle_conns: 2 | |
# max_open_conns: 0 | |
# Uncomment redis if need to customize redis db | |
# redis: | |
# # db_index 0 is for core, it's unchangeable | |
# # registry_db_index: 1 | |
# # jobservice_db_index: 2 | |
# # trivy_db_index: 5 | |
# # it's optional, the db for harbor business misc, by default is 0, uncomment it if you want to change it. | |
# # harbor_db_index: 6 | |
# # it's optional, the db for harbor cache layer, by default is 0, uncomment it if you want to change it. | |
# # cache_db_index: 7 | |
# Uncomment redis if need to customize redis db | |
# redis: | |
# # db_index 0 is for core, it's unchangeable | |
# # registry_db_index: 1 | |
# # jobservice_db_index: 2 | |
# # trivy_db_index: 5 | |
# # it's optional, the db for harbor business misc, by default is 0, uncomment it if you want to change it. | |
# # harbor_db_index: 6 | |
# # it's optional, the db for harbor cache layer, by default is 0, uncomment it if you want to change it. | |
# # cache_layer_db_index: 7 | |
# Uncomment external_redis if using external Redis server | |
# external_redis: | |
# # support redis, redis+sentinel | |
# # host for redis: <host_redis>:<port_redis> | |
# # host for redis+sentinel: | |
# # <host_sentinel1>:<port_sentinel1>,<host_sentinel2>:<port_sentinel2>,<host_sentinel3>:<port_sentinel3> | |
# host: redis:6379 | |
# password: | |
# # Redis AUTH command was extended in Redis 6, it is possible to use it in the two-arguments AUTH <username> <password> form. | |
# # there's a known issue when using external redis username ref:https://github.com/goharbor/harbor/issues/18892 | |
# # if you care about the image pull/push performance, please refer to this https://github.com/goharbor/harbor/wiki/Harbor-FAQs#external-redis-username-password-usage | |
# # username: | |
# # sentinel_master_set must be set to support redis+sentinel | |
# #sentinel_master_set: | |
# # db_index 0 is for core, it's unchangeable | |
# registry_db_index: 1 | |
# jobservice_db_index: 2 | |
# trivy_db_index: 5 | |
# idle_timeout_seconds: 30 | |
# # it's optional, the db for harbor business misc, by default is 0, uncomment it if you want to change it. | |
# # harbor_db_index: 6 | |
# # it's optional, the db for harbor cache layer, by default is 0, uncomment it if you want to change it. | |
# # cache_layer_db_index: 7 | |
# Uncomment uaa for trusting the certificate of uaa instance that is hosted via self-signed cert. | |
# uaa: | |
# ca_file: /path/to/ca | |
# Global proxy | |
# Config http proxy for components, e.g. http://my.proxy.com:3128 | |
# Components doesn't need to connect to each others via http proxy. | |
# Remove component from `components` array if want disable proxy | |
# for it. If you want use proxy for replication, MUST enable proxy | |
# for core and jobservice, and set `http_proxy` and `https_proxy`. | |
# Add domain to the `no_proxy` field, when you want disable proxy | |
# for some special registry. | |
proxy: | |
http_proxy: | |
https_proxy: | |
no_proxy: | |
components: | |
- core | |
- jobservice | |
- trivy | |
# metric: | |
# enabled: false | |
# port: 9090 | |
# path: /metrics | |
# Trace related config | |
# only can enable one trace provider(jaeger or otel) at the same time, | |
# and when using jaeger as provider, can only enable it with agent mode or collector mode. | |
# if using jaeger collector mode, uncomment endpoint and uncomment username, password if needed | |
# if using jaeger agetn mode uncomment agent_host and agent_port | |
# trace: | |
# enabled: true | |
# # set sample_rate to 1 if you wanna sampling 100% of trace data; set 0.5 if you wanna sampling 50% of trace data, and so forth | |
# sample_rate: 1 | |
# # # namespace used to differenciate different harbor services | |
# # namespace: | |
# # # attributes is a key value dict contains user defined attributes used to initialize trace provider | |
# # attributes: | |
# # application: harbor | |
# # # jaeger should be 1.26 or newer. | |
# # jaeger: | |
# # endpoint: http://hostname:14268/api/traces | |
# # username: | |
# # password: | |
# # agent_host: hostname | |
# # # export trace data by jaeger.thrift in compact mode | |
# # agent_port: 6831 | |
# # otel: | |
# # endpoint: hostname:4318 | |
# # url_path: /v1/traces | |
# # compression: false | |
# # insecure: true | |
# # # timeout is in seconds | |
# # timeout: 10 | |
# Enable purge _upload directories | |
upload_purging: | |
enabled: true | |
# remove files in _upload directories which exist for a period of time, default is one week. | |
age: 168h | |
# the interval of the purge operations | |
interval: 24h | |
dryrun: false | |
# Cache layer configurations | |
# If this feature enabled, harbor will cache the resource | |
# `project/project_metadata/repository/artifact/manifest` in the redis | |
# which can especially help to improve the performance of high concurrent | |
# manifest pulling. | |
# NOTICE | |
# If you are deploying Harbor in HA mode, make sure that all the harbor | |
# instances have the same behaviour, all with caching enabled or disabled, | |
# otherwise it can lead to potential data inconsistency. | |
cache: | |
# not enabled by default | |
enabled: false | |
# keep cache for one day by default | |
expire_hours: 24 | |
# Harbor core configurations | |
# Uncomment to enable the following harbor core related configuration items. | |
# core: | |
# # The provider for updating project quota(usage), there are 2 options, redis or db, | |
# # by default is implemented by db but you can switch the updation via redis which | |
# # can improve the performance of high concurrent pushing to the same project, | |
# # and reduce the database connections spike and occupies. | |
# # By redis will bring up some delay for quota usage updation for display, so only | |
# # suggest switch provider to redis if you were ran into the db connections spike aroud | |
# # the scenario of high concurrent pushing to same project, no improvment for other scenes. | |
# quota_update_provider: redis # Or db | |
# Configuration file of Harbor | |
# The IP address or hostname to access admin UI and registry service. | |
# DO NOT use localhost or 127.0.0.1, because Harbor needs to be accessed by external clients. | |
hostname: 0.0.0.0 | |
# http related config | |
http: | |
# port for http, default is 80. If https enabled, this port will redirect to https port | |
port: 80 | |
# # https related config | |
# https: | |
# # https port for harbor, default is 443 | |
# port: 443 | |
# # The path of cert and key files for nginx | |
# certificate: /etc/ssl/mycerts/mycert.crt | |
# private_key: /etc/ssl/mycerts/mykey.key | |
# # enable strong ssl ciphers (default: false) | |
# # strong_ssl_ciphers: false | |
# # Uncomment following will enable tls communication between all harbor components | |
# internal_tls: | |
# # set enabled to true means internal tls is enabled | |
# enabled: true | |
# # put your cert and key files on dir | |
# dir: /etc/harbor/tls/internal | |
# Uncomment external_url if you want to enable external proxy | |
# And when it enabled the hostname will no longer used | |
# external_url: https://reg.mydomain.com:8433 | |
# The initial password of Harbor admin | |
# It only works in first time to install harbor | |
# Remember Change the admin password from UI after launching Harbor. | |
harbor_admin_password: Harbor12345 | |
# Harbor DB configuration | |
database: | |
# The password for the root user of Harbor DB. Change this before any production use. | |
password: root123 | |
# The maximum number of connections in the idle connection pool. If it <=0, no idle connections are retained. | |
max_idle_conns: 100 | |
# The maximum number of open connections to the database. If it <= 0, then there is no limit on the number of open connections. | |
# Note: the default number of connections is 1024 for postgres of harbor. | |
max_open_conns: 900 | |
# The maximum amount of time a connection may be reused. Expired connections may be closed lazily before reuse. If it <= 0, connections are not closed due to a connection's age. | |
# The value is a duration string. A duration string is a possibly signed sequence of decimal numbers, each with optional fraction and a unit suffix, such as "300ms", "-1.5h" or "2h45m". Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". | |
conn_max_lifetime: 5m | |
# The maximum amount of time a connection may be idle. Expired connections may be closed lazily before reuse. If it <= 0, connections are not closed due to a connection's idle time. | |
# The value is a duration string. A duration string is a possibly signed sequence of decimal numbers, each with optional fraction and a unit suffix, such as "300ms", "-1.5h" or "2h45m". Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". | |
conn_max_idle_time: 0 | |
# The default data volume | |
# data_volume: /home/bupd/harbor/data | |
data_volume: /data | |
# Harbor Storage settings by default is using /data dir on local filesystem | |
# Uncomment storage_service setting If you want to using external storage | |
# storage_service: | |
# # ca_bundle is the path to the custom root ca certificate, which will be injected into the truststore | |
# # of registry's containers. This is usually needed when the user hosts a internal storage with self signed certificate. | |
# ca_bundle: | |
# # storage backend, default is filesystem, options include filesystem, azure, gcs, s3, swift and oss | |
# # for more info about this configuration please refer https://docs.docker.com/registry/configuration/ | |
# filesystem: | |
# maxthreads: 100 | |
# # set disable to true when you want to disable registry redirect | |
# redirect: | |
# disable: false | |
# Trivy configuration | |
# | |
# Trivy DB contains vulnerability information from NVD, Red Hat, and many other upstream vulnerability databases. | |
# It is downloaded by Trivy from the GitHub release page https://github.com/aquasecurity/trivy-db/releases and cached | |
# in the local file system. In addition, the database contains the update timestamp so Trivy can detect whether it | |
# should download a newer version from the Internet or use the cached one. Currently, the database is updated every | |
# 12 hours and published as a new release to GitHub. | |
trivy: | |
# ignoreUnfixed The flag to display only fixed vulnerabilities | |
ignore_unfixed: false | |
# skipUpdate The flag to enable or disable Trivy DB downloads from GitHub | |
# | |
# You might want to enable this flag in test or CI/CD environments to avoid GitHub rate limiting issues. | |
# If the flag is enabled you have to download the `trivy-offline.tar.gz` archive manually, extract `trivy.db` and | |
# `metadata.json` files and mount them in the `/home/scanner/.cache/trivy/db` path. | |
skip_update: false | |
# | |
# skipJavaDBUpdate If the flag is enabled you have to manually download the `trivy-java.db` file and mount it in the | |
# `/home/scanner/.cache/trivy/java-db/trivy-java.db` path | |
skip_java_db_update: false | |
# | |
# The offline_scan option prevents Trivy from sending API requests to identify dependencies. | |
# Scanning JAR files and pom.xml may require Internet access for better detection, but this option tries to avoid it. | |
# For example, the offline mode will not try to resolve transitive dependencies in pom.xml when the dependency doesn't | |
# exist in the local repositories. It means a number of detected vulnerabilities might be fewer in offline mode. | |
# It would work if all the dependencies are in local. | |
# This option doesn't affect DB download. You need to specify "skip-update" as well as "offline-scan" in an air-gapped environment. | |
offline_scan: false | |
# | |
# Comma-separated list of what security issues to detect. Possible values are `vuln`, `config` and `secret`. Defaults to `vuln`. | |
security_check: vuln | |
# | |
# insecure The flag to skip verifying registry certificate | |
insecure: false | |
# github_token The GitHub access token to download Trivy DB | |
# | |
# Anonymous downloads from GitHub are subject to the limit of 60 requests per hour. Normally such rate limit is enough | |
# for production operations. If, for any reason, it's not enough, you could increase the rate limit to 5000 | |
# requests per hour by specifying the GitHub access token. For more details on GitHub rate limiting please consult | |
# https://docs.github.com/rest/overview/resources-in-the-rest-api#rate-limiting | |
# | |
# You can create a GitHub token by following the instructions in | |
# https://help.github.com/en/github/authenticating-to-github/creating-a-personal-access-token-for-the-command-line | |
# | |
# github_token: xxx | |
jobservice: | |
# Maximum number of job workers in job service | |
max_job_workers: 10 | |
# The jobLoggers backend name, only support "STD_OUTPUT", "FILE" and/or "DB" | |
job_loggers: | |
- STD_OUTPUT | |
- FILE | |
# - DB | |
# The jobLogger sweeper duration (ignored if `jobLogger` is `stdout`) | |
logger_sweeper_duration: 1 #days | |
notification: | |
# Maximum retry count for webhook job | |
webhook_job_max_retry: 3 | |
# HTTP client timeout for webhook job | |
webhook_job_http_client_timeout: 3 #seconds | |
# Log configurations | |
log: | |
# options are debug, info, warning, error, fatal | |
level: info | |
# configs for logs in local storage | |
local: | |
# Log files are rotated log_rotate_count times before being removed. If count is 0, old versions are removed rather than rotated. | |
rotate_count: 50 | |
# Log files are rotated only if they grow bigger than log_rotate_size bytes. If size is followed by k, the size is assumed to be in kilobytes. | |
# If the M is used, the size is in megabytes, and if G is used, the size is in gigabytes. So size 100, size 100k, size 100M and size 100G | |
# are all valid. | |
rotate_size: 200M | |
# The directory on your host that store log | |
location: /data/hlogs | |
# Uncomment following lines to enable external syslog endpoint. | |
# external_endpoint: | |
# # protocol used to transmit log to external endpoint, options is tcp or udp | |
# protocol: tcp | |
# # The host of external endpoint | |
# host: localhost | |
# # Port of external endpoint | |
# port: 5140 | |
#This attribute is for migrator to detect the version of the .cfg file, DO NOT MODIFY! | |
_version: 2.10.0 | |
# Uncomment external_database if using external database. | |
# external_database: | |
# harbor: | |
# host: harbor_db_host | |
# port: harbor_db_port | |
# db_name: harbor_db_name | |
# username: harbor_db_username | |
# password: harbor_db_password | |
# ssl_mode: disable | |
# max_idle_conns: 2 | |
# max_open_conns: 0 | |
# Uncomment redis if need to customize redis db | |
# redis: | |
# # db_index 0 is for core, it's unchangeable | |
# # registry_db_index: 1 | |
# # jobservice_db_index: 2 | |
# # trivy_db_index: 5 | |
# # it's optional, the db for harbor business misc, by default is 0, uncomment it if you want to change it. | |
# # harbor_db_index: 6 | |
# # it's optional, the db for harbor cache layer, by default is 0, uncomment it if you want to change it. | |
# # cache_db_index: 7 | |
# Uncomment redis if need to customize redis db | |
# redis: | |
# # db_index 0 is for core, it's unchangeable | |
# # registry_db_index: 1 | |
# # jobservice_db_index: 2 | |
# # trivy_db_index: 5 | |
# # it's optional, the db for harbor business misc, by default is 0, uncomment it if you want to change it. | |
# # harbor_db_index: 6 | |
# # it's optional, the db for harbor cache layer, by default is 0, uncomment it if you want to change it. | |
# # cache_layer_db_index: 7 | |
# Uncomment external_redis if using external Redis server | |
# external_redis: | |
# # support redis, redis+sentinel | |
# # host for redis: <host_redis>:<port_redis> | |
# # host for redis+sentinel: | |
# # <host_sentinel1>:<port_sentinel1>,<host_sentinel2>:<port_sentinel2>,<host_sentinel3>:<port_sentinel3> | |
# host: redis:6379 | |
# password: | |
# # Redis AUTH command was extended in Redis 6, it is possible to use it in the two-arguments AUTH <username> <password> form. | |
# # there's a known issue when using external redis username ref:https://github.com/goharbor/harbor/issues/18892 | |
# # if you care about the image pull/push performance, please refer to this https://github.com/goharbor/harbor/wiki/Harbor-FAQs#external-redis-username-password-usage | |
# # username: | |
# # sentinel_master_set must be set to support redis+sentinel | |
# #sentinel_master_set: | |
# # db_index 0 is for core, it's unchangeable | |
# registry_db_index: 1 | |
# jobservice_db_index: 2 | |
# trivy_db_index: 5 | |
# idle_timeout_seconds: 30 | |
# # it's optional, the db for harbor business misc, by default is 0, uncomment it if you want to change it. | |
# # harbor_db_index: 6 | |
# # it's optional, the db for harbor cache layer, by default is 0, uncomment it if you want to change it. | |
# # cache_layer_db_index: 7 | |
# Uncomment uaa for trusting the certificate of uaa instance that is hosted via self-signed cert. | |
# uaa: | |
# ca_file: /path/to/ca | |
# Global proxy | |
# Config http proxy for components, e.g. http://my.proxy.com:3128 | |
# Components doesn't need to connect to each others via http proxy. | |
# Remove component from `components` array if want disable proxy | |
# for it. If you want use proxy for replication, MUST enable proxy | |
# for core and jobservice, and set `http_proxy` and `https_proxy`. | |
# Add domain to the `no_proxy` field, when you want disable proxy | |
# for some special registry. | |
proxy: | |
http_proxy: | |
https_proxy: | |
no_proxy: | |
components: | |
- core | |
- jobservice | |
- trivy | |
# metric: | |
# enabled: false | |
# port: 9090 | |
# path: /metrics | |
# Trace related config | |
# only can enable one trace provider(jaeger or otel) at the same time, | |
# and when using jaeger as provider, can only enable it with agent mode or collector mode. | |
# if using jaeger collector mode, uncomment endpoint and uncomment username, password if needed | |
# if using jaeger agetn mode uncomment agent_host and agent_port | |
# trace: | |
# enabled: true | |
# # set sample_rate to 1 if you wanna sampling 100% of trace data; set 0.5 if you wanna sampling 50% of trace data, and so forth | |
# sample_rate: 1 | |
# # # namespace used to differenciate different harbor services | |
# # namespace: | |
# # # attributes is a key value dict contains user defined attributes used to initialize trace provider | |
# # attributes: | |
# # application: harbor | |
# # # jaeger should be 1.26 or newer. | |
# # jaeger: | |
# # endpoint: http://hostname:14268/api/traces | |
# # username: | |
# # password: | |
# # agent_host: hostname | |
# # # export trace data by jaeger.thrift in compact mode | |
# # agent_port: 6831 | |
# # otel: | |
# # endpoint: hostname:4318 | |
# # url_path: /v1/traces | |
# # compression: false | |
# # insecure: true | |
# # # timeout is in seconds | |
# # timeout: 10 | |
# Enable purge _upload directories | |
upload_purging: | |
enabled: true | |
# remove files in _upload directories which exist for a period of time, default is one week. | |
age: 168h | |
# the interval of the purge operations | |
interval: 24h | |
dryrun: false | |
# Cache layer configurations | |
# If this feature enabled, harbor will cache the resource | |
# `project/project_metadata/repository/artifact/manifest` in the redis | |
# which can especially help to improve the performance of high concurrent | |
# manifest pulling. | |
# NOTICE | |
# If you are deploying Harbor in HA mode, make sure that all the harbor | |
# instances have the same behaviour, all with caching enabled or disabled, | |
# otherwise it can lead to potential data inconsistency. | |
cache: | |
# not enabled by default | |
enabled: false | |
# keep cache for one day by default | |
expire_hours: 24 | |
# Harbor core configurations | |
# Uncomment to enable the following harbor core related configuration items. | |
# core: | |
# # The provider for updating project quota(usage), there are 2 options, redis or db, | |
# # by default is implemented by db but you can switch the updation via redis which | |
# # can improve the performance of high concurrent pushing to the same project, | |
# # and reduce the database connections spike and occupies. | |
# # By redis will bring up some delay for quota usage updation for display, so only | |
# # suggest switch provider to redis if you were ran into the db connections spike aroud | |
# # the scenario of high concurrent pushing to same project, no improvment for other scenes. | |
# quota_update_provider: redis # Or db | |
Tools
yamllint
[error] 5-5: trailing spaces (trailing-spaces)
[error] 57-57: syntax error: expected , but found '' (syntax)
[error] 205-205: trailing spaces (trailing-spaces)
[warning] 313-313: too many blank lines (1 > 0) (empty-lines)
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.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- config.toml (1 hunks)
- internal/replicate/replicate.go (1 hunks)
- internal/store/http-fetch.go (1 hunks)
- internal/store/in-memory-store.go (1 hunks)
- registry/config.json (1 hunks)
- test/e2e/registry_test.go (1 hunks)
- test/e2e/test.go (1 hunks)
- test/e2e/testdata/config.toml (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- config.toml
- internal/store/http-fetch.go
- registry/config.json
- test/e2e/testdata/config.toml
Additional context used
golangci-lint
test/e2e/test.go
38-38: Error return value of
cmd.Process.Kill
is not checked (errcheck)internal/replicate/replicate.go
46-46: Error return value is not checked (errcheck)
test/e2e/registry_test.go
212-212: func
generateConfigFile
is unused (unused)
197-197: ineffectual assignment to err (ineffassign)
internal/store/in-memory-store.go
58-58: Error return value of
s.AddImage
is not checked (errcheck)
75-75: Error return value of
s.RemoveImage
is not checked (errcheck)
133-133: Error return value of
s.Remove
is not checked (errcheck)
225-225: Error return value of
s.Remove
is not checked (errcheck)
226-226: Error return value of
s.Add
is not checked (errcheck)
152-152: ineffectual assignment to change (ineffassign)
Additional comments not posted (1)
test/e2e/registry_test.go (1)
197-197
: Ineffectual assignment toerr
should be addressed.The assignment to
err
is ineffectual since it's overwritten immediately without being used.- prints, err := container.Stdout(ctx) + prints, _ := container.Stdout(ctx)Tools
golangci-lint
197-197: ineffectual assignment to err (ineffassign)
internal/replicate/replicate.go
Outdated
source := getPullSource(image) | ||
|
||
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.
Handle error from CopyImage
function.
The error from CopyImage
is not checked. This could lead to silent failures during the replication process.
- CopyImage(source)
+ if err := CopyImage(source); err != nil {
+ fmt.Printf("Error copying image %s: %v\n", source, err)
+ }
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
CopyImage(source) | |
if err := CopyImage(source); err != nil { | |
fmt.Printf("Error copying image %s: %v\n", source, err) | |
} |
Tools
golangci-lint
46-46: Error return value is not checked (errcheck)
test/e2e/registry_test.go
Outdated
return filepath.Abs(filepath.Join(currentDir, "../..")) | ||
} | ||
|
||
func generateConfigFile(filePath string, srvAddr 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.
Unused function generateConfigFile
should be removed or justified.
The function generateConfigFile
is declared but not used anywhere in the codebase.
- func generateConfigFile(filePath string, srvAddr string) error {
- // function implementation
- }
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
func generateConfigFile(filePath string, srvAddr string) error { |
Tools
golangci-lint
212-212: func
generateConfigFile
is unused (unused)
internal/store/in-memory-store.go
Outdated
// Check if the image already exists in the store | ||
if _, exists := s.images[img.Name]; !exists { | ||
// Add the image to the store | ||
s.AddImage(ctx, img.Name) |
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.
Consistent error handling is needed across store operations.
Multiple functions (AddImage
, RemoveImage
, Remove
, Add
) do not check the return values of operations that can fail. This might lead to unhandled errors and inconsistent state within the application.
- s.AddImage(ctx, img.Name)
+ if err := s.AddImage(ctx, img.Name); err != nil {
+ fmt.Printf("Failed to add image: %s, error: %v\n", img.Name, err)
+ }
- s.RemoveImage(ctx, image)
+ if err := s.RemoveImage(ctx, image); err != nil {
+ fmt.Printf("Failed to remove image: %s, error: %v\n", image, err)
+ }
- s.Remove(ctx, digest, image)
+ if err := s.Remove(ctx, digest, image); err != nil {
+ fmt.Printf("Failed to remove image: %s, digest: %s, error: %v\n", image, digest, err)
+ }
- s.Add(ctx, digest, image)
+ if err := s.Add(ctx, digest, image); err != nil {
+ fmt.Printf("Failed to add image: %s, digest: %s, error: %v\n", image, digest, err)
+ }
Also applies to: 75-75, 133-133, 225-225, 226-226
Tools
golangci-lint
58-58: Error return value of
s.AddImage
is not checked (errcheck)
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.
Actionable comments posted: 7
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- test/e2e/satellite_test.go (1 hunks)
- test/e2e/test.go (1 hunks)
Additional context used
golangci-lint
test/e2e/test.go
38-38: Error return value of
cmd.Process.Kill
is not checked (errcheck)test/e2e/satellite_test.go
210-210: func
generateConfigFile
is unused (unused)
195-195: ineffectual assignment to err (ineffassign)
Additional comments not posted (2)
test/e2e/satellite_test.go (2)
29-30
: Use consistent error handling inTestSetupContainerRegistry
.When a connection to Dagger fails, use
log.Fatalf
instead oflog.Fatalf
for consistent error handling.- log.Fatalf("Failed to connect to Dagger: %v", err) + log.Fatalf("Failed to connect to Dagger: %v", err)Likely invalid or redundant comment.
146-147
: Improve error handling inbuildSatellite
.Use
log.Fatalf
for error handling inbuildSatellite
to ensure that all errors are logged and the application exits gracefully.- log.Fatalf("Error getting parentDirectory: %v", err) + log.Fatalf("Error getting parent directory: %v", err)Likely invalid or redundant comment.
test/e2e/test.go
Outdated
stdout, err := cmd.StdoutPipe() | ||
if err != nil { | ||
fmt.Println("Error creating stdout pipe:", err) | ||
os.Exit(1) | ||
} |
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.
Improve error handling by exiting gracefully.
When an error occurs while creating the stdout pipe, the script prints the error and exits. Consider logging the error for better traceability and using a specific exit code for this type of error.
- fmt.Println("Error creating stdout pipe:", err)
- os.Exit(1)
+ log.Fatalf("Error creating stdout pipe: %v", err)
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
stdout, err := cmd.StdoutPipe() | |
if err != nil { | |
fmt.Println("Error creating stdout pipe:", err) | |
os.Exit(1) | |
} | |
stdout, err := cmd.StdoutPipe() | |
if err != nil { | |
log.Fatalf("Error creating stdout pipe: %v", err) | |
} |
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.
Actionable comments posted: 9
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- test/e2e/satellite_test.go (1 hunks)
- test/e2e/test.go (1 hunks)
Additional context used
golangci-lint
test/e2e/test.go
38-38: Error return value of
cmd.Process.Kill
is not checked (errcheck)test/e2e/satellite_test.go
210-210: func
generateConfigFile
is unused (unused)
195-195: ineffectual assignment to err (ineffassign)
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.
Actionable comments posted: 4
Outside diff range and nitpick comments (1)
internal/replicate/replicate.go (1)
Line range hint
152-170
: Improve error handling and logging inCopyImage
.The function lacks consistent error handling and could benefit from more detailed logging at each step of the image handling process.
+ if err != nil { + log.Printf("Failed to pull image %s: %v", imageName, err) + return err + } + log.Println("Image pulled successfully:", imageName)Also applies to: 180-180
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- internal/replicate/replicate.go (7 hunks)
- test/e2e/satellite_test.go (1 hunks)
- test/e2e/test.go (1 hunks)
- test/e2e/testdata/config.toml (1 hunks)
Files skipped from review due to trivial changes (1)
- test/e2e/testdata/config.toml
Additional context used
golangci-lint
internal/replicate/replicate.go
46-46: Error return value is not checked (errcheck)
test/e2e/satellite_test.go
45-45: S1023: redundant
return
statement (gosimple)
Signed-off-by: bupd <[email protected]> Add: Dagger integration for building and running Harbor Satellite Signed-off-by: bupd <[email protected]> create test to test the registry Signed-off-by: bupd <[email protected]> update registry test with e2e Signed-off-by: bupd <[email protected]> add testing for containers e2e Signed-off-by: bupd <[email protected]> update satellite to push to insecure registry Signed-off-by: bupd <[email protected]>
Signed-off-by: bupd <[email protected]> Signed-off-by: bupd <[email protected]>
Signed-off-by: bupd <[email protected]> update dagger test Signed-off-by: bupd <[email protected]> add complete e2e test of the satellite Signed-off-by: bupd <[email protected]>
Signed-off-by: bupd <[email protected]> add: .gitignore This commit adds standard go gitignore file to the repo. Signed-off-by: bupd <[email protected]>
@Vad1mo ready to merge. |
Overview:
This PR adds end-to-end testing with dagger.
Changes Introduced:
Prerequisites:
Tests require docker to be installed in the system.
How to Test:
go test -v ./test/e2e/registry_test.go
.Fixes: #26
Summary by CodeRabbit
New Features
Documentation
Chores
.env
and.gitignore
for environment configuration and version control.config.toml
andconfig.json
) for registry and testing settings.Tests