Merge branch 'ajk-graphql-verification' into 'master'

Validate GraphQL queries [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!50655

.gitlab-ci.yml

@@ -110,3 +110,4 @@ include:
   - local: .gitlab/ci/notify.gitlab-ci.yml
   - local: .gitlab/ci/dast.gitlab-ci.yml
   - local: .gitlab/ci/workhorse.gitlab-ci.yml
+  - local: .gitlab/ci/graphql.gitlab-ci.yml

.gitlab/ci/docs.gitlab-ci.yml

@@ -84,16 +84,3 @@ ui-docs-links lint:
   needs: []
   script:
     - bundle exec haml-lint -i DocumentationLinks
-
-graphql-reference-verify:
-  extends:
-    - .default-retry
-    - .rails-cache
-    - .default-before_script
-    - .docs:rules:graphql-reference-verify
-    - .use-pg11
-  stage: test
-  needs: ["setup-test-env"]
-  script:
-    - bundle exec rake gitlab:graphql:check_docs
-    - bundle exec rake gitlab:graphql:check_schema

.gitlab/ci/graphql.gitlab-ci.yml

+graphql-verify:
+  variables:
+    SETUP_DB: "false"
+  extends:
+    - .default-retry
+    - .rails-cache
+    - .default-before_script
+    - .graphql:rules:graphql-verify
+  stage: test
+  needs: []
+  script:
+    - bundle exec rake gitlab:graphql:validate
+    - bundle exec rake gitlab:graphql:check_docs
+    - bundle exec rake gitlab:graphql:check_schema

.gitlab/ci/rules.gitlab-ci.yml

@@ -349,7 +349,11 @@
       changes: *docs-patterns
       when: on_success
 
-.docs:rules:graphql-reference-verify:
+##################
+# GraphQL rules  #
+##################
+
+.graphql:rules:graphql-verify:
   rules:
     - <<: *if-not-ee
       when: never

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/on_demand_scans/graphql/dast_scan_create.mutation.graphql
+  - ee/app/assets/javascripts/oncall_schedules/graphql/mutations/update_oncall_schedule_rotation.mutation.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/client_unused_fragment.graphql

+#import "./thingy.fragment.graphql"
+
+query($slug: String!, $foo: String) {
+  thingy(someArg: $foo) @client {
+    ...ThingyF
+  }
+}

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/mixed_client.query.graphql

+query {
+  thingy @client
+  post(slug: "validating-queries") {
+    title
+    otherThing @client
+  }
+}

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

+query {
+  thingy @client
+  post(slug: "validating-queries") {
+    titlz
+    otherThing @client
+  }
+}

fixtures/lib/gitlab/graphql/queries/mixed_client_skipped_argument.graphql

+query($slug: String!, $foo: String) {
+  thingy(someArg: $foo) @client {
+    x
+    y
+    z
+  }
+  post(slug: $slug) {
+    title
+    otherThing @client
+  }
+}

fixtures/lib/gitlab/graphql/queries/mixed_client_unused_fragment.graphql

+#import "./thingy.fragment.graphql"
+
+query($slug: String!, $foo: String) {
+  thingy(someArg: $foo) @client {
+    ...ThingyF
+  }
+  post(slug: $slug) {
+    title
+    otherThing @client
+  }
+}

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/thingy.fragment.graphql

+fragment ThingyF on Thingy {
+  x y z
+}

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
+      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
+      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
+
+      # We need to re-write queries to remove all @client fields. Ideally we
+      # would do that as a source-to-source transformation of the AST, but doing it using a
+      # printer is much simpler.
+      class ClientFieldRedactor < GraphQL::Language::Printer
+        attr_reader :fields_printed, :skipped_arguments, :printed_arguments, :used_fragments
+
+        def initialize(skips = true)
+          @skips = skips
+          @fields_printed = 0
+          @in_operation = false
+          @skipped_arguments = [].to_set
+          @printed_arguments = [].to_set
+          @used_fragments = [].to_set
+          @skipped_fragments = [].to_set
+          @used_fragments = [].to_set
+        end
+
+        def print_variable_identifier(variable_identifier)
+          @printed_arguments << variable_identifier.name
+          super
+        end
+
+        def print_fragment_spread(fragment_spread, indent: "")
+          @used_fragments << fragment_spread.name
+          super
+        end
+
+        def print_operation_definition(op, indent: "")
+          @in_operation = true
+          out = +"#{indent}#{op.operation_type}"
+          out << " #{op.name}" if op.name
+
+          # Do these first, so that we detect any skipped arguments
+          dirs = print_directives(op.directives)
+          sels = print_selections(op.selections, indent: indent)
+
+          # remove variable definitions only used in skipped (client) fields
+          vars = op.variables.reject do |v|
+            @skipped_arguments.include?(v.name) && !@printed_arguments.include?(v.name)
+          end
+
+          if vars.any?
+            out << "(#{vars.map { |v| print_variable_definition(v) }.join(", ")})"
+          end
+
+          out + dirs + sels
+        ensure
+          @in_operation = false
+        end
+
+        def print_field(field, indent: '')
+          if skips? && field.directives.any? { |d| d.name == 'client' }
+            skipped = self.class.new(false)
+
+            skipped.print_node(field)
+            @skipped_fragments |= skipped.used_fragments
+            @skipped_arguments |= skipped.printed_arguments
+
+            return ''
+          end
+
+          ret = super
+
+          @fields_printed += 1 if @in_operation && ret != ''
+
+          ret
+        end
+
+        def print_fragment_definition(fragment_def, indent: "")
+          if skips? && @skipped_fragments.include?(fragment_def.name) && !@used_fragments.include?(fragment_def.name)
+            return ''
+          end
+
+          super
+        end
+
+        def skips?
+          @skips
+        end
+      end
+
+      class Definition
+        attr_reader :file, :imports
+
+        def initialize(path, fragments)
+          @file = path
+          @fragments = fragments
+          @imports = []
+          @errors = []
+          @ee_else_ce = []
+        end
+
+        def text(mode: :ce)
+          qs = [query] + all_imports(mode: mode).uniq.sort.map { |p| fragment(p).query }
+          t = qs.join("\n\n").gsub(/\n\n+/, "\n\n")
+
+          return t unless /@client/.match?(t)
+
+          doc = ::GraphQL.parse(t)
+          printer = ClientFieldRedactor.new
+          redacted = doc.dup.to_query_string(printer: printer)
+
+          return redacted if printer.fields_printed > 0
+        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(mode: :ce)
+          return [] if query.nil?
+
+          home = mode == :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(mode: mode) }
+        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 validate(schema)
+          return [:client_query, []] if query.present? && text.nil?
+
+          errs = all_errors.presence || schema.validate(text)
+          if @ee_else_ce.present?
+            errs += schema.validate(text(mode: :ee))
+          end
+
+          [:validated, errs]
+        rescue ::GraphQL::ParseError => e
+          [:validated, [WrappedError.new(e)]]
+        end
+
+        private
+
+        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
+      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|
+          definitions << Definition.new(path, fragments) if query?(path)
+        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
+
+      def self.query?(path)
+        path.ends_with?('.graphql') &&
+          !path.ends_with?('.fragment.graphql') &&
+          !path.ends_with?('typedefs.graphql')
+      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) => {