fix backward compatibility with full network type #4883
Conversation
WalkthroughThe changes add new test cases and modify existing ones to improve coverage of network configurations—specifically for the new Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Executor
participant NetworkModel
participant Docker
participant PortAllocator
CLI->>Executor: Run job with network flag (--network=host/--network=full)
Executor->>NetworkModel: Parse network type
NetworkModel-->>Executor: Return network type (Host or Full)
Executor->>Docker: Configure network mode
alt If network type is Host or Full
Docker-->>Executor: Set dockerNetworkHost and update ExtraHosts
else
Docker-->>Executor: Proceed with default configuration
end
Executor->>PortAllocator: AllocatePorts(networkCfg)
PortAllocator->>NetworkModel: Check SupportPortAllocation()
NetworkModel-->>PortAllocator: Return true/false
PortAllocator-->>Executor: Return allocated ports or error
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 golangci-lint (1.62.2){"Issues":[{"FromLinter":"typecheck","Text":"pattern build/: no matching files found","Severity":"","SourceLines":["//go:embed build/"],"Replacement":null,"Pos":{"Filename":"webui/webui.go","Offset":0,"Line":23,"Column":12},"ExpectNoLint":false,"ExpectedNoLintLinter":""}],"Report":{"Linters":[{"Name":"asasalint"},{"Name":"asciicheck"},{"Name":"bidichk"},{"Name":"bodyclose","Enabled":true},{"Name":"canonicalheader"},{"Name":"containedctx"},{"Name":"contextcheck"},{"Name":"copyloopvar","Enabled":true},{"Name":"cyclop"},{"Name":"decorder"},{"Name":"deadcode"},{"Name":"depguard","Enabled":true},{"Name":"dogsled","Enabled":true},{"Name":"dupl"},{"Name":"dupword"},{"Name":"durationcheck"},{"Name":"errcheck","Enabled":true,"EnabledByDefault":true},{"Name":"errchkjson"},{"Name":"errname"},{"Name":"errorlint"},{"Name":"execinquery"},{"Name":"exhaustive"},{"Name":"exhaustivestruct"},{"Name":"exhaustruct"},{"Name":"exportloopref"},{"Name":"forbidigo","Enabled":true},{"Name":"forcetypeassert"},{"Name":"fatcontext"},{"Name":"funlen","Enabled":true},{"Name":"gci"},{"Name":"ginkgolinter"},{"Name":"gocheckcompilerdirectives"},{"Name":"gochecknoglobals"},{"Name":"gochecknoinits","Enabled":true},{"Name":"gochecksumtype"},{"Name":"gocognit"},{"Name":"goconst","Enabled":true},{"Name":"gocritic"},{"Name":"gocyclo","Enabled":true},{"Name":"godot"},{"Name":"godox"},{"Name":"err113"},{"Name":"gofmt","Enabled":true},{"Name":"gofumpt"},{"Name":"goheader"},{"Name":"goimports","Enabled":true},{"Name":"golint"},{"Name":"mnd","Enabled":true},{"Name":"gomnd"},{"Name":"gomoddirectives"},{"Name":"gomodguard"},{"Name":"goprintffuncname","Enabled":true},{"Name":"gosec","Enabled":true},{"Name":"gosimple","Enabled":true,"EnabledByDefault":true},{"Name":"gosmopolitan"},{"Name":"govet","Enabled":true,"EnabledByDefault":true},{"Name":"grouper"},{"Name":"ifshort"},{"Name":"iface"},{"Name":"importas"},{"Name":"inamedparam"},{"Name":"ineffassign","Enabled":true,"EnabledByDefault":true},{"Name":"interfacebloat"},{"Name":"interfacer"},{"Name":"intrange"},{"Name":"ireturn"},{"Name":"lll","Enabled":true},{"Name":"loggercheck"},{"Name":"maintidx"},{"Name":"makezero"},{"Name":"maligned"},{"Name":"mirror"},{"Name":"misspell"},{"Name":"musttag"},{"Name":"nakedret","Enabled":true},{"Name":"nestif"},{"Name":"nilerr"},{"Name":"nilnil"},{"Name":"nlreturn"},{"Name":"noctx","Enabled":true},{"Name":"nonamedreturns"},{"Name":"nosnakecase"},{"Name":"nosprintfhostport"},{"Name":"paralleltest"},{"Name":"perfsprint"},{"Name":"prealloc"},{"Name":"predeclared"},{"Name":"promlinter"},{"Name":"protogetter"},{"Name":"reassign"},{"Name":"recvcheck"},{"Name":"revive"},{"Name":"rowserrcheck"},{"Name":"sloglint"},{"Name":"scopelint"},{"Name":"sqlclosecheck"},{"Name":"spancheck"},{"Name":"staticcheck","Enabled":true,"EnabledByDefault":true},{"Name":"structcheck"},{"Name":"stylecheck","Enabled":true},{"Name":"tagalign"},{"Name":"tagliatelle"},{"Name":"tenv"},{"Name":"testableexamples"},{"Name":"testifylint"},{"Name":"testpackage"},{"Name":"thelper"},{"Name":"tparallel"},{"Name":"typecheck","Enabled":true,"EnabledByDefault":true},{"Name":"unconvert","Enabled":true},{"Name":"unparam"},{"Name":"unused","Enabled":true,"EnabledByDefault":true},{"Name":"usestdlibvars"},{"Name":"varcheck"},{"Name":"varnamelen"},{"Name":"wastedassign"},{"Name":"whitespace","Enabled":true},{"Name":"wrapcheck"},{"Name":"wsl"},{"Name":"zerologlint"},{"Name":"nolintlint","Enabled":true}]}} 🪧 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
webui/lib/api/schema/swagger.json (1)
1885-1891: Confirm Enum Update with Clear Deprecation NoticeThe changes in the
"models.Network"definition add the newNetworkFullenum value in the enum array and its corresponding entry inx-enum-varnames. This update is intended to restore backward compatibility by ensuring that older clients that rely on theFullnetwork type get mapped correctly.Recommendations:
- Ensure that the API documentation explicitly notes that
NetworkFullis deprecated (or an alias forNetworkHost) so that users are aware of its limited, backward compatibility role.- If possible, consider adding a vendor extension (for example,
"x-deprecated": trueor a descriptive comment in the documentation) to clearly markNetworkFullas non-preferred for future integrations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (4)
cmd/cli/wasm/wasm_run_cli_test.go(1 hunks)pkg/swagger/docs.go(1 hunks)pkg/swagger/swagger.json(1 hunks)webui/lib/api/schema/swagger.json(1 hunks)
🔇 Additional comments (3)
cmd/cli/wasm/wasm_run_cli_test.go (1)
335-344: Test updated to use "host" network type instead of "full".The test case has been renamed from "with network full" to "with network host" and the network flag has been updated from "--network=full" to "--network=host". This change aligns with the PR objective to address backward compatibility issues between the old "Full" network type and the renamed "Host" type.
The assertion is correctly validating that the job's network type is set to
models.NetworkHost, which suggests this test will continue to function properly with the updated network type naming.pkg/swagger/docs.go (1)
1889-1898: Good addition for backward compatibilityThe changes add
NetworkFullbetweenNetworkHostandNetworkHTTPin the enum values formodels.Network. This aligns perfectly with the PR objective of fixing backward compatibility with the Full network type, which was previously renamed to Host.This approach allows older clients to continue using the
NetworkFulltype while newer clients can useNetworkHost, solving the issue where older clients couldn't parse theHostnetwork type in job responses.pkg/swagger/swagger.json (1)
1885-1894: New 'NetworkFull' Enum Entry Introduced
The updated enumeration now includes a new value (with integer value 4) for"NetworkFull"in both the"enum"array and the corresponding"x-enum-varnames"array. This change appears to address the backward compatibility issue by allowing older clients to continue interpreting the network types correctly, even as theFullnetwork type was previously renamed in code.Please verify that the assigned numeric values and their ordering remain consistent with the definitions in the corresponding Go source (e.g., in
pkg/models/network.go) and that any deprecation handling for the legacyFulltype is properly coordinated across the codebase.
Fullnetwork type has been renamed toHostin a previous release. While the orchestrator maintained backward compatibility by still acceptingFullnetwork types and transforming them toHost, this broke older clients as they couldn't parseHostnetwork type in the job responseSummary by CodeRabbit
New Features
Refactor
Tests
NetworkFull.