Skip to content

updated notes about approval policies and fixed links #793

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Daniellem97
Copy link
Contributor

@Daniellem97 Daniellem97 commented May 14, 2025

Description of the change

Checklist

Please make sure that the proposed change checks all the boxes below before requesting a review:

  • I have reviewed the guidelines for contributing to this repository.
  • The preview looks fine.
  • The tests pass.
  • The commit history is clean and meaningful.
  • The pull request is opened against the main branch.
  • The pull request is no longer marked as a draft.
  • You agree to license your contribution under the MIT license to Spacelift (not required for Spacelift employees).
  • You have updated the navigation files correctly:
    • No new pages have been added, or;
    • Only nav.yaml has been updated because the changes only apply to SaaS, or;
    • Only nav.self-hosted.yaml has been updated because the changes only apply to Self-Hosted, or;
    • Both nav.yaml and nav.self-hosted.yaml have been updated.

If the proposed change is ready to be merged, please request a review from @spacelift-io/solutions-engineering. Someone will review and merge the pull request.

Spacelift employees should request reviews from the relevant engineers and are allowed to merge pull requests after they got at least one approval.

Thank you for your contribution! 🙇

@@ -8,8 +8,8 @@
## Introduction

!!! info
Please note, we currently don't support importing rego.v1

Please note, we currently don't support importing rego.v1. This is because Spacelift combines its base policies with user-defined policies and evaluates them together using the Rego engine. However, enabling rego.v1 in this setup causes compatibility issues with our base policies, which may result in your policies not working as expected.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be in its own PR, as it seems unrelated?


The approval policy allows organizations to create sophisticated run review and approval flows that reflect their preferred workflow, security goals, and business objectives. Without an explicit approval policy, anyone with write access to a stack can create a [run](../run/README.md) (or a [task](../run/task.md)). An approval policy can make this way more granular and contextual.
The approval policy allows organizations to create sophisticated run review and approval flows that reflect their preferred workflow, security goals, and business objectives. Without an explicit approval policy, anyone with write access to a stack can create a [run](../run/README.md) (or a [task](../run/task.md)). An approval policy can make this process more granular and contextual.
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand "way" as in "significantly more" here, so I am not sure it is worth changing but I am fine if you want to change it.


![](<../../assets/screenshots/Mouse_Highlight_Overlay_and_Resource_in_a_separate_file_·_Bacon (3).png>)

In principle, the run review and approval process are very similar to GitHub's Pull Request review, the only exception being that it's the Rego policy (rather than a set of checkboxes and dropdowns) that defines the exact conditions to approve the run.
In principle, the run review and approval process is very similar to GitHub's Pull Request review, the only exception being that it's the Rego policy (rather than a set of checkboxes and dropdowns) that defines the exact conditions to approve the run.
Copy link
Contributor

Choose a reason for hiding this comment

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

"are" sounds better to me, but I am not a native English speaker. 😅


Runs can be reviewed when they enter one of the three states - [queued](../run/README.md#queued), [unconfirmed](../run/tracked.md#unconfirmed), or [pending review](../run/proposed.md#unconfirmed). When a [queued](../run/README.md#queued) run needs approval, it will not be scheduled before that approval is received, and if it is of a blocking type, it will block newer runs from scheduling, too. A [queued](../run/README.md#queued) run that's pending approval can be [canceled](../run/README.md#canceled) at any point.
Runs can be reviewed when they enter one of three states - [queued](../run/README.md#queued), [unconfirmed](../run/tracked.md#unconfirmed), or [pending review](../run/proposed.md#pending-review). Please note if a stack has [auto-deploy](../stack/stack-settings.md#autodeploy) enabled, then the approval policy will not be evaluated here, and you should use a [plan policy](../policy/terraform-plan-policy.md) to warn which will force the stack into an unconfirmed state and the approval policy will get evaluated as a safeguard.
Copy link
Contributor

Choose a reason for hiding this comment

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

Autodeploy is spelled with the dash elsewhere in the documentation. I like the version with the dash better, but I like consistency even more. 😄


While the 'approve' rule must be defined in order for the run to be able to progress, it's perfectly valid to not define the 'reject' rule. In that case, runs that look invalid can be cleaned up ([canceled](../run/README.md#canceled) or [discarded](../run/tracked.md#discarded)) manually.

It's also perfectly acceptable for any given policy evaluation to return 'false' on both 'approve' and 'reject' rules. This only means that the result is yet 'undecided' and more reviews will be necessary to reach the conclusion. A perfect example would be a policy that requires 2 approvals for a given job - the first review is not yet supposed to set the 'approve' value to 'true'.
It's also perfectly acceptable for any given policy evaluation to return 'false' on both 'approve' and 'reject' rules. This only means that the result is yet 'undecided,' and more reviews will be necessary to reach a conclusion. A perfect example would be a policy that requires 2 approvals for a given job - the first review is not yet supposed to set the 'approve' value to 'true.'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
It's also perfectly acceptable for any given policy evaluation to return 'false' on both 'approve' and 'reject' rules. This only means that the result is yet 'undecided,' and more reviews will be necessary to reach a conclusion. A perfect example would be a policy that requires 2 approvals for a given job - the first review is not yet supposed to set the 'approve' value to 'true.'
It's also perfectly acceptable for any given policy evaluation to return 'false' on both 'approve' and 'reject' rules. This only means that the result is yet 'undecided', and more reviews will be necessary to reach a conclusion. A perfect example would be a policy that requires 2 approvals for a given job - the first review is not yet supposed to set the 'approve' value to 'true'.

@@ -1,11 +1,13 @@
# Approval policy
# Approval Policy
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the title capitalization, but for the sake of consistency, I would recommend that we stick to the current convention for the documentation (ie Only the first word gets a capital).

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