Adds support for useHostNetwork the unity-builder#828
Adds support for useHostNetwork the unity-builder#828lupidan wants to merge 7 commits intogame-ci:mainfrom
Conversation
|
|
📝 WalkthroughWalkthroughAdded a new boolean action input Changes
Sequence Diagram(s)sequenceDiagram
participant Action as GitHub Action
participant Input as Input
participant Params as BuildParameters
participant Docker as Docker Builder
participant Engine as Docker Engine
Action->>Input: read `useHostNetwork` input
Input-->>Action: boolean value
Action->>Params: BuildParameters.create() (includes useHostNetwork)
Params-->>Action: build parameters object
Action->>Docker: construct docker run command (passes useHostNetwork)
Docker->>Docker: if useHostNetwork == true add `--net=host`
Docker->>Engine: execute `docker run ...`
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #828 +/- ##
==========================================
+ Coverage 31.23% 31.27% +0.03%
==========================================
Files 84 84
Lines 4565 4569 +4
Branches 1054 1056 +2
==========================================
+ Hits 1426 1429 +3
- Misses 3139 3140 +1
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
action.yml (1)
50-53: Nit: quote the default to match other boolean inputs.Other boolean-like inputs in this file use quoted string defaults (e.g.,
'false'forrunAsHostUser,resourceTracking,cacheUnityInstallationOnMac,skipActivation). GitHub Actions input values are always strings, so using an unquoted YAML booleanfalsehere is inconsistent with the rest of the file.Proposed change
useHostNetwork: required: false - default: false + default: 'false' description: 'Initialises Docker using the host network. (Linux only)'Also consider documenting this input in the README as the PR description notes docs weren't yet updated.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@action.yml` around lines 50 - 53, The useHostNetwork input uses an unquoted YAML boolean false; change its default to the quoted string 'false' to match other boolean-like inputs (e.g., runAsHostUser, resourceTracking) and maintain consistency because GitHub Actions inputs are strings; also add a short entry documenting useHostNetwork in the README to reflect the new input.src/model/docker.ts (1)
89-89: Consider warning whenuseHostNetworkis set on non-Linux platforms.
useHostNetworkis silently ignored on Windows (getWindowsCommanddoesn't destructure or use it). Theaction.ymldescription already marks this Linux-only, but acore.warningwhen the flag is truthy on a non-Linux host would help users debug misconfigurations rather than wondering why--net=hostisn't being applied.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/model/docker.ts` at line 89, The template includes useHostNetwork but on non-Linux hosts that flag is ignored (getWindowsCommand doesn't use it); add a runtime warning when useHostNetwork is true and process.platform !== 'linux' so users know the flag will be ignored. Locate the code that builds the docker command (the template line with ${useHostNetwork ? '--net=host' : ''}) and/or getWindowsCommand, import and call core.warning (from `@actions/core`) when useHostNetwork is truthy on non-Linux platforms, with a short message indicating that --net=host is Linux-only and will be ignored on this OS.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@action.yml`:
- Around line 50-53: The useHostNetwork input uses an unquoted YAML boolean
false; change its default to the quoted string 'false' to match other
boolean-like inputs (e.g., runAsHostUser, resourceTracking) and maintain
consistency because GitHub Actions inputs are strings; also add a short entry
documenting useHostNetwork in the README to reflect the new input.
In `@src/model/docker.ts`:
- Line 89: The template includes useHostNetwork but on non-Linux hosts that flag
is ignored (getWindowsCommand doesn't use it); add a runtime warning when
useHostNetwork is true and process.platform !== 'linux' so users know the flag
will be ignored. Locate the code that builds the docker command (the template
line with ${useHostNetwork ? '--net=host' : ''}) and/or getWindowsCommand,
import and call core.warning (from `@actions/core`) when useHostNetwork is truthy
on non-Linux platforms, with a short message indicating that --net=host is
Linux-only and will be ignored on this OS.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 59743899-61eb-4a79-9381-800dc8305a25
⛔ Files ignored due to path filters (2)
dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (6)
action.ymlsrc/model/build-parameters.test.tssrc/model/build-parameters.tssrc/model/docker.tssrc/model/input.test.tssrc/model/input.ts
| - 2021.3.45f2 | ||
| - 2022.3.62f3 | ||
| - 2023.2.22f1 | ||
| targetPlatform: |
There was a problem hiding this comment.
macos-latest with the latest Xcode was struggling trying to compile with old Unity Versions. I took the opportunity to update these.
Running in my repo actions:
1 participant
Changes
--net=hostThis allows access, for example, to licensing servers that are inside a VPN, when VPN access is configured inside the Github Action
Test Builds in home repo
Checklist
Summary by CodeRabbit
New Features
useHostNetworkoption to enable Docker host-networking (Linux only).Tests
Chores