Merge remote-tracking branch 'upstream/master' into ce-to-ee-2018-11-15

# Conflicts:
#	.gitlab-ci.yml

[ci skip]

.gitlab-ci.yml

@@ -612,6 +612,7 @@ rspec-pg-rails4:
 rspec-mysql-rails4:
   <<: *rspec-metadata-mysql-rails4
   parallel: 30
+<<<<<<< HEAD
 
 # EE jobs START
 rspec-pg-ee:
@@ -636,6 +637,8 @@ rspec-mysql-ee-rails4:
 rspec-pg-geo-rails4: *rspec-metadata-pg-geo-9-6-rails4
 rspec-pg-10-geo-rails4: *rspec-metadata-pg-geo-10-2-rails4
 # EE jobs END
+=======
+>>>>>>> upstream/master
 
 static-analysis:
   <<: *dedicated-no-docs-no-db-pull-cache-job

Gemfile.rails5

-# BUNDLE_GEMFILE=Gemfile.rails5 bundle install
-
-ENV["RAILS5"] = "true"
-
-gemfile = File.expand_path("../Gemfile", __FILE__)
-
-eval(File.read(gemfile), nil, gemfile)

app/assets/javascripts/files_comment_button.js

 /* Developer beware! Do not add logic to showButton or hideButton
- * that will force a reflow. Doing so will create a signficant performance
+ * that will force a reflow. Doing so will create a significant performance
  * bottleneck for pages with large diffs. For a comprehensive list of what
  * causes reflows, visit https://gist.github.com/paulirish/5d52fb081b3570c81e3a
  */

app/controllers/projects/blob_controller.rb

@@ -122,7 +122,7 @@ class Projects::BlobController < Projects::ApplicationController
     @lines.map! do |line|
       # These are marked as context lines but are loaded from blobs.
       # We also have context lines loaded from diffs in other places.
-      diff_line = Gitlab::Diff::Line.new(line, expanded_diff_line_type, nil, nil, nil)
+      diff_line = Gitlab::Diff::Line.new(line, nil, nil, nil, nil)
       diff_line.rich_text = line
       diff_line
     end
@@ -132,11 +132,6 @@ class Projects::BlobController < Projects::ApplicationController
     render json: DiffLineSerializer.new.represent(@lines)
   end
 
-  def expanded_diff_line_type
-    # Context lines can't receive comments.
-    Feature.enabled?(:comment_in_any_diff_line, @project) ? nil : 'context'
-  end
-
   def add_match_line
     return unless @form.unfold?
 

app/models/clusters/applications/knative.rb

@@ -41,6 +41,10 @@ module Clusters
         )
       end
 
+      def client
+        cluster.platform_kubernetes.kubeclient.knative_client
+      end
+
       private
 
       def install_script

app/models/concerns/deployable.rb

@@ -13,6 +13,10 @@ module Deployable
         name: expanded_environment_name
       )
 
