Skip to content

[code-simplifier] refactor: extract awfVersionAtLeast helper to deduplicate version-check functions #31627

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.

Sign up for GitHub

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

Merged
pelikhan merged 1 commit into from
May 12, 2026
+16 −84
Merged

[code-simplifier] refactor: extract awfVersionAtLeast helper to deduplicate version-check functions #31627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 15 additions & 83 deletions pkg/workflow/awf_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,111 +633,43 @@ func addCliProxyGHTokenToEnv(env map[string]string, workflowData *WorkflowData)
}
}

// awfSupportsExcludeEnv returns true when the effective AWF version supports --exclude-env.
//
// The --exclude-env flag was introduced in AWF v0.25.3. Any workflow that pins an explicit
// version older than v0.25.3 must not emit --exclude-env or the run will fail at startup.
//
// Special cases:
// - No version override (firewallConfig is nil or has no Version): use DefaultFirewallVersion
// which is always ≥ AWFExcludeEnvMinVersion → returns true.
// - "latest": always returns true (latest is always a new release).
// - Any semver string ≥ AWFExcludeEnvMinVersion: returns true.
// - Any semver string < AWFExcludeEnvMinVersion: returns false.
// - Non-semver string (e.g. a branch name): returns false (conservative).
// awfSupportsExcludeEnv returns true when the effective AWF version supports --exclude-env
// (introduced in AWF v0.25.3).
func awfSupportsExcludeEnv(firewallConfig *FirewallConfig) bool {
var versionStr string
if firewallConfig != nil && firewallConfig.Version != "" {
versionStr = firewallConfig.Version
} else {
// No override → use the default, which is always ≥ the minimum.
return true
}

// "latest" means the newest release — always supports the flag.
if strings.EqualFold(versionStr, "latest") {
return true
}

// Normalise the v-prefix for semverutil.Compare.
minVersion := string(constants.AWFExcludeEnvMinVersion)
return semverutil.Compare(versionStr, minVersion) >= 0
return awfVersionAtLeast(firewallConfig, constants.AWFExcludeEnvMinVersion)
Copy link
Copy Markdown
Contributor Author

@github-actions github-actions Bot May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/zoom-out] The behavioral equivalence for awfSupportsExcludeEnv with a nil/empty config relies on the invariant DefaultFirewallVersion >= AWFExcludeEnvMinVersion (currently v0.25.43 >= v0.25.3 ✅). This invariant is implicit — nothing enforces it at compile-time or in tests.

If DefaultFirewallVersion were ever rolled back below v0.25.3, this function would silently return false for unconfigured workflows instead of true, a subtle behavioral regression. Consider asserting it in the constants spec test:

// DefaultFirewallVersion must be >= all feature-flag minimum versions
assert.GreaterOrEqual(t, semverutil.Compare(string(DefaultFirewallVersion), string(AWFExcludeEnvMinVersion)), 0)

This makes the invariant visible and caught at test time rather than discovered in production.

}

