search

jreleaser / release-action

:octocat: GitHub Action for JReleaser
https://jreleaser.org
Apache License 2.0
34 stars 3 forks source link

Add input type validation #16

Closed desiderantes closed 3 weeks ago

desiderantes commented 1 month ago

This change add a simple validation for input types, fixes the doc for one of the inputs, and allows projects using typesafe github actions to use this action.

aalmiray commented 3 weeks ago

I'm still unsure how the typesafegithub/github-actions-typing action is supposed to work. Is it supposed to be configured only at this repository and will be automatically involved whenever someone consumes jreleaser/release-action? If so I'm not keen in running arbitrary Kotlin code on consumer's repositories.

desiderantes commented 3 weeks ago

I'm still unsure how the typesafegithub/github-actions-typing action is supposed to work. Is it supposed to be configured only at this repository and will be automatically involved whenever someone consumes jreleaser/release-action? If so I'm not keen in running arbitrary Kotlin code on consumer's repositories.

It is used by custom maven repo proxies to generate bindings on the fly, so no, it is not run on any consumer.

In a kotlin script, you add a maven repo from where you resolve actions as if they were packages, and the repo will try to generate bindings if missing, by using the metadata here.

aalmiray commented 3 weeks ago

This makes even less likely to apply the PR at this moment.

Vampire commented 3 weeks ago

The action types YAML is simply an emerging standard to clearly document the types of inputs and outputs of GitHub actions in a standardized way that can be consumed by other projects.

Normal users of your action are not affected in any significant way, except that they can manually read this type definition.

One of the consumers of these typings is the project https://github.com/typesafegithub/github-workflows-kt, a Kotlin DSL to write GitHub Action workflows. It uses the typings to generate more type-safe binding classes and thus makes using such actions more convenient.

The (optional) action just validates that the typings file is in the proper format and consistent with the action YAML.

With having such typings, you action would also be in good company, like e.g. my https://github.com/Vampire/setup-wsl or also Microsoft's https://github.com/microsoft/setup-msbuild.

Also "regular" users can benefit from this clear and formalized typing definition, seeing exactly what values are valid. And as the typings are made independent from the Kotlin DSL library, also other DSL or similar consumers could use these typings in the future.

When accepting this PR and in the future maintaining the typing yourself, please keep in mind that API are generated from these typing information. So if you for example recognize that a type is wrong and want to change it for example from string to integer, this would be a breaking change for such consumers and should therefore be done with a major version bump. Backwards compatible changes like new enum options, or new inputs or outputs can of course be released in a minor version as usual.

aalmiray commented 3 weeks ago

If it's an emerging standard I'll wait for an official announcement from GitHub as well as an action provided by GitHub to validate the types.

Software supply chain attacks are on the rise. This feature is closer to the infrastructure used to run actions and as such should be provided by the owners/makers and not a 3rd party IMHO.

Vampire commented 3 weeks ago

I did not say it is a standard driven by GitHub. 🙈 It would be nice if GitHub would define a standard, but they did not do so far and did not commit to do it. This is a community-driven standard to define the types for inputs and outputs, so that programmatic consumers like for example code-generators for DSLs to conveniently define workflows like the mentioned Kotlin DSL have a reliable source of type information that they can process.

Vampire commented 3 weeks ago

I'm actually not sure what you mean with supply-chain attacks in this context. If it is the validation action you are concerned about, as I said it is fully optional. It is just handy as it validates that the file is syntactically correct and in-line with the action YAML. But you can as well not have it and the only thing you have is the additional YAML file that declares the types of the inputs and outputs.

aalmiray commented 3 weeks ago

If it's not a standard driven by GitHub then it makes no sense to apply this PR at this moment for the security and provenance reasons mentioned before. I'll wait for GitHub to provide such feature, if ever.

@desiderantes thank you for taking the time to send this PR but I'll close without merging for now.

Vampire commented 3 weeks ago

Please don't get me wrong, it is fully ok not to merge it, thus I'm not trying to convince you from anything.

But I'm really just curious which "security and provenance reasons" you see not to add one YAML file where you document types in a defined way. Maybe there is something I'm just not aware of and should myself stop providing this text file for my action.