Automatic merge of gitlab-org/gitlab-ce master

app/models/ci/pipeline.rb

@@ -687,11 +687,20 @@ module Ci
       end
     end
 
+    # Returns the modified paths.
+    #
+    # The returned value is
+    # * Array: List of modified paths that should be evaluated
+    # * nil: Modified path can not be evaluated
     def modified_paths
       strong_memoize(:modified_paths) do
+        if merge_request?
+          merge_request.modified_paths
+        elsif branch_updated?
           push_details.modified_paths
         end
       end
+    end
 
     def default_branch?
       ref == project.default_branch

app/models/releases/link.rb

@@ -6,7 +6,7 @@ module Releases
 
     belongs_to :release
 
-    validates :url, presence: true, url: true, uniqueness: { scope: :release }
+    validates :url, presence: true, url: { protocols: %w(http https ftp) }, uniqueness: { scope: :release }
     validates :name, presence: true, uniqueness: { scope: :release }
 
     scope :sorted, -> { order(created_at: :desc) }

changelogs/unreleased/57410-api-create-release-link-with-ftp-address-return-400-bad-request.yml

+---
+title: Add support for FTP assets for releases
+merge_request: 25071
+author: Robert Schilling
+type: added

changelogs/unreleased/support-only-changes-on-mr-pipelines.yml

+---
+title: 'Support `only: changes:` on MR pipelines'
+merge_request: 24490
+author: Hiroyuki Sato
+type: added

doc/api/releases/links.md

@@ -3,6 +3,7 @@
 > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/41766) in GitLab 11.7.
 
 Using this API you can manipulate GitLab's [Release](../../user/project/releases/index.md) links. For manipulating other Release assets, see [Release API](index.md).
+GitLab supports links links to `http`, `https`, and `ftp` assets.
 
 ## Get links
 

doc/ci/yaml/README.md

@@ -425,10 +425,28 @@ connected with merge requests yet, and because GitLab is creating pipelines
 before an user can create a merge request we don't know a target branch at
 this point.
 
-Without a target branch, it is not possible to know what the common ancestor is,
-thus we always create a job in that case. This feature works best for stable
-branches like `master` because in that case GitLab uses the previous commit
-that is present in a branch to compare against the latest SHA that was pushed.
+#### Using `changes` with `merge_requests`
+
+With [pipelines for merge requests](../merge_request_pipelines/index.md),
+make it possible to define if a job should be created base on files modified
+in a merge request.
+
+For example:
+
+```
+docker build service one:
+  script: docker build -t my-service-one-image:$CI_COMMIT_REF_SLUG .
+  only:
+    refs:
+      - merge_requests
+    changes:
+      - Dockerfile
+      - service-one/**/*
+```
+
+In the scenario above, if you create or update a merge request that changes
+either files in `service-one` folder or `Dockerfile`, GitLab creates and triggers
+the `docker build service one` job.
 
 ## `tags`
 

doc/development/README.md

@@ -51,6 +51,7 @@ description: 'Learn how to contribute to GitLab.'
 - [Import/Export development documentation](import_export.md)
 - [Elasticsearch integration docs](elasticsearch.md)
 - [Working with Merge Request diffs](diffs.md)
+- [Kubernetes integration guidelines](kubernetes.md)
 - [Permissions](permissions.md)
 - [Prometheus metrics](prometheus_metrics.md)
 - [Guidelines for reusing abstractions](reusing_abstractions.md)

doc/development/kubernetes.md