// awfSupportsCliProxy returns true when the effective AWF version supports --difc-proxy-host
// and --difc-proxy-ca-cert.
// awfVersionAtLeast returns true when the effective AWF version is at or above minVersion.
//
// These flags were introduced in AWF v0.26.0 (replacing the earlier --enable-cli-proxy).
// Any workflow that pins an explicit version older than v0.26.0 must not emit CLI proxy
// flags or the run will fail at startup.
//
// Special cases:
// - No version override (firewallConfig is nil or has no Version): use DefaultFirewallVersion
// and compare against AWFCliProxyMinVersion.
// - "latest": always returns true (latest is always a new release).
// - Any semver string ≥ AWFCliProxyMinVersion: returns true.
// - Any semver string < AWFCliProxyMinVersion: returns false.
// - Non-semver string (e.g. a branch name): returns false (conservative).
func awfSupportsCliProxy(firewallConfig *FirewallConfig) bool {
// If firewallConfig has no version set, DefaultFirewallVersion is used. "latest" always
// returns true. Non-semver strings (e.g. branch names) return false (conservative).
func awfVersionAtLeast(firewallConfig *FirewallConfig, minVersion constants.Version) bool {
Copy link
Copy Markdown
Contributor Author

@github-actions github-actions Bot May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/improve-codebase-architecture] The helper is declared after its first caller (awfSupportsExcludeEnv at line 638). In Go the compiler does not care about declaration order within a package, but by convention helper functions are defined before the functions that use them (or at least grouped together). Moving awfVersionAtLeast above awfSupportsExcludeEnv would make the file easier to read top-to-bottom without having to scroll down to find what a one-liner delegates to.

var versionStr string
if firewallConfig != nil && firewallConfig.Version != "" {
versionStr = firewallConfig.Version
} else {
// No override → use the default version for comparison.
versionStr = string(constants.DefaultFirewallVersion)
}

// "latest" means the newest release — always supports the flag.
if strings.EqualFold(versionStr, "latest") {
return true
}
return semverutil.Compare(versionStr, string(minVersion)) >= 0
}

// Normalise the v-prefix for semverutil.Compare.
minVersion := string(constants.AWFCliProxyMinVersion)
return semverutil.Compare(versionStr, minVersion) >= 0
// awfSupportsCliProxy returns true when the effective AWF version supports --difc-proxy-host
// and --difc-proxy-ca-cert (introduced in AWF v0.26.0).
func awfSupportsCliProxy(firewallConfig *FirewallConfig) bool {
return awfVersionAtLeast(firewallConfig, constants.AWFCliProxyMinVersion)
}

// awfSupportsAllowHostPorts returns true when the effective AWF version supports
// --allow-host-ports.
//
// Special cases:
// - No version override (firewallConfig is nil or has no Version): use DefaultFirewallVersion
// and compare against AWFAllowHostPortsMinVersion (currently this returns true because
// DefaultFirewallVersion is at or above the minimum supported version).
// - "latest": always returns true (latest is always a new release).
// - Any semver string ≥ AWFAllowHostPortsMinVersion: returns true.
// - Any semver string < AWFAllowHostPortsMinVersion: returns false.
// - Non-semver string (e.g. a branch name): returns false (conservative).
func awfSupportsAllowHostPorts(firewallConfig *FirewallConfig) bool {
var versionStr string
if firewallConfig != nil && firewallConfig.Version != "" {
versionStr = firewallConfig.Version
} else {
versionStr = string(constants.DefaultFirewallVersion)
}

if strings.EqualFold(versionStr, "latest") {
return true
}

minVersion := string(constants.AWFAllowHostPortsMinVersion)
return semverutil.Compare(versionStr, minVersion) >= 0
return awfVersionAtLeast(firewallConfig, constants.AWFAllowHostPortsMinVersion)
}

// awfSupportsDockerHostPathPrefix returns true when the effective AWF version supports
// --docker-host-path-prefix.
func awfSupportsDockerHostPathPrefix(firewallConfig *FirewallConfig) bool {
var versionStr string
if firewallConfig != nil && firewallConfig.Version != "" {
versionStr = firewallConfig.Version
} else {
versionStr = string(constants.DefaultFirewallVersion)
}

if strings.EqualFold(versionStr, "latest") {
return true
}

minVersion := string(constants.AWFDockerHostPathPrefixMinVersion)
return semverutil.Compare(versionStr, minVersion) >= 0
return awfVersionAtLeast(firewallConfig, constants.AWFDockerHostPathPrefixMinVersion)
}
2 changes: 1 addition & 1 deletion pkg/workflow/compiler_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func NewCompiler(opts ...CompilerOption) *Compiler {
artifactManager: NewArtifactManager(),
actionPinWarnings: make(map[string]bool), // Initialize warning cache
priorManifests: make(map[string]*GHAWManifest),
gitRoot: gitRoot, // Auto-detected git root
gitRoot: gitRoot, // Auto-detected git root
}

// Apply functional options
Expand Down
Loading