Skip to content

feat: support CEL string expressions for custom commit statuses in v1beta3 provider type #1068

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

Conversation

kathleenfrench
Copy link
Contributor

@kathleenfrench kathleenfrench commented Mar 12, 2025

Closes #589

this change was enabled by recent work to enable RFC-0008 which allows for custom metadata to be injected in flux events.

we can leverage the flux runtime pkg cel library's support for compiling and evaluating strings (added in fluxcd/pkg#882) to compute a commit status for git provider types (github, gitlab, gitlea, bitbucketserver, bitbucket, and azuredevops) that can make use of end-user provided event metadata along with standard values accessible to event, provider, and alert.

CEL expresions with this feature can only reference: event, provider, and alert.

to accomplish this, a new optional field is added to the v1beta3 provider spec called commitStatusExpr.

following some additional discussions, it was elected to not re-add the status subresource to the provider to make way for these changes, but instead evaluate the CEL expression from the event server and forward the commit status to the relevant factory, if it exists/is applicable.

as part of this effort a few things to note:

  • the notifier factory constructor has been refactored
  • the ProviderUID notifier option has been removed from git providers in favor of CommitStatus, which is computed prior to creation of the notifier either via the custom expression or using the (status quo) default commit status generator function which combines the involved object kind, name, and provider UID.
  • all git factory function signatures have been altered to sub commitStatus for providerUID -- error handling + tests have been added to ensure the status cannot be empty on creation of the factory, though this is just a failsafe measure and should not happen given if a custom expression is used and fails we drop the alert and if no custom expression is provided we use the pre-existing logic, just evaluated sooner rather than in Post.
  • we run combineEventMetadata() prior to creating the notifier now

so to recap: if the commit status expression fails to compile or evaluate we:

  • log the error removed based on pr feedback
  • emit a warning event removed based on pr feedback
  • return an error
  • drop the alert

if no commit status expression is provided and the provider is a supported git type, we generate a default commit status using event metadata and the provider UID with the same, existing logic just sooner in our handling of the event/alert. if the provider is not a git type, the commit status forwarded to createNotifier is an empty string.

@kathleenfrench kathleenfrench force-pushed the kfrench/provider-commit-expr branch from 46640f1 to 8e37b6a Compare March 12, 2025 19:43
@kathleenfrench kathleenfrench marked this pull request as ready for review March 12, 2025 19:51
Copy link
Member

@matheuscscp matheuscscp left a comment

Choose a reason for hiding this comment

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

Thanks very much for this contribution @kathleenfrench!

First pass 😁 Will do another one tomorrow

@kathleenfrench kathleenfrench force-pushed the kfrench/provider-commit-expr branch from a2136b6 to a37949f Compare March 12, 2025 21:44
@stefanprodan stefanprodan added the area/alerting Alerting related issues and PRs label Mar 12, 2025
@kathleenfrench kathleenfrench force-pushed the kfrench/provider-commit-expr branch from 4eb9403 to 02be262 Compare March 12, 2025 22:10
Copy link
Member

@matheuscscp matheuscscp left a comment

Choose a reason for hiding this comment

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

Amazing work here @kathleenfrench!

@kathleenfrench kathleenfrench force-pushed the kfrench/provider-commit-expr branch 3 times, most recently from 0fb250c to d970b88 Compare March 13, 2025 21:15
…3 provider types

Signed-off-by: kathleen french <kfrench@groq.com>
@kathleenfrench kathleenfrench force-pushed the kfrench/provider-commit-expr branch from cb1c0a7 to 1967bc0 Compare March 14, 2025 12:54
Copy link
Member

@matheuscscp matheuscscp left a comment

Choose a reason for hiding this comment

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

LGTM!

Thanks very much for this contribution @kathleenfrench! 🚀

@matheuscscp
Copy link
Member

Btw I tested this feature in a real cluster and it works:

Screenshot from 2025-03-18 19-06-11

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @kathleenfrench 🥇

@matheuscscp matheuscscp merged commit e2eac40 into fluxcd:main Mar 18, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/alerting Alerting related issues and PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Alerts/Github - is it possible to separate add additional metadata?
3 participants