Add update to the policy store gem

Marcos Rocharequested to merge
604367-policy-store-update-and-evaluation into master

What does this MR do and why?

Adds update(id, attributes) to Gitlab::PolicyStore::Ports::PolicyRepository, the facade, and Adapters::InMemoryPolicyRepository, so the persistence-backed adapter in a follow-up MR has a contract to implement.

Most of the change reconciles a policy's two scope forms: structured data in policy_scope, which the gem compiles to Rego, and Rego authored directly in scope_rego. Four new private helpers on the port, with_updated_scope, recompile_scope?, generated_scope_rego?, and compiled_scope_rego, decide what an update does:

  • Supplying a value for one form clears the other.
  • A rename regenerates a compiled program, since the transpiler emits the policy name into it, but leaves an authored one alone.
  • Blanking scope_rego recompiles from policy_scope, or widens the policy to every project when there is none.
  • Blanking policy_scope widens a policy compiled from it, and changes nothing for one authored as Rego.
  • Values a change set merely restates are dropped before any of the above is decided, so resending a whole policy neither discards an authored program nor rewrites a compiled one.

An update supplying nothing that differs from the stored policy returns it untouched, leaving version alone, so version counts writes rather than calls.

create and update now validate the same post-compilation state, and both reject an attribute they do not know rather than dropping it. update tolerates a resent immutable value but rejects one that differs from what is stored, since ignoring that reports a move it will not make. version is exempt: comparing it would be optimistic locking reported as a validation error.

The in-memory adapter enforces what a persistence adapter gets from its schema, so a service unit-tested against the default repository cannot pass and then fail in production: unique name per organization, no explicit nil for rules, actions, mode, or lifecycle_state, a type check per scope form, whitespace as blank, version starting at 1 whatever the caller sends, and copies rather than references on every read.

How to set up and validate locally

No licence tier, feature flag, or seed data. The facade defaults to the in-memory adapter, so nothing touches the database and organization_id: 1 need not be a real organization. The store is per-process, so run everything in one gdk rails console session.

  1. Rename a compiled-scope policy, then resend the whole policy under another name
scoped = Gitlab::PolicyStore.create(
  organization_id: 1, name: "Original name", trigger_type: "deployment_requested",
  policy_scope: { compliance_frameworks: [{ id: 5 }] })

renamed = Gitlab::PolicyStore.update(scoped.id, name: "Renamed policy")
renamed.version                                # => 2
renamed.scope_rego.include?("Renamed policy")  # => true
renamed.scope_rego.include?("Original name")   # => false

resent = Gitlab::PolicyStore.update(scoped.id, renamed.to_h.merge(name: "Resent name"))
resent.policy_scope                            # => the compliance_frameworks scope, still there

Verify the program follows the name, because the engine evaluates that text, and that policy_scope survived the resend, because the resent scope_rego is a restated value rather than a newly authored program.

  1. Blank each scope form, on a compiled policy and an authored one
authored = Gitlab::PolicyStore.create(
  organization_id: 1, name: "Authored", trigger_type: "deployment_requested",
  scope_rego: "package gitlab.scope\n\n# hand written")

Gitlab::PolicyStore.update(authored.id, name: "Renamed").scope_rego     # unchanged
Gitlab::PolicyStore.update(scoped.id, scope_rego: nil).scope_rego       # recompiled, framework_id in {5}
Gitlab::PolicyStore.update(authored.id, scope_rego: nil).scope_rego     # applies to all projects

Verify an authored program survives a rename, that blanking the form a policy was compiled from recompiles it, and that blanking the only form a policy has widens it. The last is the one destructive case, and the only one.

  1. Verify the guards, each of which used to be a silent success
Gitlab::PolicyStore.update(-1, name: "Nope")            # => NotFound
Gitlab::PolicyStore.update(scoped.id, name: "   ")      # => ValidationError: Missing required attributes: name
Gitlab::PolicyStore.update(scoped.id, rules: nil)       # => ValidationError: Attributes cannot be null: rules
Gitlab::PolicyStore.update(scoped.id, nmae: "Typo")     # => ValidationError: Unknown attributes: nmae
Gitlab::PolicyStore.update(scoped.id, scope_rego: 42)   # => ValidationError: scope_rego must be a string
Gitlab::PolicyStore.update(scoped.id, organization_id: 999)
# => ValidationError: Attributes cannot be changed: organization_id

Verify the type check raises rather than storing 42 as the policy's program, because that would discard the policy_scope it was compiled from and leave no way back. Then re-run the last line with organization_id: 1 and confirm it succeeds, because a resent value is the case the tolerance exists for.

References

Edited by Marcos Rocha

Merge request reports