Add the policy store update service

Marcos Rocharequested to merge
mc_rocha-policy-store-update-service into master

What does this MR do and why?

Adds UpdateService, which changes a policy through the store's update operation added in !249133 (merged). The PATCH endpoint follows in its own MR, so this one has no route and no permission.

Design decisions

Params reach the store uncompacted, unlike CreateService. The store reads key presence to decide whether to recompile a scope, so dropping a blank scope_rego would silently lose the caller's request to retire an authored program and fall back to policy_scope. A spec pins it: adding .compact to the service fails one example and no other.

The spec covers both blank forms, since a form-encoded body sends the empty string and a JSON one sends null. It also covers the blank value arriving on its own, not just alongside a policy_scope: with a replacement scope present the store recompiles either way, so only the lone case actually distinguishes the two behaviours. Adding .compact to the service fails that example and no other.

A policy owned by another organization returns the same not-found error as a missing one. The lookup goes through BaseService#find_policy, so an id cannot be used to change a policy across organizations, matching what show and delete already guarantee. Gitlab::PolicyStore::NotFound is still rescued around the write, because the lookup and the update are not atomic.

The conflicting-scope rule and params both move to BaseService. The rule now has two callers, and holding params in the base class is what lets it read them directly. It also gives the key form one place to be settled. The store accepts symbols or strings, so a string-keyed hash used to read as an absent policy_scope, skipping the conflict check and letting the store discard the scope in favour of the Rego instead of reporting the conflict. CreateService read every attribute by symbol and had the same gap, so both are now covered by symbolize_keys in one initializer. A blank scope_rego is deliberately not a conflict: present? is false for it, which is exactly what makes retiring an authored program work.

How to set up and validate locally

Requires an Ultimate licence. The in-memory store is per-process, so every call below must reach the same Puma worker or console session.

  1. Enable the feature flag and the instance setting, then confirm the whole gate is open
Feature.enable(:security_policies_v2)
ApplicationSetting.current.update!(policy_store_experiment_enabled: true)

organization = Organizations::Organization.find(Organizations::Organization::DEFAULT_ORGANIZATION_ID)
organization.policy_store_experiment_active?   # => true

That one call ANDs the feature flag, the instance setting and the licence. If it returns false, every step below reports :experiment_not_active without saying which of the three closed.

  1. On the rails console, create a policy and change its name through the service
policy = Gitlab::PolicyStore.create(
  organization_id: organization.id,
  name: 'Framework 5 only',
  trigger_type: 'deployment_requested',
  policy_scope: { compliance_frameworks: [{ id: 5 }] }
)

def rename(organization, policy_id, name)
  Security::SecurityOrchestrationPolicies::PolicyStore::UpdateService.new(
    organization: organization, policy_id: policy_id, params: { name: name }
  ).execute
end

result = rename(organization, policy.id, 'Renamed policy')
puts(result.success? ? [result.payload[:policy].name, result.payload[:policy].version].inspect : result.message)

  1. Verify the name changed and the version is 2, and that scope_rego now carries the new name, because the transpiler emits it into the compiled program

  2. Retire the compiled program by sending scope_rego as nil on its own, the shape a JSON body sends, and verify it recompiles from policy_scope rather than clearing the scope. This is the case the uncompacted pass-through exists for, since .compact would drop the key before the store could read it

result = Security::SecurityOrchestrationPolicies::PolicyStore::UpdateService.new(
  organization: organization,
  policy_id: policy.id,
  params: { scope_rego: nil }
).execute

puts(result.success? ? result.payload[:policy].scope_rego : result.message)
# still includes "framework_id in {5}", not blank
  1. Try to change a policy that belongs to another organization, and verify it reports not found rather than changing it
other_organization = Organizations::Organization.create!(name: 'Other', path: 'other-org')
other_policy = Gitlab::PolicyStore.create(
  organization_id: other_organization.id,
  name: 'Other organization policy',
  trigger_type: 'deployment_requested'
)

result = Security::SecurityOrchestrationPolicies::PolicyStore::UpdateService.new(
  organization: organization,
  policy_id: other_policy.id,
  params: { name: 'Should not apply' }
).execute

puts [result.reason, result.message].inspect   # => [:not_found, "Policy was not found"]
puts Gitlab::PolicyStore.find(other_policy.id).name   # => "Other organization policy"
  1. Send both a policy_scope and a scope_rego and verify it reports :invalid with Only one of policy_scope or scope_rego can be provided
  2. Turn the instance setting from step 1 back off, then re-run only the update, not the create, since the policy name is taken by now
ApplicationSetting.current.update!(policy_store_experiment_enabled: false)

result = rename(organization, policy.id, 'Should not apply')
puts [result.reason, result.message].inspect
# => [:experiment_not_active, "Policy Store experiment is not active for this organization"]
puts Gitlab::PolicyStore.find(policy.id).name   # => "Renamed policy", untouched

References

Edited by Marcos Rocha

Merge request reports