-
Notifications
You must be signed in to change notification settings - Fork 0
Workfow updates #6
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
Conversation
WalkthroughUpdated GitHub Actions release workflow to generate Docker metadata tags and switch the beam-docker-release action; TCP auth handling now maps 0→regular, 1→bot, and logs+defaults to regular for unexpected values. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant S as TCP Server
participant L as Logger
rect rgba(200,230,255,0.3)
C->>S: Authenticate(Type)
alt Type == 0
S->>S: Map Type to "regular"
else Type == 1
S->>S: Map Type to "bot"
else Unknown
S->>L: Warn about unexpected auth Type
S->>S: Default mapping to "regular"
end
S-->>C: Authentication response (role)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Comment |
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: 3
🧹 Nitpick comments (1)
src/erlmur_tcp_message.erl (1)
228-230: Authenticate.client_type fallback looks good; consider explicit 0 and logging unknowns.Clearer intent and easier debugging if 0 is handled explicitly and unknown values are logged while still defaulting to regular.
- 1 -> bot; - _ -> regular + 1 -> bot; + 0 -> regular; + _ -> + logger:debug("Unknown Authenticate.client_type ~p; defaulting to regular", [Type]), + regular
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
devenv.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
.github/workflows/release.yml(2 hunks)src/erlmur_tcp_message.erl(1 hunks)
🔇 Additional comments (1)
.github/workflows/release.yml (1)
1-1: Rename LGTM.No issues with the workflow name change.
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/release.yml
24-24: undefined function "replace". available functions are "always", "cancelled", "contains", "endswith", "failure", "format", "fromjson", "hashfiles", "join", "startswith", "success", "tojson"
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (3)
.github/workflows/release.yml (3)
1-1: Workflow rename LGTMClear, accurate name.
16-16: Pin the action ref; avoid @main for supply‑chain safety and reproducibilityUse a released tag or commit SHA.
- uses: freke/beam-docker-release-action@main + uses: freke/beam-docker-release-action@<tag-or-commit-sha> + # Example: + # uses: freke/beam-docker-release-action@v1.2.3 + # or pin to a commit SHA: + # uses: freke/beam-docker-release-action@<sha>
21-21: Avoid erlang:latest; pin a specific version (ideally with a digest) for deterministic buildsMutable tags can break builds unexpectedly.
- build-image: erlang:latest + build-image: erlang:27 + # For full reproducibility, prefer a digest: + # build-image: erlang:27@sha256:<digest>
e6a568b to
a4403c2
Compare
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: 0
🧹 Nitpick comments (2)
.github/workflows/release.yml (2)
15-17: Harden Docker tag sanitization (allowable charset, length, fallback).
Current step only replaces “/”. Add lowercasing, full charset filtering, 128‑char limit, and a safe fallback.- - name: Sanitize ref_name for Docker tag - run: | - echo "SANITIZED_REF=${GITHUB_REF_NAME//\//-}" >> "$GITHUB_ENV" + - name: Sanitize ref_name for Docker tag + run: | + RAW="${GITHUB_REF_NAME}" + # lowercase, replace invalid chars with '-', trim leading/trailing '-', limit to 128, fallback to sha + SANITIZED="$(echo -n "${RAW}" \ + | tr '[:upper:]' '[:lower:]' \ + | sed -E 's/[^a-z0-9._-]+/-/g; s/^-+//; s/-+$//' \ + | cut -c1-128)" + if [ -z "$SANITIZED" ]; then SANITIZED="sha-${GITHUB_SHA:0:7}"; fi + echo "SANITIZED_REF=$SANITIZED" >> "$GITHUB_ENV"
27-27: Using SANITIZED_REF for build-tags is correct. Consider adding a ‘latest’ tag on main.
Optional: publish both the branch tag and latest when building from main.Would you like me to propose a small change to emit an extra latest tag only on main?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (3)
.github/workflows/release.yml (3)
1-1: Workflow name change looks good.
Clearer title for what the job does.
24-24: Avoid erlang:latest; pin a specific version (ideally with a digest).
Prevents surprise breakages and ensures reproducibility.- build-image: erlang:latest + build-image: erlang:27 + # or pin by digest for full reproducibility: + # build-image: erlang:27@sha256:<digest>
19-19: Pin the action ref; avoid @main for supply‑chain safety.
Use a released tag or a commit SHA.- uses: freke/beam-docker-release-action@main + uses: freke/beam-docker-release-action@<tag-or-commit-sha>#!/bin/bash # Fetch available tags to pick a stable one to pin. gh api /repos/freke/beam-docker-release-action/tags -q '.[].name' | head -n 10
a4403c2 to
4f06ef9
Compare
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
4f06ef9 to
4189e24
Compare
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/release.yml(1 hunks)src/erlmur_tcp_message.erl(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/erlmur_tcp_message.erl
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (4)
.github/workflows/release.yml (4)
1-1: LGTM: Workflow rename is fine.
29-29: Pin the action ref; avoid @main for supply‑chain safety.Pin to a release tag or commit SHA:
- uses: freke/beam-docker-release-action@main + uses: freke/beam-docker-release-action@<tag-or-commit-sha>
34-34: Avoid erlang:latest; pin a specific version (ideally by digest) for deterministic builds.Example:
- build-image: erlang:latest + build-image: erlang:27 + # or fully pinned: + # build-image: erlang:27@sha256:<digest>
37-37: No change required — comma-separated tags are supporteddocker/metadata-action's sep-tags can emit comma-separated tags; docker/build-push-action accepts CSV (comma‑delimited) tags and freke/beam-docker-release-action simply forwards build-tags to build-push-action, so sep-tags: ',' is compatible.
b54b267 to
285b755
Compare
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: 0
🧹 Nitpick comments (3)
.github/workflows/release.yml (3)
16-27: Emit a literal “latest” tag only on semver tagsdocker/metadata-action won’t produce a raw “latest” without an explicit raw rule. Gate it to semver to avoid “latest” on non-releases.
tags: | type=ref,event=branch - type=ref,event=pr type=semver,pattern={{version}} type=semver,pattern={{major}}.{{minor}} type=semver,pattern={{major}} + # publish literal latest only on semver tags + type=raw,value=latest,enable={{is_semver}}
16-27: The PR tag rule is ineffective (workflow doesn’t run on pull_request)Either remove it (shown above) or add a pull_request trigger if you truly want PR images.
Option to enable PR builds:
on: - push: + push: + pull_request:
22-22: Reduce registry churn: restrict branch tags to default branch (optional)Prevents publishing images for every feature branch push.
- type=ref,event=branch + type=ref,event=branch,enable={{is_default_branch}}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (3)
.github/workflows/release.yml (3)
27-27: Confirm separator compatibility between metadata outputs and the Beam actionYou set sep-tags to comma. Ensure the beam-docker-release action splits on commas; many tools expect newline-separated tags. If unsure, drop sep-tags to use the default newline-separated output.
- sep-tags: ',' + # Use default (newline) separation to maximize compatibility + # sep-tags: ','Also applies to: 37-37
29-29: Pin the action ref; avoid using @main for supply‑chain safety and reproducibilityUse a released tag or a commit SHA.
- uses: freke/beam-docker-release-action@main + uses: freke/beam-docker-release-action@<tag-or-commit-sha>
34-34: Avoid erlang:latest; pin a specific OTP version (ideally with a digest)Latest is mutable and can break builds.
- build-image: erlang:latest + build-image: erlang:27 + # Or pin by digest for full reproducibility: + # build-image: erlang:27@sha256:<digest>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
285b755 to
a5b3c18
Compare
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: 0
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
16-26: Emit a literal latest tag gated to semver tags.Add a raw latest tag only when the ref is a valid semver tag.
tags: | type=ref,event=branch type=semver,pattern={{version}} type=semver,pattern={{major}}.{{minor}} type=semver,pattern={{major}} + # publish "latest" only for semver tag pushes + type=raw,value=latest,enable={{is_semver}}Also confirm you intend to publish images for every branch (type=ref,event=branch). If not, restrict the workflow to tags or drop the branch tag line.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (3)
.github/workflows/release.yml (3)
33-33: Avoid erlang:latest; pin a specific version (or digest) for deterministic builds.- build-image: erlang:latest + build-image: erlang:27 + # or fully pinned: + # build-image: erlang:27@sha256:<digest>
36-36: Verify build-tags format compatibility with the action.You set sep-tags to ','; ensure freke/beam-docker-release-action accepts a comma-separated list. If it expects newline-separated tags, either drop sep-tags or split before passing.
Option A — use default newline-separated output:
- build-tags: ${{ steps.meta.outputs.tags }} + build-tags: ${{ steps.meta.outputs.tags }}(and remove
sep-tags: ','above)Option B — keep comma-separated but confirm support in the action’s docs/repo. If unsure, I can look it up and suggest the exact expected format.
28-28: Pin the action ref; avoid @main for supply‑chain safety and reproducibility.- uses: freke/beam-docker-release-action@main + uses: freke/beam-docker-release-action@<tag-or-commit-sha> + # e.g. @v1.2.3 or @<sha>; document and update periodically
Summary by CodeRabbit
Bug Fixes
Chores