Bound a policy's authored text before compiling its scope

Marcos Rocharequested to merge
mc_rocha-policy-store-split-text-limit-validation into master

What does this MR do and why?

Splits Gitlab::PolicyStore::Ports::PolicyRepository#validate_text_limits! into validate_authored_text_limits! and validate_compiled_text_limits!, and has InMemoryPolicyRepository#create call one on each side of with_compiled_scope.

A single call could not serve both sides. scope_rego does not exist until the transpiler runs, so bounding it has to happen after compilation, while name and description arrive from the caller and can be bounded before. The transpiler also embeds the policy name in its output, so checking the authored limits first means an over-limit name is rejected without first compiling a program that is about to be discarded.

COMPILED_TEXT_ATTRIBUTES names the one attribute on the far side of that step. It is private_constant because it is a detail of how the two halves divide TEXT_LIMITS rather than part of the port's contract.

Only one of the three new examples guards the change: rejects an over-limit name before the scope reaches the transpiler fails against master, because nothing stopped the transpiler running first. The other two pin behaviour the split must not alter, and pass either way by design.

Design decisions

No error message changes, deliberately. TEXT_LIMITS already iterated name ahead of scope_rego, so a policy with both an over-limit name and an over-limit compiled scope reported the name first before this change and still does. That makes the split invisible through the public API, which is why the spec asserts the transpiler is never instantiated rather than asserting on a message.

How to set up and validate locally

Every step runs on the rails console. The split changes when each limit is checked rather than which policies are valid, so the observable difference is whether the transpiler runs at all. Step 1 makes that visible.

  1. Announce every call to the transpiler, so the later steps can show whether it ran
Gitlab::PolicyStore::ScopeTranspiler.singleton_class.prepend(Module.new do
  def new(...)
    puts "ScopeTranspiler ran"
    super
  end
end)
  1. Create a policy whose name is over the limit and whose scope would compile. Verify it prints the message alone, with no ScopeTranspiler ran above it, because the authored limits are checked before compilation. On master the same call prints ScopeTranspiler ran first, having compiled a program it then discards
begin
  Gitlab::PolicyStore.create(
    organization_id: 1,
    name: "a" * 256,
    trigger_type: "deployment_requested",
    policy_scope: { "compliance_frameworks" => [{ "id" => 5 }] }
  )
rescue Gitlab::PolicyStore::ValidationError => error
  puts error.message
end
# => name exceeds maximum length of 255 characters
  1. Confirm the compiled half still runs, by authoring a scope that compiles past the scope_rego limit. Verify it prints ScopeTranspiler ran and then the message, because scope_rego is bounded after the transpiler rather than skipped
limit = Gitlab::PolicyStore::Ports::PolicyRepository::TEXT_LIMITS[:scope_rego]

begin
  Gitlab::PolicyStore.create(
    organization_id: 1, name: "Wide scope", trigger_type: "deployment_requested",
    policy_scope: { "projects" => { "including" => (1..limit).map { |id| { "id" => id } } } }
  )
rescue Gitlab::PolicyStore::ValidationError => error
  puts error.message
end
# => scope_rego exceeds maximum length of 4096 characters
  1. Confirm the opposite path still saves. Create a policy within both limits and verify it returns a compiled scope_rego, because nothing about which policies are valid has changed
Gitlab::PolicyStore.create(
  organization_id: 1, name: "Framework 5 only", trigger_type: "deployment_requested",
  policy_scope: { "compliance_frameworks" => [{ "id" => 5 }] }
).scope_rego.lines.first
# => "package gitlab.scope\n"

References

Merge request reports