Skip to content

Use version ranges for actions to not use stale cache entries #2126

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

Merged
merged 1 commit into from
Apr 27, 2025

Conversation

Vampire
Copy link
Member

@Vampire Vampire commented Mar 14, 2025

No description provided.

Copy link
Member Author

Vampire commented Mar 14, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link

codecov bot commented Mar 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.92%. Comparing base (91ab363) to head (6edf86c).
Report is 1 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2126   +/-   ##
=========================================
  Coverage     81.92%   81.92%           
  Complexity     4728     4728           
=========================================
  Files           463      463           
  Lines         14764    14764           
  Branches       1867     1867           
=========================================
  Hits          12095    12095           
  Misses         1980     1980           
  Partials        689      689           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@leonard84
Copy link
Member

What is the effect of this? Will this now be checked every time it is executed?

Copy link
Member Author

Vampire commented Mar 17, 2025

That's not so much different to before.
And the result is also the same (or the yamls would have changed).

The problem with not using version ranges is, that for Maven (and thus the main.kts script as it uses the same resolution engine) version that do not end in -SNAPSHOT are considered immutable.

So assume you execute the script on your machine today, that packs v4.0.0 into your Maven Local cache as version v4.
Now tomorrow the action adds a new input in version v4.0.1.
I recently wiped my Maven Local cache for whatever reason.
I edit the script and use the new input, because I now get v4.0.1 into my Maven Local cache as version v4.
On GHA this works perfectly fine, as Maven Local is not cached so it gets a fresh v4 which is also a v4.0.1.
But now you try to execute the script and it fails, because in your Maven Local is still the v4 which is the v4.0.0 which misses the input and thus fails to compile the script.

By using the version range the Kotlin script is using the actual latest version so uses the actual v4.0.1 in the Maven Local cache,
but due to the ___major suffix still just writes v4 to the YAML file.
So by using version ranges and the ___major suffix here, we try to avoid issues with stale cache entries (which we already had if you remember).

@Vampire Vampire force-pushed the vampire/use-version-ranges-in-workflow-scripts branch from 1b053b0 to 3ea0cad Compare April 25, 2025 13:42
@Vampire Vampire force-pushed the vampire/use-version-ranges-in-workflow-scripts branch from 3ea0cad to 6edf86c Compare April 25, 2025 23:21
@szpak
Copy link
Member

szpak commented Apr 26, 2025

I understand the problem and I don't have any "better" solution to that, however using version ranges in dependencies generate an additional moving block(s) which might make troubleshooting of accidentally misbehaving job/workflow even harder.

In addition, looking at the tj-actions/changed-files attack, it might be good to use hashes for (especially unofficial) actions.

I wanted to report a feature request to print also a version in YAML when using a dependsOn with a hash, but it has been already reported: typesafegithub/github-workflows-kt#1691

@Vampire
Copy link
Member Author

Vampire commented Apr 26, 2025

You might misunderstand this PR.
We already do use version ranges.
Using v3 for an action is effectively using a version range of "use the latest v3" version for Actions following the convention.
This PR just adjust the bindings used in the .main.kts scripts to follow suit and not risk that I use a "v3" binding where input A is present and you use a "v3" binding where input A is missing and so execution fails for you which worked fine for me.

Whether to instead use a concrete version or pin to a specific hash is a completely different topic.
You can also do that already, either by using the hash as version but then you only get untyped bindings for actions without built-in typing and loose Renovate support, or you can use the _customVersion parameter to the action to print the hash to the YAML, but you probably also loose proper Renovate support as it could still update the dependency version but not the according _customVersion I guess, unless this is configurable somehow.

The issue you linked is - as far as I understand it - more to have it in a way that Renovate can update it.
The version in the output is just a nice to have sub-part.
Actually you could already put the version there by post-processing the generated YAML in the workflow scripts, but you have to do it yourself.

@Vampire
Copy link
Member Author

Vampire commented Apr 26, 2025

This is how you for example could have codecov with hash and version printed right now, but as I said, Renovate would not anymore work like expected with it and it is not really the topic here:

