Skip to content

feat(contracts): move attestation phases policy references#2802

Closed
Piskoo wants to merge 4 commits intochainloop-dev:mainfrom
Piskoo:pfm-4736
Closed

feat(contracts): move attestation phases policy references#2802
Piskoo wants to merge 4 commits intochainloop-dev:mainfrom
Piskoo:pfm-4736

Conversation

@Piskoo
Copy link
Collaborator

@Piskoo Piskoo commented Mar 2, 2026

The attestation_phases field was incorrectly placed on PolicySpecV2 (the policy definition) instead of PolicyAttachment (the contract attachment). This moves it to PolicyAttachment so users can control at which lifecycle phase (INIT/PUSH) a policy is evaluated directly in the contract. attestation_phases in PolicySpecV2 was marked as depracted and acts as a fallback logic now.

Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
// Controls at which attestation phases this policy is evaluated.
// Empty means evaluate at all phases (INIT and PUSH) for backwards compatibility.
// Only applicable when kind is ATTESTATION.
repeated AttestationPhase attestation_phases = 5;
Copy link
Member

Choose a reason for hiding this comment

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

I don't believe this is being used anywhere. But better mark it as "deprecated" and we can remove it later on.

Copy link
Member

Choose a reason for hiding this comment

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

Or maybe we can have both behaviours (apply the one contract, and if not present, use the defaults in the policy if they exist)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added it back as fallback for now, but it's marked as depracted

Piskoo added 3 commits March 2, 2026 13:46
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
@Piskoo Piskoo marked this pull request as ready for review March 2, 2026 13:10
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 10 files

Requires human review: This PR modifies core policy evaluation logic and updates the workflow contract API (Protobuf/JSON schemas). These changes to the policy engine require human review.

Copy link
Member

@migmartri migmartri left a comment

Choose a reason for hiding this comment

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

Wait. That this feature is meant to be in the policy itself not in the contract. I am wondering if we want to support both

@migmartri
Copy link
Member

Wait. That this feature is meant to be in the policy itself not in the contract. I am wondering if we want to support both

https://docs.chainloop.dev/concepts/policies#configuring-phases-in-a-policy-spec

@jiparis
Copy link
Member

jiparis commented Mar 2, 2026

Wait. That this feature is meant to be in the policy itself not in the contract. I am wondering if we want to support both

https://docs.chainloop.dev/concepts/policies#configuring-phases-in-a-policy-spec

Having it in the policy spec makes it less reusable. I wonder then why this change was applied 13386c8

@jiparis
Copy link
Member

jiparis commented Mar 2, 2026

Wait. That this feature is meant to be in the policy itself not in the contract. I am wondering if we want to support both

We only have one use case for the release gate, and it should be placed in the contract, since it's using a policy that's also useful in different contexts.

@jiparis
Copy link
Member

jiparis commented Mar 2, 2026

@Piskoo We're going to close this issue. The former issue was a policy being executed with wrong data. Instead of skipping the policy, we are fixing the workflow to ensure the data is consistent.
Check #2807 for the actual fix.

We can come back to this PR if/when we need an actual override at the contract level for policy phases.

@jiparis jiparis closed this Mar 2, 2026
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