+      # If we failed to persist envirionment record by validation error, such as name with invalid character,
+      # the job will fall back to a non-environment job.
+      return unless environment.persisted?
+
       create_deployment!(
         project_id: environment.project_id,
         environment: environment,

app/workers/build_finished_worker.rb

@@ -15,7 +15,7 @@ class BuildFinishedWorker
       BuildTraceSectionsWorker.new.perform(build.id)
       BuildCoverageWorker.new.perform(build.id)
 
-      # We execute that async as this are two indepentent operations that can be executed after TraceSections and Coverage
+      # We execute that async as this are two independent operations that can be executed after TraceSections and Coverage
       BuildHooksWorker.perform_async(build.id)
       ArchiveTraceWorker.perform_async(build.id)
     end

app/workers/process_commit_worker.rb

 # frozen_string_literal: true
 
-# Worker for processing individiual commit messages pushed to a repository.
+# Worker for processing individual commit messages pushed to a repository.
 #
 # Jobs for this worker are scheduled for every commit that is being pushed. As a
 # result of this the workload of this worker should be kept to a bare minimum.

changelogs/unreleased/53626-update-config-map-on-install-retry.yml

+---
+title: Update config map for gitlab managed application if already present on install
+merge_request: 22969
+author:
+type: other

changelogs/unreleased/auto_devops_kubernetes_active.yml

+---
+title: Switch kubernetes:active with checking in Auto-DevOps.gitlab-ci.yml
+merge_request: 22929
+author:
+type: fixed

changelogs/unreleased/ignore-environment-validation-failure.yml

+---
+title: Ignore environment validation failure
+merge_request: 23100
+author:
+type: fixed

changelogs/unreleased/osw-fallback-on-blank-refs.yml

+---
+title: Avoid Gitaly RPC errors when fetching diff stats
+merge_request: 22995
+author:
+type: fixed

changelogs/unreleased/triggermesh-phase2-serverless.yml

+---
+title: Add knative client to kubeclient library
+merge_request: 22968
+author: cab105
+type: added

doc/development/README.md

@@ -30,6 +30,7 @@ description: 'Learn how to contribute to GitLab.'
 ## Backend guides
 
 - [GitLab utilities](utilities.md)
+- [Logging](logging.md)
 - [API styleguide](api_styleguide.md) Use this styleguide if you are
   contributing to the API.
 - [GraphQL API styleguide](api_graphql_styleguide.md) Use this

doc/development/logging.md

+# GitLab Developers Guide to Logging
+
+[GitLab Logs](../administration/logs.md) play a critical role for both
+administrators and GitLab team members to diagnose problems in the field.
+
+## Don't use `Rails.logger`
+
+Currently `Rails.logger` calls all get saved into `production.log`, which contains
+a mix of Rails' logs and other calls developers have inserted in the code base.
+For example:
+
+```
+Started GET "/gitlabhq/yaml_db/tree/master" for 168.111.56.1 at 2015-02-12 19:34:53 +0200
+Processing by Projects::TreeController#show as HTML
+  Parameters: {"project_id"=>"gitlabhq/yaml_db", "id"=>"master"}
+
+  ...
+
+  Namespaces"."created_at" DESC, "namespaces"."id" DESC LIMIT 1 [["id", 26]]
+  CACHE (0.0ms) SELECT  "members".* FROM "members"  WHERE "members"."source_type" = 'Project' AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $1 AND "members"."source_type" = $2 AND "members"."user_id" = 1  ORDER BY "members"."created_at" DESC, "members"."id" DESC LIMIT 1  [["source_id", 18], ["source_type", "Project"]]
+  CACHE (0.0ms) SELECT  "members".* FROM "members"  WHERE "members"."source_type" = 'Project' AND "members".
+  (1.4ms) SELECT COUNT(*) FROM "merge_requests"  WHERE "merge_requests"."target_project_id" = $1 AND ("merge_requests"."state" IN ('opened','reopened')) [["target_project_id", 18]]
+  Rendered layouts/nav/_project.html.haml (28.0ms)
+  Rendered layouts/_collapse_button.html.haml (0.2ms)
+  Rendered layouts/_flash.html.haml (0.1ms)
+  Rendered layouts/_page.html.haml (32.9ms)
+Completed 200 OK in 166ms (Views: 117.4ms | ActiveRecord: 27.2ms)
+```
+
+These logs suffer from a number of problems:
+
+1. They often lack timestamps or other contextual information (e.g. project ID, user)
+2. They may span multiple lines, which make them hard to find via Elasticsearch.
+3. They lack a common structure, which make them hard to parse by log
+forwarders, such as Logstash or Fluentd. This also makes them hard to
+search.
+
+Note that currently on GitLab.com, any messages in `production.log` will
+NOT get indexed by Elasticsearch due to the sheer volume and noise. They
+do end up in Google Stackdriver, but it is still harder to search for
+logs there. See the [GitLab.com logging
+documentation](https://gitlab.com/gitlab-com/runbooks/blob/master/howto/logging.md)
+for more details.
+
+## Use structured (JSON) logging
+
+Structured logging solves these problems. Consider the example from an API request:
+
+```json
+{"time":"2018-10-29T12:49:42.123Z","severity":"INFO","duration":709.08,"db":14.59,"view":694.49,"status":200,"method":"GET","path":"/api/v4/projects","params":[{"key":"action","value":"git-upload-pack"},{"key":"changes","value":"_any"},{"key":"key_id","value":"secret"},{"key":"secret_token","value":"[FILTERED]"}],"host":"localhost","ip":"::1","ua":"Ruby","route":"/api/:version/projects","user_id":1,"username":"root","queue_duration":100.31,"gitaly_calls":30}
+```
+
+In a single line, we've included all the information that a user needs
+to understand what happened: the timestamp, HTTP method and path, user
+ID, etc.
+
+### How to use JSON logging
+
+Suppose you want to log the events that happen in a project
+importer. You want to log issues created, merge requests, etc. as the
+importer progresses. Here's what to do:
+
+1. Look at [the list of GitLab Logs](../administration/logs.md) to see
+if your log message might belong with one of the existing log files.
+1. If there isn't a good place, consider creating a new filename, but
+check with a maintainer if it makes sense to do so. A log file should
+make it easy for people to search pertinent logs in one place. For
+example, `geo.log` contains all logs pertaining to GitLab Geo.
+To create a new file:
+    1. Choose a filename (e.g. `importer_json.log`).
+    1. Create a new subclass of `Gitlab::JsonLogger`:
+
+        ```ruby
+        module Gitlab
+          module Import
+            class Logger < ::Gitlab::JsonLogger
+              def self.file_name_noext
+                'importer_json'
+              end
+            end
+           end
+        end
+        ```
+
+    1. In your class where you want to log, you might initialize the logger as an instance variable:
+
+        ```ruby
+        attr_accessor :logger
+
+        def initialize
+          @logger = Gitlab::Import::Logger.build
+        end
+        ```
+
+        Note that it's useful to memoize this because creating a new logger
+        each time you log will open a file, adding unnecessary overhead.
+
+1. Now insert log messages into your code. When adding logs,
+   make sure to include all the context as key-value pairs:
+
+    ```ruby
+    # BAD
+    logger.info("Unable to create project #{project.id}")
+    ```
+
+    ```ruby
+    # GOOD
+    logger.info("Unable to create project", project_id: project.id)
+    ```
+
+1. Be sure to create a common base structure of your log messages. For example,
+   all messages might have `current_user_id` and `project_id` to make it easier
+   to search for activities by user for a given time.
+
+1. Do NOT mix and match types. Elasticsearch won't be able to index your
+   logs properly if you [mix integer and string
+   types](https://www.elastic.co/guide/en/elasticsearch/guide/current/mapping.html#_avoiding_type_gotchas):
+
+    ```ruby
+    # BAD
+    logger.info("Import error", error: 1)
+    logger.info("Import error", error: "I/O failure")
+    ```
+
+    ```ruby
+    # GOOD
+    logger.info("Import error", error_code: 1, error: "I/O failure")
+    ```
+
+## Additional steps with new log files
+
+1. Consider log retention settings. By default, Omnibus will rotate any
+logs in `/var/log/gitlab/gitlab-rails/*.log` every hour and [keep at
+most 30 compressed files](https://docs.gitlab.com/omnibus/settings/logs.html#logrotate).
+On GitLab.com, that setting is only 6 compressed files. These settings should suffice
+for most users, but you may need to tweak them in [omnibus-gitlab](https://gitlab.com/gitlab-org/omnibus-gitlab).
+
+1. If you add a new file, submit an issue to the [production
+tracker](https://gitlab.com/gitlab-com/gl-infra/production/issues) or
+a merge request to the [gitlab_fluentd](https://gitlab.com/gitlab-cookbooks/gitlab_fluentd)
+project. See [this example](https://gitlab.com/gitlab-cookbooks/gitlab_fluentd/merge_requests/51/diffs).
+
+1. Be sure to update the [GitLab CE/EE documentation](../administration/logs.md) and the [GitLab.com
+runbooks](https://gitlab.com/gitlab-com/runbooks/blob/master/howto/logging.md).

lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml

@@ -149,10 +149,10 @@ performance:
   only:
     refs:
       - branches
-    kubernetes: active
   except:
     variables:
       - $PERFORMANCE_DISABLED
+      - $KUBECONFIG == null
 
 sast:
   stage: test
@@ -227,7 +227,6 @@ dast:
   only:
     refs:
       - branches
-    kubernetes: active
     variables:
       - $GITLAB_FEATURES =~ /\bdast\b/
   except:
@@ -235,6 +234,7 @@ dast:
       - master
     variables:
       - $DAST_DISABLED
+      - $KUBECONFIG == null
 
 review:
   stage: review
@@ -256,12 +256,12 @@ review:
   only:
     refs:
       - branches
-    kubernetes: active
   except:
     refs:
       - master
     variables:
       - $REVIEW_DISABLED
+      - $KUBECONFIG == null
 
 stop_review:
   stage: cleanup
@@ -279,12 +279,12 @@ stop_review:
   only:
     refs:
       - branches
-    kubernetes: active
   except:
     refs:
       - master
     variables:
       - $REVIEW_DISABLED
+      - $KUBECONFIG == null
 
 # Staging deploys are disabled by default since
 # continuous deployment to production is enabled by default
@@ -308,9 +308,11 @@ staging:
   only:
     refs:
       - master
-    kubernetes: active
     variables:
       - $STAGING_ENABLED
+  except:
+    variables:
+      - $KUBECONFIG == null
 
 # Canaries are also disabled by default, but if you want them,
 # and know what the downsides are, you can enable this by setting
@@ -333,9 +335,11 @@ canary:
   only:
     refs:
       - master
-    kubernetes: active
     variables:
       - $CANARY_ENABLED
+  except:
+    variables:
+      - $KUBECONFIG == null
 
 .production: &production_template
   stage: production
@@ -361,13 +365,13 @@ production:
   only:
     refs:
       - master
-    kubernetes: active
   except:
     variables:
       - $STAGING_ENABLED
       - $CANARY_ENABLED
       - $INCREMENTAL_ROLLOUT_ENABLED
       - $INCREMENTAL_ROLLOUT_MODE
+      - $KUBECONFIG == null
 
 production_manual:
   <<: *production_template
@@ -376,7 +380,6 @@ production_manual:
   only:
     refs:
       - master
-    kubernetes: active
     variables:
       - $STAGING_ENABLED
       - $CANARY_ENABLED
@@ -384,6 +387,7 @@ production_manual:
     variables:
       - $INCREMENTAL_ROLLOUT_ENABLED
       - $INCREMENTAL_ROLLOUT_MODE
+      - $KUBECONFIG == null
 
 # This job implements incremental rollout on for every push to `master`.
 
@@ -413,13 +417,13 @@ production_manual:
   only:
     refs:
       - master
-    kubernetes: active
     variables:
       - $INCREMENTAL_ROLLOUT_MODE == "manual"
       - $INCREMENTAL_ROLLOUT_ENABLED
   except:
     variables:
       - $INCREMENTAL_ROLLOUT_MODE == "timed"
+      - $KUBECONFIG == null
 
 .timed_rollout_template: &timed_rollout_template
   <<: *rollout_template
@@ -428,9 +432,11 @@ production_manual:
   only:
     refs:
       - master
-    kubernetes: active
     variables:
       - $INCREMENTAL_ROLLOUT_MODE == "timed"
+  except:
+    variables:
+      - $KUBECONFIG == null
 
 timed rollout 10%:
   <<: *timed_rollout_template

lib/gitlab/git/repository.rb

@@ -419,13 +419,17 @@ module Gitlab
       end
 
       def diff_stats(left_id, right_id)
+        if [left_id, right_id].any? { |ref| ref.blank? || Gitlab::Git.blank_ref?(ref) }
+          return empty_diff_stats
+        end
+
         stats = wrapped_gitaly_errors do
           gitaly_commit_client.diff_stats(left_id, right_id)
         end
 
         Gitlab::Git::DiffStatsCollection.new(stats)
       rescue CommandError, TypeError
-        Gitlab::Git::DiffStatsCollection.new([])
+        empty_diff_stats
       end
 
       # Returns a RefName for a given SHA
@@ -962,6 +966,10 @@ module Gitlab
 
       private
 
+      def empty_diff_stats
+        Gitlab::Git::DiffStatsCollection.new([])
+      end
+
       def uncached_has_local_branches?
         wrapped_gitaly_errors do
           gitaly_repository_client.has_local_branches?

lib/gitlab/kubernetes/helm/api.rb

@@ -54,7 +54,11 @@ module Gitlab
 
         def create_config_map(command)
           command.config_map_resource.tap do |config_map_resource|
-            kubeclient.create_config_map(config_map_resource)
+            if config_map_exists?(config_map_resource)
+              kubeclient.update_config_map(config_map_resource)
+            else
+              kubeclient.create_config_map(config_map_resource)
+            end
           end
         end
 
@@ -88,6 +92,12 @@ module Gitlab
           end
         end
 
+        def config_map_exists?(resource)
+          kubeclient.get_config_map(resource.metadata.name, resource.metadata.namespace)
+        rescue ::Kubeclient::ResourceNotFoundError
+          false
+        end
+
         def service_account_exists?(resource)
           kubeclient.get_service_account(resource.metadata.name, resource.metadata.namespace)
         rescue ::Kubeclient::ResourceNotFoundError

lib/gitlab/kubernetes/kube_client.rb

@@ -16,7 +16,8 @@ module Gitlab
       SUPPORTED_API_GROUPS = {
         core: { group: 'api', version: 'v1' },
         rbac: { group: 'apis/rbac.authorization.k8s.io', version: 'v1' },
-        extensions: { group: 'apis/extensions', version: 'v1beta1' }
+        extensions: { group: 'apis/extensions', version: 'v1beta1' },
+        knative: { group: 'apis/serving.knative.dev', version: 'v1alpha1' }
       }.freeze
 
       SUPPORTED_API_GROUPS.each do |name, params|

spec/controllers/projects/blob_controller_spec.rb

@@ -141,28 +141,6 @@ describe Projects::BlobController do
           expect(lines.first).to have_key('rich_text')
         end
 
-        context 'comment in any diff line feature flag' do
-          it 'renders context lines when feature disabled' do
-            stub_feature_flags(comment_in_any_diff_line: false)
-
-            do_get(since: 1, to: 5, offset: 10, from_merge_request: true)
-            lines = JSON.parse(response.body)
-            all_context = lines.all? { |line| line['type'] == 'context' }
-
-            expect(all_context).to be(true)
-          end
-
-          it 'renders unchanged lines when feature enabled' do
-            stub_feature_flags(comment_in_any_diff_line: true)
-
-            do_get(since: 1, to: 5, offset: 10, from_merge_request: true)
-            lines = JSON.parse(response.body)
-            all_unchanged = lines.all? { |line| line['type'].nil? }
-
-            expect(all_unchanged).to be(true)
-          end
-        end
-
         context 'when rendering match lines' do
           it 'adds top match line when "since" is less than 1' do
             do_get(since: 5, to: 10, offset: 10, from_merge_request: true)

spec/lib/gitlab/git/repository_spec.rb

@@ -1095,12 +1095,26 @@ describe Gitlab::Git::Repository, :seed_helper do
     end
 
     it 'returns no Gitaly::DiffStats when there is a nil SHA' do
+      expect_any_instance_of(Gitlab::GitalyClient::CommitService)
+        .not_to receive(:diff_stats)
+
       collection = repository.diff_stats(nil, 'master')
 
       expect(collection).to be_a(Gitlab::Git::DiffStatsCollection)
       expect(collection).to be_a(Enumerable)
       expect(collection.to_a).to be_empty
     end
+
+    it 'returns no Gitaly::DiffStats when there is a BLANK_SHA' do
+      expect_any_instance_of(Gitlab::GitalyClient::CommitService)
+        .not_to receive(:diff_stats)
+
+      collection = repository.diff_stats(Gitlab::Git::BLANK_SHA, 'master')
+
+      expect(collection).to be_a(Gitlab::Git::DiffStatsCollection)
+      expect(collection).to be_a(Enumerable)
+      expect(collection.to_a).to be_empty
+    end
   end
 
   describe "#ls_files" do

spec/lib/gitlab/kubernetes/helm/api_spec.rb

@@ -36,6 +36,7 @@ describe Gitlab::Kubernetes::Helm::Api do
   describe '#install' do
     before do
       allow(client).to receive(:create_pod).and_return(nil)
+      allow(client).to receive(:get_config_map).and_return(nil)
       allow(client).to receive(:create_config_map).and_return(nil)
       allow(client).to receive(:create_service_account).and_return(nil)
       allow(client).to receive(:create_cluster_role_binding).and_return(nil)
@@ -57,6 +58,18 @@ describe Gitlab::Kubernetes::Helm::Api do
 
         subject.install(command)
       end
+
+      context 'config map already exists' do
+        before do
+          expect(client).to receive(:get_config_map).with("values-content-configuration-#{application_name}", gitlab_namespace).and_return(resource)
+        end
+
+        it 'updates the config map' do
+          expect(client).to receive(:update_config_map).with(resource).once
+
+          subject.install(command)
+        end
+      end
     end
 
     context 'without a service account' do
@@ -88,8 +101,8 @@ describe Gitlab::Kubernetes::Helm::Api do
 
         context 'service account and cluster role binding does not exist' do
           before do
-            expect(client).to receive('get_service_account').with('tiller', 'gitlab-managed-apps').and_raise(Kubeclient::ResourceNotFoundError.new(404, 'Not found', nil))
-            expect(client).to receive('get_cluster_role_binding').with('tiller-admin').and_raise(Kubeclient::ResourceNotFoundError.new(404, 'Not found', nil))
+            expect(client).to receive(:get_service_account).with('tiller', 'gitlab-managed-apps').and_raise(Kubeclient::ResourceNotFoundError.new(404, 'Not found', nil))
+            expect(client).to receive(:get_cluster_role_binding).with('tiller-admin').and_raise(Kubeclient::ResourceNotFoundError.new(404, 'Not found', nil))
           end
 
           it 'creates a service account, followed the cluster role binding on kubeclient' do
@@ -102,8 +115,8 @@ describe Gitlab::Kubernetes::Helm::Api do
 
         context 'service account already exists' do
           before do
-            expect(client).to receive('get_service_account').with('tiller', 'gitlab-managed-apps').and_return(service_account_resource)
-            expect(client).to receive('get_cluster_role_binding').with('tiller-admin').and_raise(Kubeclient::ResourceNotFoundError.new(404, 'Not found', nil))
+            expect(client).to receive(:get_service_account).with('tiller', 'gitlab-managed-apps').and_return(service_account_resource)
+            expect(client).to receive(:get_cluster_role_binding).with('tiller-admin').and_raise(Kubeclient::ResourceNotFoundError.new(404, 'Not found', nil))
           end
 
           it 'updates the service account, followed by creating the cluster role binding' do
@@ -116,8 +129,8 @@ describe Gitlab::Kubernetes::Helm::Api do
 
         context 'service account and cluster role binding already exists' do
           before do
-            expect(client).to receive('get_service_account').with('tiller', 'gitlab-managed-apps').and_return(service_account_resource)
-            expect(client).to receive('get_cluster_role_binding').with('tiller-admin').and_return(cluster_role_binding_resource)
+            expect(client).to receive(:get_service_account).with('tiller', 'gitlab-managed-apps').and_return(service_account_resource)
+            expect(client).to receive(:get_cluster_role_binding).with('tiller-admin').and_return(cluster_role_binding_resource)
           end
 
           it 'updates the service account, followed by creating the cluster role binding' do
@@ -130,7 +143,7 @@ describe Gitlab::Kubernetes::Helm::Api do
 
         context 'a non-404 error is thrown' do
           before do
-            expect(client).to receive('get_service_account').with('tiller', 'gitlab-managed-apps').and_raise(Kubeclient::HttpError.new(401, 'Unauthorized', nil))
+            expect(client).to receive(:get_service_account).with('tiller', 'gitlab-managed-apps').and_raise(Kubeclient::HttpError.new(401, 'Unauthorized', nil))
           end
 
           it 'raises an error' do

spec/lib/gitlab/kubernetes/kube_client_spec.rb

@@ -66,6 +66,20 @@ describe Gitlab::Kubernetes::KubeClient do
     end
   end
 
+  describe '#knative_client' do
+    subject { client.knative_client }
+
+    it_behaves_like 'a Kubeclient'
+
+    it 'has the extensions API group endpoint' do
+      expect(subject.api_endpoint.to_s).to match(%r{\/apis\/serving.knative.dev\Z})
+    end
+
+    it 'has the api_version' do
+      expect(subject.instance_variable_get(:@api_version)).to eq('v1alpha1')
+    end
+  end
+
   describe 'core API' do
     let(:core_client) { client.core_client }
 

spec/models/concerns/awardable_spec.rb

@@ -37,8 +37,8 @@ describe Awardable do
         create(:award_emoji, awardable: issue3, name: "star", user: award_emoji.user)
         create(:award_emoji, awardable: issue3, name: "star", user: award_emoji2.user)
 
-        expect(Issue.awarded(award_emoji.user)).to eq [issue, issue3]
-        expect(Issue.awarded(award_emoji2.user)).to eq [issue2, issue3]
+        expect(Issue.awarded(award_emoji.user)).to contain_exactly(issue, issue3)
+        expect(Issue.awarded(award_emoji2.user)).to contain_exactly(issue2, issue3)
       end
     end
 

spec/models/concerns/deployable_spec.rb

@@ -49,5 +49,26 @@ describe Deployable do
         expect(environment).to be_nil
       end
     end
+
+    context 'when environment scope contains invalid character' do
+      let(:job) do
+        create(
+          :ci_build,
+          name: 'job:deploy-to-test-site',
+          environment: '$CI_JOB_NAME',
+          options: {
+            environment: {
+              name: '$CI_JOB_NAME',
+              url: 'http://staging.example.com/$CI_JOB_NAME',
+              on_stop: 'stop_review_app'
+            }
+          })
+      end
+
+      it 'does not create a deployment and environment record' do
+        expect(deployment).to be_nil
+        expect(environment).to be_nil
+      end
+    end
   end
 end

spec/services/ci/create_pipeline_service_spec.rb

@@ -608,5 +608,53 @@ describe Ci::CreatePipelineService do
           .to eq variables_attributes.map(&:with_indifferent_access)
       end
     end
+
+    context 'when pipeline has a job with environment' do
+      let(:pipeline) { execute_service }
+
+      before do
+        stub_ci_pipeline_yaml_file(YAML.dump(config))
+      end
+
+      context 'when environment name is valid' do
+        let(:config) do
+          {
+            review_app: {
+              script: 'deploy',
+              environment: {
+                name: 'review/${CI_COMMIT_REF_NAME}',
+                url: 'http://${CI_COMMIT_REF_SLUG}-staging.example.com'
+              }
+            }
+          }
+        end
+
+        it 'has a job with environment' do
+          expect(pipeline.builds.count).to eq(1)
+          expect(pipeline.builds.first.persisted_environment.name).to eq('review/master')
+          expect(pipeline.builds.first.deployment).to be_created
+        end
+      end
+
+      context 'when environment name is invalid' do
+        let(:config) do
+          {
+            'job:deploy-to-test-site': {
+              script: 'deploy',
+              environment: {
+                name: '${CI_JOB_NAME}',
+                url: 'https://$APP_URL'
+              }
+            }
+          }
+        end
+
+        it 'has a job without environment' do
+          expect(pipeline.builds.count).to eq(1)
+          expect(pipeline.builds.first.persisted_environment).to be_nil
+          expect(pipeline.builds.first.deployment).to be_nil
+        end
+      end
+    end
   end
 end

spec/support/helpers/kubernetes_helpers.rb

@@ -21,6 +21,7 @@ module KubernetesHelpers
     WebMock.stub_request(:get, api_url + '/api/v1').to_return(kube_response(kube_v1_discovery_body))
     WebMock.stub_request(:get, api_url + '/apis/extensions/v1beta1').to_return(kube_response(kube_v1beta1_discovery_body))
     WebMock.stub_request(:get, api_url + '/apis/rbac.authorization.k8s.io/v1').to_return(kube_response(kube_v1_rbac_authorization_discovery_body))
+    WebMock.stub_request(:get, api_url + '/apis/serving.knative.dev/v1alpha1').to_return(kube_response(kube_v1alpha1_serving_knative_discovery_body))
   end
 
   def stub_kubeclient_pods(response = nil)
@@ -145,6 +146,18 @@ module KubernetesHelpers
     }
   end
 
+  def kube_v1alpha1_serving_knative_discovery_body
+    {
+      "kind" => "APIResourceList",
+      "resources" => [
+        { "name" => "revisions", "namespaced" => true, "kind" => "Revision" },
+        { "name" => "services", "namespaced" => true, "kind" => "Service" },
+        { "name" => "configurations", "namespaced" => true, "kind" => "Configuration" },
+        { "name" => "routes", "namespaced" => true, "kind" => "Route" }
+      ]
+    }
+  end
+
   def kube_pods_body
     {
       "kind" => "PodList",