Merge branch '22849-ci-build-ref-slug' into 'master'

Introduce $CI_BUILD_REF_SLUG

## What does this MR do?

Adds `$CI_BUILD_REF_SLUG` to the variables exposed to the runner. This is based on `$CI_BUILD_REF_NAME` but lowercased, shortened to 63 bytes maximum, and with characters invalid in URLs and domain names replaced with `-`. 

## Are there points in the code the reviewer needs to double check?

Slugs don't have a uniqueness guarantee. !7983 introduces an environment name slug which *is* unique, so I'm not as exercised about this as I was.

Should the slug be published in the API? It's available through the `variables` endpoint, but perhaps it should be part of `GET /project/:id/builds` ?

I've called it `ref_slug` rather than just `slug` as there are number of possibilities for slugification in a build (unlike an environment, where only the name makes sense to slugify).

## Why was this MR needed?

`$CI_BUILD_REF_NAME` is not suited for URLs and domain names, given the list of valid characters in a git ref. 

## Screenshots (if relevant)

## Does this MR meet the acceptance criteria?

- [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added
- [X] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [ ] API support added
- Tests
  - [X] Added for this feature/bug
  - [x] All builds are passing
- [X] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [X] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [X] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [X] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

## What are the relevant issue numbers?

Closes #22849

See merge request !8072

app/models/ci/build.rb

@@ -195,6 +195,17 @@ module Ci
       project.build_timeout
     end
 
+    # A slugified version of the build ref, suitable for inclusion in URLs and
+    # domain names. Rules:
+    #
+    #   * Lowercased
+    #   * Anything not matching [a-z0-9-] is replaced with a -
+    #   * Maximum length is 63 bytes
+    def ref_slug
+      slugified = ref.to_s.downcase
+      slugified.gsub(/[^a-z0-9]/, '-')[0..62]
+    end
+
     def variables
       variables = predefined_variables
       variables += project.predefined_variables
@@ -529,6 +540,7 @@ module Ci
         { key: 'CI_BUILD_REF', value: sha, public: true },
         { key: 'CI_BUILD_BEFORE_SHA', value: before_sha, public: true },
         { key: 'CI_BUILD_REF_NAME', value: ref, public: true },
+        { key: 'CI_BUILD_REF_SLUG', value: ref_slug, public: true },
         { key: 'CI_BUILD_NAME', value: name, public: true },
         { key: 'CI_BUILD_STAGE', value: stage, public: true },
         { key: 'CI_SERVER_NAME', value: 'GitLab', public: true },

changelogs/unreleased/22849-ci-build-ref-slug.yml

@@ -0,0 +1,4 @@
+---
+title: Introduce $CI_BUILD_REF_SLUG
+merge_request: 8072
+author: 

doc/ci/environments.md

@@ -248,7 +248,7 @@ deploy_review:
     - echo "Deploy a review app"
   environment:
     name: review/$CI_BUILD_REF_NAME
-    url: https://$CI_BUILD_REF_NAME.example.com
+    url: https://$CI_BUILD_REF_SLUG.example.com
   only:
     - branches
   except:
@@ -259,13 +259,14 @@ Let's break it down in pieces. The job's name is `deploy_review` and it runs
 on the `deploy` stage. The `script` at this point is fictional, you'd have to
 use your own based on your deployment. Then, we set the `environment` with the
 `environment:name` being `review/$CI_BUILD_REF_NAME`. Now that's an interesting
-one. Since the [environment name][env-name] can contain also slashes (`/`), we
-can use this pattern to distinguish between dynamic environments and the regular
+one. Since the [environment name][env-name] can contain slashes (`/`), we can
+use this pattern to distinguish between dynamic environments and the regular
 ones.
 
 So, the first part is `review`, followed by a `/` and then `$CI_BUILD_REF_NAME`
-which takes the value of the branch name. We also use the same
-`$CI_BUILD_REF_NAME` value in the `environment:url` so that the environment
+which takes the value of the branch name. Since `$CI_BUILD_REF_NAME` itself may
+also contain `/`, or other characters that would be invalid in a domain name or
+URL, we use `$CI_BUILD_REF_SLUG` in the `environment:url` so that the environment
 can get a specific and distinct URL for each branch. Again, the way you set up
 the webserver to serve these requests is based on your setup.
 
@@ -299,7 +300,7 @@ deploy_review:
     - echo "Deploy a review app"
   environment:
     name: review/$CI_BUILD_REF_NAME
-    url: https://$CI_BUILD_REF_NAME.example.com
+    url: https://$CI_BUILD_REF_SLUG.example.com
   only:
     - branches
   except:
@@ -329,16 +330,16 @@ deploy_prod:
 
 A more realistic example would include copying files to a location where a
 webserver (NGINX) could then read and serve. The example below will copy the
-`public` directory to `/srv/nginx/$CI_BUILD_REF_NAME/public`:
+`public` directory to `/srv/nginx/$CI_BUILD_REF_SLUG/public`:
 
 ```yaml
 review_app:
   stage: deploy
   script:
-    - rsync -av --delete public /srv/nginx/$CI_BUILD_REF_NAME
+    - rsync -av --delete public /srv/nginx/$CI_BUILD_REF_SLUG
   environment:
     name: review/$CI_BUILD_REF_NAME
-    url: https://$CI_BUILD_REF_NAME.example.com
+    url: https://$CI_BUILD_REF_SLUG.example.com
 ```
 
 It is assumed that the user has already setup NGINX and GitLab Runner in the
@@ -346,7 +347,7 @@ server this job will run on.
 
 >**Note:**
 Be sure to check out the [limitations](#limitations) section for some edge
-cases regarding naming of you branches and Review Apps.
+cases regarding naming of your branches and Review Apps.
 
 ---
 
@@ -418,7 +419,7 @@ deploy_review:
     - echo "Deploy a review app"
   environment:
     name: review/$CI_BUILD_REF_NAME
-    url: https://$CI_BUILD_REF_NAME.example.com
+    url: https://$CI_BUILD_REF_SLUG.example.com
     on_stop: stop_review
   only:
     - branches
@@ -480,9 +481,8 @@ exist, you should see something like:
 
 ## Checkout deployments locally
 
-Since 8.13, a reference in the git repository is saved for each deployment. So
-knowing what the state is of your current environments is only a `git fetch`
-away.
+Since 8.13, a reference in the git repository is saved for each deployment, so
+knowing the state of your current environments is only a `git fetch` away.
 
 In your git config, append the `[remote "<your-remote>"]` block with an extra
 fetch line:
@@ -493,10 +493,10 @@ fetch = +refs/environments/*:refs/remotes/origin/environments/*
 
 ## Limitations
 
-1. If the branch name contains special characters (`/`), and you use the
-   `$CI_BUILD_REF_NAME` variable to dynamically create environments, there might
-   be complications during your Review Apps deployment. Follow the
-   [issue 22849][ce-22849] for more information.
+1. `$CI_BUILD_REF_SLUG` is not *guaranteed* to be unique, so there is a small
+    chance of collisions between similarly-named branches (`fix-foo` would
+    conflict with `fix/foo`, for instance). Following a well-defined workflow
+    such as [GitLab Flow][gitlab-flow] can keep this from being a problem.
 1. You are limited to use only the [CI predefined variables][variables] in the
    `environment: name`. If you try to re-use variables defined inside `script`
    as part of the environment name, it will not work.
@@ -520,6 +520,6 @@ Below are some links you may find interesting:
 [only]: yaml/README.md#only-and-except
 [onstop]: yaml/README.md#environment-on_stop
 [ce-7015]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7015
-[ce-22849]: https://gitlab.com/gitlab-org/gitlab-ce/issues/22849
+[gitlab-flow]: ../workflow/gitlab_flow.md
 [gitlab runner]: https://docs.gitlab.com/runner/
 [git-strategy]: yaml/README.md#git-strategy

doc/ci/variables/README.md

@@ -40,6 +40,7 @@ version of Runner required.
 | **CI_BUILD_NAME**       | all    | 0.5    | The name of the build as defined in `.gitlab-ci.yml` |
 | **CI_BUILD_STAGE**      | all    | 0.5    | The name of the stage as defined in `.gitlab-ci.yml` |
 | **CI_BUILD_REF_NAME**   | all    | all    | The branch or tag name for which project is built |
+| **CI_BUILD_REF_SLUG**   | 8.15   | all    | `$CI_BUILD_REF_NAME` lowercased, shortened to 63 bytes, and with everything except `0-9` and `a-z` replaced with `-`. Use in URLs and domain names. |
 | **CI_BUILD_REPO**       | all    | all    | The URL to clone the Git repository |
 | **CI_BUILD_TRIGGERED**  | all    | 0.5    | The flag to indicate that build was [triggered] |
 | **CI_BUILD_MANUAL**     | 8.12   | all    | The flag to indicate that build was manually started |

doc/ci/yaml/README.md

@@ -690,18 +690,12 @@ The `stop_review_app` job is **required** to have the following keywords defined
 #### dynamic environments
 
 > [Introduced][ce-6323] in GitLab 8.12 and GitLab Runner 1.6.
+  `$CI_BUILD_REF_SLUG` was [introduced][ce-8072] in GitLab 8.15.
 
 `environment` can also represent a configuration hash with `name` and `url`.
 These parameters can use any of the defined [CI variables](#variables)
 (including predefined, secure variables and `.gitlab-ci.yml` variables).
 
->**Note:**
-Be aware than if the branch name contains special characters and you use the
-`$CI_BUILD_REF_NAME` variable to dynamically create environments, there might
-be complications during deployment. Follow the
-[issue 22849](https://gitlab.com/gitlab-org/gitlab-ce/issues/22849) for more
-information.
-
 For example:
 
 ```
@@ -710,7 +704,7 @@ deploy as review app:
   script: make deploy
   environment:
     name: review-apps/$CI_BUILD_REF_NAME
-    url: https://$CI_BUILD_REF_NAME.review.example.com/
+    url: https://$CI_BUILD_REF_SLUG.review.example.com/
 ```
 
 The `deploy as review app` job will be marked as deployment to dynamically
@@ -726,6 +720,10 @@ The common use case is to create dynamic environments for branches and use them
 as Review Apps. You can see a simple example using Review Apps at
 https://gitlab.com/gitlab-examples/review-apps-nginx/.
 
+`$CI_BUILD_REF_SLUG` is another environment variable set by the runner, based on
+`$CI_BUILD_REF_NAME` but lower-cased, and with some characters replaced with
+`-`, making it suitable for use in URLs and domain names.
+
 ### artifacts
 
 >**Notes:**
@@ -1245,4 +1243,5 @@ CI with various languages.
 [ce-6323]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6323
 [environment]: ../environments.md
 [ce-6669]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6669
+[ce-8072]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/xxxx
 [variables]: ../variables/README.md

spec/models/build_spec.rb

@@ -254,6 +254,24 @@ describe Ci::Build, models: true do
     end
   end
 
+  describe '#ref_slug' do
+    {
+      'master'    => 'master',
+      '1-foo'     => '1-foo',
+      'fix/1-foo' => 'fix-1-foo',
+      'fix-1-foo' => 'fix-1-foo',
+      'a' * 63    => 'a' * 63,
+      'a' * 64    => 'a' * 63,
+      'FOO'       => 'foo',
+    }.each do |ref, slug|
+      it "transforms #{ref} to #{slug}" do
+        build.ref = ref
+
+        expect(build.ref_slug).to eq(slug)
+      end
+    end
+  end
+
   describe '#variables' do
     let(:container_registry_enabled) { false }
     let(:predefined_variables) do
@@ -265,6 +283,7 @@ describe Ci::Build, models: true do
         { key: 'CI_BUILD_REF', value: build.sha, public: true },
         { key: 'CI_BUILD_BEFORE_SHA', value: build.before_sha, public: true },
         { key: 'CI_BUILD_REF_NAME', value: 'master', public: true },
+        { key: 'CI_BUILD_REF_SLUG', value: 'master', public: true },
         { key: 'CI_BUILD_NAME', value: 'test', public: true },
         { key: 'CI_BUILD_STAGE', value: 'test', public: true },
         { key: 'CI_SERVER_NAME', value: 'GitLab', public: true },