-
Notifications
You must be signed in to change notification settings - Fork 23
Add dependabot grouping/auto-merge, unit-test CI gate, and status endpoint #382
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| version: 2 | ||
| updates: | ||
| # Go module dependencies. Minor/patch bumps are grouped into a single weekly | ||
| # PR to cut down on noise; major bumps stay as individual PRs since they tend | ||
| # to need code changes and a real review. | ||
| - package-ecosystem: gomod | ||
| directory: "/" | ||
| schedule: | ||
| interval: weekly | ||
| open-pull-requests-limit: 10 | ||
| labels: | ||
| - dependencies | ||
| - go | ||
| groups: | ||
| go-minor-and-patch: | ||
| update-types: | ||
| - minor | ||
| - patch | ||
|
|
||
| # Keep GitHub Actions used in workflows up to date. | ||
| - package-ecosystem: github-actions | ||
| directory: "/" | ||
| schedule: | ||
| interval: weekly | ||
| labels: | ||
| - dependencies | ||
| groups: | ||
| github-actions: | ||
| update-types: | ||
| - minor | ||
| - patch |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| name: Dependabot auto-merge | ||
|
|
||
| # Enables auto-merge for Dependabot minor/patch bumps so they merge on their own | ||
| # once all required status checks pass. Major bumps are left for manual review. | ||
| # | ||
| # Requires, in repo settings: | ||
| # - "Allow auto-merge" enabled | ||
| # - a branch protection rule on main requiring the CI checks to pass | ||
| on: pull_request_target | ||
|
|
||
| permissions: | ||
| contents: write | ||
| pull-requests: write | ||
|
|
||
| jobs: | ||
| auto-merge: | ||
| runs-on: ubuntu-latest | ||
| if: github.actor == 'dependabot[bot]' | ||
| steps: | ||
| - name: Fetch Dependabot metadata | ||
| id: meta | ||
| uses: dependabot/fetch-metadata@v2 | ||
|
|
||
| - name: Enable auto-merge for minor and patch updates | ||
| if: steps.meta.outputs.update-type == 'version-update:semver-minor' || steps.meta.outputs.update-type == 'version-update:semver-patch' | ||
| run: gh pr merge --auto --squash "$PR_URL" | ||
| env: | ||
| PR_URL: ${{ github.event.pull_request.html_url }} | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| name: Unit tests | ||
|
|
||
| # Fast feedback gate: compiles the code (go vet) and runs the unit tests in | ||
| # tests/unit. Far quicker than the integration "validate" jobs, so build breaks | ||
| # and obvious regressions surface in a couple of minutes. | ||
| on: | ||
| push: | ||
| branches: [ main ] | ||
| pull_request: | ||
| branches: [ main ] | ||
|
|
||
| env: | ||
| CGO_ENABLED: "1" # podman bindings require CGO | ||
| BUILDTAGS: "containers_image_openpgp gssapi providerless netgo osusergo exclude_graphdriver_btrfs" | ||
|
|
||
| jobs: | ||
| test: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - uses: actions/setup-go@v5 | ||
| with: | ||
| go-version-file: go.mod | ||
|
|
||
| - name: Install build dependencies | ||
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install -y libgpgme-dev libdevmapper-dev libseccomp-dev pkg-config | ||
|
|
||
| - name: go vet | ||
| run: go vet -tags "$BUILDTAGS" ./cmd/... ./pkg/... | ||
|
|
||
| - name: Unit tests | ||
| run: go test -tags "$BUILDTAGS" ./tests/unit/... ./pkg/... |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -348,6 +348,7 @@ func getMethodTargetScheds(targetConfigs []*TargetConfig, fetchit *Fetchit) *Fet | |
| } | ||
|
|
||
| func (f *Fetchit) RunTargets() { | ||
| startStatusServer() | ||
| for method := range f.methodTargetScheds { | ||
| // ConfigReload, PodmanAutoUpdateAll, Image, Prune methods do not include git URL | ||
| if method.GetTarget().url != "" { | ||
|
|
@@ -367,7 +368,12 @@ func (f *Fetchit) RunTargets() { | |
| defer cancel() | ||
| mt := method.GetKind() | ||
| logger.Infof("Processing git target: %s Method: %s Name: %s", method.GetTarget().url, mt, method.GetName()) | ||
| s.Cron(schedInfo.schedule).Tag(mt).Do(method.Process, ctx, f.conn, skew) | ||
| status.register(method, schedInfo.schedule) | ||
| m := method | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (review_instructions): The anonymous function passed to The line below defines the function as You probably want the second parameter to be the connection, matching the original m := method
s.Cron(schedInfo.schedule).Tag(mt).Do(func(ctx context.Context, conn *grpc.ClientConn, skew int) {
status.recordRun(m)
m.Process(ctx, conn, skew)
}, ctx, f.conn, skew)This preserves the previous behavior while adding the status tracking without introducing a type mismatch. Review instructions:Path patterns: Instructions: |
||
| s.Cron(schedInfo.schedule).Tag(mt).Do(func(ctx, conn context.Context, skew int) { | ||
| status.recordRun(m) | ||
| m.Process(ctx, conn, skew) | ||
| }, ctx, f.conn, skew) | ||
| s.StartImmediately() | ||
| } | ||
| s.StartAsync() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| package engine | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "net/http" | ||
| "os" | ||
| "sort" | ||
| "sync" | ||
| "time" | ||
| ) | ||
|
|
||
| // statusAddrEnv, when set (e.g. ":8080"), makes fetchit serve a small HTTP | ||
| // status server exposing /healthz and /status. Left unset, no port is opened. | ||
| const statusAddrEnv = "FETCHIT_STATUS_ADDR" | ||
|
|
||
| // methodStatus is the per-method reconciliation state surfaced at /status. | ||
| type methodStatus struct { | ||
| Kind string `json:"kind"` | ||
| Name string `json:"name"` | ||
| URL string `json:"url,omitempty"` | ||
| Schedule string `json:"schedule"` | ||
| Runs int `json:"runs"` | ||
| LastRun *time.Time `json:"lastRun,omitempty"` | ||
| } | ||
|
|
||
| type statusRegistry struct { | ||
| mu sync.Mutex | ||
| started time.Time | ||
| methods map[string]*methodStatus | ||
| } | ||
|
|
||
| var status = &statusRegistry{methods: make(map[string]*methodStatus)} | ||
|
|
||
| func statusKey(m Method) string { | ||
| return m.GetKind() + "/" + m.GetName() + "/" + m.GetTarget().url | ||
| } | ||
|
|
||
| // register records a scheduled method so it shows up at /status before its | ||
| // first run. | ||
| func (r *statusRegistry) register(m Method, schedule string) { | ||
| r.mu.Lock() | ||
| defer r.mu.Unlock() | ||
| if r.started.IsZero() { | ||
| r.started = time.Now() | ||
| } | ||
| r.methods[statusKey(m)] = &methodStatus{ | ||
| Kind: m.GetKind(), | ||
| Name: m.GetName(), | ||
| URL: m.GetTarget().url, | ||
| Schedule: schedule, | ||
| } | ||
| } | ||
|
|
||
| // recordRun stamps the most recent execution of a method. | ||
| func (r *statusRegistry) recordRun(m Method) { | ||
| r.mu.Lock() | ||
| defer r.mu.Unlock() | ||
| s, ok := r.methods[statusKey(m)] | ||
| if !ok { | ||
| return | ||
| } | ||
| now := time.Now() | ||
| s.Runs++ | ||
| s.LastRun = &now | ||
| } | ||
|
|
||
| func (r *statusRegistry) writeJSON(w http.ResponseWriter) { | ||
|
sourcery-ai[bot] marked this conversation as resolved.
sourcery-ai[bot] marked this conversation as resolved.
|
||
| r.mu.Lock() | ||
| // Copy by value under the lock so concurrent recordRun/register calls | ||
| // cannot mutate what we encode below. | ||
| ms := make([]methodStatus, 0, len(r.methods)) | ||
| for _, s := range r.methods { | ||
| ms = append(ms, *s) | ||
| } | ||
| started := r.started | ||
| r.mu.Unlock() | ||
|
|
||
| sort.Slice(ms, func(i, j int) bool { | ||
| if ms[i].Kind != ms[j].Kind { | ||
| return ms[i].Kind < ms[j].Kind | ||
| } | ||
| return ms[i].Name < ms[j].Name | ||
| }) | ||
|
|
||
| // Before the first method registers, started is zero; report a sane state | ||
| // instead of an uptime measured from year 1. | ||
| state := "running" | ||
| var uptime int64 | ||
| if started.IsZero() { | ||
| state = "initializing" | ||
| } else { | ||
| uptime = int64(time.Since(started).Seconds()) | ||
| } | ||
|
|
||
| resp := struct { | ||
| Status string `json:"status"` | ||
| StartedAt time.Time `json:"startedAt"` | ||
| UptimeSeconds int64 `json:"uptimeSeconds"` | ||
| Methods []methodStatus `json:"methods"` | ||
| }{ | ||
| Status: state, | ||
| StartedAt: started, | ||
| UptimeSeconds: uptime, | ||
| Methods: ms, | ||
| } | ||
|
|
||
| w.Header().Set("Content-Type", "application/json") | ||
| enc := json.NewEncoder(w) | ||
|
sourcery-ai[bot] marked this conversation as resolved.
|
||
| enc.SetIndent("", " ") | ||
| if err := enc.Encode(resp); err != nil { | ||
| logger.Warnf("status: failed to encode response: %v", err) | ||
| } | ||
| } | ||
|
|
||
| // startStatusServer launches the status/health HTTP server in a goroutine if | ||
| // FETCHIT_STATUS_ADDR is set. It is a no-op otherwise. | ||
| func startStatusServer() { | ||
|
sourcery-ai[bot] marked this conversation as resolved.
|
||
| addr := os.Getenv(statusAddrEnv) | ||
|
sourcery-ai[bot] marked this conversation as resolved.
|
||
| if addr == "" { | ||
| return | ||
| } | ||
| mux := http.NewServeMux() | ||
| mux.HandleFunc("/healthz", func(w http.ResponseWriter, _ *http.Request) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (review_instructions): The writes in the /healthz handler ignore write errors, which can silently hide I/O problems. In the Consider at least logging the error from Review instructions:Path patterns: Instructions: |
||
| w.WriteHeader(http.StatusOK) | ||
| _, _ = w.Write([]byte("ok\n")) | ||
| }) | ||
| mux.HandleFunc("/status", func(w http.ResponseWriter, _ *http.Request) { | ||
| status.writeJSON(w) | ||
| }) | ||
| go func() { | ||
| logger.Infof("Status server listening on %s (/healthz, /status)", addr) | ||
| if err := http.ListenAndServe(addr, mux); err != nil { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (review_instructions): The HTTP status server uses http.ListenAndServe with default settings, which lacks timeouts and can be vulnerable to slow-client/DoS-style attacks; consider using an http.Server with sensible timeouts. Right now the status server is created via A more defensive pattern is to construct an srv := &http.Server{
Addr: addr,
Handler: mux,
ReadTimeout: 5 * time.Second,
WriteTimeout: 10 * time.Second,
IdleTimeout: 60 * time.Second,
}
if err := srv.ListenAndServe(); err != nil {
logger.Errorf("status server error: %v", err)
}If you expect this endpoint to ever be exposed beyond localhost, these timeouts help reduce the risk of malicious or misbehaving clients tying up connections indefinitely. Review instructions:Path patterns: Instructions: |
||
| logger.Errorf("status server error: %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.
suggestion (review_instructions): The double slash in
podman//podman.socklooks like a typographical error in the command and may confuse readers.The path
podman//podman.sockincludes a double slash, which is unusual and likely a typo rather than intentional.To make the example clearer and avoid confusion, consider correcting it to use a single slash:
This keeps the path syntactically clean while preserving the intended mount point.
Review instructions:
Path patterns:
*Instructions:
Identify and point out typographical errors in code, especially in commands or scripts.