Add GraphQL verification tooling

This adds query validation for GraphQL, exposed in
a new raketask (`gitlab:graphql:validate`).

Tests are added for the validation system, which handles common patterns
in our front-end code (such as Apollo client directives, and the use of
ee_else_ce).

The new graphql files used in the tests are excluded from prettier
formatting.

A couple of clearly incorrect graphql files (discovered during testing)
have been fixed. One remaining one has been marked as a known failure.

app/assets/javascripts/repository/queries/project_path.query.graphql

 query getProjectPath {
-  projectPath
+  projectPath @client
 }

config/known_invalid_graphql_queries.yml

+---
+filenames:
+  - ee/app/assets/javascripts/security_configuration/dast_profiles/graphql/dast_site_profiles_extended.query.graphql

ee/app/assets/javascripts/security_dashboard/graphql/group_specific_scanners.query.graphql

-#import "./vulnerablity_scanner.fragment.graphql"
+#import "./vulnerability_scanner.fragment.graphql"
 
 query groupSpecificScanners($fullPath: ID!) {
   group(fullPath: $fullPath) {

ee/app/assets/javascripts/security_dashboard/graphql/instance_specific_scanners.query.graphql

-#import "./vulnerablity_scanner.fragment.graphql"
+#import "./vulnerability_scanner.fragment.graphql"
 
 query instanceSpecificScanners {
   instanceSecurityDashboard {

ee/app/assets/javascripts/security_dashboard/graphql/project_specific_scanners.query.graphql

-#import "./vulnerablity_scanner.fragment.graphql"
+#import "./vulnerability_scanner.fragment.graphql"
 
-query projectSpecificScanners($fullpath: id!) {
-  project(fullPath: $fullPath) {
+query projectSpecificScanners($fullpath: ID!) {
+  project(fullPath: $fullpath) {
     vulnerabilityScanners {
       nodes {
         ...VulnerabilityScanner

fixtures/lib/gitlab/graphql/queries/author.fragment.graphql

+fragment AuthorF on Author {
+  name
+  handle
+}

fixtures/lib/gitlab/graphql/queries/bad.fragment.graphql

+fragment BadF on Blog {
+  wibble
+  wobble
+}

fixtures/lib/gitlab/graphql/queries/bad_argument.graphql

+query($bad: String) {
+  blog(title: $bad) {
+    description
+  }
+}

fixtures/lib/gitlab/graphql/queries/client.query.graphql

+query {
+  thingy @client
+}

fixtures/lib/gitlab/graphql/queries/connection.query.graphql

+query($slug: String!) {
+  post(slug: $slug) {
+    author {
+      posts @connection(key: "posts") {
+        title
+      }
+    }
+  }
+}

fixtures/lib/gitlab/graphql/queries/deeply/nested/bad_import.graphql

+# import "../author.fragment.graphql"
+
+query($slug: String!) {
+  post(slug: $slug) {
+    author { ...AuthorF }
+  }
+}

fixtures/lib/gitlab/graphql/queries/deeply/nested/query.graphql

+# import "../../author.fragment.graphql"
+
+query($slug: String!) {
+  post(slug: $slug) {
+    author { ...AuthorF }
+  }
+}

fixtures/lib/gitlab/graphql/queries/duplicate_imports.graphql

+# import "./author.fragment.graphql"
+# import "./post.fragment.graphql"
+
+query($title: String!) {
+  blog(title: $title) {
+    description
+    mainAuthor { ...AuthorF }
+    posts { ...PostF }
+  }
+}

fixtures/lib/gitlab/graphql/queries/ee/author.fragment.graphql

+fragment AuthorF on Author {
+  name
+  handle
+  verified
+}

fixtures/lib/gitlab/graphql/queries/ee_else_ce.import.graphql

+#import "ee_else_ce/author.fragment.graphql"
+
+query {
+  post(slug: "validating-queries") {
+    title
+    content
+    author { ...AuthorF }
+  }
+}

fixtures/lib/gitlab/graphql/queries/missing_argument.graphql

+query {
+  blog {
+    title
+    posts {
+      title
+    }
+  }
+}

fixtures/lib/gitlab/graphql/queries/post.fragment.graphql

+# import "./author.fragment.graphql"
+
+fragment PostF on Post {
+  name
+  title
+  content
+  author { ...AuthorF }
+}

fixtures/lib/gitlab/graphql/queries/post_by_slug.graphql

+query {
+  post(slug: "validating-queries") {
+    title
+    content
+    author { name }
+  }
+}

fixtures/lib/gitlab/graphql/queries/post_by_slug.with_import.graphql

+#import "./author.fragment.graphql"
+
+query {
+  post(slug: "validating-queries") {
+    title
+    content
+    author { ...AuthorF }
+  }
+}

fixtures/lib/gitlab/graphql/queries/post_by_slug.with_import.misspelled.graphql

+# import "./auther.fragment.graphql"
+
+query {
+  post(slug: "validating-queries") {
+    title
+    content
+    author { ...AuthorF }
+  }
+}

fixtures/lib/gitlab/graphql/queries/syntax-error.graphql

+query }
+  blog(title: "boom") {
+    description
+  }
+}

fixtures/lib/gitlab/graphql/queries/transitive_bad_import.fragment.graphql

+# import "does-not-exist.graphql"
+
+fragment AuthorF on Author {
+  name
+  handle
+}

fixtures/lib/gitlab/graphql/queries/transitive_bad_import.graphql

+# import "./transitive_bad_import.fragment.graphql"
+
+query($slug: String!) {
+  post(slug: $slug) {
+    title
+    content
+    author { ...AuthorF }
+  }
+}

fixtures/lib/gitlab/graphql/queries/typedefs.graphql

+type Author {
+  name: String
+}

fixtures/lib/gitlab/graphql/queries/unused_import.graphql

+# import "./author.fragment.graphql"
+
+query($slug: String!) {
+  post(slug: $slug) {
+    title
+    content
+  }
+}

fixtures/lib/gitlab/graphql/queries/wrong_field.graphql

+query {
+  blog(title: "A history of GraphQL") {
+    title
+    createdAt
+    categories { name }
+  }
+}

fixtures/lib/gitlab/graphql/queries/wrong_field.import.graphql

+# import "./bad.fragment.graphql"
+
+query($title: String!) {
+  blog(title: $title) {
+    ...BadF
+  }
+}

lib/gitlab/graphql/queries.rb

+# frozen_string_literal: true
+
+require 'find'
+
+module Gitlab
+  module Graphql
+    module Queries
+      IMPORT_RE = /^#\s*import "(?<path>[^"]+)"$/m.freeze
+      # TODO: validate both the EE and CE versions
+      EE_ELSE_CE = /^ee_else_ce/.freeze
+      HOME_RE = /^~/.freeze
+      HOME_EE = %r{^ee/}.freeze
+      DOTS_RE = %r{^(\.\./)+}.freeze
+      DOT_RE = %r{^\./}.freeze
+      IMPLICIT_ROOT = %r{^app/}.freeze
+      CLIENT_DIRECTIVE = /@client/.freeze
+      CONN_DIRECTIVE = /@connection\(key: "\w+"\)/.freeze
+
+      class WrappedError
+        delegate :message, to: :@error
+
+        def initialize(error)
+          @error = error
+        end
+
+        def path
+          []
+        end
+      end
+
+      class FileNotFound
+        def initialize(file)
+          @file = file
+        end
+
+        def message
+          "File not found: #{@file}"
+        end
+
+        def path
+          []
+        end
+      end
+
+      class Definition
+        attr_reader :file, :imports
+
+        def initialize(path, fragments)
+          @file = path
+          @fragments = fragments
+          @imports = []
+          @errors = []
+          @ee_else_ce = []
+        end
+
+        def text(ee = false)
+          qs = [query] + all_imports(ee).uniq.sort.map { |p| fragment(p).query }
+          qs.join("\n\n").gsub(/\n\n+/, "\n\n")
+        end
+
+        def query
+          return @query if defined?(@query)
+
+          # CONN_DIRECTIVEs are purely client-side constructs
+          @query = File.read(file).gsub(CONN_DIRECTIVE, '').gsub(IMPORT_RE) do
+            path = $~[:path]
+
+            if EE_ELSE_CE.match?(path)
+              @ee_else_ce << path.gsub(EE_ELSE_CE, '')
+            else
+              @imports << fragment_path(path)
+            end
+
+            ''
+          end
+        rescue Errno::ENOENT
+          @errors << FileNotFound.new(file)
+          @query = nil
+        end
+
+        def all_imports(ee = false)
+          return [] if query.nil?
+
+          home = ee ? @fragments.home_ee : @fragments.home
+          eithers = @ee_else_ce.map { |p| home + p }
+
+          (imports + eithers).flat_map { |p| [p] + @fragments.get(p).all_imports(ee) }
+        end
+
+        def all_errors
+          return @errors.to_set if query.nil?
+
+          paths = imports + @ee_else_ce.flat_map { |p| [@fragments.home + p, @fragments.home_ee + p] }
+
+          paths.map { |p| fragment(p).all_errors }.reduce(@errors.to_set) { |a, b| a | b }
+        end
+
+        def fragment(path)
+          @fragments.get(path)
+        end
+
+        def fragment_path(import_path)
+          frag_path = import_path.gsub(HOME_RE, @fragments.home)
+          frag_path = frag_path.gsub(HOME_EE, @fragments.home_ee + '/')
+          frag_path = frag_path.gsub(DOT_RE) do
+            Pathname.new(file).parent.to_s + '/'
+          end
+          frag_path = frag_path.gsub(DOTS_RE) do |dots|
+            rel_dir(dots.split('/').count)
+          end
+          frag_path = frag_path.gsub(IMPLICIT_ROOT) do
+            (Rails.root / 'app').to_s + '/'
+          end
+
+          frag_path
+        end
+
+        def rel_dir(n_steps_up)
+          path = Pathname.new(file).parent
+          while n_steps_up > 0
+            path = path.parent
+            n_steps_up -= 1
+          end
+
+          path.to_s + '/'
+        end
+
+        # TODO: move these warnings to the rake task
+        def validate(schema)
+          return [:client_query, []] if CLIENT_DIRECTIVE.match?(text)
+
+          errs = all_errors.presence || schema.validate(text)
+          if @ee_else_ce.present?
+            errs += schema.validate(text(true))
+          end
+
+          [:validated, errs]
+        rescue ::GraphQL::ParseError => e
+          [:validated, [WrappedError.new(e)]]
+        end
+      end
+
+      class Fragments
+        def initialize(root, dir = 'app/assets/javascripts')
+          @root = root
+          @store = {}
+          @dir = dir
+        end
+
+        def home
+          @home ||= (@root / @dir).to_s
+        end
+
+        def home_ee
+          @home_ee ||= (@root / 'ee' / @dir).to_s
+        end
+
+        def get(frag_path)
+          @store[frag_path] ||= Definition.new(frag_path, self)
+        end
+      end
+
+      def self.find(root)
+        definitions = []
+
+        ::Find.find(root.to_s) do |path|
+          next unless path.ends_with?('.graphql')
+          next if path.ends_with?('.fragment.graphql')
+          next if path.ends_with?('typedefs.graphql')
+
+          definitions << Definition.new(path, fragments)
+        end
+
+        definitions
+      rescue Errno::ENOENT
+        [] # root does not exist
+      end
+
+      def self.fragments
+        @fragments ||= Fragments.new(Rails.root)
+      end
+
+      def self.all
+        ['.', 'ee'].flat_map do |prefix|
+          find(Rails.root / prefix / 'app/assets/javascripts')
+        end
+      end
+
+      def self.known_failure?(path)
+        @known_failures ||= YAML.safe_load(File.read(Rails.root.join('config', 'known_invalid_graphql_queries.yml')))
+
+        @known_failures.fetch('filenames', []).any? { |known_failure| path.to_s.ends_with?(known_failure) }
+      end
+    end
+  end
+end

lib/tasks/gitlab/graphql.rake

@@ -33,6 +33,44 @@ namespace :gitlab do
   )
 
   namespace :graphql do
+    desc 'Gitlab | GraphQL | Validate queries'
+    task validate: [:environment, :enable_feature_flags] do |t, args|
+      queries = if args.to_a.present?
+                  args.to_a.flat_map { |path| Gitlab::Graphql::Queries.find(path) }
+                else
+                  Gitlab::Graphql::Queries.all
+                end
+
+      failed = queries.flat_map do |defn|
+        summary, errs = defn.validate(GitlabSchema)
+
+        case summary
+        when :client_query
+          warn("SKIP  #{defn.file}: client query")
+        else
+          warn("OK    #{defn.file}") if errs.empty?
+          errs.each do |err|
+            warn(<<~MSG)
+            ERROR #{defn.file}: #{err.message} (at #{err.path.join('.')})
+            MSG
+          end
+        end
+
+        errs.empty? ? [] : [defn.file]
+      end
+
+      if failed.present?
+        format_output(
+          "#{failed.count} GraphQL #{'query'.pluralize(failed.count)} out of #{queries.count} failed validation:",
+          *failed.map do |name|
+            known_failure = Gitlab::Graphql::Queries.known_failure?(name)
+            "- #{name}" + (known_failure ? ' (known failure)' : '')
+          end
+        )
+        abort unless failed.all? { |name| Gitlab::Graphql::Queries.known_failure?(name) }
+      end
+    end
+
     desc 'GitLab | GraphQL | Generate GraphQL docs'
     task compile_docs: [:environment, :enable_feature_flags] do
       renderer = Gitlab::Graphql::Docs::Renderer.new(GitlabSchema.graphql_definition, render_options)
@@ -78,11 +116,11 @@ def render_options
   }
 end
 
-def format_output(str)
+def format_output(*strs)
   heading = '#' * 10
   puts heading
   puts '#'
-  puts "# #{str}"
+  strs.each { |str| puts "# #{str}" }
   puts '#'
   puts heading
 end

scripts/frontend/prettier.js

@@ -7,7 +7,7 @@ const matchExtensions = ['js', 'vue', 'graphql'];
 
 // This will improve glob performance by excluding certain directories.
 // The .prettierignore file will also be respected, but after the glob has executed.
-const globIgnore = ['**/node_modules/**', 'vendor/**', 'public/**'];
+const globIgnore = ['**/node_modules/**', 'vendor/**', 'public/**', 'fixtures/**'];
 
 const readFileAsync = (file, options) =>
   new Promise((resolve, reject) => {

spec/lib/gitlab/graphql/queries_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require "test_prof/recipes/rspec/let_it_be"
+
+RSpec.describe Gitlab::Graphql::Queries do
+  shared_examples 'a valid GraphQL query for the blog schema' do
+    it 'is valid' do
+      expect(subject.validate(schema).second).to be_empty
+    end
+  end
+
+  shared_examples 'an invalid GraphQL query for the blog schema' do
+    it 'is invalid' do
+      expect(subject.validate(schema).second).to match errors
+    end
+  end
+
+  # Toy schema to validate queries against
+  let_it_be(:schema) do
+    author = Class.new(GraphQL::Schema::Object) do
+      graphql_name 'Author'
+      field :name, GraphQL::STRING_TYPE, null: true
+      field :handle, GraphQL::STRING_TYPE, null: false
+      field :verified, GraphQL::BOOLEAN_TYPE, null: false
+    end
+
+    post = Class.new(GraphQL::Schema::Object) do
+      graphql_name 'Post'
+      field :name, GraphQL::STRING_TYPE, null: false
+      field :title, GraphQL::STRING_TYPE, null: false
+      field :content, GraphQL::STRING_TYPE, null: true
+      field :author, author, null: false
+    end
+    author.field :posts, [post], null: false do
+      argument :blog_title, GraphQL::STRING_TYPE, required: false
+    end
+
+    blog = Class.new(GraphQL::Schema::Object) do
+      graphql_name 'Blog'
+      field :title, GraphQL::STRING_TYPE, null: false
+      field :description, GraphQL::STRING_TYPE, null: false
+      field :main_author, author, null: false
+      field :posts, [post], null: false
+      field :post, post, null: true do
+        argument :slug, GraphQL::STRING_TYPE, required: true
+      end
+    end
+
+    Class.new(GraphQL::Schema) do
+      query(Class.new(GraphQL::Schema::Object) do
+        graphql_name 'Query'
+        field :blog, blog, null: true do
+          argument :title, GraphQL::STRING_TYPE, required: true
+        end
+        field :post, post, null: true do
+          argument :slug, GraphQL::STRING_TYPE, required: true
+        end
+      end)
+    end
+  end
+
+  let(:root) do
+    Rails.root / 'fixtures/lib/gitlab/graphql/queries'
+  end
+
+  describe Gitlab::Graphql::Queries::Fragments do
+    subject { described_class.new(root) }
+
+    it 'has the right home' do
+      expect(subject.home).to eq (root / 'app/assets/javascripts').to_s
+    end
+
+    it 'has the right EE home' do
+      expect(subject.home_ee).to eq (root / 'ee/app/assets/javascripts').to_s
+    end
+
+    it 'caches query definitions' do
+      fragment = subject.get('foo')
+
+      expect(fragment).to be_a(::Gitlab::Graphql::Queries::Definition)
+      expect(subject.get('foo')).to be fragment
+    end
+  end
+
+  describe '.all' do
+    it 'is the combination of finding queries in CE and EE' do
+      expect(described_class)
+        .to receive(:find).with(Rails.root / 'app/assets/javascripts').and_return([:ce])
+      expect(described_class)
+        .to receive(:find).with(Rails.root / 'ee/app/assets/javascripts').and_return([:ee])
+
+      expect(described_class.all).to eq([:ce, :ee])
+    end
+  end
+
+  describe '.find' do
+    def definition_of(path)
+      be_a(::Gitlab::Graphql::Queries::Definition)
+        .and(have_attributes(file: path.to_s))
+    end
+
+    it 'find a single specific file' do
+      path = root / 'post_by_slug.graphql'
+
+      expect(described_class.find(path)).to contain_exactly(definition_of(path))
+    end
+
+    it 'ignores files that do not exist' do
+      path = root / 'not_there.graphql'
+
+      expect(described_class.find(path)).to be_empty
+    end
+
+    it 'ignores fragments' do
+      path = root / 'author.fragment.graphql'
+
+      expect(described_class.find(path)).to be_empty
+    end
+
+    it 'ignores typedefs' do
+      path = root / 'typedefs.graphql'
+
+      expect(described_class.find(path)).to be_empty
+    end
+
+    it 'finds all query definitions under a root directory' do
+      found = described_class.find(root)
+
+      expect(found).to include(
+        definition_of(root / 'post_by_slug.graphql'),
+        definition_of(root / 'post_by_slug.with_import.graphql'),
+        definition_of(root / 'post_by_slug.with_import.misspelled.graphql'),
+        definition_of(root / 'duplicate_imports.graphql'),
+        definition_of(root / 'deeply/nested/query.graphql')
+      )
+
+      expect(found).not_to include(
+        definition_of(root / 'typedefs.graphql'),
+        definition_of(root / 'author.fragment.graphql')
+      )
+    end
+  end
+
+  describe Gitlab::Graphql::Queries::Definition do
+    let(:fragments) { Gitlab::Graphql::Queries::Fragments.new(root, '.') }
+
+    subject { described_class.new(root / path, fragments) }
+
+    context 'a simple query' do
+      let(:path) { 'post_by_slug.graphql' }
+
+      it_behaves_like 'a valid GraphQL query for the blog schema'
+    end
+
+    context 'a query with an import' do
+      let(:path) { 'post_by_slug.with_import.graphql' }
+
+      it_behaves_like 'a valid GraphQL query for the blog schema'
+    end
+
+    context 'a query with duplicate imports' do
+      let(:path) { 'duplicate_imports.graphql' }
+
+      it_behaves_like 'a valid GraphQL query for the blog schema'
+    end
+
+    context 'a query importing from ee_else_ce' do
+      let(:path) { 'ee_else_ce.import.graphql' }
+
+      it_behaves_like 'a valid GraphQL query for the blog schema'
+
+      it 'can resolve the ee fields' do
+        expect(subject.text(false)).not_to include('verified')
+        expect(subject.text(true)).to include('verified')
+      end
+    end
+
+    context 'a query refering to parent directories' do
+      let(:path) { 'deeply/nested/query.graphql' }
+
+      it_behaves_like 'a valid GraphQL query for the blog schema'
+    end
+
+    context 'a query refering to parent directories, incorrectly' do
+      let(:path) { 'deeply/nested/bad_import.graphql' }
+
+      it_behaves_like 'an invalid GraphQL query for the blog schema' do
+        let(:errors) do
+          contain_exactly(
+            be_a(::Gitlab::Graphql::Queries::FileNotFound)
+              .and(have_attributes(message: include('deeply/author.fragment.graphql')))
+          )
+        end
+      end
+    end
+
+    context 'a query with a broken import' do
+      let(:path) { 'post_by_slug.with_import.misspelled.graphql' }
+
+      it_behaves_like 'an invalid GraphQL query for the blog schema' do
+        let(:errors) do
+          contain_exactly(
+            be_a(::Gitlab::Graphql::Queries::FileNotFound)
+              .and(have_attributes(message: include('auther.fragment.graphql')))
+          )
+        end
+      end
+    end
+
+    context 'a query which imports a file with a broken import' do
+      let(:path) { 'transitive_bad_import.graphql' }
+
+      it_behaves_like 'an invalid GraphQL query for the blog schema' do
+        let(:errors) do
+          contain_exactly(
+            be_a(::Gitlab::Graphql::Queries::FileNotFound)
+              .and(have_attributes(message: include('does-not-exist.graphql')))
+          )
+        end
+      end
+    end
+
+    context 'a query containing a client directive' do
+      let(:path) { 'client.query.graphql' }
+
+      it_behaves_like 'a valid GraphQL query for the blog schema'
+
+      it 'is tagged as a client query' do
+        expect(subject.validate(schema).first).to eq :client_query
+      end
+    end
+
+    context 'a query containing a connection directive' do
+      let(:path) { 'connection.query.graphql' }
+
+      it_behaves_like 'a valid GraphQL query for the blog schema'
+    end
+
+    context 'a query which mentions an incorrect field' do
+      let(:path) { 'wrong_field.graphql' }
+
+      it_behaves_like 'an invalid GraphQL query for the blog schema' do
+        let(:errors) do
+          contain_exactly(
+            have_attributes(message: /'createdAt' doesn't exist/),
+            have_attributes(message: /'categories' doesn't exist/)
+          )
+        end
+      end
+    end
+
+    context 'a query which has a missing argument' do
+      let(:path) { 'missing_argument.graphql' }
+
+      it_behaves_like 'an invalid GraphQL query for the blog schema' do
+        let(:errors) do
+          contain_exactly(
+            have_attributes(message: include('blog'))
+          )
+        end
+      end
+    end
+
+    context 'a query which has a bad argument' do
+      let(:path) { 'bad_argument.graphql' }
+
+      it_behaves_like 'an invalid GraphQL query for the blog schema' do
+        let(:errors) do
+          contain_exactly(
+            have_attributes(message: include('Nullability mismatch on variable $bad'))
+          )
+        end
+      end
+    end
+
+    context 'a query which has a syntax error' do
+      let(:path) { 'syntax-error.graphql' }
+
+      it_behaves_like 'an invalid GraphQL query for the blog schema' do
+        let(:errors) do
+          contain_exactly(
+            have_attributes(message: include('Parse error'))
+          )
+        end
+      end
+    end
+
+    context 'a query which has an unused import' do
+      let(:path) { 'unused_import.graphql' }
+
+      it_behaves_like 'an invalid GraphQL query for the blog schema' do
+        let(:errors) do
+          contain_exactly(
+            have_attributes(message: include('AuthorF was defined, but not used'))
+          )
+        end
+      end
+    end
+  end
+end