Skip to content

fix(policy-controller): Define failurePolicy variable for cosign webhook - #1206

Open
ttryf wants to merge 1 commit into
sigstore:mainfrom
ttryf:fix/policy-controller-webook-config
Open

fix(policy-controller): Define failurePolicy variable for cosign webhook#1206
ttryf wants to merge 1 commit into
sigstore:mainfrom
ttryf:fix/policy-controller-webook-config

Conversation

@ttryf

@ttryf ttryf commented Jul 31, 2026

Copy link
Copy Markdown

Description of the change

Introduce failurePolicy variable for the cosign webhook to parameterise the default action (from the hardcoded Fail value).

Existing or Associated Issue(s)

Relates to #1182 issue

Additional Information

Checklist

  • Chart version bumped in Chart.yaml according to semver. Where applicable, update and bump the versions in any associated umbrella chart
  • Variables are documented in the values.yaml and added to the README.md. The helm-docs utility can be used to generate the necessary content. Use helm-docs --dry-run to preview the content.
  • JSON Schema generated.
  • List tests pass for Chart using the Chart Testing tool and the ct lint command.

Introduce `failurePolicy` variable for the cosign webhook to parameterise the default action (from the hardcoded `Fail` value).

Fixes sigstore#1182

Signed-off-by: Thanos Tryfonidis <thanos.tryfonidis@allwyn-lotterysolutions.com>
@ttryf
ttryf requested review from a team as code owners July 31, 2026 10:37

@cmurphy cmurphy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Based on #553 it seems like there was an intent at some point to unify these two webhooks and not have separate configuration for them. Could the new cosign.failurePolicy be removed and could the other webhoook just use webhook.failurePolicy?


name: policy-controller
version: 0.10.6
version: 0.10.7

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you bump this to 0.11.0 since this is adding a feature?

@ttryf

ttryf commented Aug 3, 2026

Copy link
Copy Markdown
Author

Based on #553 it seems like there was an intent at some point to unify these two webhooks and not have separate configuration for them. Could the new cosign.failurePolicy be removed and could the other webhoook just use webhook.failurePolicy?

Thanks for the context @cmurphy, this is perfectly fine and it was my initial implementation (before submitting this PR).

One thing I noticed while working on this and calling it out for visibility (my rationale for separate configuration for failurePolicy):

The webhook is used to make calls to Fulcio/Rekor/TUF etc. and could fail independently of pod health, compared to ClusterImagePolicy webhook that does local validation - no dependencies. Keeping them separately configurable, reduces the blast radius (during an outage or service unavailability) of each functionality. Not blocking this PR either way, just flagging in case it changes your view on #553.

This is based on the three failure scenarios I considered:

  1. Pod is fully down/unavailable: both cases fail regardless of the defined failurePolicy value.
  2. Pod is healthy, external 3rd party verification is degraded - cosign webhook fails per request while ClusterImagePolicy (CIP) remains unaffected. This is the case where the asymmetry adds value with the Ignore case to mitigate a failure that is out of someone's control and it doesn't relate with the correctness of the policy object I'm admitting in the cluster.
  3. An object fails admission and gets Ignored/Failed based on the CIP configuration.

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.

2 participants