Skip to content

Forwards compatible test changes for framework authorization [RUN AS-IF-FOSS]

Alex Kalderimis requested to merge ajk-13984-resolver-test-changes into master

What does this MR do?

This MR includes non-observable changes to resolvers to prepare them for upcoming changes to the GraphQL authorization framework.

Background

This is part of drive to adopt the authorization of the GraphQL framework rather than our own solution. We have a working (green-pipeline) version of this in !40088 (merged) for #13984 (closed) but it is too large to merge as is.

The changes to authorization have the effect that:

  • rather than passing in arguments with ruby-land keys and the wide array of types that ruby will accept, the arguments will need to be using graphql-land keys and using the specific types advertised on the fields. This is explained in a couple of notes.
  • rather than receiving the raw value from the resolvers (generally relations) we will receive the fully processed values (generally lazy promises for connections). In !47967 (merged) we added compatibility methods so that relations share some methods, allowing us to, for example call expect(thing).to include(x) on both a relation and connection of that relation.

Our resolver specs are generally written in a way that expects the relation, and they could be more abstract. This MR is mainly about adding the necessary to_a and contains_exactly calls so that the tests that work today will keep working when we change the test resolution method.

The rationale for this is that it is much safer to change the tests first and then the code, since this proves that the (new) tests work under the existing implementation, and that when we do change the code, our specs will be guarantees that the refactoring does not change behaviour.

There are a couple of drive-by fixes (simplifications of resolvers to use authorize for example) and a couple of cases where we change the behaviour or resolvers from raising errors to returning nil (the correct behaviour), but mainly this is about preparing the ground for when all specs suddenly start returning connections.

Does this MR meet the acceptance criteria?

Conformity

As pure backend testing changes, this does not have user-facing effects. No changelog is required.

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Alex Kalderimis

Merge request reports