+# Kubernetes integration - development guidelines
+
+This document provides various guidelines when developing for GitLab's
+[Kubernetes integration](../user/project/clusters/index.md).
+
+## Development
+
+### Architecture
+
+Some Kubernetes operations, such as creating restricted project
+namespaces are performed on the GitLab Rails application. These
+operations are performed using a [client library](#client-library).
+These operations will carry an element of risk as the operations will be
+run as the same user running the GitLab Rails application, see the
+[security](#security) section below.
+
+Some Kubernetes operations, such as installing cluster applications are
+performed on one-off pods on the Kubernetes cluster itself. These
+installation pods are currently named `install-<application_name>` and
+are created within the `gitlab-managed-apps` namespace.
+
+In terms of code organization, we generally add objects that represent
+Kubernetes resources in
+[`lib/gitlab/kubernetes`](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/lib/gitlab/kubernetes).
+
+### Client library
+
+We use the [`kubeclient`](https://rubygems.org/gems/kubeclient) gem to
+perform Kubernetes API calls. As the `kubeclient` gem does not support
+different API Groups (e.g. `apis/rbac.authorization.k8s.io`) from a
+single client, we have created a wrapper class,
+[`Gitlab::Kubernetes::KubeClient`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/gitlab/kubernetes/kube_client.rb)
+that will enable you to achieve this.
+
+Selected Kubernetes API groups are currently supported. Do add support
+for new API groups or methods to
+[`Gitlab::Kubernetes::KubeClient`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/gitlab/kubernetes/kube_client.rb)
+if you need to use them. New API groups or API group versions can be
+added to `SUPPORTED_API_GROUPS` - internally, this will create an
+internal client for that group. New methods can be added as a delegation
+to the relevant internal client.
+
+### Performance considerations
+
+All calls to the Kubernetes API must be in a background process. Do not
+perform Kubernetes API calls within a web request as this will block
+unicorn and can easily lead to a Denial Of Service (DoS) attack in GitLab as
+the Kubernetes cluster response times are outside of our control.
+
+The easiest way to ensure your calls happen a background process is to
+delegate any such work to happen in a [sidekiq
+worker](sidekiq_style_guide.md).
+
+There are instances where you would like to make calls to Kubernetes and
+return the response and as such a background worker does not seem to be
+a good fit. For such cases you should make use of [reactive
+caching](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/models/concerns/reactive_caching.rb).
+For example:
+
+```ruby
+  def calculate_reactive_cache!
+    { pods: cluster.platform_kubernetes.kubeclient.get_pods }
+  end
+
+  def pods
+    with_reactive_cache do |data|
+      data[:pods]
+    end
+  end
+```
+
+### Testing
+
+We have some Webmock stubs in
+[`KubernetesHelpers`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/support/helpers/kubernetes_helpers.rb)
+which can help with mocking out calls to Kubernetes API in your tests.
+
+## Security
+
+### SSRF
+
+As URLs for Kubernetes clusters are user controlled it is easily
+susceptible to Server Side Request Forgery (SSRF) attacks. You should
+understand the mitigation strategies if you are adding more API calls to
+a cluster.
+
+Mitigation strategies include:
+
+1. Not allowing redirects to attacker controller resources:
+   [`Kubeclient::KubeClient`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/gitlab/kubernetes/kube_client.rb#)
+   can be configured to disallow any redirects by passing in
+   `http_max_redirects: 0` as an option.
+1. Not exposing error messages: by doing so, we
+   prevent attackers from triggering errors to expose results from
+   attacker controlled requests. For example, we do not expose (or store)
+   raw error messages:
+
+   ```ruby
+   rescue Kubernetes::HttpError => e
+     # bad
+     # app.make_errored!("Kubernetes error: #{e.message}")
+
+     # good
+     app.make_errored!("Kubernetes error: #{e.error_code}")
+   ```
+
+## Debugging
+
+Logs related to the Kubernetes integration can be found in
+[kubernetes.log](../administration/logs.md#kuberneteslog). On a local
+GDK install, this will be present in `log/kubernetes.log`.
+
+Some services such as
+[`Clusters::Applications::InstallService`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/services/clusters/applications/install_service.rb#L18)
+rescues `StandardError` which can make it harder to debug issues in an
+development environment. The current workaround is to temporarily
+comment out the `rescue` in your local development source.
+
+You can also follow the installation pod logs to debug issues related to
+installation. Once the installation/upgrade is underway, wait for the
+pod to be created. Then run the following to obtain the pods logs as
+they are written:
+
+```bash
+kubectl logs <pod_name> --follow -n gitlab-managed-apps
+```

lib/gitlab/ci/build/policy/changes.rb

@@ -10,7 +10,7 @@ module Gitlab
           end
 
           def satisfied_by?(pipeline, seed)
-            return true unless pipeline.branch_updated?
+            return true if pipeline.modified_paths.nil?
 
             pipeline.modified_paths.any? do |path|
               @globs.any? do |glob|

spec/lib/gitlab/ci/build/policy/changes_spec.rb

@@ -73,9 +73,9 @@ describe Gitlab::Ci::Build::Policy::Changes do
         expect(policy).not_to be_satisfied_by(pipeline, seed)
       end
 
-      context 'when pipelines does not run for a branch update' do
+      context 'when modified paths can not be evaluated' do
         before do
-          pipeline.before_sha = Gitlab::Git::BLANK_SHA
+          allow(pipeline).to receive(:modified_paths) { nil }
         end
 
         it 'is always satisfied' do
@@ -115,5 +115,57 @@ describe Gitlab::Ci::Build::Policy::Changes do
         expect(policy).not_to be_satisfied_by(pipeline, seed)
       end
     end
+
+    context 'when branch is created' do
+      let(:pipeline) do
+        create(:ci_empty_pipeline, project: project,
+                                   ref: 'feature',
+                                   source: source,
+                                   sha: '0b4bc9a4',
+                                   before_sha: Gitlab::Git::BLANK_SHA,
+                                   merge_request: merge_request)
+      end
+
+      let(:ci_build) do
+        build(:ci_build, pipeline: pipeline, project: project, ref: 'feature')
+      end
+
+      let(:seed) { double('build seed', to_resource: ci_build) }
+
+      context 'when source is merge request' do
+        let(:source) { :merge_request }
+
+        let(:merge_request) do
+          create(:merge_request,
+                 source_project: project,
+                 source_branch: 'feature',
+                 target_project: project,
+                 target_branch: 'master')
+        end
+
+        it 'is satified by changes in the merge request' do
+          policy = described_class.new(%w[files/ruby/feature.rb])
+
+          expect(policy).to be_satisfied_by(pipeline, seed)
+        end
+
+        it 'is not satified by changes not in the merge request' do
+          policy = described_class.new(%w[foo.rb])
+
+          expect(policy).not_to be_satisfied_by(pipeline, seed)
+        end
+      end
+
+      context 'when source is push' do
+        let(:source) { :push }
+        let(:merge_request) { nil }
+
+        it 'is always satified' do
+          policy = described_class.new(%w[foo.rb])
+
+          expect(policy).to be_satisfied_by(pipeline, seed)
+        end
+      end
+    end
   end
 end

spec/models/ci/pipeline_spec.rb

@@ -1181,8 +1181,26 @@ describe Ci::Pipeline, :mailer do
         pipeline.update_column(:before_sha, Gitlab::Git::BLANK_SHA)
       end
 
-      it 'raises an error' do
-        expect { pipeline.modified_paths }.to raise_error(ArgumentError)
+      it 'returns nil' do
+        expect(pipeline.modified_paths).to be_nil
+      end
+    end
+
+    context 'when source is merge request' do
+      let(:pipeline) do
+        create(:ci_pipeline, source: :merge_request, merge_request: merge_request)
+      end
+
+      let(:merge_request) do
+        create(:merge_request,
+               source_project: project,
+               source_branch: 'feature',
+               target_project: project,
+               target_branch: 'master')
+      end
+
+      it 'returns merge request modified paths' do
+        expect(pipeline.modified_paths).to match(merge_request.modified_paths)
       end
     end
   end

spec/models/releases/link_spec.rb

@@ -77,4 +77,28 @@ describe Releases::Link do
 
     it { is_expected.to be_truthy }
   end
+
+  describe 'supported protocols' do
+    where(:protocol) do
+      %w(http https ftp)
+    end
+
+    with_them do
+      let(:link) { build(:release_link, url: protocol + '://assets.com/download') }
+
+      it 'will be valid' do
+        expect(link).to be_valid
+      end
+    end
+  end
+
+  describe 'unsupported protocol' do
+    context 'for torrent' do
+      let(:link) { build(:release_link, url: 'torrent://assets.com/download') }
+
+      it 'will be invalid' do
+        expect(link).to be_invalid
+      end
+    end
+  end
 end