Filter policy store list by trigger type

Marcos Rocharequested to merge
mc_rocha-policy-store-trigger-filter into master

What does this MR do and why?

Adds an optional trigger_type filter to GET /organizations/:id/security/policy_store. Requested by @arfedoro on !249148 (merged): glaz fetches the organization's policies to evaluate rules against (glaz!133) and wants only the policies for the trigger it is evaluating, rather than fetching everything and filtering client side.

The change runs through three layers and is additive at each. Gitlab::PolicyStore.list, Ports::PolicyRepository#list, and InMemoryPolicyRepository#list gain a trigger_type: keyword defaulting to nil, so every existing caller keeps returning the whole organization and no other call site moves. ListService takes the same keyword and passes it through. The route declares it with values: drawn from the trigger catalogue.

It also carries a follow-up to a suggestion @minac left on !249148 (merged): the fallback branch of render_policy_store_error! returned a 4XX for a reason the endpoint never anticipated, which is our bug rather than the caller's. It now returns 500 and tracks an UnmappedReasonError. The response body does not change.

Design decisions

The parameter is trigger_type, not trigger_id. It matches Gitlab::PolicyStore::Policy#trigger_type and the field API::Entities::Govern::Policy exposes, so the filter names the same thing the response does. GET /security/policy_store/triggers returns those same values under id, because id is that catalogue's own key, so the two line up despite the different names.

An unknown trigger is a 400, not an empty list. values: constrains the parameter to Gitlab::PolicyStore::Triggers::ALL, so a typo fails loudly instead of looking like an organization with no matching policies. The catalogue holds only deployment_requested today, so it is also the only value the route accepts.

An unanticipated reason is tracked, not just relabelled. Grape's error! returns a response rather than raising, so changing the status code alone would leave the branch invisible: nothing reaches an exception tracker and nothing pages. The exception's message is the unmapped reason rather than the service's own text, because the fix is always to map that reason, and one bug should not fragment into one tracked issue per message the service happened to fail with. The service text rides along as an extra, so it reaches the tracker rather than the caller and the generic body still holds.

The store is not constrained the same way. Gitlab::PolicyStore validates presence rather than membership, so it can hold a policy whose trigger_type is outside the catalogue, which is what the request spec seeds to prove the filter excludes rather than merely returns everything.

Omitting the filter returns every trigger, and the port contract now asserts it. The existing #list examples all seeded a single trigger, so nothing would have caught a nil that silently meant one specific trigger. The shared example added here lists two policies with different triggers and no filter.

Neither error branch is reachable through this route today, so both are spec-only. ListService#execute returns either success or :experiment_not_active; :invalid comes from CreateService and :not_found from BaseService, and neither has a route yet. The helper is shared with the show, create, and delete routes later in the stack, which is why it maps reasons it cannot currently receive. Verifying the 500 means stubbing the service, which the request spec does, rather than a curl.

How to set up and validate locally

Requires an Ultimate licence.

Gitlab::PolicyStore is backed by an in-memory repository built lazily per process, and nothing configures it at boot, so a policy created on the rails console is invisible to the process serving the request. Seeding therefore happens in an initializer, which runs in every process including Puma. The ActiveRecord-backed repository in https://gitlab.com/gitlab-org/gitlab/-/work_items/606969 removes the need for this.

  1. Enable the security_policies_v2 feature flag on the rails console
Feature.enable(:security_policies_v2)
  1. As an administrator, go to Admin > Settings > Security and compliance and turn on the policy store experiment
  2. On the rails console, confirm the gate is satisfied, pick an organization you own, and note your GDK's base URL
organization = Organizations::Organization.find(1)
organization.policy_store_experiment_active?   # => true

puts Gitlab.config.gitlab.url   # for example https://gdk.test:3443
  1. Make yourself an owner of that organization, if you are not already
user = User.find_by_username('root')
Organizations::OrganizationUser.find_or_create_by!(organization: organization, user: user) do |organization_user|
  organization_user.access_level = Gitlab::Access::OWNER
end
  1. Seed two policies with different triggers into the process that serves the request, by adding config/initializers/zz_temporary_policy_store_seed.rb, then running gdk restart rails-web. Delete the file when you are done
# frozen_string_literal: true

Rails.application.config.after_initialize do
  next unless Rails.env.development?

  Gitlab::PolicyStore.create(
    organization_id: 1,
    name: 'Block deployments on critical findings',
    trigger_type: 'deployment_requested'
  )

  Gitlab::PolicyStore.create(
    organization_id: 1,
    name: 'Require approval on merge requests',
    trigger_type: 'merge_request'
  )
end
  1. Create a personal access token for yourself: select your avatar in the upper-right corner, select Edit profile, then in the left sidebar select Access > Personal access tokens
  2. From the Generate token dropdown list select Legacy token, enter a Token name, leave Expiration date empty, select the api scope, then select Generate token. Copy the token, since it is shown only once
  3. List the policies without a filter, against the base URL from step 3, and verify both come back
curl --header "PRIVATE-TOKEN: <your token>" \
  --url "<your GDK URL>/api/v4/organizations/1/security/policy_store"
  1. Repeat with the filter, and verify only the deployment policy comes back, which is the behaviour this MR adds
curl --header "PRIVATE-TOKEN: <your token>" \
  --url "<your GDK URL>/api/v4/organizations/1/security/policy_store?trigger_type=deployment_requested"
  1. Pass a trigger outside the catalogue and verify it returns 400, so a typo fails loudly rather than looking like an organization with no matching policies
curl --header "PRIVATE-TOKEN: <your token>" \
  --url "<your GDK URL>/api/v4/organizations/1/security/policy_store?trigger_type=nonsense"

References

Edited by Marcos Rocha

Merge request reports