diff --git a/.github/workflows/common.main.kts b/.github/workflows/common.main.kts
index 48eda517c0..7aeaae5b8e 100755
--- a/.github/workflows/common.main.kts
+++ b/.github/workflows/common.main.kts
@@ -19,17 +19,47 @@
 @file:Repository("https://repo.maven.apache.org/maven2/")
 @file:DependsOn("io.github.typesafegithub:github-workflows-kt:3.3.0")

+import io.github.typesafegithub.workflows.domain.Concurrency
 import io.github.typesafegithub.workflows.domain.Job
 import io.github.typesafegithub.workflows.domain.JobOutputs.EMPTY
 import io.github.typesafegithub.workflows.domain.RunnerType
 import io.github.typesafegithub.workflows.domain.actions.Action.Outputs
 import io.github.typesafegithub.workflows.domain.actions.LocalAction
+import io.github.typesafegithub.workflows.domain.triggers.Trigger
 import io.github.typesafegithub.workflows.dsl.JobBuilder
 import io.github.typesafegithub.workflows.dsl.WorkflowBuilder
 import io.github.typesafegithub.workflows.dsl.expressions.Contexts.secrets
 import io.github.typesafegithub.workflows.dsl.expressions.expr
+import io.github.typesafegithub.workflows.dsl.workflow
+import java.io.File
 import java.util.Properties

+fun workflowWithCommentedHashes(
+    name: String,
+    on: List<Trigger>,
+    sourceFile: File? = null,
+    concurrency: Concurrency? = null,
+    block: WorkflowBuilder.() -> Unit,
+) {
+    workflow(
+        name = name,
+        on = on,
+        sourceFile = sourceFile,
+        concurrency = concurrency,
+        block = block,
+    )
+    sourceFile!!
+        .let { it.resolveSibling(it.nameWithoutExtension.substringBeforeLast('.') + ".yaml") }
+        .apply {
+            readText()
+                .replace(
+                    "@ad3126e916f78f00edff4ed0317cf185271ccc2d'",
+                    "@ad3126e916f78f00edff4ed0317cf185271ccc2d' #5.4.2"
+                )
+                .also { writeText(it) }
+        }
+}
+
 val GRADLE_ENTERPRISE_ACCESS_KEY by secrets

 val commonCredentials = mapOf(
diff --git a/.github/workflows/release.main.kts b/.github/workflows/release.main.kts
index 59f1f25895..668936063c 100755
--- a/.github/workflows/release.main.kts
+++ b/.github/workflows/release.main.kts
@@ -24,19 +24,18 @@

 @file:Repository("https://bindings.krzeminski.it/")
 @file:DependsOn("actions:checkout:v4")
-@file:DependsOn("codecov:codecov-action:v5")
+@file:DependsOn("codecov:codecov-action:ad3126e916f78f00edff4ed0317cf185271ccc2d")

 import io.github.typesafegithub.workflows.actions.actions.Checkout
 import io.github.typesafegithub.workflows.actions.actions.Checkout.FetchDepth
-import io.github.typesafegithub.workflows.actions.codecov.CodecovAction
+import io.github.typesafegithub.workflows.actions.codecov.CodecovAction_Untyped
 import io.github.typesafegithub.workflows.domain.RunnerType
 import io.github.typesafegithub.workflows.domain.triggers.Push
 import io.github.typesafegithub.workflows.dsl.expressions.Contexts.github
 import io.github.typesafegithub.workflows.dsl.expressions.Contexts.secrets
 import io.github.typesafegithub.workflows.dsl.expressions.expr
-import io.github.typesafegithub.workflows.dsl.workflow

-workflow(
+workflowWithCommentedHashes(
     name = "Build and Release Spock",
     on = listOf(
         Push(
@@ -97,8 +96,8 @@ workflow(
         )
         uses(
             name = "Upload to Codecov.io",
-            action = CodecovAction(
-                failCiIfError = true
+            action = CodecovAction_Untyped(
+                failCiIfError_Untyped = "true"
             )
         )
     }

@leonard84 leonard84 merged commit f6a2a86 into master Apr 27, 2025
52 checks passed
@leonard84 leonard84 deleted the vampire/use-version-ranges-in-workflow-scripts branch April 27, 2025 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants