feat: Dockerize application and add CI workflow#11
Conversation
This commit introduces Docker support for the sigrab application and sets up a GitHub Actions CI workflow. Key changes: - Added a multi-stage Dockerfile to build a minimal runtime image for sigrab. - Created a GitHub Actions workflow (`.github/workflows/ci.yml`) that: - Triggers on push to main and pull requests to main. - Runs Go tests using `go test ./...`. - Builds the Docker image. - Pushes the Docker image to GitHub Container Registry (ghcr.io) on pushes to main. - Updated `README.md` with a "Docker Usage" section, explaining how to build, run, and pull the Docker image. The CI workflow uses a placeholder for JIRA_API_TOKEN for testing purposes. You will need to configure the actual JIRA_API_TOKEN secret in your repository settings for the image to function correctly when run, and for tests that might require it if they were integration tests (current tests appear to be unit tests).
WalkthroughA new continuous integration workflow is added using GitHub Actions, featuring jobs for testing and Docker image management. A multi-stage Dockerfile for building and running the Go application is introduced. The README is updated with a section detailing Docker-based usage, including build, run, and image pull instructions. The Changes
Possibly related PRs
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
This commit corrects the GitHub Actions workflow file to trigger builds and pushes based on the `master` branch instead of `main`. - Changed push trigger from `main` to `master`. - Changed pull_request trigger from `main` to `master`. - Ensured Docker image push condition correctly references `refs/heads/master`.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
Dockerfile (1)
15-19: Run container as a non-root user for better security.
Consider creating a dedicated user and switching to it after copying the binary to reduce attack surface.# After copying /sigrab RUN adduser -D -u 1000 sigrab && \ chown sigrab:sigrab /sigrab USER sigrabREADME.md (2)
52-53: Suggest tagging images for reproducibility.
Encourage users to tag with a version or SHA alongsidelatest:docker build -t sigrab:latest -t sigrab:<version-or-sha> .
74-74: Grammar nit: add missing article.
Change to:Pre-built Docker images are available on the GitHub Container Registry.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~74-~74: You might be missing the article “the” here.
Context: ...re-built Docker images are available on GitHub Container Registry. You can pull the la...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/ci.yml(1 hunks)Dockerfile(1 hunks)README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~74-~74: You might be missing the article “the” here.
Context: ...re-built Docker images are available on GitHub Container Registry. You can pull the la...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 actionlint (1.7.7)
.github/workflows/ci.yml
21-21: the runner of "actions/setup-go@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (3)
README.md (1)
43-48: Approve Docker usage section.
The new "🐳 Docker Usage" is clear and aligns with existing documentation style..github/workflows/ci.yml (2)
21-24: Setup-Go action version is appropriate.
actions/setup-go@v4is the latest major release and compatible with current runners.🧰 Tools
🪛 actionlint (1.7.7)
21-21: the runner of "actions/setup-go@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
41-46: Verify Docker image naming and tagging.
You're using${{ github.repository_owner }}/sigrab. Confirm the repository is namedsigrabso the image path matches. Alternatively, you could use${{ github.repository }}to deriveowner/repo.
Dockerfile
Outdated
| FROM golang:1.21-alpine AS builder | ||
|
|
||
| WORKDIR /app | ||
|
|
||
| # Copy all project files | ||
| COPY . . | ||
|
|
||
| # Build the Go project | ||
| RUN go build -o /sigrab cmd/sigrab/main.go |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enable efficient Docker layer caching and optimize build steps.
To leverage Docker caching for Go modules and produce a smaller static binary, copy go.mod/go.sum first, download dependencies, then build with CGO_ENABLED=0 and linker flags.
FROM golang:1.21-alpine AS builder
WORKDIR /app
-# Copy all project files
-COPY . .
-# Build the Go project
-RUN go build -o /sigrab cmd/sigrab/main.go
+# Cache dependencies
+COPY go.mod go.sum ./
+RUN go mod download
+
+# Copy source and build
+COPY . .
+RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 \
+ go build -ldflags="-s -w" -o /sigrab cmd/sigrab/main.go📝 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.
| FROM golang:1.21-alpine AS builder | |
| WORKDIR /app | |
| # Copy all project files | |
| COPY . . | |
| # Build the Go project | |
| RUN go build -o /sigrab cmd/sigrab/main.go | |
| FROM golang:1.21-alpine AS builder | |
| WORKDIR /app | |
| # Cache dependencies | |
| COPY go.mod go.sum ./ | |
| RUN go mod download | |
| # Copy source and build | |
| COPY . . | |
| RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 \ | |
| go build -ldflags="-s -w" -o /sigrab cmd/sigrab/main.go |
🤖 Prompt for AI Agents
In the Dockerfile from lines 2 to 10, optimize Docker layer caching and build
efficiency by first copying only the go.mod and go.sum files, then running 'go
mod download' to cache dependencies separately. Next, copy the rest of the
project files and build the Go binary with 'CGO_ENABLED=0' and appropriate
linker flags to produce a smaller static binary. This approach improves build
speed and results in a more efficient Docker image.
This commit addresses several issues that were causing test failures and improves the project structure by initializing Go modules. Changes include: - Ran `go mod init github.com/tutunak/sigrab` and `go mod tidy` to create `go.mod` and `go.sum`, enabling proper dependency management. - Corrected function signatures and return value handling in `internal/jira/fetcher.go` and its tests (`fetcher_test.go`). - Fixed a duplicate import in `internal/jira/fetcher.go`. - Resolved an incorrect file path construction logic in `internal/jira/fetcher.go`. - Corrected an assignment mismatch in `internal/cli/root.go`. - Adjusted test expectations in `internal/jira/fetcher_test.go` regarding the order of returned issues and behavior on write errors. All tests under `./...` are now passing.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/jira/fetcher.go (1)
29-33: Consider adding logging for skipped issues.While silently skipping failed issue fetches matches the test expectations, this could make debugging difficult in production. Consider adding optional logging to track which issues are being skipped and why.
if err != nil { // If an issue is not found or there's an error, skip it and continue // This matches the test expectation for "skip non-existent issues" - // Consider logging this error if necessary + // TODO: Add optional debug logging for skipped issues continue }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
go.mod(1 hunks)internal/cli/root.go(1 hunks)internal/jira/fetcher.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- go.mod
🧰 Additional context used
🧬 Code Graph Analysis (1)
internal/jira/fetcher.go (3)
internal/utils/parser.go (1)
ParseIssueKey(9-26)internal/jira/client.go (1)
GetIssue(21-27)internal/output/writer.go (1)
NewWriter(12-14)
🔇 Additional comments (3)
internal/jira/fetcher.go (2)
18-18: LGTM: Function signature updated correctly.The method signature change to return both issues and error is implemented correctly and aligns with the broader changes in the codebase.
24-24: Issue collection logic is correct.The logic to collect fetched issues in order is well-implemented and maintains the expected sequence (N, N-1, N-2) as mentioned in the comments.
Also applies to: 36-36, 45-45
internal/cli/root.go (1)
50-50: LGTM: Correctly adapted to new method signature.The change properly adapts to the updated
FetchBackwardmethod signature by explicitly discarding the returned issues slice while preserving the existing error handling logic.
| func (f *Fetcher) FetchBackward(to string, path string) ([]goJira.Issue, error) { | ||
| prefix, endNum, err := utils.ParseIssueKey(to) | ||
| if err != nil { | ||
|
|
||
| return fmt.Errorf("failed to parse issue key %s: %w", to, err) | ||
| return nil, fmt.Errorf("failed to parse issue key %s: %w", to, err) | ||
| } | ||
|
|
||
| var fetchedIssues []goJira.Issue | ||
| for current := endNum; current >= 1; current-- { | ||
| issueKey := fmt.Sprintf("%s-%d", prefix, current) | ||
| issue, err := GetIssue(f.client, issueKey) | ||
| if err != nil { | ||
| panic(err) | ||
| // If an issue is not found or there's an error, skip it and continue | ||
| // This matches the test expectation for "skip non-existent issues" | ||
| // Consider logging this error if necessary | ||
| continue | ||
| } | ||
|
|
||
| // Append to maintain fetched order (e.g., N, N-1, N-2) as expected by tests. | ||
| fetchedIssues = append(fetchedIssues, *issue) | ||
|
|
||
| writer := output.NewWriter() | ||
| // writer.WriteToFile expects the directory as the first argument and constructs the filename itself. | ||
| if err := writer.WriteToFile(path, *issue); err != nil { | ||
|
|
||
| return fmt.Errorf("failed to write issue %s to file: %w", issueKey, err) | ||
| // On write error, the test expects nil for the issues slice. | ||
| return nil, fmt.Errorf("failed to write issue %s to directory %s: %w", issueKey, path, err) | ||
| } | ||
|
|
||
| // Prepend to maintain order | ||
| } | ||
| return nil | ||
| return fetchedIssues, nil | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider refactoring to separate concerns.
The FetchBackward method is currently handling both fetching issues and writing them to files, which violates the single responsibility principle. This tight coupling makes the method harder to test and reuse.
Consider refactoring to separate these concerns:
-func (f *Fetcher) FetchBackward(to string, path string) ([]goJira.Issue, error) {
+func (f *Fetcher) FetchBackward(to string) ([]goJira.Issue, error) {
prefix, endNum, err := utils.ParseIssueKey(to)
if err != nil {
return nil, fmt.Errorf("failed to parse issue key %s: %w", to, err)
}
var fetchedIssues []goJira.Issue
for current := endNum; current >= 1; current-- {
issueKey := fmt.Sprintf("%s-%d", prefix, current)
issue, err := GetIssue(f.client, issueKey)
if err != nil {
// If an issue is not found or there's an error, skip it and continue
// This matches the test expectation for "skip non-existent issues"
// Consider logging this error if necessary
continue
}
// Append to maintain fetched order (e.g., N, N-1, N-2) as expected by tests.
fetchedIssues = append(fetchedIssues, *issue)
-
- writer := output.NewWriter()
- // writer.WriteToFile expects the directory as the first argument and constructs the filename itself.
- if err := writer.WriteToFile(path, *issue); err != nil {
- // On write error, the test expects nil for the issues slice.
- return nil, fmt.Errorf("failed to write issue %s to directory %s: %w", issueKey, path, err)
- }
}
return fetchedIssues, nil
}Then handle file writing in the calling code or create a separate method for batch writing.
📝 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 (f *Fetcher) FetchBackward(to string, path string) ([]goJira.Issue, error) { | |
| prefix, endNum, err := utils.ParseIssueKey(to) | |
| if err != nil { | |
| return fmt.Errorf("failed to parse issue key %s: %w", to, err) | |
| return nil, fmt.Errorf("failed to parse issue key %s: %w", to, err) | |
| } | |
| var fetchedIssues []goJira.Issue | |
| for current := endNum; current >= 1; current-- { | |
| issueKey := fmt.Sprintf("%s-%d", prefix, current) | |
| issue, err := GetIssue(f.client, issueKey) | |
| if err != nil { | |
| panic(err) | |
| // If an issue is not found or there's an error, skip it and continue | |
| // This matches the test expectation for "skip non-existent issues" | |
| // Consider logging this error if necessary | |
| continue | |
| } | |
| // Append to maintain fetched order (e.g., N, N-1, N-2) as expected by tests. | |
| fetchedIssues = append(fetchedIssues, *issue) | |
| writer := output.NewWriter() | |
| // writer.WriteToFile expects the directory as the first argument and constructs the filename itself. | |
| if err := writer.WriteToFile(path, *issue); err != nil { | |
| return fmt.Errorf("failed to write issue %s to file: %w", issueKey, err) | |
| // On write error, the test expects nil for the issues slice. | |
| return nil, fmt.Errorf("failed to write issue %s to directory %s: %w", issueKey, path, err) | |
| } | |
| // Prepend to maintain order | |
| } | |
| return nil | |
| return fetchedIssues, nil | |
| } | |
| func (f *Fetcher) FetchBackward(to string) ([]goJira.Issue, error) { | |
| prefix, endNum, err := utils.ParseIssueKey(to) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to parse issue key %s: %w", to, err) | |
| } | |
| var fetchedIssues []goJira.Issue | |
| for current := endNum; current >= 1; current-- { | |
| issueKey := fmt.Sprintf("%s-%d", prefix, current) | |
| issue, err := GetIssue(f.client, issueKey) | |
| if err != nil { | |
| // If an issue is not found or there's an error, skip it and continue | |
| continue | |
| } | |
| // Append to maintain fetched order (e.g., N, N-1, N-2) as expected by tests. | |
| fetchedIssues = append(fetchedIssues, *issue) | |
| } | |
| return fetchedIssues, nil | |
| } |
🤖 Prompt for AI Agents
In internal/jira/fetcher.go around lines 18 to 46, the FetchBackward method
mixes fetching issues and writing them to files, violating single responsibility
principle. Refactor by removing the file writing logic from FetchBackward so it
only fetches and returns issues. Move the file writing code to the caller or a
new dedicated method that takes the fetched issues and writes them to files,
improving testability and reusability.
This commit resolves a Docker build failure caused by a Go version mismatch between the `go.mod` file and the Go version specified in the Dockerfile. Changes: - Updated `Dockerfile`: The builder stage now uses `golang:1.22-alpine` (previously `golang:1.21-alpine`) to match the Go version (>=1.22.2) required by `go.mod`. - Updated `.github/workflows/ci.yml`: The Go version for the `test` job (using `actions/setup-go`) has been changed from `1.21.x` to `1.22.x` to maintain consistency with the Docker build environment.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
Dockerfile (1)
6-10: 🛠️ Refactor suggestionOptimize Docker build caching and binary size
Refactor the build stage to first copy onlygo.mod/go.sumand download dependencies, then copy the rest of the source. Also enable static linking and strip debug symbols for a smaller binary.FROM golang:1.22-alpine AS builder WORKDIR /app -# Copy all project files -COPY . . +# Cache dependencies +COPY go.mod go.sum ./ +RUN go mod download + +# Copy source files +COPY . . -RUN go build -o /sigrab cmd/sigrab/main.go +RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 \ + go build -ldflags="-s -w" -o /sigrab cmd/sigrab/main.go.github/workflows/ci.yml (2)
11-13: Use GitHub secrets for JIRA_API_TOKEN
Avoid hardcoded placeholders—reference the actual secret instead:env: - JIRA_API_TOKEN: SUPER_SECRET_TOKEN_FOR_TESTING_ONLY + JIRA_API_TOKEN: ${{ secrets.JIRA_API_TOKEN }}
4-9:⚠️ Potential issueCritical: Workflow triggers reference
master, but repo usesmain
The workflow is configured formaster, so it won’t run on pushes or PRs againstmain. Update both triggers to the correct branch:-on: - push: - branches: - - master - pull_request: - branches: - - master +on: + push: + branches: + - main + pull_request: + branches: + - main
🧹 Nitpick comments (2)
Dockerfile (1)
13-20: Consider a more minimal and secure runtime
For an even smaller attack surface, you could switch to ascratchor distroless base image and run as a non-root user..github/workflows/ci.yml (1)
20-25: Speed up CI by caching Go modules
Before running tests, cache downloaded modules to reduce setup time:- name: Cache Go modules uses: actions/cache@v3 with: path: ~/go/pkg/mod key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} restore-keys: | ${{ runner.os }}-go-🧰 Tools
🪛 actionlint (1.7.7)
21-21: the runner of "actions/setup-go@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks)Dockerfile(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/ci.yml
21-21: the runner of "actions/setup-go@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (1)
Dockerfile (1)
13-14: Good: CA certificates installed for HTTPS support
Includingca-certificatesensures runtime HTTPS calls (e.g., to Jira) succeed.
1 participant
This commit introduces Docker support for the sigrab application and sets up a GitHub Actions CI workflow.
Key changes:
.github/workflows/ci.yml) that:go test ./....README.mdwith a "Docker Usage" section, explaining how to build, run, and pull the Docker image.The CI workflow uses a placeholder for JIRA_API_TOKEN for testing purposes. You will need to configure the actual JIRA_API_TOKEN secret in your repository settings for the image to function correctly when run, and for tests that might require it if they were integration tests (current tests appear to be unit tests).
Summary by CodeRabbit