-
Notifications
You must be signed in to change notification settings - Fork 3
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
chore: daggerize #55
base: next
Are you sure you want to change the base?
chore: daggerize #55
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request introduce a new Go module named Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Harbor
participant Dagger
User->>Harbor: New(source)
activate Harbor
Harbor->>Dagger: BuildAllImages()
Dagger-->>Harbor: Return built images
Harbor->>User: PublishAndSignAllImages()
activate Harbor
Harbor->>Dagger: PublishImage()
Dagger-->>Harbor: Return published image data
Harbor->>User: Return success message
deactivate Harbor
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Quality Gate passedIssues Measures |
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: 5
🧹 Outside diff range and nitpick comments (1)
.dagger/main.go (1)
116-155
: Refactor duplicated code inPublishAllImages
andPublishImage
functionsThe functions
PublishAllImages
(lines 116-155) andPublishImage
(lines 157-197) contain similar logic, leading to code duplication. This can make maintenance more challenging and increase the risk of inconsistent behavior.Consider extracting the common functionality into a shared helper function or method to reduce duplication and improve maintainability.
Also applies to: 157-197
🧰 Tools
🪛 golangci-lint (1.62.2)
140-140: undefined: dag
(typecheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
.dagger/go.sum
is excluded by!**/*.sum
src/go.sum
is excluded by!**/*.sum
📒 Files selected for processing (9)
.dagger/.gitattributes
(1 hunks).dagger/.gitignore
(1 hunks).dagger/go.mod
(1 hunks).dagger/main.go
(1 hunks).gitignore
(1 hunks).golangci.yaml
(1 hunks)dagger.json
(1 hunks)src/core/main.go
(1 hunks)src/go.mod
(2 hunks)
✅ Files skipped from review due to trivial changes (5)
- .dagger/.gitattributes
- dagger.json
- .dagger/.gitignore
- src/core/main.go
- .gitignore
🧰 Additional context used
🪛 golangci-lint (1.62.2)
.dagger/main.go
93-93: undefined: dag
(typecheck)
140-140: undefined: dag
(typecheck)
184-184: undefined: dag
(typecheck)
🔇 Additional comments (4)
src/go.mod (2)
81-84
: Review new dependencies for Dagger integration
The addition of golang.org/x/crypto/x509roots/fallback
suggests certificate handling requirements. Please ensure:
- This is a required dependency for Dagger integration
- The version pinning (
0241127184453-8c4e668694cc
) is stable and secure
24-24
: Verify direct usage of go-openapi packages
The removal of // indirect
comments for go-openapi
packages suggests they are now directly imported. Please ensure these dependencies are actually used directly in the codebase, particularly in relation to the Dagger integration.
Also applies to: 26-26, 29-29
✅ Verification successful
The go-openapi
packages are directly used in the codebase
The search results confirm extensive direct usage of go-openapi
packages throughout the codebase:
go-openapi/runtime/middleware
is used in numerous handler files underserver/v2.0/handler/
go-openapi/strfmt
is used in model definitions and handlersgo-openapi/errors
is used in error handlinggo-openapi/swag
is used in registry implementation
The removal of // indirect
comments is correct as these dependencies are indeed directly imported and used in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify direct usage of go-openapi packages
# Search for direct imports of the packages
echo "Checking for direct imports of go-openapi packages..."
rg --type go "^import.*go-openapi/(loads|spec|validate)"
Length of output: 175
Script:
#!/bin/bash
# Let's try a broader search for go-openapi packages without restricting to import statements
echo "Searching for any usage of go-openapi packages..."
rg --type go "go-openapi/(loads|spec|validate)" -A 2
# Also check for any other go-openapi packages that might be used
echo -e "\nChecking all go-openapi related imports..."
rg --type go "go-openapi/"
# Let's also check if there are any .go files that might contain OpenAPI/Swagger related code
echo -e "\nLooking for potential OpenAPI/Swagger related files..."
fd -e go -x grep -l -i "openapi\|swagger" {}
Length of output: 9372
.golangci.yaml (1)
1-17
: LGTM!
The GolangCI-Lint configuration is properly set up with relevant linters and appropriate exclusions.
.dagger/go.mod (1)
10-11
:
Verify the versions of OpenTelemetry dependencies and replace directives
The dependencies and replace directives include versions with future timestamps, such as v0.0.0-20240518090000-14441aefdf88
. These versions may not be valid or available, leading to issues during dependency resolution.
Please confirm that the specified versions are correct and accessible. If these are placeholders or intended for future releases, update them to use the appropriate existing versions to ensure compatibility.
Also applies to: 16-21, 44-50
var imageAddresses []string | ||
for pkg, imgs := range platformVariantsContainer { | ||
for _, imageTag := range imageTags { | ||
container := dag.Container().WithRegistryAuth(registry, registryUsername, registryPassword) |
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.
Fix the undefined dag
identifier in multiple locations
The variable dag
is also undefined at lines 140 and 184, causing compilation errors. Replace dag
with the correct package or variable name.
Apply these diffs to fix the issues:
At line 140:
- container := dag.Container().WithRegistryAuth(registry, registryUsername, registryPassword)
+ container := dagger.Container().WithRegistryAuth(registry, registryUsername, registryPassword)
At line 184:
- addr, err := dag.Container().WithRegistryAuth(registry, registryUsername, registryPassword).
+ addr, err := dagger.Container().WithRegistryAuth(registry, registryUsername, registryPassword).
Also applies to: 184-184
🧰 Tools
🪛 golangci-lint (1.62.2)
140-140: undefined: dag
(typecheck)
func (m *Harbor) buildBinary(ctx context.Context, platform Platform, pkg Package) *BuildMetadata { | ||
|
||
os, arch, err := parsePlatform(string(platform)) | ||
if err != nil { | ||
log.Fatalf("Error parsing platform: %v", err) | ||
} | ||
|
||
outputPath := fmt.Sprintf("bin/%s/%s", platform, pkg) | ||
src := fmt.Sprintf("%s/main.go", pkg) | ||
builder := dag.Container(). | ||
From("golang:latest"). | ||
WithMountedCache("/go/pkg/mod", dag.CacheVolume("go-mod-"+GO_VERSION)). | ||
WithEnvVariable("GOMODCACHE", "/go/pkg/mod"). | ||
WithMountedCache("/go/build-cache", dag.CacheVolume("go-build-"+GO_VERSION)). | ||
WithEnvVariable("GOCACHE", "/go/build-cache"). | ||
WithMountedDirectory("/harbor", m.Source). | ||
WithWorkdir("/harbor/src/"). | ||
WithEnvVariable("GOOS", os). | ||
WithEnvVariable("GOARCH", arch). | ||
WithEnvVariable("CGO_ENABLED", "0"). | ||
WithExec([]string{"go", "build", "-o", outputPath, "-ldflags", "-extldflags=-static -s -w", src}) | ||
|
||
return &BuildMetadata{ | ||
Package: pkg, | ||
BinaryPath: outputPath, | ||
Container: builder, | ||
Platform: platform, | ||
} | ||
} |
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 suggestion
Set explicit Go version and cache keys in the build container
In the buildBinary
function, the Go version used in the build container is set to "latest"
. For reproducible builds and to avoid unexpected issues due to version changes, it is better to specify an explicit Go version. Additionally, using the Go version in cache keys ensures that caches are invalidated appropriately when the Go version changes.
Apply this diff to specify the Go version and update cache keys:
builder := dag.Container().
- From("golang:latest").
+ From(fmt.Sprintf("golang:%s", GO_VERSION)).
WithMountedCache("/go/pkg/mod", dag.CacheVolume("go-mod-"+GO_VERSION)).
WithEnvVariable("GOMODCACHE", "/go/pkg/mod").
WithMountedCache("/go/build-cache", dag.CacheVolume("go-build-"+GO_VERSION)).
WithEnvVariable("GOCACHE", "/go/build-cache").
Ensure that GO_VERSION
is properly defined with a specific version (e.g., "1.20"
).
📝 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 (m *Harbor) buildBinary(ctx context.Context, platform Platform, pkg Package) *BuildMetadata { | |
os, arch, err := parsePlatform(string(platform)) | |
if err != nil { | |
log.Fatalf("Error parsing platform: %v", err) | |
} | |
outputPath := fmt.Sprintf("bin/%s/%s", platform, pkg) | |
src := fmt.Sprintf("%s/main.go", pkg) | |
builder := dag.Container(). | |
From("golang:latest"). | |
WithMountedCache("/go/pkg/mod", dag.CacheVolume("go-mod-"+GO_VERSION)). | |
WithEnvVariable("GOMODCACHE", "/go/pkg/mod"). | |
WithMountedCache("/go/build-cache", dag.CacheVolume("go-build-"+GO_VERSION)). | |
WithEnvVariable("GOCACHE", "/go/build-cache"). | |
WithMountedDirectory("/harbor", m.Source). | |
WithWorkdir("/harbor/src/"). | |
WithEnvVariable("GOOS", os). | |
WithEnvVariable("GOARCH", arch). | |
WithEnvVariable("CGO_ENABLED", "0"). | |
WithExec([]string{"go", "build", "-o", outputPath, "-ldflags", "-extldflags=-static -s -w", src}) | |
return &BuildMetadata{ | |
Package: pkg, | |
BinaryPath: outputPath, | |
Container: builder, | |
Platform: platform, | |
} | |
} | |
func (m *Harbor) buildBinary(ctx context.Context, platform Platform, pkg Package) *BuildMetadata { | |
os, arch, err := parsePlatform(string(platform)) | |
if err != nil { | |
log.Fatalf("Error parsing platform: %v", err) | |
} | |
outputPath := fmt.Sprintf("bin/%s/%s", platform, pkg) | |
src := fmt.Sprintf("%s/main.go", pkg) | |
builder := dag.Container(). | |
From(fmt.Sprintf("golang:%s", GO_VERSION)). | |
WithMountedCache("/go/pkg/mod", dag.CacheVolume("go-mod-"+GO_VERSION)). | |
WithEnvVariable("GOMODCACHE", "/go/pkg/mod"). | |
WithMountedCache("/go/build-cache", dag.CacheVolume("go-build-"+GO_VERSION)). | |
WithEnvVariable("GOCACHE", "/go/build-cache"). | |
WithMountedDirectory("/harbor", m.Source). | |
WithWorkdir("/harbor/src/"). | |
WithEnvVariable("GOOS", os). | |
WithEnvVariable("GOARCH", arch). | |
WithEnvVariable("CGO_ENABLED", "0"). | |
WithExec([]string{"go", "build", "-o", outputPath, "-ldflags", "-extldflags=-static -s -w", src}) | |
return &BuildMetadata{ | |
Package: pkg, | |
BinaryPath: outputPath, | |
Container: builder, | |
Platform: platform, | |
} | |
} |
@@ -0,0 +1,50 @@ | |||
module dagger/harbor | |||
|
|||
go 1.23.2 |
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.
💡 Codebase verification
Invalid Go version specified in go.mod
The specified Go version 1.23.2
is incorrect as:
- Go 1.23 is not yet released (currently in development)
- Go version numbers don't use three components (major.minor.patch) in go.mod files, they only use major.minor
Please update the Go version in go.mod to use a currently released and stable version (e.g., go 1.21
or go 1.22
).
🔗 Analysis chain
Verify the Go version specified
The Go version is specified as go 1.23.2
, which may not be released or widely available yet. Using an unavailable Go version can cause build failures.
Run the following script to check the latest stable Go version:
Ensure that the Go version specified in go.mod
is correct and supported in your build environment.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Retrieve the latest stable Go version.
curl -s https://go.dev/dl/ | grep -oP 'go[0-9]+\.[0-9]+' | head -1
Length of output: 75
Daggerize the build pipeline
Summary by CodeRabbit
New Features
dagger/harbor
with functionalities for managing container image builds and publishing.dagger.json
for project settings.Bug Fixes
.gitignore
to exclude thebin/
directory from version control.Documentation
.golangci.yaml
to improve code quality checks.Chores
go.mod
for better readability and management.